On Thu, Jan 26, 2006 at 03:17:02PM -0500, Alan Stern wrote:
> Greg:
>
> This is a revised version (as641b) of the earlier patch that James
> Bottomley didn't like. It goes in the direction of eliminating
> klist_remove entirely. (One more patch is still needed...)
>
> Add an is_registered flag to struct device, so that drivers
> won't get bound to a device after it is gone.
>
> When unregistering a driver, use the drv->unloaded completion
> to wait for the device_driver structure to be removed from
> the bus's klist instead of using klist_remove. This also
> eliminates the need for the klist_drivers_get method. (It's
> not a violation of the refcounting credo, because we have to
> wait in any case for the driver to be completely idle before
> driver_unregister can return.)
>
> Likewise, the klist_devices_get and klist_devices_put methods
> in drivers.c aren't needed, because we always have to wait for
> a device to be completely removed from its driver's klist. In
> fact, this is the last remaining usage of klist_remove.
>
> Move the call to a klist's put method outside the scope of the
> spinlock (i.e., move it from klist_release to klist_del and
> klist_next).
>
> The one unpalatable aspect of this patch is that it adds a new single-bit
> flag to struct device, thereby increasing the structure's size by at least
> 4 bytes.
>
> Alan Stern
>
>
>
> Signed-off-by: Alan Stern <[email protected]>
>
> ---
>
> Index: usb-2.6/drivers/base/dd.c
> ===================================================================
> --- usb-2.6.orig/drivers/base/dd.c
> +++ usb-2.6/drivers/base/dd.c
> @@ -72,6 +72,8 @@ int driver_probe_device(struct device_dr
> {
> int ret = 0;
>
> + if (!device_is_registered(dev))
> + return -ENODEV;
> if (drv->bus->match && !drv->bus->match(dev, drv))
> goto Done;
>
> Index: usb-2.6/drivers/base/bus.c
> ===================================================================
> --- usb-2.6.orig/drivers/base/bus.c
> +++ usb-2.6/drivers/base/bus.c
> @@ -367,6 +367,7 @@ int bus_add_device(struct device * dev)
>
> if (bus) {
> pr_debug("bus %s: add device %s\n", bus->name, dev->bus_id);
> + dev->is_registered = 1;
> device_attach(dev);
> klist_add_tail(&dev->knode_bus, &bus->klist_devices);
> error = device_add_attrs(bus, dev);
> @@ -393,7 +394,8 @@ void bus_remove_device(struct device * d
> sysfs_remove_link(&dev->kobj, "bus");
> sysfs_remove_link(&dev->bus->devices.kobj, dev->bus_id);
> device_remove_attrs(dev->bus, dev);
> - klist_remove(&dev->knode_bus);
> + klist_del(&dev->knode_bus);
> + dev->is_registered = 0;
Don't we have a race between these two lines? How is that protected?
thanks,
greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
