LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@kernel.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org, john.stultz@linaro.org,
	kernel-team@fb.com, ak@linux.intel.com, rong.a.chen@intel.com,
	sboyd@kernel.org
Subject: Re: [GIT PULL clocksource] Clocksource watchdog commits for v5.15
Date: Thu, 12 Aug 2021 09:37:53 -0700	[thread overview]
Message-ID: <20210812163753.GW4126399@paulmck-ThinkPad-P17-Gen-1> (raw)
In-Reply-To: <87czqiixml.ffs@tglx>

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;

  reply	other threads:[~2021-08-12 16:38 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-12  0:01 Paul E. McKenney
2021-08-12 13:46 ` Thomas Gleixner
2021-08-12 16:37   ` Paul E. McKenney [this message]
2021-08-12 17:11     ` Thomas Gleixner
2021-08-12 20:01       ` Paul E. McKenney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210812163753.GW4126399@paulmck-ThinkPad-P17-Gen-1 \
    --to=paulmck@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=john.stultz@linaro.org \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rong.a.chen@intel.com \
    --cc=sboyd@kernel.org \
    --cc=tglx@linutronix.de \
    --subject='Re: [GIT PULL clocksource] Clocksource watchdog commits for v5.15' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).