* [GIT PULL clocksource] Clocksource watchdog commits for v5.15 @ 2021-08-12 0:01 Paul E. McKenney 2021-08-12 13:46 ` Thomas Gleixner 0 siblings, 1 reply; 5+ messages in thread From: Paul E. McKenney @ 2021-08-12 0:01 UTC (permalink / raw) To: tglx; +Cc: linux-kernel, john.stultz, kernel-team, ak, rong.a.chen, sboyd Hello, Thomas, This pull request contains a single change that prevents clocksource watchdog testing on systems with HZ < 100, thus preventing the integer underflow that can occur on leisurely HZed systems. This has been posted to LKML: https://lore.kernel.org/lkml/20210721212755.GA2066078@paulmck-ThinkPad-P17-Gen-1/ This has been tested by Rong Chen of Intel. It has also been subjected to subjected to the kbuild test robot and -next testing, and are available in the git repository based on v5.14-rc2 at: git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git clocksource for you to fetch changes up to 9b073961afabcf70d0804e472ea02fc6c739dcce: clocksource: Prohibit clocksource watchdog test when HZ<100 (2021-07-20 13:54:34 -0700) ---------------------------------------------------------------- Paul E. McKenney (1): clocksource: Prohibit clocksource watchdog test when HZ<100 lib/Kconfig.debug | 1 + 1 file changed, 1 insertion(+) ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [GIT PULL clocksource] Clocksource watchdog commits for v5.15 2021-08-12 0:01 [GIT PULL clocksource] Clocksource watchdog commits for v5.15 Paul E. McKenney @ 2021-08-12 13:46 ` Thomas Gleixner 2021-08-12 16:37 ` Paul E. McKenney 0 siblings, 1 reply; 5+ messages in thread From: Thomas Gleixner @ 2021-08-12 13:46 UTC (permalink / raw) To: paulmck; +Cc: linux-kernel, john.stultz, kernel-team, ak, rong.a.chen, sboyd On Wed, Aug 11 2021 at 17:01, Paul E. McKenney wrote: > This pull request contains a single change that prevents clocksource > watchdog testing on systems with HZ < 100, thus preventing the integer > underflow that can occur on leisurely HZed systems. This has been > posted to LKML: > > https://lore.kernel.org/lkml/20210721212755.GA2066078@paulmck-ThinkPad-P17-Gen-1/ So with HZ < 100 .mult overflows, but why not simply adjusting the mult, shift value to be .mult = TICK_NSEC, .shift = 0, which is effectively the same as .mult = TICK_NSEC << 8, .shift = 8, Hmm? Thanks, tglx ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [GIT PULL clocksource] Clocksource watchdog commits for v5.15 2021-08-12 13:46 ` Thomas Gleixner @ 2021-08-12 16:37 ` Paul E. McKenney 2021-08-12 17:11 ` Thomas Gleixner 0 siblings, 1 reply; 5+ messages in thread From: Paul E. McKenney @ 2021-08-12 16:37 UTC (permalink / raw) To: Thomas Gleixner Cc: linux-kernel, john.stultz, kernel-team, ak, rong.a.chen, sboyd On Thu, Aug 12, 2021 at 03:46:42PM +0200, Thomas Gleixner wrote: > On Wed, Aug 11 2021 at 17:01, Paul E. McKenney wrote: > > This pull request contains a single change that prevents clocksource > > watchdog testing on systems with HZ < 100, thus preventing the integer > > underflow that can occur on leisurely HZed systems. This has been > > posted to LKML: > > > > https://lore.kernel.org/lkml/20210721212755.GA2066078@paulmck-ThinkPad-P17-Gen-1/ > > So with HZ < 100 .mult overflows, but why not simply adjusting the > mult, shift value to be > > .mult = TICK_NSEC, > .shift = 0, > > which is effectively the same as > > .mult = TICK_NSEC << 8, > .shift = 8, > > Hmm? Another option would be for me to be less lazy and to move this code: /* Since jiffies uses a simple TICK_NSEC multiplier * conversion, the .shift value could be zero. However * this would make NTP adjustments impossible as they are * in units of 1/2^.shift. Thus we use JIFFIES_SHIFT to * shift both the nominator and denominator the same * amount, and give ntp adjustments in units of 1/2^8 * * The value 8 is somewhat carefully chosen, as anything * larger can result in overflows. TICK_NSEC grows as HZ * shrinks, so values greater than 8 overflow 32bits when * HZ=100. */ #if HZ < 34 #define JIFFIES_SHIFT 6 #elif HZ < 67 #define JIFFIES_SHIFT 7 #else #define JIFFIES_SHIFT 8 #endif from kernel/time/jiffies.c to include/linux/clocksource.h. Then remove this from kernel/time/clocksource-wdtest.c: /* Assume HZ > 100. */ #define JIFFIES_SHIFT 8 Then I could get rid of the HZ < 100 restriction. So how about as shown below? Thanx, Paul ------------------------------------------------------------------------ commit 413933be37676419414fc7cd03e333c8eaf8a2db Author: Paul E. McKenney <paulmck@kernel.org> Date: Thu Aug 12 09:31:28 2021 -0700 clocksource: Make clocksource-wdtest.c safe for slow-HZ systems Currently, clocksource-wdtest.c sets a local JIFFIES_SHIFT macro for operation at HZ>=67, which can cause this test suite to fail on systems with HZ<67. Therefore, move the HZ-based definitions of JIFFIES_SHIFT from kernel/time/jiffies.c to include/linux/clocksource.h, allowing the local JIFFIES_SHIFT macro to be removed from clocksource-wdtest.c in favor of a properly HZ-based definition. This in turn makes clocksource-wdtest.c safe for slow-HZ systems. Cc: John Stultz <john.stultz@linaro.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Stephen Boyd <sboyd@kernel.org> Cc: Rong Chen <rong.a.chen@intel.com> Signed-off-by: Paul E. McKenney <paulmck@kernel.org> diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h index 1d42d4b173271..61b9a132a7e00 100644 --- a/include/linux/clocksource.h +++ b/include/linux/clocksource.h @@ -294,4 +294,24 @@ static inline void timer_probe(void) {} extern ulong max_cswd_read_retries; void clocksource_verify_percpu(struct clocksource *cs); +/* Since jiffies uses a simple TICK_NSEC multiplier + * conversion, the .shift value could be zero. However + * this would make NTP adjustments impossible as they are + * in units of 1/2^.shift. Thus we use JIFFIES_SHIFT to + * shift both the nominator and denominator the same + * amount, and give ntp adjustments in units of 1/2^8 + * + * The value 8 is somewhat carefully chosen, as anything + * larger can result in overflows. TICK_NSEC grows as HZ + * shrinks, so values greater than 8 overflow 32bits when + * HZ=100. + */ +#if HZ < 34 +#define JIFFIES_SHIFT 6 +#elif HZ < 67 +#define JIFFIES_SHIFT 7 +#else +#define JIFFIES_SHIFT 8 +#endif + #endif /* _LINUX_CLOCKSOURCE_H */ diff --git a/kernel/time/clocksource-wdtest.c b/kernel/time/clocksource-wdtest.c index b72a969f7b938..781d8dc69be47 100644 --- a/kernel/time/clocksource-wdtest.c +++ b/kernel/time/clocksource-wdtest.c @@ -34,9 +34,6 @@ static u64 wdtest_jiffies_read(struct clocksource *cs) return (u64)jiffies; } -/* Assume HZ > 100. */ -#define JIFFIES_SHIFT 8 - static struct clocksource clocksource_wdtest_jiffies = { .name = "wdtest-jiffies", .rating = 1, /* lowest valid rating*/ diff --git a/kernel/time/jiffies.c b/kernel/time/jiffies.c index 01935aafdb460..74f4b292900d1 100644 --- a/kernel/time/jiffies.c +++ b/kernel/time/jiffies.c @@ -12,26 +12,6 @@ #include "timekeeping.h" -/* Since jiffies uses a simple TICK_NSEC multiplier - * conversion, the .shift value could be zero. However - * this would make NTP adjustments impossible as they are - * in units of 1/2^.shift. Thus we use JIFFIES_SHIFT to - * shift both the nominator and denominator the same - * amount, and give ntp adjustments in units of 1/2^8 - * - * The value 8 is somewhat carefully chosen, as anything - * larger can result in overflows. TICK_NSEC grows as HZ - * shrinks, so values greater than 8 overflow 32bits when - * HZ=100. - */ -#if HZ < 34 -#define JIFFIES_SHIFT 6 -#elif HZ < 67 -#define JIFFIES_SHIFT 7 -#else -#define JIFFIES_SHIFT 8 -#endif - static u64 jiffies_read(struct clocksource *cs) { return (u64) jiffies; ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [GIT PULL clocksource] Clocksource watchdog commits for v5.15 2021-08-12 16:37 ` Paul E. McKenney @ 2021-08-12 17:11 ` Thomas Gleixner 2021-08-12 20:01 ` Paul E. McKenney 0 siblings, 1 reply; 5+ messages in thread From: Thomas Gleixner @ 2021-08-12 17:11 UTC (permalink / raw) To: paulmck; +Cc: linux-kernel, john.stultz, kernel-team, ak, rong.a.chen, sboyd On Thu, Aug 12 2021 at 09:37, Paul E. McKenney wrote: > On Thu, Aug 12, 2021 at 03:46:42PM +0200, Thomas Gleixner wrote: >> On Wed, Aug 11 2021 at 17:01, Paul E. McKenney wrote: >> > This pull request contains a single change that prevents clocksource >> > watchdog testing on systems with HZ < 100, thus preventing the integer >> > underflow that can occur on leisurely HZed systems. This has been >> > posted to LKML: >> > >> > https://lore.kernel.org/lkml/20210721212755.GA2066078@paulmck-ThinkPad-P17-Gen-1/ >> >> So with HZ < 100 .mult overflows, but why not simply adjusting the >> mult, shift value to be >> >> .mult = TICK_NSEC, >> .shift = 0, >> >> which is effectively the same as >> >> .mult = TICK_NSEC << 8, >> .shift = 8, >> >> Hmm? > > Another option would be for me to be less lazy and to move this code: > > /* Since jiffies uses a simple TICK_NSEC multiplier > * conversion, the .shift value could be zero. However > * this would make NTP adjustments impossible as they are > * in units of 1/2^.shift. Thus we use JIFFIES_SHIFT to > * shift both the nominator and denominator the same > * amount, and give ntp adjustments in units of 1/2^8 > * > * The value 8 is somewhat carefully chosen, as anything > * larger can result in overflows. TICK_NSEC grows as HZ > * shrinks, so values greater than 8 overflow 32bits when > * HZ=100. > */ > #if HZ < 34 > #define JIFFIES_SHIFT 6 > #elif HZ < 67 > #define JIFFIES_SHIFT 7 > #else > #define JIFFIES_SHIFT 8 > #endif > > from kernel/time/jiffies.c to include/linux/clocksource.h. No need to expose this globaly. kernel/time/tick-internal.h or kernel/time/jiffies.h Thanks, tglx ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [GIT PULL clocksource] Clocksource watchdog commits for v5.15 2021-08-12 17:11 ` Thomas Gleixner @ 2021-08-12 20:01 ` Paul E. McKenney 0 siblings, 0 replies; 5+ messages in thread From: Paul E. McKenney @ 2021-08-12 20:01 UTC (permalink / raw) To: Thomas Gleixner Cc: linux-kernel, john.stultz, kernel-team, ak, rong.a.chen, sboyd On Thu, Aug 12, 2021 at 07:11:55PM +0200, Thomas Gleixner wrote: > On Thu, Aug 12 2021 at 09:37, Paul E. McKenney wrote: > > > On Thu, Aug 12, 2021 at 03:46:42PM +0200, Thomas Gleixner wrote: > >> On Wed, Aug 11 2021 at 17:01, Paul E. McKenney wrote: > >> > This pull request contains a single change that prevents clocksource > >> > watchdog testing on systems with HZ < 100, thus preventing the integer > >> > underflow that can occur on leisurely HZed systems. This has been > >> > posted to LKML: > >> > > >> > https://lore.kernel.org/lkml/20210721212755.GA2066078@paulmck-ThinkPad-P17-Gen-1/ > >> > >> So with HZ < 100 .mult overflows, but why not simply adjusting the > >> mult, shift value to be > >> > >> .mult = TICK_NSEC, > >> .shift = 0, > >> > >> which is effectively the same as > >> > >> .mult = TICK_NSEC << 8, > >> .shift = 8, > >> > >> Hmm? > > > > Another option would be for me to be less lazy and to move this code: > > > > /* Since jiffies uses a simple TICK_NSEC multiplier > > * conversion, the .shift value could be zero. However > > * this would make NTP adjustments impossible as they are > > * in units of 1/2^.shift. Thus we use JIFFIES_SHIFT to > > * shift both the nominator and denominator the same > > * amount, and give ntp adjustments in units of 1/2^8 > > * > > * The value 8 is somewhat carefully chosen, as anything > > * larger can result in overflows. TICK_NSEC grows as HZ > > * shrinks, so values greater than 8 overflow 32bits when > > * HZ=100. > > */ > > #if HZ < 34 > > #define JIFFIES_SHIFT 6 > > #elif HZ < 67 > > #define JIFFIES_SHIFT 7 > > #else > > #define JIFFIES_SHIFT 8 > > #endif > > > > from kernel/time/jiffies.c to include/linux/clocksource.h. > > No need to expose this globaly. > > kernel/time/tick-internal.h or kernel/time/jiffies.h How about like this? Thanx, Paul ------------------------------------------------------------------------ clocksource-wdtest.c | 5 ++--- jiffies.c | 21 +-------------------- tick-internal.h | 20 ++++++++++++++++++++ 3 files changed, 23 insertions(+), 23 deletions(-) commit 2419f70beeb885ad96b832c64648e42559d7cf1d Author: Paul E. McKenney <paulmck@kernel.org> Date: Thu Aug 12 09:31:28 2021 -0700 clocksource: Make clocksource-wdtest.c safe for slow-HZ systems Currently, clocksource-wdtest.c sets a local JIFFIES_SHIFT macro for operation at HZ>=67, which can cause this test suite to fail on systems with HZ<67. Therefore, move the HZ-based definitions of JIFFIES_SHIFT from kernel/time/jiffies.c to kernel/time/tick-internal.h, allowing the local JIFFIES_SHIFT macro to be removed from clocksource-wdtest.c in favor of a properly HZ-based definition. This in turn makes clocksource-wdtest.c safe for slow-HZ systems. [ paulmck: Moved JIFFIES_SHIFT from include/linux/clocksource.h. ] Link: https://lore.kernel.org/lkml/20210812000133.GA402890@paulmck-ThinkPad-P17-Gen-1/ Cc: John Stultz <john.stultz@linaro.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Stephen Boyd <sboyd@kernel.org> Cc: Rong Chen <rong.a.chen@intel.com> Signed-off-by: Paul E. McKenney <paulmck@kernel.org> diff --git a/kernel/time/clocksource-wdtest.c b/kernel/time/clocksource-wdtest.c index b72a969f7b938..7e82500c400b9 100644 --- a/kernel/time/clocksource-wdtest.c +++ b/kernel/time/clocksource-wdtest.c @@ -19,6 +19,8 @@ #include <linux/prandom.h> #include <linux/cpu.h> +#include "tick-internal.h" + MODULE_LICENSE("GPL"); MODULE_AUTHOR("Paul E. McKenney <paulmck@kernel.org>"); @@ -34,9 +36,6 @@ static u64 wdtest_jiffies_read(struct clocksource *cs) return (u64)jiffies; } -/* Assume HZ > 100. */ -#define JIFFIES_SHIFT 8 - static struct clocksource clocksource_wdtest_jiffies = { .name = "wdtest-jiffies", .rating = 1, /* lowest valid rating*/ diff --git a/kernel/time/jiffies.c b/kernel/time/jiffies.c index 01935aafdb460..bc4db9e5ab70c 100644 --- a/kernel/time/jiffies.c +++ b/kernel/time/jiffies.c @@ -10,28 +10,9 @@ #include <linux/init.h> #include "timekeeping.h" +#include "tick-internal.h" -/* Since jiffies uses a simple TICK_NSEC multiplier - * conversion, the .shift value could be zero. However - * this would make NTP adjustments impossible as they are - * in units of 1/2^.shift. Thus we use JIFFIES_SHIFT to - * shift both the nominator and denominator the same - * amount, and give ntp adjustments in units of 1/2^8 - * - * The value 8 is somewhat carefully chosen, as anything - * larger can result in overflows. TICK_NSEC grows as HZ - * shrinks, so values greater than 8 overflow 32bits when - * HZ=100. - */ -#if HZ < 34 -#define JIFFIES_SHIFT 6 -#elif HZ < 67 -#define JIFFIES_SHIFT 7 -#else -#define JIFFIES_SHIFT 8 -#endif - static u64 jiffies_read(struct clocksource *cs) { return (u64) jiffies; diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h index 6a742a29e545f..5459bab2f4f70 100644 --- a/kernel/time/tick-internal.h +++ b/kernel/time/tick-internal.h @@ -165,3 +165,23 @@ DECLARE_PER_CPU(struct hrtimer_cpu_base, hrtimer_bases); extern u64 get_next_timer_interrupt(unsigned long basej, u64 basem); void timer_clear_idle(void); + +/* Since jiffies uses a simple TICK_NSEC multiplier + * conversion, the .shift value could be zero. However + * this would make NTP adjustments impossible as they are + * in units of 1/2^.shift. Thus we use JIFFIES_SHIFT to + * shift both the nominator and denominator the same + * amount, and give ntp adjustments in units of 1/2^8 + * + * The value 8 is somewhat carefully chosen, as anything + * larger can result in overflows. TICK_NSEC grows as HZ + * shrinks, so values greater than 8 overflow 32bits when + * HZ=100. + */ +#if HZ < 34 +#define JIFFIES_SHIFT 6 +#elif HZ < 67 +#define JIFFIES_SHIFT 7 +#else +#define JIFFIES_SHIFT 8 +#endif ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-08-12 20:01 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-12 0:01 [GIT PULL clocksource] Clocksource watchdog commits for v5.15 Paul E. McKenney 2021-08-12 13:46 ` Thomas Gleixner 2021-08-12 16:37 ` Paul E. McKenney 2021-08-12 17:11 ` Thomas Gleixner 2021-08-12 20:01 ` Paul E. McKenney
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).