From b2a644b435de1d45bd8b5d4e391c894a662eb1f3 Mon Sep 17 00:00:00 2001 From: Ian Abbott Date: Thu, 4 Apr 2013 14:58:56 +0100 Subject: [PATCH] staging: comedi: simplify driver module counting For a legacy device attachment with the `COMEDI_DEVCONFIG` ioctl, `do_devconfig_ioctl()` calls `comedi_device_attach()` to find a matching device driver and attach the device. It then tries to increment the device driver's module count and if that fails it detaches the device. So on successful attachment of a device by the `COMEDI_DEVCONFIG` ioctl, the device driver's module count will have been incremented. `comedi_device_attach()` is called from nowhere else. It already increments the device driver's module count temporarily and decrements it again; if it gets as far as calling `comedi_device_postconfig()` the module count is decremented within that function. Simplify the above by removing the decrement of the device driver module count from `comedi_device_postconfig()`. If the call to `comedi_device_postconfig()` succeeds, `comedi_device_attach()` will return with the module count still incremented, otherwise decrement the module count before returning the error. Don't try and increment the module count in `do_devconfig_ioctl()` after a successful return from `comedi_device_attach()` as the module count has now already been incremented. `comedi_device_postconfig()` is also called by `comedi_auto_config()` which currently has to increment the device driver's module count temporarily so that `comedi_device_postconfig()` can decrement it, but always returns with no overall change to the module count. Remove all the module count manipulations from `comedi_device_postconfig()`. There is no other reason for `comedi_auto_config()` to increment the device driver's module count temporarily, since it is only called (indirectly) from the device driver itself (usually via one of the wrappers `comedi_pci_auto_config()` or `comedi_usb_auto_config()`). Signed-off-by: Ian Abbott Reviewed-by: H Hartley Sweeten Signed-off-by: Greg Kroah-Hartman --- drivers/staging/comedi/comedi_fops.c | 12 ++--------- drivers/staging/comedi/drivers.c | 32 +++++++++++++--------------- 2 files changed, 17 insertions(+), 27 deletions(-) diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c index 1c310e790b6..229be419b01 100644 --- a/drivers/staging/comedi/comedi_fops.c +++ b/drivers/staging/comedi/comedi_fops.c @@ -520,7 +520,6 @@ static int do_devconfig_ioctl(struct comedi_device *dev, struct comedi_devconfig __user *arg) { struct comedi_devconfig it; - int ret; if (!capable(CAP_SYS_ADMIN)) return -EPERM; @@ -551,15 +550,8 @@ static int do_devconfig_ioctl(struct comedi_device *dev, /* don't re-use dynamically allocated comedi devices */ return -EBUSY; - ret = comedi_device_attach(dev, &it); - if (ret == 0) { - if (!try_module_get(dev->driver->module)) { - comedi_device_detach(dev); - ret = -ENOSYS; - } - } - - return ret; + /* This increments the driver module count on success. */ + return comedi_device_attach(dev, &it); } /* diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c index caadd3bd72a..73727a73bd3 100644 --- a/drivers/staging/comedi/drivers.c +++ b/drivers/staging/comedi/drivers.c @@ -277,11 +277,11 @@ static int __comedi_device_postconfig(struct comedi_device *dev) } /* do a little post-config cleanup */ -/* called with module refcount incremented, decrements it */ static int comedi_device_postconfig(struct comedi_device *dev) { - int ret = __comedi_device_postconfig(dev); - module_put(dev->driver->module); + int ret; + + ret = __comedi_device_postconfig(dev); if (ret < 0) { __comedi_device_detach(dev); return ret; @@ -400,7 +400,11 @@ int comedi_device_attach(struct comedi_device *dev, struct comedi_devconfig *it) __comedi_device_detach(dev); return ret; } - return comedi_device_postconfig(dev); + ret = comedi_device_postconfig(dev); + if (ret < 0) + module_put(dev->driver->module); + /* On success, the driver module count has been incremented. */ + return ret; } int comedi_auto_config(struct device *hardware_device, @@ -431,19 +435,13 @@ int comedi_auto_config(struct device *hardware_device, return PTR_ERR(comedi_dev); /* Note: comedi_alloc_board_minor() locked comedi_dev->mutex. */ - if (!try_module_get(driver->module)) - ret = -EIO; - else { - comedi_set_hw_dev(comedi_dev, hardware_device); - comedi_dev->driver = driver; - ret = driver->auto_attach(comedi_dev, context); - if (ret < 0) { - module_put(driver->module); - __comedi_device_detach(comedi_dev); - } else { - ret = comedi_device_postconfig(comedi_dev); - } - } + comedi_set_hw_dev(comedi_dev, hardware_device); + comedi_dev->driver = driver; + ret = driver->auto_attach(comedi_dev, context); + if (ret < 0) + __comedi_device_detach(comedi_dev); + else + ret = comedi_device_postconfig(comedi_dev); mutex_unlock(&comedi_dev->mutex); if (ret < 0) -- 2.41.0