lkml.org 
[lkml]   [2018]   [Apr]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] PM / Sleep: only update last time for active wakeup sources
On Thu, Apr 26, 2018 at 1:40 AM, Doug Berger <opendmb@gmail.com> wrote:
> When wakelock support was added, the wakeup_source_add() function
> was updated to set the last_time value of the wakeup source. This
> has the unintended side effect of producing confusing output from
> pm_print_active_wakeup_sources() when a wakeup source is added
> prior to a sleep that is blocked by a different wakeup source.
>
> The function pm_print_active_wakeup_sources() will search for the
> most recently active wakeup source when no active source is found.
> If a wakeup source is added after a different wakeup source blocks
> the system from going to sleep it may have a later last_time value
> than the blocking source and be output as the last active wakeup
> source even if it has never actually been active.
>
> It looks to me like the change to wakeup_source_add() was made to
> prevent the wakelock garbage collection from accidentally dropping
> a wakelock during the narrow window between adding the wakelock to
> the wakelock list in wakelock_lookup_add() and the activation of
> the wakeup source in pm_wake_lock().
>
> This commit changes the behavior so that only the last_time of the
> wakeup source used by a wakelock is initialized prior to adding it
> to the wakeup source list. This preserves the meaning of the
> last_time value as the last time the wakeup source was active and
> allows a wakeup source that has never been active to have a
> last_time value of 0.
>
> Fixes: b86ff982 ("PM / Sleep: Add user space interface for manipulating wakeup sources, v3")
> Signed-off-by: Doug Berger <opendmb@gmail.com>
> ---
> drivers/base/power/wakeup.c | 1 -
> kernel/power/wakelock.c | 1 +
> 2 files changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index ea01621..230160e 100644
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -183,7 +183,6 @@ void wakeup_source_add(struct wakeup_source *ws)
> spin_lock_init(&ws->lock);
> timer_setup(&ws->timer, pm_wakeup_timer_fn, 0);
> ws->active = false;
> - ws->last_time = ktime_get();

If it is not initialized here, max_time may not be updated correctly later on.

If you don't want to initialize it to ktime_get() (to avoid the issue
you're trying to avoid), initialize it to something special and then
check for that explicitly in wakeup_source_deactivate() when computing
max_time.

>
> spin_lock_irqsave(&events_lock, flags);
> list_add_rcu(&ws->entry, &wakeup_sources);
> diff --git a/kernel/power/wakelock.c b/kernel/power/wakelock.c
> index dfba59b..4210152 100644
> --- a/kernel/power/wakelock.c
> +++ b/kernel/power/wakelock.c
> @@ -188,6 +188,7 @@ static struct wakelock *wakelock_lookup_add(const char *name, size_t len,
> return ERR_PTR(-ENOMEM);
> }
> wl->ws.name = wl->name;
> + wl->ws.last_time = ktime_get();
> wakeup_source_add(&wl->ws);
> rb_link_node(&wl->node, parent, node);
> rb_insert_color(&wl->node, &wakelocks_tree);
> --
> 2.7.4
>

\
 
 \ /
  Last update: 2018-04-26 08:31    [W:0.048 / U:0.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site