Re: Problems with get_driver() and driver_attach() (and new_id too)

[Alan Stern]

On Thu, 5 Jan 2012, Dmitry Torokhov wrote:

> > > I think pinning driver so that it can't be unregistered (and
> > > consequently module unload hangs) its a mis-feature.
> > 
> > I suspect that references obtained from get_driver() aren't held very 
> > long.  However I haven't checked every case.
> 
> Unless we stop exporting them we can not make any assumptions on how
> long they will be held - code is changing constantly.

Something we need to watch out for: get_driver and put_driver are used
in a bunch of other places, unrelated to driver_attach.  Here's what
I found:

lib/dma-debug.c:173:  drv = get_driver(dev->driver);
lib/dma-debug.c:188:  put_driver(drv);
drivers/pci/xen-pcifront.c:596:       if (get_driver(&pdrv->driver)) {
drivers/pci/xen-pcifront.c:626:               put_driver(&pdrv->driver);
drivers/media/video/s5p-fimc/fimc-mdevice.c:348:      put_driver(driver);
drivers/media/video/s5p-fimc/fimc-mdevice.c:356:              put_driver(driver);
drivers/media/video/ivtv/ivtvfb.c:1296:       put_driver(drv);
drivers/media/video/ivtv/ivtvfb.c:1313:       put_driver(drv);
drivers/media/video/cx18/cx18-alsa-main.c:288:        put_driver(drv);
drivers/media/video/s5p-tv/mixer_video.c:61:  put_driver(drv);
drivers/s390/cio/ccwgroup.c:583:      get_driver(&cdriver->driver);
drivers/s390/cio/ccwgroup.c:595:      put_driver(&cdriver->driver);
drivers/s390/cio/device.c:1681:       drv = get_driver(&cdrv->driver);
drivers/s390/cio/device.c:1687:       put_driver(drv);
drivers/s390/net/smsgiucv_app.c:199:  put_driver(smsgiucv_drv);
drivers/ssb/main.c:146:               get_driver(&drv->drv);
drivers/ssb/main.c:153:               put_driver(&drv->drv);
drivers/net/phy/phy_device.c:934:     drv = get_driver(phydev->dev.driver);
drivers/net/phy/phy_device.c:975:     put_driver(dev->driver);

I don't think any of those calls actually accomplish anything, but it's
hard to be certain.  Some of them appear to be futile attempts to
prevent the driver from being unregistered or unloaded, others are
there simply to drop the reference taken by driver_find().

In a few of them it's obvious that the driver can't be unregistered 
while the critical section runs, but in the others I can't tell.  On 
the other hand, if a critical section can race with unregistration 
then the code is buggy now.

What do you think?

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/