On Friday, May 15, 2015 06:08:32 PM Jin Qian wrote: > Some wakeup event happens every frequently between reading > wakeup_count and writing back wakeup_count. > They changed wakeup event count so writes fail and usespace doesn't > continue to suspend. However, such > occurrences are not counted in ws->wakeup_count.
So I hope you've read the comment in wakeup_source_report_event(). > I spent quite sometime finding out the problematic wakeup > event with inaccurate wakeup_count : ) But your patch doesn't fix the problem. ws->wakeup_count is still racy and kind of even more after the patch than before. > On Fri, May 15, 2015 at 5:34 PM, Rafael J. Wysocki <[email protected]> wrote: > > On Wednesday, April 22, 2015 05:50:11 PM Jin Qian wrote: > >> user-space aborts suspend attempt if writing wakeup_count failed. > >> Count the write failure towards wakeup_count. > > > > A use case, please? > > > >> Signed-off-by: Jin Qian <[email protected]> > >> --- > >> drivers/base/power/wakeup.c | 17 +++++++++++++++++ > >> 1 file changed, 17 insertions(+) > >> > >> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c > >> index f24c622..bdb45f3 100644 > >> --- a/drivers/base/power/wakeup.c > >> +++ b/drivers/base/power/wakeup.c > >> @@ -57,6 +57,8 @@ static LIST_HEAD(wakeup_sources); > >> > >> static DECLARE_WAIT_QUEUE_HEAD(wakeup_count_wait_queue); > >> > >> +static ktime_t last_read_time; > >> + > >> /** > >> * wakeup_source_prepare - Prepare a new wakeup source for initialization. > >> * @ws: Wakeup source to prepare. > >> @@ -771,10 +773,15 @@ void pm_wakeup_clear(void) > >> bool pm_get_wakeup_count(unsigned int *count, bool block) > >> { > >> unsigned int cnt, inpr; > >> + unsigned long flags; > >> > >> if (block) { > >> DEFINE_WAIT(wait); > >> > >> + spin_lock_irqsave(&events_lock, flags); > >> + last_read_time = ktime_get(); > >> + spin_unlock_irqrestore(&events_lock, flags); > >> + > >> for (;;) { > >> prepare_to_wait(&wakeup_count_wait_queue, &wait, > >> TASK_INTERRUPTIBLE); > >> @@ -806,6 +813,7 @@ bool pm_save_wakeup_count(unsigned int count) > >> { > >> unsigned int cnt, inpr; > >> unsigned long flags; > >> + struct wakeup_source *ws; > >> > >> events_check_enabled = false; > >> spin_lock_irqsave(&events_lock, flags); > >> @@ -813,6 +821,15 @@ bool pm_save_wakeup_count(unsigned int count) > >> if (cnt == count && inpr == 0) { > >> saved_count = count; > >> events_check_enabled = true; > >> + } else { > >> + rcu_read_lock(); > >> + list_for_each_entry_rcu(ws, &wakeup_sources, entry) { > >> + if (ws->active || > >> + ktime_compare(ws->last_time, last_read_time) > > >> 0) { > >> + ws->wakeup_count++; > >> + } > >> + } > >> + rcu_read_unlock(); > >> } > >> spin_unlock_irqrestore(&events_lock, flags); > >> return events_check_enabled; > >> > > > > -- > > I speak only for myself. > > Rafael J. Wysocki, Intel Open Source Technology Center. > -- > 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/ -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- 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/
