Re: [GIT PULL clocksource] Clocksource watchdog commits for v5.15

From: Paul E. McKenney
Date: Thu Aug 12 2021 - 12:38:01 EST


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@xxxxxxxxxx>
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@xxxxxxxxxx>
Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Cc: Stephen Boyd <sboyd@xxxxxxxxxx>
Cc: Rong Chen <rong.a.chen@xxxxxxxxx>
Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxx>

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;