From: Johan Hovold <johan@hovoldconsulting.com>
Date: Tue, 13 Oct 2015 17:10:24 +0000 (+0200)
Subject: greybus: protocol: warn on bad deregistration
X-Git-Url: http://git.maquefel.me/?a=commitdiff_plain;h=78033844daa64c83d91dca73eb1fbcae56c42fac;p=linux.git

greybus: protocol: warn on bad deregistration

A protocol should be deregistered exactly once when the protocol module
is being unloaded. This means that protocol deregister will never be
called with active users as we take a module reference when looking up a
protocol.

Remove comment suggesting that we could one day forcefully stop a user
of a protocol, and issue a big warning if a protocol is deregistered
more than once or at some other time than during module unload (e.g.
with active users).

Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
---

diff --git a/drivers/staging/greybus/protocol.c b/drivers/staging/greybus/protocol.c
index c93f963656674..140baa6854698 100644
--- a/drivers/staging/greybus/protocol.c
+++ b/drivers/staging/greybus/protocol.c
@@ -102,36 +102,24 @@ EXPORT_SYMBOL_GPL(__gb_protocol_register);
 
 /*
  * De-register a previously registered protocol.
- *
- * XXX Currently this fails (and reports an error to the caller) if
- * XXX the protocol is currently in use.  We may want to forcefully
- * XXX kill off a protocol and all its active users at some point.
- * XXX But I think that's better handled by quiescing modules that
- * XXX have users and having those users drop their reference.
- *
- * Returns true if successful, false otherwise.
  */
-int gb_protocol_deregister(struct gb_protocol *protocol)
+void gb_protocol_deregister(struct gb_protocol *protocol)
 {
-	u8 protocol_count = 0;
-
 	if (!protocol)
-		return 0;
+		return;
 
 	spin_lock_irq(&gb_protocols_lock);
 	protocol = gb_protocol_find(protocol->id, protocol->major,
 				    protocol->minor);
-	if (protocol) {
-		protocol_count = protocol->count;
-		if (!protocol_count)
-			list_del(&protocol->links);
+	if (WARN_ON(!protocol || protocol->count)) {
+		spin_unlock_irq(&gb_protocols_lock);
+		return;
 	}
-	spin_unlock_irq(&gb_protocols_lock);
 
-	if (protocol)
-		pr_info("Deregistered %s protocol.\n", protocol->name);
+	list_del(&protocol->links);
+	spin_unlock_irq(&gb_protocols_lock);
 
-	return protocol && !protocol_count;
+	pr_info("Deregistered %s protocol.\n", protocol->name);
 }
 EXPORT_SYMBOL_GPL(gb_protocol_deregister);
 
diff --git a/drivers/staging/greybus/protocol.h b/drivers/staging/greybus/protocol.h
index d856885a89e1f..2e0f4d667897a 100644
--- a/drivers/staging/greybus/protocol.h
+++ b/drivers/staging/greybus/protocol.h
@@ -46,7 +46,7 @@ struct gb_protocol {
 };
 
 int __gb_protocol_register(struct gb_protocol *protocol, struct module *module);
-int gb_protocol_deregister(struct gb_protocol *protocol);
+void gb_protocol_deregister(struct gb_protocol *protocol);
 
 #define gb_protocol_register(protocol) \
 	__gb_protocol_register(protocol, THIS_MODULE)