On 09/11/2012 02:18 AM, John Stultz wrote: > On 09/10/2012 12:45 PM, Daniel Lezcano wrote: >> On 09/10/2012 07:14 PM, John Stultz wrote: >>> In the meantime, I'll try to reproduce on my T61. If you could send me >>> your .config, I'd appreciate it. >> http://pastebin.com/qSxqfdDK >> >> The header of the config file shows for a v3.5-rc7 because it is the >> result of the git-bisect. If you keep this config file for the latest >> kernel that should reproduce the problem. >> >> Let me know if you were able to reproduce the problem. > Great! With this I was able to quickly reproduce the problem and I think > I have a fix. Cool ! > Would you mind testing the following patch? It seems to resolve the > issue, but I've not yet run it through my test suite to make sure it > didn't break anything else. No problem, I will try it this evening. Is this problem related to all 32bits arch ? Thanks ! -- Daniel > If both your and my testing comes back ok, I'll submit it to Thomas. > > thanks > -john > > From f10a285a5b532a14d3330f6e60e4d7bd5627932a Mon Sep 17 00:00:00 2001 > From: John Stultz <john.stultz@xxxxxxxxxx> > Date: Mon, 10 Sep 2012 20:00:15 -0400 > Subject: [PATCH] time: Fix timeekeping_get_ns overflow on 32bit systems > > Daniel Lezcano reported seeing multi-second stalls from > keyboard input on his T61 laptop when NOHZ and CPU_IDLE > were enabled on a 32bit kernel. > > He bisected the problem down to > 1e75fa8be9fb61e1af46b5b3b176347a4c958ca1 (time: Condense > timekeeper.xtime into xtime_sec). > > After reproducing this issue, I narrowed the problem down > to the fact that timekeeping_get_ns() returns a 64bit > nsec value that hasn't been accumulated. In some cases > this value was being then stored in timespec.tv_nsec > (which is a long). > > On 32bit systems, With idle times larger then 4 seconds > (or less, depending on the value of xtime_nsec), the > returned nsec value would overflow 32bits. This limited > kept time from increasing, causing timers to not expire. > > The fix is to make sure we don't directly store the > result of timekeeping_get_ns() into a tv_nsec field, > instead using a 64bit nsec value which can then be > added into the timespec via timespec_add_ns(). > > With this patch I cannot reproduce the issue. > > Cc: Ingo Molnar <mingo@xxxxxxxxxx> > Cc: Richard Cochran <richardcochran@xxxxxxxxx> > Cc: Prarit Bhargava <prarit@xxxxxxxxxx> > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx> > Cc: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> > Reported-and-bisected-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> > Signed-off-by: John Stultz <john.stultz@xxxxxxxxxx> > --- > kernel/time/timekeeping.c | 19 ++++++++++++------- > 1 file changed, 12 insertions(+), 7 deletions(-) > > diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c > index 34e5eac..d3b91e7 100644 > --- a/kernel/time/timekeeping.c > +++ b/kernel/time/timekeeping.c > @@ -303,10 +303,11 @@ void getnstimeofday(struct timespec *ts) > seq = read_seqbegin(&tk->lock); > > ts->tv_sec = tk->xtime_sec; > - ts->tv_nsec = timekeeping_get_ns(tk); > + nsecs = timekeeping_get_ns(tk); > > } while (read_seqretry(&tk->lock, seq)); > > + ts->tv_nsec = 0; > timespec_add_ns(ts, nsecs); > } > EXPORT_SYMBOL(getnstimeofday); > @@ -345,6 +346,7 @@ void ktime_get_ts(struct timespec *ts) > { > struct timekeeper *tk = &timekeeper; > struct timespec tomono; > + s64 nsec; > unsigned int seq; > > WARN_ON(timekeeping_suspended); > @@ -352,13 +354,14 @@ void ktime_get_ts(struct timespec *ts) > do { > seq = read_seqbegin(&tk->lock); > ts->tv_sec = tk->xtime_sec; > - ts->tv_nsec = timekeeping_get_ns(tk); > + nsec = timekeeping_get_ns(tk); > tomono = tk->wall_to_monotonic; > > } while (read_seqretry(&tk->lock, seq)); > > - set_normalized_timespec(ts, ts->tv_sec + tomono.tv_sec, > - ts->tv_nsec + tomono.tv_nsec); > + ts->tv_sec += tomono.tv_sec; > + ts->tv_nsec = 0; > + timespec_add_ns(ts, nsec + tomono.tv_nsec); > } > EXPORT_SYMBOL_GPL(ktime_get_ts); > > @@ -1244,6 +1247,7 @@ void get_monotonic_boottime(struct timespec *ts) > { > struct timekeeper *tk = &timekeeper; > struct timespec tomono, sleep; > + s64 nsec; > unsigned int seq; > > WARN_ON(timekeeping_suspended); > @@ -1251,14 +1255,15 @@ void get_monotonic_boottime(struct timespec *ts) > do { > seq = read_seqbegin(&tk->lock); > ts->tv_sec = tk->xtime_sec; > - ts->tv_nsec = timekeeping_get_ns(tk); > + nsec = timekeeping_get_ns(tk); > tomono = tk->wall_to_monotonic; > sleep = tk->total_sleep_time; > > } while (read_seqretry(&tk->lock, seq)); > > - set_normalized_timespec(ts, ts->tv_sec + tomono.tv_sec + sleep.tv_sec, > - ts->tv_nsec + tomono.tv_nsec + sleep.tv_nsec); > + ts->tv_sec += tomono.tv_sec + sleep.tv_sec; > + ts->tv_nsec = 0; > + timespec_add_ns(ts, nsec + tomono.tv_nsec + sleep.tv_nsec); > } > EXPORT_SYMBOL_GPL(get_monotonic_boottime); > -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
