[PATCH] try_module_get(THIS_MODULE) is bogus

In most cases the fix is to add an struct module * member to the operations
vector instead and manipulate the refcounts in the callers context.

For the ALSA cases it was completly superflous (when will people get it that
using an exported symbol will make it's module unloadable?..)

drivers/char/ipmi/ipmi_kcs_intf.c

@@ -613,18 +613,6 @@ static void request_events(void *send_info)
 	atomic_set(&kcs_info->req_events, 1);
 }
 
-static int new_user(void *send_info)
-{
-	if (!try_module_get(THIS_MODULE))
-		return -EBUSY;
-	return 0;
-}
-
-static void user_left(void *send_info)
-{
-	module_put(THIS_MODULE);
-}
-
 /* Call every 10 ms. */
 #define KCS_TIMEOUT_TIME_USEC	10000
 #define KCS_USEC_PER_JIFFY	(1000000/HZ)
@@ -718,11 +706,10 @@ static void kcs_irq_handler(int irq, void *data, struct pt_regs *regs)
 
 static struct ipmi_smi_handlers handlers =
 {
-	sender:		       sender,
-	request_events:        request_events,
-	new_user:	       new_user,
-	user_left:	       user_left,
-	set_run_to_completion: set_run_to_completion
+	.owner			= THIS_MODULE,
+	.sender			= sender,
+	.request_events		= request_events,
+	.set_run_to_completion	= set_run_to_completion,
 };
 
 static unsigned char ipmi_kcs_dev_rev;

drivers/char/ipmi/ipmi_msghandler.c

@@ -485,13 +485,14 @@ int ipmi_create_user(unsigned int          if_num,
 	new_user->intf = ipmi_interfaces[if_num];
 	new_user->gets_events = 0;
 
-	rv = new_user->intf->handlers->new_user(new_user->intf->send_info);
-	if (rv)
+	if (!try_module_get(new_user->intf->handlers->owner)) {
+		rv = -ENODEV;
 		goto out_unlock;
+	}
 
-	write_lock_irqsave(&(new_user->intf->users_lock), flags);
-	list_add_tail(&(new_user->link), &(new_user->intf->users));
-	write_unlock_irqrestore(&(new_user->intf->users_lock), flags);
+	write_lock_irqsave(&new_user->intf->users_lock, flags);
+	list_add_tail(&new_user->link, &new_user->intf->users);
+	write_unlock_irqrestore(&new_user->intf->users_lock, flags);
 
  out_unlock:	
 	if (rv) {
@@ -563,12 +564,12 @@ int ipmi_destroy_user(ipmi_user_t user)
 	unsigned long flags;
 
 	down_read(&interfaces_sem);
-	write_lock_irqsave(&(intf->users_lock), flags);
+	write_lock_irqsave(&intf->users_lock, flags);
 	rv = ipmi_destroy_user_nolock(user);
 	if (!rv)
-		intf->handlers->user_left(intf->send_info);
+		module_put(intf->handlers->owner);
 		
-	write_unlock_irqrestore(&(intf->users_lock), flags);
+	write_unlock_irqrestore(&intf->users_lock, flags);
 	up_read(&interfaces_sem);
 	return rv;
 }

drivers/ieee1394/hosts.c

@@ -11,6 +11,7 @@
  */
 
 #include <linux/config.h>
+#include <linux/module.h>
 #include <linux/types.h>
 #include <linux/list.h>
 #include <linux/init.h>
@@ -69,8 +70,10 @@ int hpsb_ref_host(struct hpsb_host *host)
         spin_lock_irqsave(&hosts_lock, flags);
         list_for_each(lh, &hosts) {
                 if (host == list_entry(lh, struct hpsb_host, host_list)) {
-			if (host->driver->devctl(host, MODIFY_USAGE, 1)) {
-				host->driver->devctl(host, MODIFY_USAGE, 1);
+			if (try_module_get(host->driver->owner)) {
+				/* we're doing this twice and don't seem
+				   to undo it..  --hch */
+				(void)try_module_get(host->driver->owner);
 				host->refcount++;
 				retval = 1;
 			}
@@ -95,7 +98,7 @@ void hpsb_unref_host(struct hpsb_host *host)
 {
         unsigned long flags;
 
-        host->driver->devctl(host, MODIFY_USAGE, 0);
+	module_put(host->driver->owner);
 
         spin_lock_irqsave(&hosts_lock, flags);
         host->refcount--;

drivers/ieee1394/hosts.h

@@ -92,12 +92,6 @@ enum devctl_cmd {
          * Return void. */
         CANCEL_REQUESTS,
 
-        /* Decrease host usage count if arg == 0, increase otherwise.  Return
-         * 1 for success, 0 for failure.  Increase usage may fail if the driver
-         * is in the process of shutting itself down.  Decrease usage can not
-         * fail. */
-        MODIFY_USAGE,
-
         /* Start or stop receiving isochronous channel in arg.  Return void.
          * This acts as an optimization hint, hosts are not required not to
          * listen on unrequested channels. */
@@ -147,6 +141,7 @@ enum reset_types {
 };
 
 struct hpsb_host_driver {
+	struct module *owner;
 	const char *name;
 
         /* This function must store a pointer to the configuration ROM into the

drivers/ieee1394/ohci1394.c

@@ -966,16 +966,6 @@ static int ohci_devctl(struct hpsb_host *host, enum devctl_cmd cmd, int arg)
 		dma_trm_reset(&ohci->at_resp_context);
 		break;
 
-	case MODIFY_USAGE:
-		if (arg) {
-			if (try_module_get(THIS_MODULE))
-				retval = 1;
-		} else {
-			module_put(THIS_MODULE);
-			retval = 1;
-		}
-                break;
-
 	case ISO_LISTEN_CHANNEL:
         {
 		u64 mask;
@@ -3202,6 +3192,7 @@ static quadlet_t ohci_hw_csr_reg(struct hpsb_host *host, int reg,
 }
 
 static struct hpsb_host_driver ohci1394_driver = {
+	.owner =		THIS_MODULE,
 	.name =			OHCI1394_DRIVER_NAME,
 	.get_rom =		ohci_get_rom,
 	.transmit_packet =	ohci_transmit,

drivers/ieee1394/pcilynx.c

@@ -801,17 +801,6 @@ static int lynx_devctl(struct hpsb_host *host, enum devctl_cmd cmd, int arg)
 
                 break;
 
-        case MODIFY_USAGE:
-		if (arg) {
-			if (try_module_get(THIS_MODULE))
-				retval = 1;
-		} else {
-			module_put(THIS_MODULE);
-			retval = 1;
-		}
-
-                break;
-
         case ISO_LISTEN_CHANNEL:
                 spin_lock_irqsave(&lynx->iso_rcv.lock, flags);
                 
@@ -1904,6 +1893,7 @@ static struct pci_driver lynx_pci_driver = {
 };
 
 static struct hpsb_host_driver lynx_driver = {
+	.owner =	   THIS_MODULE,
 	.name =		   PCILYNX_DRIVER_NAME,
         .get_rom =         get_lynx_rom,
         .transmit_packet = lynx_transmit,

drivers/pci/hotplug.c

@@ -172,44 +172,6 @@ int pci_visit_dev (struct pci_visit *fn, struct pci_dev_wrapped *wrapped_dev,
 }
 EXPORT_SYMBOL(pci_visit_dev);
 
-/**
- * pci_is_dev_in_use - query devices' usage
- * @dev: PCI device to query
- *
- * Queries whether a given PCI device is in use by a driver or not.
- * Returns 1 if the device is in use, 0 if it is not.
- */
-int pci_is_dev_in_use(struct pci_dev *dev)
-{
-	/* 
-	 * dev->driver will be set if the device is in use by a new-style 
-	 * driver -- otherwise, check the device's regions to see if any
-	 * driver has claimed them.
-	 */
-
-	int i;
-	int inuse = 0;
-
-	if (dev->driver) {
-		/* Assume driver feels responsible */
-		return 1;
-	}
-
-	for (i = 0; !dev->driver && !inuse && (i < 6); i++) {
-		if (!pci_resource_start(dev, i))
-			continue;
-		if (pci_resource_flags(dev, i) & IORESOURCE_IO) {
-			inuse = check_region(pci_resource_start(dev, i),
-					     pci_resource_len(dev, i));
-		} else if (pci_resource_flags(dev, i) & IORESOURCE_MEM) {
-			inuse = check_mem_region(pci_resource_start(dev, i),
-						 pci_resource_len(dev, i));
-		}
-	}
-	return inuse;
-}
-EXPORT_SYMBOL(pci_is_dev_in_use);
-
 /**
  * pci_remove_device_safe - remove an unused hotplug device
  * @dev: the device to remove
@@ -221,9 +183,8 @@ EXPORT_SYMBOL(pci_is_dev_in_use);
  */
 int pci_remove_device_safe(struct pci_dev *dev)
 {
-	if (pci_is_dev_in_use(dev)) {
+	if (pci_dev_driver(dev))
 		return -EBUSY;
-	}
 	pci_remove_device(dev);
 	return 0;
 }

include/linux/ipmi_smi.h

@@ -78,6 +78,8 @@ struct ipmi_smi_msg
 
 struct ipmi_smi_handlers
 {
+	struct module *owner;
+
 	/* Called to enqueue an SMI message to be sent.  This
 	   operation is not allowed to fail.  If an error occurs, it
 	   should report back the error in a received message.  It may
@@ -93,15 +95,6 @@ struct ipmi_smi_handlers
 	   events from the BMC we are attached to. */
 	void (*request_events)(void *send_info);
 
-	/* Called when someone is using the interface, so the module can
-	   adjust it's use count.  Return zero if successful, or an
-	   errno if not. */
-	int (*new_user)(void *send_info);
-
-	/* Called when someone is no longer using the interface, so the
-	   module can adjust it's use count. */
-	void (*user_left)(void *send_info);
-
 	/* Called when the interface should go into "run to
 	   completion" mode.  If this call sets the value to true, the
 	   interface should make sure that all messages are flushed

include/linux/sunrpc/auth.h

@@ -84,6 +84,7 @@ struct rpc_auth {
  * Client authentication ops
  */
 struct rpc_authops {
+	struct module		*owner;
 	rpc_authflavor_t	au_flavor;	/* flavor (RPC_AUTH_*) */
 #ifdef RPC_DEBUG
 	char *			au_name;

net/sunrpc/auth.c

@@ -8,6 +8,7 @@
 
 #include <linux/types.h>
 #include <linux/sched.h>
+#include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/errno.h>
 #include <linux/socket.h>
@@ -65,6 +66,8 @@ rpcauth_create(rpc_authflavor_t pseudoflavor, struct rpc_clnt *clnt)
 
 	if (flavor >= RPC_AUTH_MAXFLAVOR || !(ops = auth_flavors[flavor]))
 		return NULL;
+	if (!try_module_get(ops->owner))
+		return NULL;
 	clnt->cl_auth = ops->create(clnt, pseudoflavor);
 	return clnt->cl_auth;
 }
@@ -73,6 +76,8 @@ void
 rpcauth_destroy(struct rpc_auth *auth)
 {
 	auth->au_ops->destroy(auth);
+	module_put(auth->au_ops->owner);
+	kfree(auth);
 }
 
 static spinlock_t rpc_credcache_lock = SPIN_LOCK_UNLOCKED;

net/sunrpc/auth_gss/auth_gss.c

@@ -438,8 +438,6 @@ gss_create(struct rpc_clnt *clnt, rpc_authflavor_t flavor)
 	struct rpc_auth * auth;
 
 	dprintk("RPC: creating GSS authenticator for client %p\n",clnt);
-	if (!try_module_get(THIS_MODULE))
-		return NULL;
 	if (!(gss_auth = kmalloc(sizeof(*gss_auth), GFP_KERNEL)))
 		goto out_dec;
 	gss_auth->mech = gss_pseudoflavor_to_mech(flavor);
@@ -470,7 +468,6 @@ gss_create(struct rpc_clnt *clnt, rpc_authflavor_t flavor)
 err_free:
 	kfree(gss_auth);
 out_dec:
-	module_put(THIS_MODULE);
 	return NULL;
 }
 
@@ -485,9 +482,6 @@ gss_destroy(struct rpc_auth *auth)
 	rpc_unlink(gss_auth->path);
 
 	rpcauth_free_credcache(auth);
-
-	kfree(auth);
-	module_put(THIS_MODULE);
 }
 
 /* gss_destroy_cred (and gss_destroy_ctx) are used to clean up after failure
@@ -691,6 +685,7 @@ gss_validate(struct rpc_task *task, u32 *p)
 }
 
 static struct rpc_authops authgss_ops = {
+	.owner		= THIS_MODULE,
 	.au_flavor	= RPC_AUTH_GSS,
 #ifdef RPC_DEBUG
 	.au_name	= "RPCSEC_GSS",

net/sunrpc/auth_null.c

@@ -8,6 +8,7 @@
 
 #include <linux/types.h>
 #include <linux/socket.h>
+#include <linux/module.h>
 #include <linux/in.h>
 #include <linux/utsname.h>
 #include <linux/sunrpc/clnt.h>
@@ -41,7 +42,6 @@ nul_destroy(struct rpc_auth *auth)
 {
 	dprintk("RPC: destroying NULL authenticator %p\n", auth);
 	rpcauth_free_credcache(auth);
-	kfree(auth);
 }
 
 /*
@@ -125,6 +125,7 @@ nul_validate(struct rpc_task *task, u32 *p)
 }
 
 struct rpc_authops	authnull_ops = {
+	.owner		= THIS_MODULE,
 	.au_flavor	= RPC_AUTH_NULL,
 #ifdef RPC_DEBUG
 	.au_name	= "NULL",

net/sunrpc/auth_unix.c

@@ -8,6 +8,7 @@
 
 #include <linux/types.h>
 #include <linux/sched.h>
+#include <linux/module.h>
 #include <linux/socket.h>
 #include <linux/in.h>
 #include <linux/sunrpc/clnt.h>
@@ -60,7 +61,6 @@ unx_destroy(struct rpc_auth *auth)
 {
 	dprintk("RPC: destroying UNIX authenticator %p\n", auth);
 	rpcauth_free_credcache(auth);
-	kfree(auth);
 }
 
 static struct rpc_cred *
@@ -219,6 +219,7 @@ unx_validate(struct rpc_task *task, u32 *p)
 }
 
 struct rpc_authops	authunix_ops = {
+	.owner		= THIS_MODULE,
 	.au_flavor	= RPC_AUTH_UNIX,
 #ifdef RPC_DEBUG
 	.au_name	= "UNIX",

sound/pci/rme9652/hammerfall_mem.c

@@ -150,8 +150,6 @@ void *snd_hammerfall_get_buffer (struct pci_dev *pcidev, dma_addr_t *dmaaddr)
 	for (i = 0; i < NBUFS; i++) {
 		rbuf = &hammerfall_buffers[i];
 		if (rbuf->flags == HAMMERFALL_BUF_ALLOCATED) {
-			if (! try_module_get(THIS_MODULE))
-				return NULL;
 			rbuf->flags |= HAMMERFALL_BUF_USED;
 			rbuf->pci = pcidev;
 			*dmaaddr = rbuf->addr;
@@ -171,7 +169,6 @@ void snd_hammerfall_free_buffer (struct pci_dev *pcidev, void *addr)
 		rbuf = &hammerfall_buffers[i];
 		if (rbuf->buf == addr && rbuf->pci == pcidev) {
 			rbuf->flags &= ~HAMMERFALL_BUF_USED;
-			module_put(THIS_MODULE);
 			return;
 		}
 	}