[Sorry for resending, but it looks like this message didn't reach the lists.]
On Tuesday, 22 of April 2008, Linus Torvalds wrote:
>
> On Tue, 22 Apr 2008, Rafael J. Wysocki wrote:
> >
> > There is a bug in device_add() that IMO can be fixed this way:
>
> Ok, looks fine. Greg?
>
> > Index: linux-2.6/drivers/base/core.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/base/core.c
> > +++ linux-2.6/drivers/base/core.c
> > @@ -820,11 +820,11 @@ int device_add(struct device *dev)
> > error = bus_add_device(dev);
> > if (error)
> > goto BusError;
> > + bus_attach_device(dev);
> > error = device_pm_add(dev);
> > if (error)
> > goto PMError;
> > kobject_uevent(&dev->kobj, KOBJ_ADD);
> > - bus_attach_device(dev);
> > if (parent)
> > klist_add_tail(&dev->knode_parent, &parent->klist_children);
> >
> > The problem is that bus_remove_device() assumes bus_attach_device() to have
> > run, AFAICS.
>
> As to the other issue:
>
> > > So I would suggest reverting that commit, or at least just making it a
> > > warning (while still registering the device).
> >
> > Are drivers supposed to register children of suspended devices? That doesn't
> > make much sense IMO ...
>
> Well, that's why I think the warning itself makes sense - and then we can
> decide whether it makes sense for that particular case or not. Clearly it
> happens (since it triggered), now we need to figure out _why_ it happened.
Well, this particular case looks like a race to me.
> But I don't think debugging messages should change behaviour.
Okay, below is a more complete patch that changes device_pm_add() so that
it doesn't refuse to add children of suspended devices. Note, however, that
even if such a registration succeeds, it will probably lead to some future
problems.
Thanks,
Rafael
---
Do not refuse to actually register children of suspended devices,
but still warn about attempts to do that.
Signed-off-by: Rafael J. Wysocki <rjw@xxxxxxx>
---
drivers/base/power/main.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
Index: linux-2.6/drivers/base/power/main.c
===================================================================
--- linux-2.6.orig/drivers/base/power/main.c
+++ linux-2.6/drivers/base/power/main.c
@@ -62,7 +62,7 @@ static bool all_sleeping;
*/
int device_pm_add(struct device *dev)
{
- int error = 0;
+ int error;
pr_debug("PM: Adding info for %s:%s\n",
dev->bus ? dev->bus->name : "No Bus",
@@ -70,18 +70,15 @@ int device_pm_add(struct device *dev)
mutex_lock(&dpm_list_mtx);
if ((dev->parent && dev->parent->power.sleeping) || all_sleeping) {
if (dev->parent->power.sleeping)
- dev_warn(dev,
- "parent %s is sleeping, will not add\n",
+ dev_warn(dev, "parent %s is sleeping\n",
dev->parent->bus_id);
else
- dev_warn(dev, "devices are sleeping, will not add\n");
+ dev_warn(dev, "all devices are sleeping\n");
WARN_ON(true);
- error = -EBUSY;
- } else {
- error = dpm_sysfs_add(dev);
- if (!error)
- list_add_tail(&dev->power.entry, &dpm_active);
}
+ error = dpm_sysfs_add(dev);
+ if (!error)
+ list_add_tail(&dev->power.entry, &dpm_active);
mutex_unlock(&dpm_list_mtx);
return error;
}
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
