| Message ID | 1471993702-29148-3-git-send-email-john.stultz@linaro.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Hi John, some small typos below, others should be OK. On Tue, Aug 23, 2016 at 04:08:22PM -0700, John Stultz wrote: > It was reported that hibernation could fail on the 2nd attempt, > where the system hangs at hibernate() -> syscore_resume() -> > i8237A_resume() -> claim_dma_lock(), because the lock has > already been taken. > > However there is actually no other process would like to grab > this lock on that problematic platform. > > Further investigation showed that the problem is triggered by > setting /sys/power/pm_trace to 1 before the 1st hibernation. > > Since once pm_trace is enabled, the rtc becomes unmeaningful > after suspend, and meanwhile some BIOSes would like to adjust > the 'invalid' tsc(e.g, smaller than 1970) to the release date I checked the previous commit log, and I have made a mistake, it should be: s/tsc/RTC > of that motherboard during POST stage, thus after resumed, it > may seem that the system had a significant long sleep time might > due to meaningless tsc or RTC delta. s/tsc or RTC/RTC > > Then in timekeeping_resume -> tk_debug_account_sleep_time, if > the bit31 of the sleep time happened to be set to 1, the fls > returns 32 and then we add 1 to sleep_time_bin[32], which > caused a memory overwritten. > > As depicted by System.map: > 0xffffffff81c9d080 b sleep_time_bin > 0xffffffff81c9d100 B dma_spin_lock > the dma_spin_lock.val is set to 1, which caused this problem. > > This patch adds a sanity check in tk_debug_account_sleep_time() > to ensure we don't index past the sleep_time_bin array. > BTW, I've also post a fix to deal with pm_trace which might break timekeeping at: https://patchwork.kernel.org/patch/9287347/ could you please hel take a glance? thanks. Yu -- 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 Tue, Aug 23, 2016 at 5:58 PM, Chen Yu <yu.c.chen@intel.com> wrote:
> Hi John, some small typos below, others should be OK.
Thanks. I realize you mailed me before on this, but I forgot to
integrate the changes. Apologies.
I've now integrated the changes and updated the git branch. The HEAD
for the pull request is now: 4efd3c8faf189674eeafb0c14ff4c556ed6adcf7.
I'll send an updated v2 patch here shortly.
thanks
-john
--
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
diff --git a/kernel/time/timekeeping_debug.c b/kernel/time/timekeeping_debug.c index f6bd652..107310a6 100644 --- a/kernel/time/timekeeping_debug.c +++ b/kernel/time/timekeeping_debug.c @@ -23,7 +23,9 @@ #include "timekeeping_internal.h" -static unsigned int sleep_time_bin[32] = {0}; +#define NUM_BINS 32 + +static unsigned int sleep_time_bin[NUM_BINS] = {0}; static int tk_debug_show_sleep_time(struct seq_file *s, void *data) { @@ -69,6 +71,9 @@ late_initcall(tk_debug_sleep_time_init); void tk_debug_account_sleep_time(struct timespec64 *t) { - sleep_time_bin[fls(t->tv_sec)]++; + /* Cap bin index so we don't overflow the array */ + int bin = min(fls(t->tv_sec), NUM_BINS-1); + + sleep_time_bin[bin]++; }
It was reported that hibernation could fail on the 2nd attempt, where the system hangs at hibernate() -> syscore_resume() -> i8237A_resume() -> claim_dma_lock(), because the lock has already been taken. However there is actually no other process would like to grab this lock on that problematic platform. Further investigation showed that the problem is triggered by setting /sys/power/pm_trace to 1 before the 1st hibernation. Since once pm_trace is enabled, the rtc becomes unmeaningful after suspend, and meanwhile some BIOSes would like to adjust the 'invalid' tsc(e.g, smaller than 1970) to the release date of that motherboard during POST stage, thus after resumed, it may seem that the system had a significant long sleep time might due to meaningless tsc or RTC delta. Then in timekeeping_resume -> tk_debug_account_sleep_time, if the bit31 of the sleep time happened to be set to 1, the fls returns 32 and then we add 1 to sleep_time_bin[32], which caused a memory overwritten. As depicted by System.map: 0xffffffff81c9d080 b sleep_time_bin 0xffffffff81c9d100 B dma_spin_lock the dma_spin_lock.val is set to 1, which caused this problem. This patch adds a sanity check in tk_debug_account_sleep_time() to ensure we don't index past the sleep_time_bin array. Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Ingo Molnar <mingo@kernel.org> Cc: Rafael J. Wysocki <rjw@rjwysocki.net> Cc: Janek Kozicki <cosurgi@gmail.com> Cc: Chen Yu <yu.c.chen@intel.com> Cc: Xunlei Pang <xpang@redhat.com> Cc: Zhang Rui <rui.zhang@intel.com> Cc: linux-pm@vger.kernel.org Cc: stable <stable@vger.kernel.org> Reported-by: Janek Kozicki <cosurgi@gmail.com> Reported-by: Chen Yu <yu.c.chen@intel.com> [jstultz: Problem diagnosed and original patch by Chen Yu, I've solved the issue slightly differently, but borrowed his excelent explanation of of the issue here.] Signed-off-by: John Stultz <john.stultz@linaro.org> --- kernel/time/timekeeping_debug.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)