linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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-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-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 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

* 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

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;
as well as URLs for NNTP newsgroup(s).