Re: [PATCH 1/4] PM: Introduce destroy_suspended_device()
On Saturday, 5 of January 2008, Alan Stern wrote:
> On Fri, 4 Jan 2008, Rafael J. Wysocki wrote:
>
> > I have rebased
> > gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch
> > on top of the $subject series, the result is appended. It has only been
> > compilation tested for now, but I'll be testing it for the next couple of
> > days.
> >
> > Please review.
>
> I would prefer it if you could also merge in this patch at the same
> time:
>
> https://lists.linux-foundation.org/pipermail/linux-pm/2007-December/015921.html
Makes sense, I will.
> > +void device_resume(void)
> > {
> > - sysdev_resume();
> > - dpm_power_up();
> > + might_sleep();
> > + dpm_resume();
> > + unlock_all_devices();
> > + unregister_dropped_devices();
> > + up_write(_sleep_rwsem);
> > }
>
> With the aforementioned patch merged in, this will generate a
> warning for each dropped device. The call to
> unregister_dropped_devices() should come after the up_write().
>
> You might also consider adding a call to unregister_dropped_devices()
> in the error path of device_suspend() -- in theory even an aborted
> suspend might cause a device to malfunction.
In fact it already works like this, since device_suspend() now calls the entire
device_resume() on error.
> Otherwise this looks okay.
However, I think we don't need to wait with unregistering suspended devices
until after the other ones are resumed. We only need a special function for
unregistering suspended devices that will make the PM core release the device's
semaphore before unregistering it.
I have already sent a replacemet for
gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch that includes
some code from the $subject patch and implements the above idea:
http://lkml.org/lkml/2008/1/4/278
I'm going to merge it with your patch at:
https://lists.linux-foundation.org/pipermail/linux-pm/2007-December/015921.html
and with patches [2/4] and [4/4] from the $subject series. I'll post the
result for a review later today.
Thanks,
Rafael
--
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/
Re: [PATCH 1/4] PM: Introduce destroy_suspended_device()
On Saturday, 5 of January 2008, Alan Stern wrote:
On Fri, 4 Jan 2008, Rafael J. Wysocki wrote:
I have rebased
gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch
on top of the $subject series, the result is appended. It has only been
compilation tested for now, but I'll be testing it for the next couple of
days.
Please review.
I would prefer it if you could also merge in this patch at the same
time:
https://lists.linux-foundation.org/pipermail/linux-pm/2007-December/015921.html
Makes sense, I will.
+void device_resume(void)
{
- sysdev_resume();
- dpm_power_up();
+ might_sleep();
+ dpm_resume();
+ unlock_all_devices();
+ unregister_dropped_devices();
+ up_write(pm_sleep_rwsem);
}
With the aforementioned patch merged in, this will generate a
warning for each dropped device. The call to
unregister_dropped_devices() should come after the up_write().
You might also consider adding a call to unregister_dropped_devices()
in the error path of device_suspend() -- in theory even an aborted
suspend might cause a device to malfunction.
In fact it already works like this, since device_suspend() now calls the entire
device_resume() on error.
Otherwise this looks okay.
However, I think we don't need to wait with unregistering suspended devices
until after the other ones are resumed. We only need a special function for
unregistering suspended devices that will make the PM core release the device's
semaphore before unregistering it.
I have already sent a replacemet for
gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch that includes
some code from the $subject patch and implements the above idea:
http://lkml.org/lkml/2008/1/4/278
I'm going to merge it with your patch at:
https://lists.linux-foundation.org/pipermail/linux-pm/2007-December/015921.html
and with patches [2/4] and [4/4] from the $subject series. I'll post the
result for a review later today.
Thanks,
Rafael
--
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/
Re: [PATCH 1/4] PM: Introduce destroy_suspended_device()
On Fri, 4 Jan 2008, Rafael J. Wysocki wrote:
> I have rebased gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch
> on top of the $subject series, the result is appended. It has only been
> compilation tested for now, but I'll be testing it for the next couple of
> days.
>
> Please review.
I would prefer it if you could also merge in this patch at the same
time:
https://lists.linux-foundation.org/pipermail/linux-pm/2007-December/015921.html
> +void device_resume(void)
> {
> - sysdev_resume();
> - dpm_power_up();
> + might_sleep();
> + dpm_resume();
> + unlock_all_devices();
> + unregister_dropped_devices();
> + up_write(_sleep_rwsem);
> }
With the aforementioned patch merged in, this will generate a
warning for each dropped device. The call to
unregister_dropped_devices() should come after the up_write().
You might also consider adding a call to unregister_dropped_devices()
in the error path of device_suspend() -- in theory even an aborted
suspend might cause a device to malfunction.
Otherwise this looks okay.
Alan Stern
--
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/
[RFC][PATCH] PM: Acquire device locks on suspend (was: Re: [PATCH 1/4] PM: Introduce destroy_suspended_device())
On Friday, 4 of January 2008, Rafael J. Wysocki wrote:
> On Wednesday, 2 of January 2008, Rafael J. Wysocki wrote:
> > On Wednesday, 2 of January 2008, Alan Stern wrote:
> > > On Wed, 2 Jan 2008, Rafael J. Wysocki wrote:
> > >
> > > > On Wednesday, 2 of January 2008, Rafael J. Wysocki wrote:
> > > > > From: Rafael J. Wysocki <[EMAIL PROTECTED]>
> > > > >
> > > > > It sometimes is necessary to destroy a device object during a suspend
> > > > > or
> > > > > hibernation, but the PM core is supposed to control all device
> > > > > objects in that
> > > > > cases. For this reason, it is necessary to introduce a mechanism
> > > > > allowing one
> > > > > to ask the PM core to remove a device object corresponding to a
> > > > > suspended
> > > > > device on one's behalf.
> > > > >
> > > > > Define function destroy_suspended_device() that will schedule the
> > > > > removal of
> > > > > a device object corresponding to a suspended device by the PM core
> > > > > during the
> > > > > subsequent resume.
> > > > >
> > > > > Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
> > > >
> > > > Sorry, a small fix is needed for this patch. Namely,
> > > > dpm_sysfs_remove(dev)
> > > > should not be called by device_pm_schedule_removal(), because it will
> > > > be called
> > > > anyway from device_pm_remove() when the device object is finally
> > > > unregistered
> > > > (we're talking here about unlikely error paths only, but still).
> > >
> > > The situation is a little confusing, because the source files under
> > > drivers/base/power are maintained in Greg's tree and he already has
> > > gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch
> > > installed. That patch conflicts with this one.
> > >
> > > One of the these two patches will have to be rewritten to apply on top
> > > of the other. Which do you think should be changed?
> >
> > Well, from the bisectability point of view, it would be better to adjust
> > gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch and let the
> > $subject patch series go first, if you don't mind.
>
> I have rebased gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch
> on top of the $subject series, the result is appended. It has only been
> compilation tested for now, but I'll be testing it for the next couple of
> days.
>
> Please review.
Actually, please scratch that. We can do better.
The appended patch (compilation tested) combines patch [1/4] with the
gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch in such a way
that it is now possible to remove a suspended device at once via the PM core
using the destroy_suspended_device() callback.
Thus, the drivers that need to remove device objects while suspended (eg.
from the CPU hotplug notifiers) can use destroy_suspended_device() for this
purpose without deadlocking, but using device_destroy() to remove suspended
devices is prohibited.
Please review.
Thanks,
Rafael
---
From: Alan Stern <[EMAIL PROTECTED]>, Rafael J. Wysocki <[EMAIL PROTECTED]>
This patch reorganizes the way suspend and resume notifications are
sent to drivers. The major changes are that now the PM core acquires
every device semaphore before calling the methods, and calls to
device_add() during suspends will fail.
It also provides a way to safely remove a suspended device with the help of
the PM core, by using the destroy_suspended_device() callback introduced
specifically for this purpose.
Not-yet-signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
---
drivers/base/core.c| 57 -
drivers/base/power/main.c | 452 ++---
drivers/base/power/power.h | 12 +
include/linux/device.h |8
4 files changed, 336 insertions(+), 193 deletions(-)
Index: linux-2.6/drivers/base/core.c
===
--- linux-2.6.orig/drivers/base/core.c
+++ linux-2.6/drivers/base/core.c
@@ -726,11 +726,17 @@ int device_add(struct device *dev)
{
struct device *parent = NULL;
struct class_interface *class_intf;
- int error = -EINVAL;
+ int error;
+
+ error = pm_sleep_lock();
+ if (error)
+ return error;
dev = get_device(dev);
- if (!dev || !strlen(dev->bus_id))
- goto Error;
+ if (!dev || !strlen(dev->bus_id)) {
+ error = -EINVAL;
+ goto Done;
+ }
pr_debug("DEV: registering device: ID = '%s'\n", dev->bus_id);
@@ -795,6 +801,7 @@ int device_add(struct device *dev)
}
Done:
put_device(dev);
+ pm_sleep_unlock();
return error;
BusError:
device_pm_remove(dev);
@@ -1156,14 +1163,11 @@ error:
EXPORT_SYMBOL_GPL(device_create);
/**
- * device_destroy - removes a device that was created with device_create()
+ * find_device - finds a device that was created with device_create()
* @class: pointer to the struct class that this
Re: [PATCH 1/4] PM: Introduce destroy_suspended_device()
On Wednesday, 2 of January 2008, Rafael J. Wysocki wrote:
> On Wednesday, 2 of January 2008, Alan Stern wrote:
> > On Wed, 2 Jan 2008, Rafael J. Wysocki wrote:
> >
> > > On Wednesday, 2 of January 2008, Rafael J. Wysocki wrote:
> > > > From: Rafael J. Wysocki <[EMAIL PROTECTED]>
> > > >
> > > > It sometimes is necessary to destroy a device object during a suspend or
> > > > hibernation, but the PM core is supposed to control all device objects
> > > > in that
> > > > cases. For this reason, it is necessary to introduce a mechanism
> > > > allowing one
> > > > to ask the PM core to remove a device object corresponding to a
> > > > suspended
> > > > device on one's behalf.
> > > >
> > > > Define function destroy_suspended_device() that will schedule the
> > > > removal of
> > > > a device object corresponding to a suspended device by the PM core
> > > > during the
> > > > subsequent resume.
> > > >
> > > > Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
> > >
> > > Sorry, a small fix is needed for this patch. Namely,
> > > dpm_sysfs_remove(dev)
> > > should not be called by device_pm_schedule_removal(), because it will be
> > > called
> > > anyway from device_pm_remove() when the device object is finally
> > > unregistered
> > > (we're talking here about unlikely error paths only, but still).
> >
> > The situation is a little confusing, because the source files under
> > drivers/base/power are maintained in Greg's tree and he already has
> > gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch
> > installed. That patch conflicts with this one.
> >
> > One of the these two patches will have to be rewritten to apply on top
> > of the other. Which do you think should be changed?
>
> Well, from the bisectability point of view, it would be better to adjust
> gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch and let the
> $subject patch series go first, if you don't mind.
I have rebased gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch
on top of the $subject series, the result is appended. It has only been
compilation tested for now, but I'll be testing it for the next couple of days.
Please review.
Thanks,
Rafael
---
From: Alan Stern <[EMAIL PROTECTED]>, Rafael J. Wysocki <[EMAIL PROTECTED]>
This patch reorganizes the way suspend and resume notifications are
sent to drivers. The major changes are that now the PM core acquires
every device semaphore before calling the methods, and calls to
device_add() during suspends will fail.
---
drivers/base/core.c| 13 -
drivers/base/power/main.c | 452 ++---
drivers/base/power/power.h | 11 +
3 files changed, 290 insertions(+), 186 deletions(-)
Index: linux-2.6/drivers/base/core.c
===
--- linux-2.6.orig/drivers/base/core.c
+++ linux-2.6/drivers/base/core.c
@@ -726,11 +726,17 @@ int device_add(struct device *dev)
{
struct device *parent = NULL;
struct class_interface *class_intf;
- int error = -EINVAL;
+ int error;
+
+ error = pm_sleep_lock();
+ if (error)
+ return error;
dev = get_device(dev);
- if (!dev || !strlen(dev->bus_id))
- goto Error;
+ if (!dev || !strlen(dev->bus_id)) {
+ error = -EINVAL;
+ goto Done;
+ }
pr_debug("DEV: registering device: ID = '%s'\n", dev->bus_id);
@@ -795,6 +801,7 @@ int device_add(struct device *dev)
}
Done:
put_device(dev);
+ pm_sleep_unlock();
return error;
BusError:
device_pm_remove(dev);
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
@@ -24,18 +24,39 @@
#include
#include
#include
+#include
#include "../base.h"
#include "power.h"
+/*
+ * The entries in the dpm_active list are in a depth first order, simply
+ * because children are guaranteed to be discovered after parents, and
+ * are inserted at the back of the list on discovery.
+ *
+ * All the other lists are kept in the same order, for consistency.
+ * However the lists aren't always traversed in the same order.
+ * Semaphores must be acquired from the top (i.e., front) down
+ * and released in the opposite order. Devices must be suspended
+ * from the bottom (i.e., end) up and resumed in the opposite order.
+ * That way no parent will be suspended while it still has an active
+ * child.
+ *
+ * Since device_pm_add() may be called with a device semaphore held,
+ * we must never try to acquire a device semaphore while holding
+ * dpm_list_mutex.
+ */
+
LIST_HEAD(dpm_active);
+static LIST_HEAD(dpm_locked);
static LIST_HEAD(dpm_off);
static LIST_HEAD(dpm_off_irq);
static LIST_HEAD(dpm_destroy);
-static DEFINE_MUTEX(dpm_mtx);
static
Re: [PATCH 1/4] PM: Introduce destroy_suspended_device()
On Wednesday, 2 of January 2008, Rafael J. Wysocki wrote:
On Wednesday, 2 of January 2008, Alan Stern wrote:
On Wed, 2 Jan 2008, Rafael J. Wysocki wrote:
On Wednesday, 2 of January 2008, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki [EMAIL PROTECTED]
It sometimes is necessary to destroy a device object during a suspend or
hibernation, but the PM core is supposed to control all device objects
in that
cases. For this reason, it is necessary to introduce a mechanism
allowing one
to ask the PM core to remove a device object corresponding to a
suspended
device on one's behalf.
Define function destroy_suspended_device() that will schedule the
removal of
a device object corresponding to a suspended device by the PM core
during the
subsequent resume.
Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED]
Sorry, a small fix is needed for this patch. Namely,
dpm_sysfs_remove(dev)
should not be called by device_pm_schedule_removal(), because it will be
called
anyway from device_pm_remove() when the device object is finally
unregistered
(we're talking here about unlikely error paths only, but still).
The situation is a little confusing, because the source files under
drivers/base/power are maintained in Greg's tree and he already has
gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch
installed. That patch conflicts with this one.
One of the these two patches will have to be rewritten to apply on top
of the other. Which do you think should be changed?
Well, from the bisectability point of view, it would be better to adjust
gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch and let the
$subject patch series go first, if you don't mind.
I have rebased gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch
on top of the $subject series, the result is appended. It has only been
compilation tested for now, but I'll be testing it for the next couple of days.
Please review.
Thanks,
Rafael
---
From: Alan Stern [EMAIL PROTECTED], Rafael J. Wysocki [EMAIL PROTECTED]
This patch reorganizes the way suspend and resume notifications are
sent to drivers. The major changes are that now the PM core acquires
every device semaphore before calling the methods, and calls to
device_add() during suspends will fail.
---
drivers/base/core.c| 13 -
drivers/base/power/main.c | 452 ++---
drivers/base/power/power.h | 11 +
3 files changed, 290 insertions(+), 186 deletions(-)
Index: linux-2.6/drivers/base/core.c
===
--- linux-2.6.orig/drivers/base/core.c
+++ linux-2.6/drivers/base/core.c
@@ -726,11 +726,17 @@ int device_add(struct device *dev)
{
struct device *parent = NULL;
struct class_interface *class_intf;
- int error = -EINVAL;
+ int error;
+
+ error = pm_sleep_lock();
+ if (error)
+ return error;
dev = get_device(dev);
- if (!dev || !strlen(dev-bus_id))
- goto Error;
+ if (!dev || !strlen(dev-bus_id)) {
+ error = -EINVAL;
+ goto Done;
+ }
pr_debug(DEV: registering device: ID = '%s'\n, dev-bus_id);
@@ -795,6 +801,7 @@ int device_add(struct device *dev)
}
Done:
put_device(dev);
+ pm_sleep_unlock();
return error;
BusError:
device_pm_remove(dev);
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
@@ -24,18 +24,39 @@
#include linux/mutex.h
#include linux/pm.h
#include linux/resume-trace.h
+#include linux/rwsem.h
#include ../base.h
#include power.h
+/*
+ * The entries in the dpm_active list are in a depth first order, simply
+ * because children are guaranteed to be discovered after parents, and
+ * are inserted at the back of the list on discovery.
+ *
+ * All the other lists are kept in the same order, for consistency.
+ * However the lists aren't always traversed in the same order.
+ * Semaphores must be acquired from the top (i.e., front) down
+ * and released in the opposite order. Devices must be suspended
+ * from the bottom (i.e., end) up and resumed in the opposite order.
+ * That way no parent will be suspended while it still has an active
+ * child.
+ *
+ * Since device_pm_add() may be called with a device semaphore held,
+ * we must never try to acquire a device semaphore while holding
+ * dpm_list_mutex.
+ */
+
LIST_HEAD(dpm_active);
+static LIST_HEAD(dpm_locked);
static LIST_HEAD(dpm_off);
static LIST_HEAD(dpm_off_irq);
static LIST_HEAD(dpm_destroy);
-static DEFINE_MUTEX(dpm_mtx);
static DEFINE_MUTEX(dpm_list_mtx);
+static DECLARE_RWSEM(pm_sleep_rwsem);
+
int (*platform_enable_wakeup)(struct
[RFC][PATCH] PM: Acquire device locks on suspend (was: Re: [PATCH 1/4] PM: Introduce destroy_suspended_device())
On Friday, 4 of January 2008, Rafael J. Wysocki wrote:
On Wednesday, 2 of January 2008, Rafael J. Wysocki wrote:
On Wednesday, 2 of January 2008, Alan Stern wrote:
On Wed, 2 Jan 2008, Rafael J. Wysocki wrote:
On Wednesday, 2 of January 2008, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki [EMAIL PROTECTED]
It sometimes is necessary to destroy a device object during a suspend
or
hibernation, but the PM core is supposed to control all device
objects in that
cases. For this reason, it is necessary to introduce a mechanism
allowing one
to ask the PM core to remove a device object corresponding to a
suspended
device on one's behalf.
Define function destroy_suspended_device() that will schedule the
removal of
a device object corresponding to a suspended device by the PM core
during the
subsequent resume.
Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED]
Sorry, a small fix is needed for this patch. Namely,
dpm_sysfs_remove(dev)
should not be called by device_pm_schedule_removal(), because it will
be called
anyway from device_pm_remove() when the device object is finally
unregistered
(we're talking here about unlikely error paths only, but still).
The situation is a little confusing, because the source files under
drivers/base/power are maintained in Greg's tree and he already has
gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch
installed. That patch conflicts with this one.
One of the these two patches will have to be rewritten to apply on top
of the other. Which do you think should be changed?
Well, from the bisectability point of view, it would be better to adjust
gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch and let the
$subject patch series go first, if you don't mind.
I have rebased gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch
on top of the $subject series, the result is appended. It has only been
compilation tested for now, but I'll be testing it for the next couple of
days.
Please review.
Actually, please scratch that. We can do better.
The appended patch (compilation tested) combines patch [1/4] with the
gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch in such a way
that it is now possible to remove a suspended device at once via the PM core
using the destroy_suspended_device() callback.
Thus, the drivers that need to remove device objects while suspended (eg.
from the CPU hotplug notifiers) can use destroy_suspended_device() for this
purpose without deadlocking, but using device_destroy() to remove suspended
devices is prohibited.
Please review.
Thanks,
Rafael
---
From: Alan Stern [EMAIL PROTECTED], Rafael J. Wysocki [EMAIL PROTECTED]
This patch reorganizes the way suspend and resume notifications are
sent to drivers. The major changes are that now the PM core acquires
every device semaphore before calling the methods, and calls to
device_add() during suspends will fail.
It also provides a way to safely remove a suspended device with the help of
the PM core, by using the destroy_suspended_device() callback introduced
specifically for this purpose.
Not-yet-signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED]
---
drivers/base/core.c| 57 -
drivers/base/power/main.c | 452 ++---
drivers/base/power/power.h | 12 +
include/linux/device.h |8
4 files changed, 336 insertions(+), 193 deletions(-)
Index: linux-2.6/drivers/base/core.c
===
--- linux-2.6.orig/drivers/base/core.c
+++ linux-2.6/drivers/base/core.c
@@ -726,11 +726,17 @@ int device_add(struct device *dev)
{
struct device *parent = NULL;
struct class_interface *class_intf;
- int error = -EINVAL;
+ int error;
+
+ error = pm_sleep_lock();
+ if (error)
+ return error;
dev = get_device(dev);
- if (!dev || !strlen(dev-bus_id))
- goto Error;
+ if (!dev || !strlen(dev-bus_id)) {
+ error = -EINVAL;
+ goto Done;
+ }
pr_debug(DEV: registering device: ID = '%s'\n, dev-bus_id);
@@ -795,6 +801,7 @@ int device_add(struct device *dev)
}
Done:
put_device(dev);
+ pm_sleep_unlock();
return error;
BusError:
device_pm_remove(dev);
@@ -1156,14 +1163,11 @@ error:
EXPORT_SYMBOL_GPL(device_create);
/**
- * device_destroy - removes a device that was created with device_create()
+ * find_device - finds a device that was created with device_create()
* @class: pointer to the struct class that this device was registered with
* @devt: the dev_t of the device that was previously registered
- *
- * This call unregisters and cleans up a device that was created with a
- * call to
Re: [PATCH 1/4] PM: Introduce destroy_suspended_device()
On Fri, 4 Jan 2008, Rafael J. Wysocki wrote:
I have rebased gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch
on top of the $subject series, the result is appended. It has only been
compilation tested for now, but I'll be testing it for the next couple of
days.
Please review.
I would prefer it if you could also merge in this patch at the same
time:
https://lists.linux-foundation.org/pipermail/linux-pm/2007-December/015921.html
+void device_resume(void)
{
- sysdev_resume();
- dpm_power_up();
+ might_sleep();
+ dpm_resume();
+ unlock_all_devices();
+ unregister_dropped_devices();
+ up_write(pm_sleep_rwsem);
}
With the aforementioned patch merged in, this will generate a
warning for each dropped device. The call to
unregister_dropped_devices() should come after the up_write().
You might also consider adding a call to unregister_dropped_devices()
in the error path of device_suspend() -- in theory even an aborted
suspend might cause a device to malfunction.
Otherwise this looks okay.
Alan Stern
--
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/
Re: [PATCH 1/4] PM: Introduce destroy_suspended_device()
On Wednesday, 2 of January 2008, Alan Stern wrote: > On Wed, 2 Jan 2008, Rafael J. Wysocki wrote: > > > On Wednesday, 2 of January 2008, Rafael J. Wysocki wrote: > > > From: Rafael J. Wysocki <[EMAIL PROTECTED]> > > > > > > It sometimes is necessary to destroy a device object during a suspend or > > > hibernation, but the PM core is supposed to control all device objects in > > > that > > > cases. For this reason, it is necessary to introduce a mechanism > > > allowing one > > > to ask the PM core to remove a device object corresponding to a suspended > > > device on one's behalf. > > > > > > Define function destroy_suspended_device() that will schedule the removal > > > of > > > a device object corresponding to a suspended device by the PM core during > > > the > > > subsequent resume. > > > > > > Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]> > > > > Sorry, a small fix is needed for this patch. Namely, dpm_sysfs_remove(dev) > > should not be called by device_pm_schedule_removal(), because it will be > > called > > anyway from device_pm_remove() when the device object is finally > > unregistered > > (we're talking here about unlikely error paths only, but still). > > The situation is a little confusing, because the source files under > drivers/base/power are maintained in Greg's tree and he already has > gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch > installed. That patch conflicts with this one. > > One of the these two patches will have to be rewritten to apply on top > of the other. Which do you think should be changed? Well, from the bisectability point of view, it would be better to adjust gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch and let the $subject patch series go first, if you don't mind. Rafael -- 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/
Re: [PATCH 1/4] PM: Introduce destroy_suspended_device()
On Wed, 2 Jan 2008, Rafael J. Wysocki wrote: > On Wednesday, 2 of January 2008, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <[EMAIL PROTECTED]> > > > > It sometimes is necessary to destroy a device object during a suspend or > > hibernation, but the PM core is supposed to control all device objects in > > that > > cases. For this reason, it is necessary to introduce a mechanism allowing > > one > > to ask the PM core to remove a device object corresponding to a suspended > > device on one's behalf. > > > > Define function destroy_suspended_device() that will schedule the removal of > > a device object corresponding to a suspended device by the PM core during > > the > > subsequent resume. > > > > Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]> > > Sorry, a small fix is needed for this patch. Namely, dpm_sysfs_remove(dev) > should not be called by device_pm_schedule_removal(), because it will be > called > anyway from device_pm_remove() when the device object is finally unregistered > (we're talking here about unlikely error paths only, but still). The situation is a little confusing, because the source files under drivers/base/power are maintained in Greg's tree and he already has gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch installed. That patch conflicts with this one. One of the these two patches will have to be rewritten to apply on top of the other. Which do you think should be changed? Alan Stern -- 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/
Re: [PATCH 1/4] PM: Introduce destroy_suspended_device()
On Wednesday, 2 of January 2008, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[EMAIL PROTECTED]>
>
> It sometimes is necessary to destroy a device object during a suspend or
> hibernation, but the PM core is supposed to control all device objects in that
> cases. For this reason, it is necessary to introduce a mechanism allowing one
> to ask the PM core to remove a device object corresponding to a suspended
> device on one's behalf.
>
> Define function destroy_suspended_device() that will schedule the removal of
> a device object corresponding to a suspended device by the PM core during the
> subsequent resume.
>
> Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
Sorry, a small fix is needed for this patch. Namely, dpm_sysfs_remove(dev)
should not be called by device_pm_schedule_removal(), because it will be called
anyway from device_pm_remove() when the device object is finally unregistered
(we're talking here about unlikely error paths only, but still).
Corrected patch follows.
Thanks,
Rafael
---
From: Rafael J. Wysocki <[EMAIL PROTECTED]>
It sometimes is necessary to destroy a device object during a suspend or
hibernation, but the PM core is supposed to control all device objects in that
cases. For this reason, it is necessary to introduce a mechanism allowing one
to ask the PM core to remove a device object corresponding to a suspended
device on one's behalf.
Define function destroy_suspended_device() that will schedule the removal of
a device object corresponding to a suspended device by the PM core during the
subsequent resume.
Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
---
drivers/base/core.c| 44 +++-
drivers/base/power/main.c | 35 ++-
drivers/base/power/power.h |1 +
include/linux/device.h |8
4 files changed, 74 insertions(+), 14 deletions(-)
Index: linux-2.6/drivers/base/core.c
===
--- linux-2.6.orig/drivers/base/core.c
+++ linux-2.6/drivers/base/core.c
@@ -1156,14 +1156,11 @@ error:
EXPORT_SYMBOL_GPL(device_create);
/**
- * device_destroy - removes a device that was created with device_create()
+ * find_device - finds a device that was created with device_create()
* @class: pointer to the struct class that this device was registered with
* @devt: the dev_t of the device that was previously registered
- *
- * This call unregisters and cleans up a device that was created with a
- * call to device_create().
*/
-void device_destroy(struct class *class, dev_t devt)
+static struct device *find_device(struct class *class, dev_t devt)
{
struct device *dev = NULL;
struct device *dev_tmp;
@@ -1176,12 +1173,49 @@ void device_destroy(struct class *class,
}
}
up(>sem);
+ return dev;
+}
+/**
+ * device_destroy - removes a device that was created with device_create()
+ * @class: pointer to the struct class that this device was registered with
+ * @devt: the dev_t of the device that was previously registered
+ *
+ * This call unregisters and cleans up a device that was created with a
+ * call to device_create().
+ */
+void device_destroy(struct class *class, dev_t devt)
+{
+ struct device *dev;
+
+ dev = find_device(class, devt);
if (dev)
device_unregister(dev);
}
EXPORT_SYMBOL_GPL(device_destroy);
+#ifdef CONFIG_PM_SLEEP
+/**
+ * destroy_suspended_device - schedules the removal of a suspended device
+ * @class: pointer to the struct class that this device was registered with
+ * @devt: the dev_t of the device that was previously registered
+ *
+ * This call notifies the PM core of the necessity to unregister a suspended
+ * device created with a call to device_create() (devices cannot be
+ * unregistered directly while suspended, since the PM core controls them in
+ * that cases).
+ */
+void destroy_suspended_device(struct class *class, dev_t devt)
+{
+ struct device *dev;
+
+ dev = find_device(class, devt);
+ if (dev)
+ device_pm_schedule_removal(dev);
+}
+EXPORT_SYMBOL_GPL(destroy_suspended_device);
+#endif /* CONFIG_PM_SLEEP */
+
/**
* device_rename - renames a device
* @dev: the pointer to the struct device to be renamed
Index: linux-2.6/include/linux/device.h
===
--- linux-2.6.orig/include/linux/device.h
+++ linux-2.6/include/linux/device.h
@@ -521,6 +521,14 @@ extern struct device *device_create(stru
dev_t devt, const char *fmt, ...)
__attribute__((format(printf,4,5)));
extern void device_destroy(struct class *cls, dev_t devt);
+#ifdef CONFIG_PM_SLEEP
+extern void destroy_suspended_device(struct class *cls, dev_t devt);
+#else /* !CONFIG_PM_SLEEP */
+static inline void destroy_suspended_device(struct class *cls, dev_t devt)
Re: [PATCH 1/4] PM: Introduce destroy_suspended_device()
On Wednesday, 2 of January 2008, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki [EMAIL PROTECTED]
It sometimes is necessary to destroy a device object during a suspend or
hibernation, but the PM core is supposed to control all device objects in that
cases. For this reason, it is necessary to introduce a mechanism allowing one
to ask the PM core to remove a device object corresponding to a suspended
device on one's behalf.
Define function destroy_suspended_device() that will schedule the removal of
a device object corresponding to a suspended device by the PM core during the
subsequent resume.
Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED]
Sorry, a small fix is needed for this patch. Namely, dpm_sysfs_remove(dev)
should not be called by device_pm_schedule_removal(), because it will be called
anyway from device_pm_remove() when the device object is finally unregistered
(we're talking here about unlikely error paths only, but still).
Corrected patch follows.
Thanks,
Rafael
---
From: Rafael J. Wysocki [EMAIL PROTECTED]
It sometimes is necessary to destroy a device object during a suspend or
hibernation, but the PM core is supposed to control all device objects in that
cases. For this reason, it is necessary to introduce a mechanism allowing one
to ask the PM core to remove a device object corresponding to a suspended
device on one's behalf.
Define function destroy_suspended_device() that will schedule the removal of
a device object corresponding to a suspended device by the PM core during the
subsequent resume.
Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED]
---
drivers/base/core.c| 44 +++-
drivers/base/power/main.c | 35 ++-
drivers/base/power/power.h |1 +
include/linux/device.h |8
4 files changed, 74 insertions(+), 14 deletions(-)
Index: linux-2.6/drivers/base/core.c
===
--- linux-2.6.orig/drivers/base/core.c
+++ linux-2.6/drivers/base/core.c
@@ -1156,14 +1156,11 @@ error:
EXPORT_SYMBOL_GPL(device_create);
/**
- * device_destroy - removes a device that was created with device_create()
+ * find_device - finds a device that was created with device_create()
* @class: pointer to the struct class that this device was registered with
* @devt: the dev_t of the device that was previously registered
- *
- * This call unregisters and cleans up a device that was created with a
- * call to device_create().
*/
-void device_destroy(struct class *class, dev_t devt)
+static struct device *find_device(struct class *class, dev_t devt)
{
struct device *dev = NULL;
struct device *dev_tmp;
@@ -1176,12 +1173,49 @@ void device_destroy(struct class *class,
}
}
up(class-sem);
+ return dev;
+}
+/**
+ * device_destroy - removes a device that was created with device_create()
+ * @class: pointer to the struct class that this device was registered with
+ * @devt: the dev_t of the device that was previously registered
+ *
+ * This call unregisters and cleans up a device that was created with a
+ * call to device_create().
+ */
+void device_destroy(struct class *class, dev_t devt)
+{
+ struct device *dev;
+
+ dev = find_device(class, devt);
if (dev)
device_unregister(dev);
}
EXPORT_SYMBOL_GPL(device_destroy);
+#ifdef CONFIG_PM_SLEEP
+/**
+ * destroy_suspended_device - schedules the removal of a suspended device
+ * @class: pointer to the struct class that this device was registered with
+ * @devt: the dev_t of the device that was previously registered
+ *
+ * This call notifies the PM core of the necessity to unregister a suspended
+ * device created with a call to device_create() (devices cannot be
+ * unregistered directly while suspended, since the PM core controls them in
+ * that cases).
+ */
+void destroy_suspended_device(struct class *class, dev_t devt)
+{
+ struct device *dev;
+
+ dev = find_device(class, devt);
+ if (dev)
+ device_pm_schedule_removal(dev);
+}
+EXPORT_SYMBOL_GPL(destroy_suspended_device);
+#endif /* CONFIG_PM_SLEEP */
+
/**
* device_rename - renames a device
* @dev: the pointer to the struct device to be renamed
Index: linux-2.6/include/linux/device.h
===
--- linux-2.6.orig/include/linux/device.h
+++ linux-2.6/include/linux/device.h
@@ -521,6 +521,14 @@ extern struct device *device_create(stru
dev_t devt, const char *fmt, ...)
__attribute__((format(printf,4,5)));
extern void device_destroy(struct class *cls, dev_t devt);
+#ifdef CONFIG_PM_SLEEP
+extern void destroy_suspended_device(struct class *cls, dev_t devt);
+#else /* !CONFIG_PM_SLEEP */
+static inline void destroy_suspended_device(struct class *cls, dev_t devt)
+{
+
Re: [PATCH 1/4] PM: Introduce destroy_suspended_device()
On Wed, 2 Jan 2008, Rafael J. Wysocki wrote: On Wednesday, 2 of January 2008, Rafael J. Wysocki wrote: From: Rafael J. Wysocki [EMAIL PROTECTED] It sometimes is necessary to destroy a device object during a suspend or hibernation, but the PM core is supposed to control all device objects in that cases. For this reason, it is necessary to introduce a mechanism allowing one to ask the PM core to remove a device object corresponding to a suspended device on one's behalf. Define function destroy_suspended_device() that will schedule the removal of a device object corresponding to a suspended device by the PM core during the subsequent resume. Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED] Sorry, a small fix is needed for this patch. Namely, dpm_sysfs_remove(dev) should not be called by device_pm_schedule_removal(), because it will be called anyway from device_pm_remove() when the device object is finally unregistered (we're talking here about unlikely error paths only, but still). The situation is a little confusing, because the source files under drivers/base/power are maintained in Greg's tree and he already has gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch installed. That patch conflicts with this one. One of the these two patches will have to be rewritten to apply on top of the other. Which do you think should be changed? Alan Stern -- 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/
Re: [PATCH 1/4] PM: Introduce destroy_suspended_device()
On Wednesday, 2 of January 2008, Alan Stern wrote: On Wed, 2 Jan 2008, Rafael J. Wysocki wrote: On Wednesday, 2 of January 2008, Rafael J. Wysocki wrote: From: Rafael J. Wysocki [EMAIL PROTECTED] It sometimes is necessary to destroy a device object during a suspend or hibernation, but the PM core is supposed to control all device objects in that cases. For this reason, it is necessary to introduce a mechanism allowing one to ask the PM core to remove a device object corresponding to a suspended device on one's behalf. Define function destroy_suspended_device() that will schedule the removal of a device object corresponding to a suspended device by the PM core during the subsequent resume. Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED] Sorry, a small fix is needed for this patch. Namely, dpm_sysfs_remove(dev) should not be called by device_pm_schedule_removal(), because it will be called anyway from device_pm_remove() when the device object is finally unregistered (we're talking here about unlikely error paths only, but still). The situation is a little confusing, because the source files under drivers/base/power are maintained in Greg's tree and he already has gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch installed. That patch conflicts with this one. One of the these two patches will have to be rewritten to apply on top of the other. Which do you think should be changed? Well, from the bisectability point of view, it would be better to adjust gregkh-driver-pm-acquire-device-locks-prior-to-suspending.patch and let the $subject patch series go first, if you don't mind. Rafael -- 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/
