linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] sched_clock: Fixes for frequency reporting
@ 2022-04-24 11:47 Maciej W. Rozycki
  2022-04-24 11:47 ` [PATCH 1/3] sched_clock: Round the frequency reported to nearest rather than down Maciej W. Rozycki
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Maciej W. Rozycki @ 2022-04-24 11:47 UTC (permalink / raw)
  To: John Stultz, Thomas Gleixner, Stephen Boyd, Russell King, Ingo Molnar
  Cc: linux-kernel

Hi,

 Hmm, kernel/time/sched_clock.c lacks a MAINTAINERS entry, but I guess it 
goes along with either high-resolution timers or clocksource core.  Please 
advise otherwise.

 In the course of reviewing the MIPS part of the `random_get_entropy' 
patch set recently posted I have noticed the odd rounding mode used for 
clock source frequency reporting, and then upon inspecting code involved 
I've come across a couple of further small issues.

 This patch set addresses these issues.  Please see individual change 
descriptions for details.

  Maciej

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] sched_clock: Round the frequency reported to nearest rather than down
  2022-04-24 11:47 [PATCH 0/3] sched_clock: Fixes for frequency reporting Maciej W. Rozycki
@ 2022-04-24 11:47 ` Maciej W. Rozycki
  2022-04-26 20:38   ` John Stultz
  2022-05-02 12:31   ` [tip: timers/core] time/sched_clock: " tip-bot2 for Maciej W. Rozycki
  2022-04-24 11:47 ` [PATCH 2/3] sched_clock: Use Hz as the unit for clock rate reporting below 4kHz Maciej W. Rozycki
  2022-04-24 11:47 ` [PATCH 3/3] sched_clock: Fix formatting of frequency reporting code Maciej W. Rozycki
  2 siblings, 2 replies; 9+ messages in thread
From: Maciej W. Rozycki @ 2022-04-24 11:47 UTC (permalink / raw)
  To: John Stultz, Thomas Gleixner, Stephen Boyd, Russell King, Ingo Molnar
  Cc: linux-kernel

We currently round the frequency reported for clock sources down, which 
gives misleading figures, e.g.:

I/O ASIC clock frequency 24999480Hz
clocksource: dec-ioasic: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 76452008078 ns
sched_clock: 32 bits at 24MHz, resolution 40ns, wraps every 85901132779ns
MIPS counter frequency 59998512Hz
clocksource: MIPS: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 31855130776 ns
sched_clock: 32 bits at 59MHz, resolution 16ns, wraps every 35792281591ns

Rounding to nearest seems more adequate:

I/O ASIC clock frequency 24999664Hz
clocksource: dec-ioasic: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 76451445358 ns
sched_clock: 32 bits at 25MHz, resolution 40ns, wraps every 85900499947ns
MIPS counter frequency 59999728Hz
clocksource: MIPS: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 31854485176 ns
sched_clock: 32 bits at 60MHz, resolution 16ns, wraps every 35791556599ns

Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Fixes: 112f38a4a316 ("ARM: sched_clock: provide common infrastructure for sched_clock()")
---
 kernel/time/sched_clock.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

linux-sched-clock-rate-round.diff
Index: linux-macro/kernel/time/sched_clock.c
===================================================================
--- linux-macro.orig/kernel/time/sched_clock.c
+++ linux-macro/kernel/time/sched_clock.c
@@ -8,6 +8,7 @@
 #include <linux/jiffies.h>
 #include <linux/ktime.h>
 #include <linux/kernel.h>
+#include <linux/math.h>
 #include <linux/moduleparam.h>
 #include <linux/sched.h>
 #include <linux/sched/clock.h>
@@ -199,11 +200,11 @@ sched_clock_register(u64 (*read)(void),
 
 	r = rate;
 	if (r >= 4000000) {
-		r /= 1000000;
+		r = DIV_ROUND_CLOSEST(r, 1000000);
 		r_unit = 'M';
 	} else {
 		if (r >= 1000) {
-			r /= 1000;
+			r = DIV_ROUND_CLOSEST(r, 1000);
 			r_unit = 'k';
 		} else {
 			r_unit = ' ';

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/3] sched_clock: Use Hz as the unit for clock rate reporting below 4kHz
  2022-04-24 11:47 [PATCH 0/3] sched_clock: Fixes for frequency reporting Maciej W. Rozycki
  2022-04-24 11:47 ` [PATCH 1/3] sched_clock: Round the frequency reported to nearest rather than down Maciej W. Rozycki
@ 2022-04-24 11:47 ` Maciej W. Rozycki
  2022-05-02 12:31   ` [tip: timers/core] time/sched_clock: " tip-bot2 for Maciej W. Rozycki
  2022-04-24 11:47 ` [PATCH 3/3] sched_clock: Fix formatting of frequency reporting code Maciej W. Rozycki
  2 siblings, 1 reply; 9+ messages in thread
From: Maciej W. Rozycki @ 2022-04-24 11:47 UTC (permalink / raw)
  To: John Stultz, Thomas Gleixner, Stephen Boyd, Russell King, Ingo Molnar
  Cc: linux-kernel

We have always used kHz as the unit for clock rates reported between 
1MHz (inclusive) and 4MHz (exclusive), e.g.:

riscv_timer_init_dt: Registering clocksource cpuid [0] hartid [1]
clocksource: riscv_clocksource: mask: 0xffffffffffffffff max_cycles: 0x1d854df40, max_idle_ns: 3526361616960 ns
sched_clock: 64 bits at 1000kHz, resolution 1000ns, wraps every 2199023255500ns

This reduces the amount of data lost due to rounding, but hasn't been 
replicated for the kHz range when support was added for proper reporting 
of sub-kHz clock rates.  Take the same approach then for rates between 
1kHz (inclusive) and 4kHz (exclusive), also for consistency.

Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Fixes: 2f0778afac79 ("ARM: 7205/2: sched_clock: allow sched_clock to be selected at runtime")
---
Russell,

 Please correct me if I am wrong with my guess for the origin of the 4MHz 
boundary as there's no mention of it with your commit 112f38a4a316 ("ARM: 
sched_clock: provide common infrastructure for sched_clock()"), which is 
where this code has come from.

  Maciej
---
 kernel/time/sched_clock.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

linux-sched-clock-rate-4khz.diff
Index: linux-macro/kernel/time/sched_clock.c
===================================================================
--- linux-macro.orig/kernel/time/sched_clock.c
+++ linux-macro/kernel/time/sched_clock.c
@@ -203,7 +203,7 @@ sched_clock_register(u64 (*read)(void),
 		r = DIV_ROUND_CLOSEST(r, 1000000);
 		r_unit = 'M';
 	} else {
-		if (r >= 1000) {
+		if (r >= 4000) {
 			r = DIV_ROUND_CLOSEST(r, 1000);
 			r_unit = 'k';
 		} else {

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/3] sched_clock: Fix formatting of frequency reporting code
  2022-04-24 11:47 [PATCH 0/3] sched_clock: Fixes for frequency reporting Maciej W. Rozycki
  2022-04-24 11:47 ` [PATCH 1/3] sched_clock: Round the frequency reported to nearest rather than down Maciej W. Rozycki
  2022-04-24 11:47 ` [PATCH 2/3] sched_clock: Use Hz as the unit for clock rate reporting below 4kHz Maciej W. Rozycki
@ 2022-04-24 11:47 ` Maciej W. Rozycki
  2022-04-26 20:41   ` John Stultz
  2022-05-02 12:31   ` [tip: timers/core] time/sched_clock: " tip-bot2 for Maciej W. Rozycki
  2 siblings, 2 replies; 9+ messages in thread
From: Maciej W. Rozycki @ 2022-04-24 11:47 UTC (permalink / raw)
  To: John Stultz, Thomas Gleixner, Stephen Boyd, Russell King, Ingo Molnar
  Cc: linux-kernel

Use flat rather than nested indentation for chained else/if clauses as 
per coding-style.rst:

	if (x == y) {
		..
	} else if (x > y) {
		...
	} else {
		....
	}

This also improves readability.

Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Fixes: 32fea568aec5b ("timers, sched/clock: Clean up the code a bit")
---
Hi,

 I guess this got broken with 32fea568aec5b by mistake.

  Maciej
---
 kernel/time/sched_clock.c |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

linux-sched-clock-rate-cond.diff
Index: linux-macro/kernel/time/sched_clock.c
===================================================================
--- linux-macro.orig/kernel/time/sched_clock.c
+++ linux-macro/kernel/time/sched_clock.c
@@ -202,13 +202,11 @@ sched_clock_register(u64 (*read)(void),
 	if (r >= 4000000) {
 		r = DIV_ROUND_CLOSEST(r, 1000000);
 		r_unit = 'M';
+	} else if (r >= 4000) {
+		r = DIV_ROUND_CLOSEST(r, 1000);
+		r_unit = 'k';
 	} else {
-		if (r >= 4000) {
-			r = DIV_ROUND_CLOSEST(r, 1000);
-			r_unit = 'k';
-		} else {
-			r_unit = ' ';
-		}
+		r_unit = ' ';
 	}
 
 	/* Calculate the ns resolution of this counter */

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] sched_clock: Round the frequency reported to nearest rather than down
  2022-04-24 11:47 ` [PATCH 1/3] sched_clock: Round the frequency reported to nearest rather than down Maciej W. Rozycki
@ 2022-04-26 20:38   ` John Stultz
  2022-05-02 12:31   ` [tip: timers/core] time/sched_clock: " tip-bot2 for Maciej W. Rozycki
  1 sibling, 0 replies; 9+ messages in thread
From: John Stultz @ 2022-04-26 20:38 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Thomas Gleixner, Stephen Boyd, Russell King, Ingo Molnar, linux-kernel

On Sun, Apr 24, 2022 at 4:47 AM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>
> We currently round the frequency reported for clock sources down, which
> gives misleading figures, e.g.:
>
> I/O ASIC clock frequency 24999480Hz
> clocksource: dec-ioasic: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 76452008078 ns
> sched_clock: 32 bits at 24MHz, resolution 40ns, wraps every 85901132779ns
> MIPS counter frequency 59998512Hz
> clocksource: MIPS: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 31855130776 ns
> sched_clock: 32 bits at 59MHz, resolution 16ns, wraps every 35792281591ns
>
> Rounding to nearest seems more adequate:
>
> I/O ASIC clock frequency 24999664Hz
> clocksource: dec-ioasic: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 76451445358 ns
> sched_clock: 32 bits at 25MHz, resolution 40ns, wraps every 85900499947ns
> MIPS counter frequency 59999728Hz
> clocksource: MIPS: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 31854485176 ns
> sched_clock: 32 bits at 60MHz, resolution 16ns, wraps every 35791556599ns
>
> Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
> Fixes: 112f38a4a316 ("ARM: sched_clock: provide common infrastructure for sched_clock()")
> ---

This seems sane to me.

Acked-by: John Stultz <jstultz@google.com>

thanks
-john

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] sched_clock: Fix formatting of frequency reporting code
  2022-04-24 11:47 ` [PATCH 3/3] sched_clock: Fix formatting of frequency reporting code Maciej W. Rozycki
@ 2022-04-26 20:41   ` John Stultz
  2022-05-02 12:31   ` [tip: timers/core] time/sched_clock: " tip-bot2 for Maciej W. Rozycki
  1 sibling, 0 replies; 9+ messages in thread
From: John Stultz @ 2022-04-26 20:41 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Thomas Gleixner, Stephen Boyd, Russell King, Ingo Molnar, linux-kernel

On Sun, Apr 24, 2022 at 4:47 AM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>
> Use flat rather than nested indentation for chained else/if clauses as
> per coding-style.rst:
>
>         if (x == y) {
>                 ..
>         } else if (x > y) {
>                 ...
>         } else {
>                 ....
>         }
>
> This also improves readability.
>
> Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
> Fixes: 32fea568aec5b ("timers, sched/clock: Clean up the code a bit")

This patch seems fine to me. Though as Ingo was the one to introduce
the change, his style preference may override in this case.

Acked-by: John Stultz <jstultz@google.com>

thanks
-john

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [tip: timers/core] time/sched_clock: Fix formatting of frequency reporting code
  2022-04-24 11:47 ` [PATCH 3/3] sched_clock: Fix formatting of frequency reporting code Maciej W. Rozycki
  2022-04-26 20:41   ` John Stultz
@ 2022-05-02 12:31   ` tip-bot2 for Maciej W. Rozycki
  1 sibling, 0 replies; 9+ messages in thread
From: tip-bot2 for Maciej W. Rozycki @ 2022-05-02 12:31 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Maciej W. Rozycki, Thomas Gleixner, John Stultz, x86, linux-kernel

The following commit has been merged into the timers/core branch of tip:

Commit-ID:     f4b62e1e1137507268c2c63dc4e6da279dc58e9f
Gitweb:        https://git.kernel.org/tip/f4b62e1e1137507268c2c63dc4e6da279dc58e9f
Author:        Maciej W. Rozycki <macro@orcam.me.uk>
AuthorDate:    Sun, 24 Apr 2022 12:47:30 +01:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Mon, 02 May 2022 14:29:04 +02:00

time/sched_clock: Fix formatting of frequency reporting code

Use flat rather than nested indentation for chained else/if clauses as 
per coding-style.rst:

	if (x == y) {
		..
	} else if (x > y) {
		...
	} else {
		....
	}

This also improves readability.

Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: John Stultz <jstultz@google.com>
Link: https://lore.kernel.org/r/alpine.DEB.2.21.2204240148220.9383@angie.orcam.me.uk

---
 kernel/time/sched_clock.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
index 4a95c0b..8464c5a 100644
--- a/kernel/time/sched_clock.c
+++ b/kernel/time/sched_clock.c
@@ -202,13 +202,11 @@ sched_clock_register(u64 (*read)(void), int bits, unsigned long rate)
 	if (r >= 4000000) {
 		r = DIV_ROUND_CLOSEST(r, 1000000);
 		r_unit = 'M';
+	} else if (r >= 4000) {
+		r = DIV_ROUND_CLOSEST(r, 1000);
+		r_unit = 'k';
 	} else {
-		if (r >= 4000) {
-			r = DIV_ROUND_CLOSEST(r, 1000);
-			r_unit = 'k';
-		} else {
-			r_unit = ' ';
-		}
+		r_unit = ' ';
 	}
 
 	/* Calculate the ns resolution of this counter */

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [tip: timers/core] time/sched_clock: Use Hz as the unit for clock rate reporting below 4kHz
  2022-04-24 11:47 ` [PATCH 2/3] sched_clock: Use Hz as the unit for clock rate reporting below 4kHz Maciej W. Rozycki
@ 2022-05-02 12:31   ` tip-bot2 for Maciej W. Rozycki
  0 siblings, 0 replies; 9+ messages in thread
From: tip-bot2 for Maciej W. Rozycki @ 2022-05-02 12:31 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Maciej W. Rozycki, Thomas Gleixner, x86, linux-kernel

The following commit has been merged into the timers/core branch of tip:

Commit-ID:     cc1b923a4e378c943386e7fe6205918d43e5fede
Gitweb:        https://git.kernel.org/tip/cc1b923a4e378c943386e7fe6205918d43e5fede
Author:        Maciej W. Rozycki <macro@orcam.me.uk>
AuthorDate:    Sun, 24 Apr 2022 12:47:26 +01:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Mon, 02 May 2022 14:29:04 +02:00

time/sched_clock: Use Hz as the unit for clock rate reporting below 4kHz

The kernel uses kHz as the unit for clock rates reported between 1MHz
(inclusive) and 4MHz (exclusive), e.g.:

 sched_clock: 64 bits at 1000kHz, resolution 1000ns, wraps every 2199023255500ns

This reduces the amount of data lost due to rounding, but hasn't been
replicated for the kHz range when support was added for proper reporting of
sub-kHz clock rates.  Take the same approach for rates between 1kHz
(inclusive) and 4kHz (exclusive), which makes it consistent.

Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/r/alpine.DEB.2.21.2204240106380.9383@angie.orcam.me.uk

---
 kernel/time/sched_clock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
index ee07f3a..4a95c0b 100644
--- a/kernel/time/sched_clock.c
+++ b/kernel/time/sched_clock.c
@@ -203,7 +203,7 @@ sched_clock_register(u64 (*read)(void), int bits, unsigned long rate)
 		r = DIV_ROUND_CLOSEST(r, 1000000);
 		r_unit = 'M';
 	} else {
-		if (r >= 1000) {
+		if (r >= 4000) {
 			r = DIV_ROUND_CLOSEST(r, 1000);
 			r_unit = 'k';
 		} else {

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [tip: timers/core] time/sched_clock: Round the frequency reported to nearest rather than down
  2022-04-24 11:47 ` [PATCH 1/3] sched_clock: Round the frequency reported to nearest rather than down Maciej W. Rozycki
  2022-04-26 20:38   ` John Stultz
@ 2022-05-02 12:31   ` tip-bot2 for Maciej W. Rozycki
  1 sibling, 0 replies; 9+ messages in thread
From: tip-bot2 for Maciej W. Rozycki @ 2022-05-02 12:31 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Maciej W. Rozycki, Thomas Gleixner, John Stultz, x86, linux-kernel

The following commit has been merged into the timers/core branch of tip:

Commit-ID:     92067440f1311dfa4d77b57a9da6b3706f5da32e
Gitweb:        https://git.kernel.org/tip/92067440f1311dfa4d77b57a9da6b3706f5da32e
Author:        Maciej W. Rozycki <macro@orcam.me.uk>
AuthorDate:    Sun, 24 Apr 2022 12:47:20 +01:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Mon, 02 May 2022 14:29:04 +02:00

time/sched_clock: Round the frequency reported to nearest rather than down

The frequency reported for clock sources are rounded down, which gives
misleading figures, e.g.:

 I/O ASIC clock frequency 24999480Hz
 sched_clock: 32 bits at 24MHz, resolution 40ns, wraps every 85901132779ns
 MIPS counter frequency 59998512Hz
 sched_clock: 32 bits at 59MHz, resolution 16ns, wraps every 35792281591ns

Rounding to nearest is more adequate:

 I/O ASIC clock frequency 24999664Hz
 sched_clock: 32 bits at 25MHz, resolution 40ns, wraps every 85900499947ns
 MIPS counter frequency 59999728Hz
 sched_clock: 32 bits at 60MHz, resolution 16ns, wraps every 35791556599ns

Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: John Stultz <jstultz@google.com>
Link: https://lore.kernel.org/r/alpine.DEB.2.21.2204240055590.9383@angie.orcam.me.uk

---
 kernel/time/sched_clock.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
index b1b9b12..ee07f3a 100644
--- a/kernel/time/sched_clock.c
+++ b/kernel/time/sched_clock.c
@@ -8,6 +8,7 @@
 #include <linux/jiffies.h>
 #include <linux/ktime.h>
 #include <linux/kernel.h>
+#include <linux/math.h>
 #include <linux/moduleparam.h>
 #include <linux/sched.h>
 #include <linux/sched/clock.h>
@@ -199,11 +200,11 @@ sched_clock_register(u64 (*read)(void), int bits, unsigned long rate)
 
 	r = rate;
 	if (r >= 4000000) {
-		r /= 1000000;
+		r = DIV_ROUND_CLOSEST(r, 1000000);
 		r_unit = 'M';
 	} else {
 		if (r >= 1000) {
-			r /= 1000;
+			r = DIV_ROUND_CLOSEST(r, 1000);
 			r_unit = 'k';
 		} else {
 			r_unit = ' ';

^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-05-02 12:31 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-24 11:47 [PATCH 0/3] sched_clock: Fixes for frequency reporting Maciej W. Rozycki
2022-04-24 11:47 ` [PATCH 1/3] sched_clock: Round the frequency reported to nearest rather than down Maciej W. Rozycki
2022-04-26 20:38   ` John Stultz
2022-05-02 12:31   ` [tip: timers/core] time/sched_clock: " tip-bot2 for Maciej W. Rozycki
2022-04-24 11:47 ` [PATCH 2/3] sched_clock: Use Hz as the unit for clock rate reporting below 4kHz Maciej W. Rozycki
2022-05-02 12:31   ` [tip: timers/core] time/sched_clock: " tip-bot2 for Maciej W. Rozycki
2022-04-24 11:47 ` [PATCH 3/3] sched_clock: Fix formatting of frequency reporting code Maciej W. Rozycki
2022-04-26 20:41   ` John Stultz
2022-05-02 12:31   ` [tip: timers/core] time/sched_clock: " tip-bot2 for Maciej W. Rozycki

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