Messages in this thread Patch in this message | | | Date | Tue, 10 Jan 2012 11:21:43 +0100 (CET) | From | Sebastian Ott <> | Subject | Re: Incorrect uses of get_driver()/put_driver() |
| |
On Tue, 10 Jan 2012, Martin Schwidefsky wrote:
> On Mon, 9 Jan 2012 12:35:09 -0500 (EST) > Alan Stern <stern@rowland.harvard.edu> wrote: > > > drivers/s390/cio/device.c:1681: drv = get_driver(&cdrv->driver); > > drivers/s390/cio/device.c:1687: put_driver(drv); > > > > Martin, these calls seem to be useless. The calls in ccwgroup.c are > > definitely useless; there's no reason to take a reference to a driver > > while it's being unregistered, since it can't go away until the > > unregistration is finished. > > The get_driver/put_driver in ccwgroup.c are obviously useless, the caller > passed ccwgroup_driver_unregister a ccwgroup_driver reference. > I am not so sure about the code in device.c. get_ccwdev_by_busid() gets > used e.g. by vmur like this: > > static struct ccw_driver ur_driver = { .. }; > > static struct urdev *urdev_get_from_devno(u16 devno) > { > .. > sprintf(bus_id, "0.0.%04x", devno); > cdev = get_ccwdev_by_busid(&ur_driver, bus_id); > .. > } > > static int ur_open(struct inode *inode, struct file *file) > { > .. > urd = urdev_get_from_devno(devno); > .. > } > > For vmur we should be safe by the fact that ur_open is only possible if > a try_module_get has been successful and the ccw_driver is unregistered > only in the module exit function. But we have to check if this is true > for all users of the get_ccwdev_by_busid() function.
Done. [PATCH] cio: remove {get,put}_driver
Remove useless {get,put}_driver - the caller of the functions has to ensure valid driver pointers.
Signed-off-by: Sebastian Ott <sebott@linux.vnet.ibm.com> --- drivers/s390/cio/ccwgroup.c | 2 -- drivers/s390/cio/device.c | 8 +------- 2 files changed, 1 insertion(+), 9 deletions(-)
--- a/drivers/s390/cio/ccwgroup.c +++ b/drivers/s390/cio/ccwgroup.c @@ -580,7 +580,6 @@ void ccwgroup_driver_unregister(struct c struct device *dev; /* We don't want ccwgroup devices to live longer than their driver. */ - get_driver(&cdriver->driver); while ((dev = driver_find_device(&cdriver->driver, NULL, NULL, __ccwgroup_match_all))) { struct ccwgroup_device *gdev = to_ccwgroupdev(dev); @@ -592,7 +591,6 @@ void ccwgroup_driver_unregister(struct c mutex_unlock(&gdev->reg_mutex); put_device(dev); } - put_driver(&cdriver->driver); driver_unregister(&cdriver->driver); } EXPORT_SYMBOL(ccwgroup_driver_unregister); --- a/drivers/s390/cio/device.c +++ b/drivers/s390/cio/device.c @@ -1676,15 +1676,9 @@ struct ccw_device *get_ccwdev_by_busid(s const char *bus_id) { struct device *dev; - struct device_driver *drv; - drv = get_driver(&cdrv->driver); - if (!drv) - return NULL; - - dev = driver_find_device(drv, NULL, (void *)bus_id, + dev = driver_find_device(&cdrv->driver, NULL, (void *)bus_id, __ccwdev_check_busid); - put_driver(drv); return dev ? to_ccwdev(dev) : NULL; }
> > -- > blue skies, > Martin. > > "Reality continues to ruin my life." - Calvin. > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > >
| |