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 <abbotti@xxxxxxxxx>
---
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 1c310e7..229be41 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 caadd3b..73727a7 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)
--
1.8.1.5
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
