hangcheck-timer is broken on x86
in [Kernel]
|
From: Andrew Morton on 26 Mar 2010 17:30 On Tue, 23 Mar 2010 23:36:11 -0400 Yury Polyanskiy <ypolyans(a)princeton.edu> wrote: > The drivers/char/hangcheck-timer.c is doubly broken. First, the > following line overflows unsigned long: > # define TIMER_FREQ (HZ*loops_per_jiffy) > > Second, and more importantly, loops_per_jiffy has little to do with the conversion from the > the time scale of get_cycles() (aka rdtsc) to the time scale of jiffies. It's a bit odd to have a driver be this broken on x86_32 for five years without anyone noticing. What are the user-visible effects of these shortcomings? Also, please do send us a Signed-off-by: for this patch, as explained in Documentation/SubmittingPatches, thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Joel Becker on 26 Mar 2010 17:50 On Tue, Mar 23, 2010 at 11:36:11PM -0400, Yury Polyanskiy wrote: > Second, and more importantly, loops_per_jiffy has little to do with the conversion from the > the time scale of get_cycles() (aka rdtsc) to the time scale of jiffies. It used to! Fundamentally, of course, we didn't have a monotonic clock everywhere that satisfied hangcheck-timer's needs. So we had to use different approaches on different architectures. > @@ -130,7 +129,9 @@ extern unsigned long long monotonic_clock(void); > #else > static inline unsigned long long monotonic_clock(void) > { > - return get_cycles(); > + struct timespec ts; > + getrawmonotonic(&ts); > + return timespec_to_ns(&ts); > } > #endif /* HAVE_MONOTONIC */ I have two questions: 1) Does getrawmonotonic() satisfy hangcheck-timer? What I mean is, will it always return the wallclock nanoseconds even in the face of CPU speed changes, suspend, udelay, or any other suspension of kernel operation? Yes, I know this is a tougher standard than rdtsc(), but this is what hangcheck-timer wants. rdtsc() at least satisfied udelay and PCI hangs. 2) If it does satisfy, why not use it for all hangcheck usage instead of any ifdefs? Joel -- "The cynics are right nine times out of ten." - H. L. Mencken Joel Becker Principal Software Developer Oracle E-mail: joel.becker(a)oracle.com Phone: (650) 506-8127 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Yury Polyanskiy on 26 Mar 2010 18:00 On Fri, 26 Mar 2010 14:24:23 -0700 Andrew Morton <akpm(a)linux-foundation.org> wrote: > On Tue, 23 Mar 2010 23:36:11 -0400 > Yury Polyanskiy <ypolyans(a)princeton.edu> wrote: > > > The drivers/char/hangcheck-timer.c is doubly broken. First, the > > following line overflows unsigned long: > > # define TIMER_FREQ (HZ*loops_per_jiffy) > > > > Second, and more importantly, loops_per_jiffy has little to do with the conversion from the > > the time scale of get_cycles() (aka rdtsc) to the time scale of jiffies. > > It's a bit odd to have a driver be this broken on x86_32 for five years > without anyone noticing. What are the user-visible effects of these > shortcomings? When the overflown value of TIMER_FREQ is abnormally low, it spams the syslog with KERN_CRIT messages "Hangcheck: hangcheck value past margin!" But whether it happens or not depends on HZ and lpj in a complex way. People have hit it occasionally as far as google search can tell. > > Also, please do send us a Signed-off-by: for this patch, as explained > in Documentation/SubmittingPatches, thanks. > Sorry. Signed-off-by: Yury Polyanskiy <polyanskiy(a)gmail.com> Thank you Andrew! Yury
From: Yury Polyanskiy on 26 Mar 2010 18:10 On Fri, 26 Mar 2010 14:46:49 -0700 Joel Becker <Joel.Becker(a)oracle.com> wrote: > On Tue, Mar 23, 2010 at 11:36:11PM -0400, Yury Polyanskiy wrote: > 1) Does getrawmonotonic() satisfy hangcheck-timer? What I mean is, will > it always return the wallclock nanoseconds even in the face of CPU speed > changes, suspend, udelay, or any other suspension of kernel operation? > Yes, I know this is a tougher standard than rdtsc(), but this is what > hangcheck-timer wants. rdtsc() at least satisfied udelay and PCI hangs. Yes, as far as I can tell. Note that rdtsc is hosed on suspend-resume. > > 2) If it does satisfy, why not use it for all hangcheck usage instead of > any ifdefs? On my part, I didn't want to touch the S390 code since I can't test it. Yury
From: Joel Becker on 26 Mar 2010 21:00 On Fri, Mar 26, 2010 at 06:00:25PM -0400, Yury Polyanskiy wrote: > On Fri, 26 Mar 2010 14:46:49 -0700 > Joel Becker <Joel.Becker(a)oracle.com> wrote: > > > On Tue, Mar 23, 2010 at 11:36:11PM -0400, Yury Polyanskiy wrote: > > > 1) Does getrawmonotonic() satisfy hangcheck-timer? What I mean is, will > > it always return the wallclock nanoseconds even in the face of CPU speed > > changes, suspend, udelay, or any other suspension of kernel operation? > > Yes, I know this is a tougher standard than rdtsc(), but this is what > > hangcheck-timer wants. rdtsc() at least satisfied udelay and PCI hangs. > > Yes, as far as I can tell. Note that rdtsc is hosed on suspend-resume. Yeah, I know. rdtsc hangcheck-timer really required no suspend or cpufreq. Since it is only really used by servers, this wasn't a terrible restriction. Then virtualization came along... > > 2) If it does satisfy, why not use it for all hangcheck usage instead of > > any ifdefs? > > On my part, I didn't want to touch the S390 code since I can't test it. Makes sense. Andrew, would you mind pushing this through your tree? Acked-by: Joel Becker <joel.becker(a)oracle.com> Joel -- Dort wo man B�cher verbrennt, verbrennt man am Ende auch Mensch. (Wherever they burn books, they will also end up burning people.) - Heinrich Heine Joel Becker Principal Software Developer Oracle E-mail: joel.becker(a)oracle.com Phone: (650) 506-8127 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|
Next
|
Last
Pages: 1 2 3 4 Prev: Protect prefetch macro arguments. Next: dmar: section mismatch cleanup |