This patch (as1271) affects when new devices get linked into their
bus's list of devices. Currently this happens after probing, and it
doesn't happen at all if probing fails. Clearly this is wrong,
because at that point quite a few symbolic links have already been
created in sysfs. We are committed to adding the device, so it should
be linked into the bus's list regardless.
In addition, this needs to happen before the uevent announcing the new
device gets issued. Otherwise user programs might try to access the
device before it has been added to the bus.
To fix both these problems, the patch moves the call to
klist_add_tail() forward from bus_attach_device() to bus_add_device().
Since bus_attach_device() now does nothing but probe for drivers, it
has been renamed to bus_probe_device(). And lastly, the kerneldoc is
updated.
Signed-off-by: Alan Stern <[email protected]>
CC: Kay Sievers <[email protected]>
---
Kay, do you want this merged into 2.6.31 or are you okay with waiting
for 2.6.32-rc1? It changes a major core routine. On the other hand,
the problem it fixes does affect real users.
Index: usb-2.6/drivers/base/base.h
===================================================================
--- usb-2.6.orig/drivers/base/base.h
+++ usb-2.6/drivers/base/base.h
@@ -109,7 +109,7 @@ extern int system_bus_init(void);
extern int cpu_dev_init(void);
extern int bus_add_device(struct device *dev);
-extern void bus_attach_device(struct device *dev);
+extern void bus_probe_device(struct device *dev);
extern void bus_remove_device(struct device *dev);
extern int bus_add_driver(struct device_driver *drv);
Index: usb-2.6/drivers/base/bus.c
===================================================================
--- usb-2.6.orig/drivers/base/bus.c
+++ usb-2.6/drivers/base/bus.c
@@ -459,8 +459,9 @@ static inline void remove_deprecated_bus
* bus_add_device - add device to bus
* @dev: device being added
*
+ * - Add device's bus attributes.
+ * - Create links to device's bus.
* - Add the device to its bus's list of devices.
- * - Create link to device's bus.
*/
int bus_add_device(struct device *dev)
{
@@ -483,6 +484,7 @@ int bus_add_device(struct device *dev)
error = make_deprecated_bus_links(dev);
if (error)
goto out_deprecated;
+ klist_add_tail(&dev->p->knode_bus, &bus->p->klist_devices);
}
return 0;
@@ -498,24 +500,19 @@ out_put:
}
/**
- * bus_attach_device - add device to bus
- * @dev: device tried to attach to a driver
+ * bus_probe_device - probe drivers for a new device
+ * @dev: device to probe
*
- * - Add device to bus's list of devices.
- * - Try to attach to driver.
+ * - Automatically probe for a driver if the bus allows it.
*/
-void bus_attach_device(struct device *dev)
+void bus_probe_device(struct device *dev)
{
struct bus_type *bus = dev->bus;
- int ret = 0;
+ int ret;
- if (bus) {
- if (bus->p->drivers_autoprobe)
- ret = device_attach(dev);
+ if (bus && bus->p->drivers_autoprobe) {
+ ret = device_attach(dev);
WARN_ON(ret < 0);
- if (ret >= 0)
- klist_add_tail(&dev->p->knode_bus,
- &bus->p->klist_devices);
}
}
Index: usb-2.6/drivers/base/core.c
===================================================================
--- usb-2.6.orig/drivers/base/core.c
+++ usb-2.6/drivers/base/core.c
@@ -955,7 +955,7 @@ int device_add(struct device *dev)
BUS_NOTIFY_ADD_DEVICE, dev);
kobject_uevent(&dev->kobj, KOBJ_ADD);
- bus_attach_device(dev);
+ bus_probe_device(dev);
if (parent)
klist_add_tail(&dev->p->knode_parent,
&parent->p->klist_children);
On Thu, Jul 30, 2009 at 15:27, Alan Stern<[email protected]> wrote:
> This patch (as1271) affects when new devices get linked into their
> bus's list of devices. Currently this happens after probing, and it
> doesn't happen at all if probing fails. Clearly this is wrong,
> because at that point quite a few symbolic links have already been
> created in sysfs. We are committed to adding the device, so it should
> be linked into the bus's list regardless.
>
> In addition, this needs to happen before the uevent announcing the new
> device gets issued. Otherwise user programs might try to access the
> device before it has been added to the bus.
>
> To fix both these problems, the patch moves the call to
> klist_add_tail() forward from bus_attach_device() to bus_add_device().
> Since bus_attach_device() now does nothing but probe for drivers, it
> has been renamed to bus_probe_device(). And lastly, the kerneldoc is
> updated.
Thanks for doing this that quickly. You are doing a really great job.
> Kay, do you want this merged into 2.6.31 or are you okay with waiting
> for 2.6.32-rc1? It changes a major core routine. On the other hand,
> the problem it fixes does affect real users.
I think it should go into -next and we wait a few days. It seems like
the proper fix, but we should make sure, we didn't miss something.
After that, it would be nice if we can get that into 2.6.31, as we
have several problems already, which are likely solved by this.
Thanks,
Kay
On Thu, Jul 30, 2009 at 04:57:06PM -0400, Kay Sievers wrote:
> On Thu, Jul 30, 2009 at 15:27, Alan Stern<[email protected]> wrote:
> > This patch (as1271) affects when new devices get linked into their
> > bus's list of devices. ?Currently this happens after probing, and it
> > doesn't happen at all if probing fails. ?Clearly this is wrong,
> > because at that point quite a few symbolic links have already been
> > created in sysfs. ?We are committed to adding the device, so it should
> > be linked into the bus's list regardless.
> >
> > In addition, this needs to happen before the uevent announcing the new
> > device gets issued. ?Otherwise user programs might try to access the
> > device before it has been added to the bus.
> >
> > To fix both these problems, the patch moves the call to
> > klist_add_tail() forward from bus_attach_device() to bus_add_device().
> > Since bus_attach_device() now does nothing but probe for drivers, it
> > has been renamed to bus_probe_device(). ?And lastly, the kerneldoc is
> > updated.
>
> Thanks for doing this that quickly. You are doing a really great job.
>
> > Kay, do you want this merged into 2.6.31 or are you okay with waiting
> > for 2.6.32-rc1? ?It changes a major core routine. ?On the other hand,
> > the problem it fixes does affect real users.
>
> I think it should go into -next and we wait a few days. It seems like
> the proper fix, but we should make sure, we didn't miss something.
>
> After that, it would be nice if we can get that into 2.6.31, as we
> have several problems already, which are likely solved by this.
But as this isn't a regression (it's how things always have worked,
right?), I'm a bit leary of pushing it to .31 right now, so late in the
release cycle. How about it goes to Linus for .32, and we backport it
to -stable if it looks ok?
thanks,
greg k-h
On Fri, Jul 31, 2009 at 12:05, Greg KH<[email protected]> wrote:
> On Thu, Jul 30, 2009 at 04:57:06PM -0400, Kay Sievers wrote:
>> On Thu, Jul 30, 2009 at 15:27, Alan Stern<[email protected]> wrote:
>> > Kay, do you want this merged into 2.6.31 or are you okay with waiting
>> > for 2.6.32-rc1? It changes a major core routine. On the other hand,
>> > the problem it fixes does affect real users.
>>
>> I think it should go into -next and we wait a few days. It seems like
>> the proper fix, but we should make sure, we didn't miss something.
>>
>> After that, it would be nice if we can get that into 2.6.31, as we
>> have several problems already, which are likely solved by this.
>
> But as this isn't a regression (it's how things always have worked,
> right?),
Yeah, that sounds right, but I guess we have just been the needed bit
slower in the past, and with the recent speedups I would expect more
cases to uncover this issue.
> I'm a bit leary of pushing it to .31 right now, so late in the
> release cycle. How about it goes to Linus for .32, and we backport it
> to -stable if it looks ok?
Sure, sounds fine too.
Would be good though to have it in -next soon. Three major distros are
preparing a new distro release at the moment, and they will likely
need to add that to their kernel to make this issue, USB firmware
loading, and similar things working again.
Thanks,
Kay
On Fri, Jul 31, 2009 at 12:30:03PM -0400, Kay Sievers wrote:
> On Fri, Jul 31, 2009 at 12:05, Greg KH<[email protected]> wrote:
> > On Thu, Jul 30, 2009 at 04:57:06PM -0400, Kay Sievers wrote:
> >> On Thu, Jul 30, 2009 at 15:27, Alan Stern<[email protected]> wrote:
>
> >> > Kay, do you want this merged into 2.6.31 or are you okay with waiting
> >> > for 2.6.32-rc1? ?It changes a major core routine. ?On the other hand,
> >> > the problem it fixes does affect real users.
> >>
> >> I think it should go into -next and we wait a few days. It seems like
> >> the proper fix, but we should make sure, we didn't miss something.
> >>
> >> After that, it would be nice if we can get that into 2.6.31, as we
> >> have several problems already, which are likely solved by this.
> >
> > But as this isn't a regression (it's how things always have worked,
> > right?),
>
> Yeah, that sounds right, but I guess we have just been the needed bit
> slower in the past, and with the recent speedups I would expect more
> cases to uncover this issue.
>
> > I'm a bit leary of pushing it to .31 right now, so late in the
> > release cycle. ?How about it goes to Linus for .32, and we backport it
> > to -stable if it looks ok?
>
> Sure, sounds fine too.
>
> Would be good though to have it in -next soon. Three major distros are
> preparing a new distro release at the moment, and they will likely
> need to add that to their kernel to make this issue, USB firmware
> loading, and similar things working again.
It will show up in the next linux-next release.
thanks,
greg k-h