On Saturday, 23 of February 2008, Greg KH wrote:
> On Sat, Feb 23, 2008 at 12:53:11AM +0100, Rafael J. Wysocki wrote:
> > Hi Greg,
> >
> > The appended patch fixes the issue with the new code for suspending/resuming
> > devices, related to the fact that some device drivers and CPU hotplug notifiers
> > unregister device objects while suspend is in progress, which leads to
> > deadlocks.
> >
> > Please consider taking it for 2.6.25.
> >
> > Thanks,
> > Rafael
> >
> > ---
> > From: Rafael J. Wysocki <rjw@xxxxxxx>
> >
> > Introduce a mechanism preventing drivers and CPU hotplug notifiers
> > from deadlocking suspend/hibernation by unregistering device objects
> > while it is in progress. Specifically, make device_del() detect if
> > it has been called by the suspending task and automatically defer the
> > removal of the device object if that's the case.
> >
> > Signed-off-by: Rafael J. Wysocki <rjw@xxxxxxx>
> > Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> > ---
> > drivers/base/core.c | 5 +++++
> > drivers/base/power/main.c | 9 +++++++++
> > drivers/base/power/power.h | 5 +++++
> > 3 files changed, 19 insertions(+)
> >
> > 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
> > @@ -59,6 +59,13 @@ static DECLARE_RWSEM(pm_sleep_rwsem);
> >
> > int (*platform_enable_wakeup)(struct device *dev, int is_on);
> >
> > +static struct task_struct *suspending_task;
> > +
> > +bool in_suspend_context(void)
> > +{
> > + return (suspending_task == current);
> > +}
> > +
> > /**
> > * device_pm_add - add a device to the list of active devices
> > * @dev: Device to be added to the list
> > @@ -272,6 +279,7 @@ static void dpm_resume(void)
> > mutex_lock(&dpm_list_mtx);
> > }
> > mutex_unlock(&dpm_list_mtx);
> > + suspending_task = NULL;
> > }
> >
> > /**
> > @@ -460,6 +468,7 @@ static int dpm_suspend(pm_message_t stat
> > {
> > int error = 0;
> >
> > + suspending_task = current;
> > mutex_lock(&dpm_list_mtx);
> > while (!list_empty(&dpm_locked)) {
> > struct list_head *entry = dpm_locked.prev;
> > Index: linux-2.6/drivers/base/core.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/base/core.c
> > +++ linux-2.6/drivers/base/core.c
> > @@ -929,6 +929,11 @@ void device_del(struct device *dev)
> > struct device *parent = dev->parent;
> > struct class_interface *class_intf;
> >
> > + if (in_suspend_context()) {
> > + get_device(dev);
> > + device_pm_schedule_removal(dev);
> > + return;
> > + }
>
> Why are you grabbing an additional reference to the device here? That
> would seem to get out of balance when the device is later scheduled for
> removal, right?
No, IMO the reference is necessary, because unregister_dropped_devices() uses
device_unregister() that does the put_device() eventually.
If we are called by device_unregister(), the get_device() is needed to balance
the put_device() that will be called by device_unregister() after we return.
OTOH, if we are called directly, then we need to balance the put_device()
that will be done by device_unregister() called from
unregister_dropped_devices().
Thanks,
Rafael
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
