| Message ID | 1369944137-30948-1-git-send-email-jwerner@chromium.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Thursday, May 30, 2013 01:02:17 PM Julius Werner wrote: > Commit a938da06 introduced a useful little log message to tell > users/debuggers which wakeup source aborted a suspend. However, this > message is only printed if the abort happens during the in-kernel > suspend path (after writing /sys/power/state). > > The full specification of the /sys/power/wakeup_count facility allows > user-space power managers to double-check if wakeups have already > happened before it actually tries to suspend (e.g. while it was running > user-space pre-suspend hooks), by writing the last known wakeup_count > value to /sys/power/wakeup_count. This patch changes the sysfs handler > for that node to also print said log message if that write fails, so > that we can figure out the offending wakeup source for both kinds of > suspend aborts. > > Signed-off-by: Julius Werner <jwerner@chromium.org> > --- > drivers/base/power/wakeup.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c > index 79715e7..b0b7886 100644 > --- a/drivers/base/power/wakeup.c > +++ b/drivers/base/power/wakeup.c > @@ -772,6 +772,8 @@ bool pm_save_wakeup_count(unsigned int count) > events_check_enabled = true; > } > spin_unlock_irqrestore(&events_lock, flags); > + if (!events_check_enabled) > + print_active_wakeup_sources(); This is going to be done in the autosleep case too, which I'm slightly concerned about. If you wanted to cover /sys/power/wakeup_count only, it would be better to put that into wakeup_count_store(), but perhaps you wanted to cover autosleep too? > return events_check_enabled; > } Thanks, Rafael
Resending as plain text... sorry for the spam, Gmail just likes to silently mess up my settings every once in a while. On Tue, Jun 11, 2013 at 4:17 PM, Julius Werner <jwerner@chromium.org> wrote: > >> This is going to be done in the autosleep case too, which I'm slightly >> concerned about. >> >> If you wanted to cover /sys/power/wakeup_count only, it would be better >> to put that into wakeup_count_store(), but perhaps you wanted to cover >> autosleep too? > > > Hmm... no, I did not really take the autosleep case into account. I can't > quite tell with what frequency the autosleep try_to_suspend() handler on an > active system would run... do you think it might be a useful feature to have > there, or would it lead to logspam? > > I can move it to wakeup_count_store() if you prefer, but then I will have to > turn print_active_wakeup_sources() into an exported function, which is also > a little inconvenient. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday, June 11, 2013 04:17:35 PM Julius Werner wrote: > > This is going to be done in the autosleep case too, which I'm slightly > > concerned about. > > > > If you wanted to cover /sys/power/wakeup_count only, it would be better > > to put that into wakeup_count_store(), but perhaps you wanted to cover > > autosleep too? > > > > Hmm... no, I did not really take the autosleep case into account. I can't > quite tell with what frequency the autosleep try_to_suspend() handler on an > active system would run... do you think it might be a useful feature to > have there, or would it lead to logspam? On Android systems it may be really frequent, because they try to suspend very aggressively. That's why I asked about it. > I can move it to wakeup_count_store() if you prefer, but then I will have > to turn print_active_wakeup_sources() into an exported function, which is > also a little inconvenient. That's OK. Thanks, Rafael
diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c index 79715e7..b0b7886 100644 --- a/drivers/base/power/wakeup.c +++ b/drivers/base/power/wakeup.c @@ -772,6 +772,8 @@ bool pm_save_wakeup_count(unsigned int count) events_check_enabled = true; } spin_unlock_irqrestore(&events_lock, flags); + if (!events_check_enabled) + print_active_wakeup_sources(); return events_check_enabled; }
Commit a938da06 introduced a useful little log message to tell users/debuggers which wakeup source aborted a suspend. However, this message is only printed if the abort happens during the in-kernel suspend path (after writing /sys/power/state). The full specification of the /sys/power/wakeup_count facility allows user-space power managers to double-check if wakeups have already happened before it actually tries to suspend (e.g. while it was running user-space pre-suspend hooks), by writing the last known wakeup_count value to /sys/power/wakeup_count. This patch changes the sysfs handler for that node to also print said log message if that write fails, so that we can figure out the offending wakeup source for both kinds of suspend aborts. Signed-off-by: Julius Werner <jwerner@chromium.org> --- drivers/base/power/wakeup.c | 2 ++ 1 file changed, 2 insertions(+)