Re: Incorrect uses of get_driver()/put_driver()

[Alan Stern]

On Mon, 9 Jan 2012, Dmitry Torokhov wrote:

> On Mon, Jan 09, 2012 at 07:03:21PM +0100, Michael Büsch wrote:
> > On Mon, 9 Jan 2012 12:35:09 -0500 (EST)
> > Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> > 
> > > drivers/ssb/main.c:146:               get_driver(&drv->drv);
> > > drivers/ssb/main.c:153:               put_driver(&drv->drv);
> > > 
> > > Michael, these are part of ssb_driver_get() and ssb_driver_put(), used
> > > in ssb_devices_freeze() and ssb_devices_thaw().  They don't currently
> > > do anything, but it looks as if they are meant to prevent the driver
> > > from being unloaded.  Should they be replaced with try_module_get()?  
> > > Or would it be good enough to call device_attach() in 
> > > ssb_devices_thaw()?
> > 
> > Hm, It seems that this code is trying to pin the ssb_driver, so that
> > it doesn't become invalid during the freeze period. So it most likely wants
> > to protect against module unload and driver unbind here. Not sure
> > if that actually works, though :/
> > 
> 
> Not at all ;)

Maybe you want to call device_lock(&sdev->dev) here?  It will prevent
the driver from being unbound (and therefore from being unloaded), and
it's likely that sdrv's remove and probe routines expect to be called
with this lock held, because that's what the device core does.  The
drawback is that holding the lock prevents other things from happening
as well, like unregistering sdev.

Alternatively, we can simply remove ssb_driver_get/put.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/