[PATCH/RFC] Modify sched_clock() to make CONFIG_PRINTK_TIME more sane.

[PATCH/RFC] Modify sched_clock() to make CONFIG_PRINTK_TIME more sane.

[Tony Breeds]

When booting a current kernel with CONFIG_PRINTK_TIME enabled you'll
see messages like:

[    0.000000] time_init: decrementer frequency = 188.044000 MHz
[    0.000000] time_init: processor frequency   = 1504.352000 MHz
[3712914.436297] Console: colour dummy device 80x25

This patch modifies sched_clock() to report the offset since the machine booted
so the same printk's now look like:

[    0.000000] time_init: decrementer frequency = 188.044000 MHz
[    0.000000] time_init: processor frequency   = 1504.352000 MHz
[    0.000135] Console: colour dummy device 80x25

Effectivly including the uptime in printk()s.

Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>

---

 arch/powerpc/kernel/time.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Index: working/arch/powerpc/kernel/time.c
===================================================================
--- working.orig/arch/powerpc/kernel/time.c
+++ working/arch/powerpc/kernel/time.c
@@ -115,6 +115,7 @@ EXPORT_SYMBOL_GPL(rtc_lock);
 
 u64 tb_to_ns_scale;
 unsigned tb_to_ns_shift;
+unsigned long boot_tb;
 
 struct gettimeofday_struct do_gtod;
 
@@ -735,7 +736,7 @@ unsigned long long sched_clock(void)
 {
 	if (__USE_RTC())
 		return get_rtc();
-	return mulhdu(get_tb(), tb_to_ns_scale) << tb_to_ns_shift;
+	return mulhdu(get_tb() - boot_tb, tb_to_ns_scale) << tb_to_ns_shift;
 }
 
 int do_settimeofday(struct timespec *tv)
@@ -960,6 +961,8 @@ void __init time_init(void)
 	}
 	tb_to_ns_scale = scale;
 	tb_to_ns_shift = shift;
+	/* Save the current timebase to pretty up CONFIG_PRINTK_TIME */
+	boot_tb = get_tb();
 
 	tm = get_boot_time();
 
Yours Tony

  linux.conf.au        http://linux.conf.au/ || http://lca2008.linux.org.au/
  Jan 28 - Feb 02 2008 The Australian Linux Technical Conference!

Re: [PATCH/RFC] Modify sched_clock() to make CONFIG_PRINTK_TIME more sane.

[Segher Boessenkool]

> When booting a current kernel with CONFIG_PRINTK_TIME enabled you'll
> see messages like:
>
> [    0.000000] time_init: decrementer frequency = 188.044000 MHz
> [    0.000000] time_init: processor frequency   = 1504.352000 MHz
> [3712914.436297] Console: colour dummy device 80x25
>
> This patch modifies sched_clock() to report the offset since the 
> machine booted
> so the same printk's now look like:
>
> [    0.000000] time_init: decrementer frequency = 188.044000 MHz
> [    0.000000] time_init: processor frequency   = 1504.352000 MHz
> [    0.000135] Console: colour dummy device 80x25
>
> Effectivly including the uptime in printk()s.

This was long overdue, thank you Tony!


Segher

Re: [PATCH/RFC] Modify sched_clock() to make CONFIG_PRINTK_TIME more sane.

[Olof Johansson]

Hi,

On Tue, Jun 19, 2007 at 04:35:57PM +1000, Tony Breeds wrote:
> When booting a current kernel with CONFIG_PRINTK_TIME enabled you'll
> see messages like:
> 
> [    0.000000] time_init: decrementer frequency = 188.044000 MHz
> [    0.000000] time_init: processor frequency   = 1504.352000 MHz
> [3712914.436297] Console: colour dummy device 80x25
> 
> This patch modifies sched_clock() to report the offset since the machine booted
> so the same printk's now look like:
> 
> [    0.000000] time_init: decrementer frequency = 188.044000 MHz
> [    0.000000] time_init: processor frequency   = 1504.352000 MHz
> [    0.000135] Console: colour dummy device 80x25
> 
> Effectivly including the uptime in printk()s.

Nice!

> Index: working/arch/powerpc/kernel/time.c
> ===================================================================
> --- working.orig/arch/powerpc/kernel/time.c
> +++ working/arch/powerpc/kernel/time.c
> @@ -115,6 +115,7 @@ EXPORT_SYMBOL_GPL(rtc_lock);
>  
>  u64 tb_to_ns_scale;
>  unsigned tb_to_ns_shift;
> +unsigned long boot_tb;

It only seems to be used in this file, so it can be static, right?


-Olof

Re: [PATCH/RFC] Modify sched_clock() to make CONFIG_PRINTK_TIME more sane.

[Tony Breeds]

On Tue, Jun 19, 2007 at 09:53:56AM -0500, Olof Johansson wrote:
 
> It only seems to be used in this file, so it can be static, right?

True, I was following suit with tb_to_ns_scale, tb_to_ns_shift which
could also be static.

Here's an updated patch that makes them all static and updates the
commit message as well.

From: Tony Breeds <tony@bakeyournoodle.com>

Modify sched_clock() to make CONFIG_PRINTK_TIME more sane.

When booting a current kernel with CONFIG_PRINTK_TIME enabled you'll
see messages like:

[    0.000000] time_init: decrementer frequency = 188.044000 MHz
[    0.000000] time_init: processor frequency   = 1504.352000 MHz
[3712914.436297] Console: colour dummy device 80x25

This cause by the initialisation of tb_to_ns_scale in time_init(), suddenly the
multiplication in sched_clock() now does something :).  This patch modifies
sched_clock() to report the offset since the machine booted so the same
printk's now look like:

[    0.000000] time_init: decrementer frequency = 188.044000 MHz
[    0.000000] time_init: processor frequency   = 1504.352000 MHz
[    0.000135] Console: colour dummy device 80x25

Effectively including the uptime in printk()s.

Also this patch makes tb_to_ns_scale and tb_to_ns_shift static for good
measure.

Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>

---
There looks to be other variables that could be made static, I think
that's a job for another day though.

 arch/powerpc/kernel/time.c |    9 ++++++---
  1 file changed, 6 insertions(+), 3 deletions(-)

Index: working/arch/powerpc/kernel/time.c
===================================================================
--- working.orig/arch/powerpc/kernel/time.c
+++ working/arch/powerpc/kernel/time.c
@@ -113,8 +113,9 @@ u64 ticklen_to_xs;	/* 0.64 fraction */
 DEFINE_SPINLOCK(rtc_lock);
 EXPORT_SYMBOL_GPL(rtc_lock);
 
-u64 tb_to_ns_scale;
-unsigned tb_to_ns_shift;
+static u64 tb_to_ns_scale;
+static unsigned tb_to_ns_shift;
+static unsigned long boot_tb;
 
 struct gettimeofday_struct do_gtod;
 
@@ -735,7 +736,7 @@ unsigned long long sched_clock(void)
 {
 	if (__USE_RTC())
 		return get_rtc();
-	return mulhdu(get_tb(), tb_to_ns_scale) << tb_to_ns_shift;
+	return mulhdu(get_tb() - boot_tb, tb_to_ns_scale) << tb_to_ns_shift;
 }
 
 int do_settimeofday(struct timespec *tv)
@@ -960,6 +961,8 @@ void __init time_init(void)
 	}
 	tb_to_ns_scale = scale;
 	tb_to_ns_shift = shift;
+	/* Save the current timebase to pretty up CONFIG_PRINTK_TIME */
+	boot_tb = get_tb();
 
 	tm = get_boot_time();
 
Yours Tony

  linux.conf.au        http://linux.conf.au/ || http://lca2008.linux.org.au/
  Jan 28 - Feb 02 2008 The Australian Linux Technical Conference!

Re: [PATCH/RFC] Modify sched_clock() to make CONFIG_PRINTK_TIME more sane.

[Olof Johansson]

On Wed, Jun 20, 2007 at 12:02:11PM +1000, Tony Breeds wrote:
> On Tue, Jun 19, 2007 at 09:53:56AM -0500, Olof Johansson wrote:
>  
> > It only seems to be used in this file, so it can be static, right?
> 
> True, I was following suit with tb_to_ns_scale, tb_to_ns_shift which
> could also be static.

I wasn't sure of those since I was only looking at the patch in question,
not the full source file. But yes, that's the right thing to do.

> Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>

Acked-by: Olof Johansson <olof@lixom.net>


-Olof

[PATCH/RFC] Make certain timekeeping variables __read_mostly

[Tony Breeds]

On Wed, Jun 20, 2007 at 12:02:11PM +1000, Tony Breeds wrote:
 
> Also this patch makes tb_to_ns_scale and tb_to_ns_shift static for good
> measure.

While we're ate it these 3 variables can be marked __read_mostly.

Depends on previous patch.

From: Tony Breeds <tony@bakeyournoodle.com>

These really are read mostly as they're only written to in time_init()

Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>

---
There looks to be other variables that can be made read_mostly, I think
that's a job for another day though (perhaps while doing the static cleanup).

 arch/powerpc/kernel/time.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Index: working/arch/powerpc/kernel/time.c
===================================================================
--- working.orig/arch/powerpc/kernel/time.c	2007-06-20 12:08:27.000000000 +1000
+++ working/arch/powerpc/kernel/time.c	2007-06-20 12:08:32.000000000 +1000
@@ -113,9 +113,9 @@ u64 ticklen_to_xs;	/* 0.64 fraction */
 DEFINE_SPINLOCK(rtc_lock);
 EXPORT_SYMBOL_GPL(rtc_lock);
 
-static u64 tb_to_ns_scale;
-static unsigned tb_to_ns_shift;
-static unsigned long boot_tb;
+static u64 tb_to_ns_scale __read_mostly;
+static unsigned tb_to_ns_shift __read_mostly;
+static unsigned long boot_tb __read_mostly;
 
 struct gettimeofday_struct do_gtod;
 

Yours Tony

  linux.conf.au        http://linux.conf.au/ || http://lca2008.linux.org.au/
  Jan 28 - Feb 02 2008 The Australian Linux Technical Conference!

Re: [PATCH/RFC] Make certain timekeeping variables __read_mostly

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 970 bytes --]

On Wed, 20 Jun 2007, Tony Breeds wrote:
> On Wed, Jun 20, 2007 at 12:02:11PM +1000, Tony Breeds wrote:
> > Also this patch makes tb_to_ns_scale and tb_to_ns_shift static for good
> > measure.
> 
> While we're ate it these 3 variables can be marked __read_mostly.

There's no __read_mostly support for powerpc yet (is there?), so __read_mostly
just expands to nothing?

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

Re: [PATCH/RFC] Make certain timekeeping variables __read_mostly

[Tony Breeds]

On Wed, Jun 20, 2007 at 09:41:47AM +0200, Geert Uytterhoeven wrote:

Hi Geert,
 
> There's no __read_mostly support for powerpc yet (is there?), so __read_mostly
> just expands to nothing?

That's true but we may in the future! :)

Perhaps something like the patch below will start discussion.

From: Tony Breeds <tony@bakeyournoodle.com>

Initial cut to add __read_mostly support for powerpc.

Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>

---

 arch/powerpc/kernel/vmlinux.lds.S |    6 ++++++
 include/asm-powerpc/cache.h       |    2 ++
 2 files changed, 8 insertions(+)

Index: working/arch/powerpc/kernel/vmlinux.lds.S
===================================================================
--- working.orig/arch/powerpc/kernel/vmlinux.lds.S
+++ working/arch/powerpc/kernel/vmlinux.lds.S
@@ -7,6 +7,7 @@
 #define PROVIDE32(x)	PROVIDE(x)
 #endif
 #include <asm-generic/vmlinux.lds.h>
+#include <asm/cache.h>
 
 ENTRY(_stext)
 
@@ -211,6 +212,11 @@ SECTIONS
 		*(.data.cacheline_aligned)
 	}
 
+	. = ALIGN(L1_CACHE_BYTES);
+	.data.read_mostly : {
+		*(.data.read_mostly)
+	}
+
 	. = ALIGN(PAGE_SIZE);
 	__data_nosave : {
 		__nosave_begin = .;
Index: working/include/asm-powerpc/cache.h
===================================================================
--- working.orig/include/asm-powerpc/cache.h
+++ working/include/asm-powerpc/cache.h
@@ -34,5 +34,9 @@ struct ppc64_caches {
 extern struct ppc64_caches ppc64_caches;
 #endif /* __powerpc64__ && ! __ASSEMBLY__ */
 
+#if !defined(__ASSEMBLY__)
+#define __read_mostly __attribute__((__section__(".data.read_mostly")))
+#endif
+
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_CACHE_H */

Yours Tony

  linux.conf.au        http://linux.conf.au/ || http://lca2008.linux.org.au/
  Jan 28 - Feb 02 2008 The Australian Linux Technical Conference!

Re: [PATCH/RFC] Modify sched_clock() to make CONFIG_PRINTK_TIME more sane.

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2189 bytes --]

On Wed, 20 Jun 2007, Tony Breeds wrote:
> On Tue, Jun 19, 2007 at 09:53:56AM -0500, Olof Johansson wrote:
> > It only seems to be used in this file, so it can be static, right?
> 
> True, I was following suit with tb_to_ns_scale, tb_to_ns_shift which
> could also be static.
> 
> Here's an updated patch that makes them all static and updates the
> commit message as well.
> 
> From: Tony Breeds <tony@bakeyournoodle.com>
> 
> Modify sched_clock() to make CONFIG_PRINTK_TIME more sane.
> 
> When booting a current kernel with CONFIG_PRINTK_TIME enabled you'll
> see messages like:
> 
> [    0.000000] time_init: decrementer frequency = 188.044000 MHz
> [    0.000000] time_init: processor frequency   = 1504.352000 MHz
> [3712914.436297] Console: colour dummy device 80x25
> 
> This cause by the initialisation of tb_to_ns_scale in time_init(), suddenly the
> multiplication in sched_clock() now does something :).  This patch modifies
> sched_clock() to report the offset since the machine booted so the same
> printk's now look like:
> 
> [    0.000000] time_init: decrementer frequency = 188.044000 MHz
> [    0.000000] time_init: processor frequency   = 1504.352000 MHz
> [    0.000135] Console: colour dummy device 80x25
> 
> Effectively including the uptime in printk()s.

Just wondering, does the INITIAL_JIFFIES mechanism to catch wrap bugs still
work after this patch?

include/linux/jiffies.h:
| /*
|  * Have the 32 bit jiffies value wrap 5 minutes after boot
|  * so jiffies wrap bugs show up earlier.
|  */
| #define INITIAL_JIFFIES ((unsigned long)(unsigned int) (-300*HZ))

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

Re: [PATCH/RFC] Modify sched_clock() to make CONFIG_PRINTK_TIME more sane.

[Tony Breeds]

On Wed, Jun 20, 2007 at 09:37:51AM +0200, Geert Uytterhoeven wrote:

Hi Geert,
 
> Just wondering, does the INITIAL_JIFFIES mechanism to catch wrap bugs still
> work after this patch?
> 
> include/linux/jiffies.h:
> | /*
> |  * Have the 32 bit jiffies value wrap 5 minutes after boot
> |  * so jiffies wrap bugs show up earlier.
> |  */
> | #define INITIAL_JIFFIES ((unsigned long)(unsigned int) (-300*HZ))

I've done a quick check and I can't see how my patch would break
anything,  given that sched_clock() on powerpc returns a timebase value
generally not related to jiffies.

I hope I haven't missed anything.

Yours Tony

  linux.conf.au        http://linux.conf.au/ || http://lca2008.linux.org.au/
  Jan 28 - Feb 02 2008 The Australian Linux Technical Conference!