* [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 read-only IMAP folder(s) and NNTP newsgroup(s).