Re: device model update 2/2

From: Patrick Mochel (mochel@osdl.org)
Date: Thu Jun 06 2002 - 11:56:13 EST


On Thu, 6 Jun 2002, Linus Torvalds wrote:

> In article <Pine.LNX.4.33.0206060808050.654-100000@geena.pdx.osdl.net>,
> Patrick Mochel <mochel@osdl.org> wrote:
> >-
> > /* detach from driver */
> > if (dev->driver->remove)
> > dev->driver->remove(dev);
> > put_driver(dev->driver);
> >+
> >+ lock_device(dev);
> >+ dev->driver = NULL;
> >+ unlock_device(dev);
>
> Code like the above just basically can _never_ be correct.
>
> The locking just doesn't make any sense like that.
>
> Real locking looks something like this:
>
> lock_device(dev);
> driver = dev->driver;
> dev->driver = NULL;
> unlock_device(dev);
>
> if (driver->remove)
> driver->remove(dev);
> put_driver(driver);
>
> together with some promise that "dev" cannot go away from under us (ie a
> refcount on "dev" itself).

The device is already going away; the refcount has hit 0 and we're
detaching it from the driver.

Incremental patch below; also in bk://ldm.bkbits.net/linux-2.5

        -pat

ChangeSet@1.457, 2002-06-06 09:50:30-07:00, mochel@osdl.org
  device detach locking, one more time: get driver and reset it in struct device before calling remove()

 drivers/base/core.c | 21 ++++++++++++---------
 1 files changed, 12 insertions, 9 deletions

diff -Nru a/drivers/base/core.c b/drivers/base/core.c
--- a/drivers/base/core.c Thu Jun 6 09:52:43 2002
+++ b/drivers/base/core.c Thu Jun 6 09:52:43 2002
@@ -98,19 +98,22 @@
 
 static void device_detach(struct device * dev)
 {
- if (dev->driver) {
- write_lock(&dev->driver->lock);
- list_del_init(&dev->driver_list);
- write_unlock(&dev->driver->lock);
-
- /* detach from driver */
- if (dev->driver->remove)
- dev->driver->remove(dev);
- put_driver(dev->driver);
+ struct device_driver * drv;
 
+ if (dev->driver) {
                 lock_device(dev);
+ drv = dev->driver;
                 dev->driver = NULL;
                 unlock_device(dev);
+
+ write_lock(&drv->lock);
+ list_del_init(&dev->driver_list);
+ write_unlock(&drv->lock);
+
+ /* detach from driver */
+ if (drv->remove)
+ drv->remove(dev);
+ put_driver(drv);
         }
 }
 

-
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/



This archive was generated by hypermail 2b29 : Fri Jun 07 2002 - 22:00:28 EST