* [PATCH] PM: Prevent dpm_prepare() from returning errors unnecessarily
@ 2010-11-28 12:12 Rafael J. Wysocki
2010-11-28 15:35 ` Alan Stern
2010-11-30 13:07 ` Ming Lei
0 siblings, 2 replies; 13+ messages in thread
From: Rafael J. Wysocki @ 2010-11-28 12:12 UTC (permalink / raw)
To: Linux-pm mailing list; +Cc: LKML, Alan Stern
From: Rafael J. Wysocki <rjw@sisk.pl>
Currently dpm_prepare() returns error code if it finds that a device
being suspended has a pending runtime resume request. However, it
should not do that if the checking for wakeup events is not enabled.
On the other hand, if the checking for wakeup events is enabled, it
can return error when a wakeup event is detected, regardless of its
source.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
drivers/base/power/main.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 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
@@ -26,6 +26,7 @@
#include <linux/interrupt.h>
#include <linux/sched.h>
#include <linux/async.h>
+#include <linux/suspend.h>
#include "../base.h"
#include "power.h"
@@ -1052,8 +1053,10 @@ static int dpm_prepare(pm_message_t stat
mutex_unlock(&dpm_list_mtx);
pm_runtime_get_noresume(dev);
- if (pm_runtime_barrier(dev) && device_may_wakeup(dev)) {
- /* Wake-up requested during system sleep transition. */
+ if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
+ pm_wakeup_event(dev, 0);
+
+ if (pm_check_wakeup_events()) {
pm_runtime_put_sync(dev);
error = -EBUSY;
} else {
@@ -1068,8 +1071,8 @@ static int dpm_prepare(pm_message_t stat
error = 0;
continue;
}
- printk(KERN_ERR "PM: Failed to prepare device %s "
- "for power transition: error %d\n",
+ printk(KERN_INFO "PM: Device %s not prepared "
+ "for power transition: code %d\n",
kobject_name(&dev->kobj), error);
put_device(dev);
break;
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] PM: Prevent dpm_prepare() from returning errors unnecessarily 2010-11-28 12:12 [PATCH] PM: Prevent dpm_prepare() from returning errors unnecessarily Rafael J. Wysocki @ 2010-11-28 15:35 ` Alan Stern 2010-11-28 22:52 ` Rafael J. Wysocki 2010-11-30 13:07 ` Ming Lei 1 sibling, 1 reply; 13+ messages in thread From: Alan Stern @ 2010-11-28 15:35 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Linux-pm mailing list, LKML On Sun, 28 Nov 2010, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rjw@sisk.pl> > > Currently dpm_prepare() returns error code if it finds that a device > being suspended has a pending runtime resume request. However, it > should not do that if the checking for wakeup events is not enabled. It doesn't. The line you changed _does_ check device_may_wakeup(). > On the other hand, if the checking for wakeup events is enabled, it > can return error when a wakeup event is detected, regardless of its > source. Will adding this call to pm_wakeup_event() end up double-counting some events? Alan Stern ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PM: Prevent dpm_prepare() from returning errors unnecessarily 2010-11-28 15:35 ` Alan Stern @ 2010-11-28 22:52 ` Rafael J. Wysocki 2010-11-29 3:05 ` Alan Stern 0 siblings, 1 reply; 13+ messages in thread From: Rafael J. Wysocki @ 2010-11-28 22:52 UTC (permalink / raw) To: Alan Stern; +Cc: Linux-pm mailing list, LKML On Sunday, November 28, 2010, Alan Stern wrote: > On Sun, 28 Nov 2010, Rafael J. Wysocki wrote: > > > From: Rafael J. Wysocki <rjw@sisk.pl> > > > > Currently dpm_prepare() returns error code if it finds that a device > > being suspended has a pending runtime resume request. However, it > > should not do that if the checking for wakeup events is not enabled. > > It doesn't. The line you changed _does_ check device_may_wakeup(). That's not the point. The problem is that it shouldn't abort suspend when events_check_enabled is unset. > > On the other hand, if the checking for wakeup events is enabled, it > > can return error when a wakeup event is detected, regardless of its > > source. > > Will adding this call to pm_wakeup_event() end up double-counting some > events? Yes, it will, if the event has already been reported by the subsystem or driver. I don't think it's a very big issue and I'm not sure trying to avoid it is worth the effort (we can check if the device's wakeup source object is active and skip reporting the wakeup event in that case, but that doesn't guarantee that the event won't be counted twice anyway). Thanks, Rafael ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PM: Prevent dpm_prepare() from returning errors unnecessarily 2010-11-28 22:52 ` Rafael J. Wysocki @ 2010-11-29 3:05 ` Alan Stern 2010-11-29 22:04 ` Rafael J. Wysocki 0 siblings, 1 reply; 13+ messages in thread From: Alan Stern @ 2010-11-29 3:05 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Linux-pm mailing list, LKML On Sun, 28 Nov 2010, Rafael J. Wysocki wrote: > On Sunday, November 28, 2010, Alan Stern wrote: > > On Sun, 28 Nov 2010, Rafael J. Wysocki wrote: > > > > > From: Rafael J. Wysocki <rjw@sisk.pl> > > > > > > Currently dpm_prepare() returns error code if it finds that a device > > > being suspended has a pending runtime resume request. However, it > > > should not do that if the checking for wakeup events is not enabled. > > > > It doesn't. The line you changed _does_ check device_may_wakeup(). > > That's not the point. The problem is that it shouldn't abort suspend > when events_check_enabled is unset. Oh, I see. This is a tricky issue. Every driver for a device that can have wakeup-enabled children needs to worry about the race between suspending the device and receiving a wakeup request from a child. For example, in drivers/usb/core/hcd-pci.c, the suspend_common() routine goes out of its way to return -EBUSY if device_may_wakeup() is true and the controller's root hub has a pending wakeup request. How should drivers handle this in general? Should we make an effort to convert them to use the wakeup framework so they they can let the PM core take care of these races? Do we have to consider similar races during runtime suspend? > > > On the other hand, if the checking for wakeup events is enabled, it > > > can return error when a wakeup event is detected, regardless of its > > > source. > > > > Will adding this call to pm_wakeup_event() end up double-counting some > > events? > > Yes, it will, if the event has already been reported by the subsystem or driver. > > I don't think it's a very big issue and I'm not sure trying to avoid it is > worth the effort (we can check if the device's wakeup source object is active > and skip reporting the wakeup event in that case, but that doesn't guarantee > that the event won't be counted twice anyway). I agree that it's not a big issue. Wakeups reported twice because they occur just before a system sleep won't cause serious accounting problems and probably won't happen very often anyway. I just wanted to make sure that the issue wasn't being ignored by mistake. Alan Stern ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PM: Prevent dpm_prepare() from returning errors unnecessarily 2010-11-29 3:05 ` Alan Stern @ 2010-11-29 22:04 ` Rafael J. Wysocki 2010-11-30 15:13 ` Alan Stern 0 siblings, 1 reply; 13+ messages in thread From: Rafael J. Wysocki @ 2010-11-29 22:04 UTC (permalink / raw) To: Alan Stern; +Cc: Linux-pm mailing list, LKML On Monday, November 29, 2010, Alan Stern wrote: > On Sun, 28 Nov 2010, Rafael J. Wysocki wrote: > > > On Sunday, November 28, 2010, Alan Stern wrote: > > > On Sun, 28 Nov 2010, Rafael J. Wysocki wrote: > > > > > > > From: Rafael J. Wysocki <rjw@sisk.pl> > > > > > > > > Currently dpm_prepare() returns error code if it finds that a device > > > > being suspended has a pending runtime resume request. However, it > > > > should not do that if the checking for wakeup events is not enabled. > > > > > > It doesn't. The line you changed _does_ check device_may_wakeup(). > > > > That's not the point. The problem is that it shouldn't abort suspend > > when events_check_enabled is unset. > > Oh, I see. This is a tricky issue. Every driver for a device that can > have wakeup-enabled children needs to worry about the race between > suspending the device and receiving a wakeup request from a child. > For example, in drivers/usb/core/hcd-pci.c, the suspend_common() > routine goes out of its way to return -EBUSY if device_may_wakeup() is > true and the controller's root hub has a pending wakeup request. > > How should drivers handle this in general? Should we make an effort to > convert them to use the wakeup framework so they they can let the PM > core take care of these races? I think so. We also need to put a pm_check_wakeup_events() check into dpm_suspend() IMO, so that we abort the suspending of devices as soon as a wakeup event is reported. > Do we have to consider similar races during runtime suspend? Ideally, yes, but I'm not sure if that's generally possible. IMO, it won't be a big deal if a parent device is suspended and immediately resumed occasionally due to a pending wakeup signal from one of its children. It may be a problem if that happens too often, though. > > > > On the other hand, if the checking for wakeup events is enabled, it > > > > can return error when a wakeup event is detected, regardless of its > > > > source. > > > > > > Will adding this call to pm_wakeup_event() end up double-counting some > > > events? > > > > Yes, it will, if the event has already been reported by the subsystem or driver. > > > > I don't think it's a very big issue and I'm not sure trying to avoid it is > > worth the effort (we can check if the device's wakeup source object is active > > and skip reporting the wakeup event in that case, but that doesn't guarantee > > that the event won't be counted twice anyway). > > I agree that it's not a big issue. Wakeups reported twice because they > occur just before a system sleep won't cause serious accounting > problems and probably won't happen very often anyway. I just wanted to > make sure that the issue wasn't being ignored by mistake. OK Does it mean you're fine with the patch? Rafael ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PM: Prevent dpm_prepare() from returning errors unnecessarily 2010-11-29 22:04 ` Rafael J. Wysocki @ 2010-11-30 15:13 ` Alan Stern 2010-11-30 22:27 ` Rafael J. Wysocki 0 siblings, 1 reply; 13+ messages in thread From: Alan Stern @ 2010-11-30 15:13 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Linux-pm mailing list, LKML On Mon, 29 Nov 2010, Rafael J. Wysocki wrote: > > Oh, I see. This is a tricky issue. Every driver for a device that can > > have wakeup-enabled children needs to worry about the race between > > suspending the device and receiving a wakeup request from a child. > > For example, in drivers/usb/core/hcd-pci.c, the suspend_common() > > routine goes out of its way to return -EBUSY if device_may_wakeup() is > > true and the controller's root hub has a pending wakeup request. > > > > How should drivers handle this in general? Should we make an effort to > > convert them to use the wakeup framework so they they can let the PM > > core take care of these races? > > I think so. > > We also need to put a pm_check_wakeup_events() check into dpm_suspend() IMO, > so that we abort the suspending of devices as soon as a wakeup event is > reported. You might as well add that into this patch. > > Do we have to consider similar races during runtime suspend? > > Ideally, yes, but I'm not sure if that's generally possible. IMO, it won't be > a big deal if a parent device is suspended and immediately resumed occasionally > due to a pending wakeup signal from one of its children. It may be a problem > if that happens too often, though. Okay. > Does it mean you're fine with the patch? Provided you repair the error that Lei Ming pointed out. That's the problem with functions that return Boolean values -- you have to name them very carefully. Ideally the name should be a predicate or a question. Alan Stern ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PM: Prevent dpm_prepare() from returning errors unnecessarily 2010-11-30 15:13 ` Alan Stern @ 2010-11-30 22:27 ` Rafael J. Wysocki 2010-12-01 15:15 ` Alan Stern 0 siblings, 1 reply; 13+ messages in thread From: Rafael J. Wysocki @ 2010-11-30 22:27 UTC (permalink / raw) To: Alan Stern; +Cc: Linux-pm mailing list, LKML, Ming Lei On Tuesday, November 30, 2010, Alan Stern wrote: > On Mon, 29 Nov 2010, Rafael J. Wysocki wrote: > > > > Oh, I see. This is a tricky issue. Every driver for a device that can > > > have wakeup-enabled children needs to worry about the race between > > > suspending the device and receiving a wakeup request from a child. > > > For example, in drivers/usb/core/hcd-pci.c, the suspend_common() > > > routine goes out of its way to return -EBUSY if device_may_wakeup() is > > > true and the controller's root hub has a pending wakeup request. > > > > > > How should drivers handle this in general? Should we make an effort to > > > convert them to use the wakeup framework so they they can let the PM > > > core take care of these races? > > > > I think so. > > > > We also need to put a pm_check_wakeup_events() check into dpm_suspend() IMO, > > so that we abort the suspending of devices as soon as a wakeup event is > > reported. > > You might as well add that into this patch. I'll do that in a separate patch. > > > Do we have to consider similar races during runtime suspend? > > > > Ideally, yes, but I'm not sure if that's generally possible. IMO, it won't be > > a big deal if a parent device is suspended and immediately resumed occasionally > > due to a pending wakeup signal from one of its children. It may be a problem > > if that happens too often, though. > > Okay. > > > Does it mean you're fine with the patch? > > Provided you repair the error that Lei Ming pointed out. That's the > problem with functions that return Boolean values -- you have to name > them very carefully. Ideally the name should be a predicate or a > question. I already have fixed it. The name is unfortunate indeed, perhaps it's better to call that function pm_new_wakeup_events() or something like this. Thanks, Rafael ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PM: Prevent dpm_prepare() from returning errors unnecessarily 2010-11-30 22:27 ` Rafael J. Wysocki @ 2010-12-01 15:15 ` Alan Stern 2010-12-01 23:50 ` Rafael J. Wysocki 0 siblings, 1 reply; 13+ messages in thread From: Alan Stern @ 2010-12-01 15:15 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Linux-pm mailing list, LKML, Ming Lei On Tue, 30 Nov 2010, Rafael J. Wysocki wrote: > > Provided you repair the error that Lei Ming pointed out. That's the > > problem with functions that return Boolean values -- you have to name > > them very carefully. Ideally the name should be a predicate or a > > question. > > I already have fixed it. > > The name is unfortunate indeed, perhaps it's better to call that function > pm_new_wakeup_events() or something like this. Or pm_any_wakeup_events(). And reverse the meaning of the return value, of course -- it would be good to explain the return value in the kerneldoc. Alan Stern ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PM: Prevent dpm_prepare() from returning errors unnecessarily 2010-12-01 15:15 ` Alan Stern @ 2010-12-01 23:50 ` Rafael J. Wysocki 2010-12-02 15:38 ` Alan Stern 0 siblings, 1 reply; 13+ messages in thread From: Rafael J. Wysocki @ 2010-12-01 23:50 UTC (permalink / raw) To: Alan Stern; +Cc: Linux-pm mailing list, LKML, Ming Lei On Wednesday, December 01, 2010, Alan Stern wrote: > On Tue, 30 Nov 2010, Rafael J. Wysocki wrote: > > > > Provided you repair the error that Lei Ming pointed out. That's the > > > problem with functions that return Boolean values -- you have to name > > > them very carefully. Ideally the name should be a predicate or a > > > question. > > > > I already have fixed it. > > > > The name is unfortunate indeed, perhaps it's better to call that function > > pm_new_wakeup_events() or something like this. > > Or pm_any_wakeup_events(). And reverse the meaning of the return > value, of course -- it would be good to explain the return value in > the kerneldoc. OK, so please let me know what you think of the appended patch (on top of the previous one). Thanks, Rafael --- From: Rafael J. Wysocki <rjw@sisk.pl> Subject: PM / Wakeup: Replace pm_check_wakeup_events() with pm_wakeup_pending() To avoid confusion with the meaning and return value of pm_check_wakeup_events() replace it with pm_wakeup_pending() that will work the other way around (ie. return true when system-wide power transition should be aborted). Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> --- drivers/base/power/main.c | 2 +- drivers/base/power/wakeup.c | 20 ++++++++++---------- include/linux/suspend.h | 4 ++-- kernel/power/hibernate.c | 4 ++-- kernel/power/process.c | 2 +- kernel/power/suspend.c | 2 +- 6 files changed, 17 insertions(+), 17 deletions(-) Index: linux-2.6/drivers/base/power/wakeup.c =================================================================== --- linux-2.6.orig/drivers/base/power/wakeup.c +++ linux-2.6/drivers/base/power/wakeup.c @@ -542,26 +542,26 @@ static void pm_wakeup_update_hit_counts( } /** - * pm_check_wakeup_events - Check for new wakeup events. + * pm_wakeup_pending - Check if power transition in progress should be aborted. * * Compare the current number of registered wakeup events with its preserved - * value from the past to check if new wakeup events have been registered since - * the old value was stored. Check if the current number of wakeup events being - * processed is zero. + * value from the past and return true if new wakeup events have been registered + * since the old value was stored. Also return true if the current number of + * wakeup events being processed is different from zero. */ -bool pm_check_wakeup_events(void) +bool pm_wakeup_pending(void) { unsigned long flags; - bool ret = true; + bool ret = false; spin_lock_irqsave(&events_lock, flags); if (events_check_enabled) { - ret = ((unsigned int)atomic_read(&event_count) == saved_count) - && !atomic_read(&events_in_progress); - events_check_enabled = ret; + ret = ((unsigned int)atomic_read(&event_count) != saved_count) + || atomic_read(&events_in_progress); + events_check_enabled = !ret; } spin_unlock_irqrestore(&events_lock, flags); - if (!ret) + if (ret) pm_wakeup_update_hit_counts(); return ret; } Index: linux-2.6/include/linux/suspend.h =================================================================== --- linux-2.6.orig/include/linux/suspend.h +++ linux-2.6/include/linux/suspend.h @@ -292,7 +292,7 @@ extern int unregister_pm_notifier(struct /* drivers/base/power/wakeup.c */ extern bool events_check_enabled; -extern bool pm_check_wakeup_events(void); +extern bool pm_wakeup_pending(void); extern bool pm_get_wakeup_count(unsigned int *count); extern bool pm_save_wakeup_count(unsigned int count); #else /* !CONFIG_PM_SLEEP */ @@ -309,7 +309,7 @@ static inline int unregister_pm_notifier #define pm_notifier(fn, pri) do { (void)(fn); } while (0) -static inline bool pm_check_wakeup_events(void) { return true; } +static inline bool pm_wakeup_pending(void) { return true; } #endif /* !CONFIG_PM_SLEEP */ extern struct mutex pm_mutex; 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 @@ -1056,7 +1056,7 @@ static int dpm_prepare(pm_message_t stat if (pm_runtime_barrier(dev) && device_may_wakeup(dev)) pm_wakeup_event(dev, 0); - if (!pm_check_wakeup_events()) { + if (pm_wakeup_pending()) { pm_runtime_put_sync(dev); error = -EBUSY; } else { Index: linux-2.6/kernel/power/hibernate.c =================================================================== --- linux-2.6.orig/kernel/power/hibernate.c +++ linux-2.6/kernel/power/hibernate.c @@ -278,7 +278,7 @@ static int create_image(int platform_mod goto Enable_irqs; } - if (hibernation_test(TEST_CORE) || !pm_check_wakeup_events()) + if (hibernation_test(TEST_CORE) || pm_wakeup_pending()) goto Power_up; in_suspend = 1; @@ -516,7 +516,7 @@ int hibernation_platform_enter(void) local_irq_disable(); sysdev_suspend(PMSG_HIBERNATE); - if (!pm_check_wakeup_events()) { + if (pm_wakeup_pending()) { error = -EAGAIN; goto Power_up; } Index: linux-2.6/kernel/power/process.c =================================================================== --- linux-2.6.orig/kernel/power/process.c +++ linux-2.6/kernel/power/process.c @@ -85,7 +85,7 @@ static int try_to_freeze_tasks(bool sig_ if (!todo || time_after(jiffies, end_time)) break; - if (!pm_check_wakeup_events()) { + if (pm_wakeup_pending()) { wakeup = true; break; } Index: linux-2.6/kernel/power/suspend.c =================================================================== --- linux-2.6.orig/kernel/power/suspend.c +++ linux-2.6/kernel/power/suspend.c @@ -163,7 +163,7 @@ static int suspend_enter(suspend_state_t error = sysdev_suspend(PMSG_SUSPEND); if (!error) { - if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) { + if (!(suspend_test(TEST_CORE) || pm_wakeup_pending())) { error = suspend_ops->enter(state); events_check_enabled = false; } ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PM: Prevent dpm_prepare() from returning errors unnecessarily 2010-12-01 23:50 ` Rafael J. Wysocki @ 2010-12-02 15:38 ` Alan Stern 2010-12-02 19:42 ` Rafael J. Wysocki 0 siblings, 1 reply; 13+ messages in thread From: Alan Stern @ 2010-12-02 15:38 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Linux-pm mailing list, LKML, Ming Lei On Thu, 2 Dec 2010, Rafael J. Wysocki wrote: > OK, so please let me know what you think of the appended patch (on top of the > previous one). > > Thanks, > Rafael > Index: linux-2.6/include/linux/suspend.h > =================================================================== > --- linux-2.6.orig/include/linux/suspend.h > +++ linux-2.6/include/linux/suspend.h > @@ -292,7 +292,7 @@ extern int unregister_pm_notifier(struct > /* drivers/base/power/wakeup.c */ > extern bool events_check_enabled; > > -extern bool pm_check_wakeup_events(void); > +extern bool pm_wakeup_pending(void); > extern bool pm_get_wakeup_count(unsigned int *count); > extern bool pm_save_wakeup_count(unsigned int count); > #else /* !CONFIG_PM_SLEEP */ > @@ -309,7 +309,7 @@ static inline int unregister_pm_notifier > > #define pm_notifier(fn, pri) do { (void)(fn); } while (0) > > -static inline bool pm_check_wakeup_events(void) { return true; } > +static inline bool pm_wakeup_pending(void) { return true; } Shouldn't this return false? Alan Stern ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PM: Prevent dpm_prepare() from returning errors unnecessarily 2010-12-02 15:38 ` Alan Stern @ 2010-12-02 19:42 ` Rafael J. Wysocki 0 siblings, 0 replies; 13+ messages in thread From: Rafael J. Wysocki @ 2010-12-02 19:42 UTC (permalink / raw) To: Alan Stern; +Cc: Linux-pm mailing list, LKML, Ming Lei On Thursday, December 02, 2010, Alan Stern wrote: > On Thu, 2 Dec 2010, Rafael J. Wysocki wrote: > > > OK, so please let me know what you think of the appended patch (on top of the > > previous one). > > > > Thanks, > > Rafael > > Index: linux-2.6/include/linux/suspend.h > > =================================================================== > > --- linux-2.6.orig/include/linux/suspend.h > > +++ linux-2.6/include/linux/suspend.h > > @@ -292,7 +292,7 @@ extern int unregister_pm_notifier(struct > > /* drivers/base/power/wakeup.c */ > > extern bool events_check_enabled; > > > > -extern bool pm_check_wakeup_events(void); > > +extern bool pm_wakeup_pending(void); > > extern bool pm_get_wakeup_count(unsigned int *count); > > extern bool pm_save_wakeup_count(unsigned int count); > > #else /* !CONFIG_PM_SLEEP */ > > @@ -309,7 +309,7 @@ static inline int unregister_pm_notifier > > > > #define pm_notifier(fn, pri) do { (void)(fn); } while (0) > > > > -static inline bool pm_check_wakeup_events(void) { return true; } > > +static inline bool pm_wakeup_pending(void) { return true; } > > Shouldn't this return false? Yes, it should, thanks! Rafael ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PM: Prevent dpm_prepare() from returning errors unnecessarily 2010-11-28 12:12 [PATCH] PM: Prevent dpm_prepare() from returning errors unnecessarily Rafael J. Wysocki 2010-11-28 15:35 ` Alan Stern @ 2010-11-30 13:07 ` Ming Lei 2010-11-30 22:23 ` Rafael J. Wysocki 1 sibling, 1 reply; 13+ messages in thread From: Ming Lei @ 2010-11-30 13:07 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Linux-pm mailing list, LKML, Alan Stern 2010/11/28 Rafael J. Wysocki <rjw@sisk.pl>: > From: Rafael J. Wysocki <rjw@sisk.pl> > > Currently dpm_prepare() returns error code if it finds that a device > being suspended has a pending runtime resume request. However, it > should not do that if the checking for wakeup events is not enabled. > On the other hand, if the checking for wakeup events is enabled, it > can return error when a wakeup event is detected, regardless of its > source. > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > --- > drivers/base/power/main.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 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 > @@ -26,6 +26,7 @@ > #include <linux/interrupt.h> > #include <linux/sched.h> > #include <linux/async.h> > +#include <linux/suspend.h> > > #include "../base.h" > #include "power.h" > @@ -1052,8 +1053,10 @@ static int dpm_prepare(pm_message_t stat > mutex_unlock(&dpm_list_mtx); > > pm_runtime_get_noresume(dev); > - if (pm_runtime_barrier(dev) && device_may_wakeup(dev)) { > - /* Wake-up requested during system sleep transition. */ > + if (pm_runtime_barrier(dev) && device_may_wakeup(dev)) > + pm_wakeup_event(dev, 0); > + > + if (pm_check_wakeup_events()) { If pm_check_wakeup_events returns true, it means there is no wakeup event coming, so seems should handle normal suspend prepare, but why abort the suspend here? > pm_runtime_put_sync(dev); > error = -EBUSY; > } else { thanks, -- Lei Ming ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PM: Prevent dpm_prepare() from returning errors unnecessarily 2010-11-30 13:07 ` Ming Lei @ 2010-11-30 22:23 ` Rafael J. Wysocki 0 siblings, 0 replies; 13+ messages in thread From: Rafael J. Wysocki @ 2010-11-30 22:23 UTC (permalink / raw) To: Ming Lei; +Cc: Linux-pm mailing list, LKML, Alan Stern On Tuesday, November 30, 2010, Ming Lei wrote: > 2010/11/28 Rafael J. Wysocki <rjw@sisk.pl>: > > From: Rafael J. Wysocki <rjw@sisk.pl> > > > > Currently dpm_prepare() returns error code if it finds that a device > > being suspended has a pending runtime resume request. However, it > > should not do that if the checking for wakeup events is not enabled. > > On the other hand, if the checking for wakeup events is enabled, it > > can return error when a wakeup event is detected, regardless of its > > source. > > > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> > > --- > > drivers/base/power/main.c | 11 +++++++---- > > 1 file changed, 7 insertions(+), 4 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 > > @@ -26,6 +26,7 @@ > > #include <linux/interrupt.h> > > #include <linux/sched.h> > > #include <linux/async.h> > > +#include <linux/suspend.h> > > > > #include "../base.h" > > #include "power.h" > > @@ -1052,8 +1053,10 @@ static int dpm_prepare(pm_message_t stat > > mutex_unlock(&dpm_list_mtx); > > > > pm_runtime_get_noresume(dev); > > - if (pm_runtime_barrier(dev) && device_may_wakeup(dev)) { > > - /* Wake-up requested during system sleep transition. */ > > + if (pm_runtime_barrier(dev) && device_may_wakeup(dev)) > > + pm_wakeup_event(dev, 0); > > + > > + if (pm_check_wakeup_events()) { > > If pm_check_wakeup_events returns true, it means there is no wakeup event > coming, so seems should handle normal suspend prepare, but why > abort the suspend here? That's a bug, it should be !pm_check_wakeup_events() instead. Thanks, Rafael ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2010-12-02 19:43 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-11-28 12:12 [PATCH] PM: Prevent dpm_prepare() from returning errors unnecessarily Rafael J. Wysocki 2010-11-28 15:35 ` Alan Stern 2010-11-28 22:52 ` Rafael J. Wysocki 2010-11-29 3:05 ` Alan Stern 2010-11-29 22:04 ` Rafael J. Wysocki 2010-11-30 15:13 ` Alan Stern 2010-11-30 22:27 ` Rafael J. Wysocki 2010-12-01 15:15 ` Alan Stern 2010-12-01 23:50 ` Rafael J. Wysocki 2010-12-02 15:38 ` Alan Stern 2010-12-02 19:42 ` Rafael J. Wysocki 2010-11-30 13:07 ` Ming Lei 2010-11-30 22:23 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox