arm: Replace arm sched_clock by clocksource based sched_clock

Index: linux-2.6-tip/kernel/time/Kconfig
===================================================================

kernel/time/Kconfig +4 -0

@@ -27,3 +27,7 @@ config GENERIC_CLOCKEVENTS_BUILD
 	default y
 	depends on GENERIC_CLOCKEVENTS || GENERIC_CLOCKEVENTS_MIGR
 
+# Selectable by architectures which want to reuse the clocksource as
+# sched_clock
+config HAVE_CLKSRC_SCHED_CLOCK
+       bool

Quoted 8 lines · Thomas Gleixner

Index: linux-2.6-tip/kernel/time/Kconfig
===================================================================

kernel/time/Kconfig +0 -0

@@ -27,3 +27,7 @@ config GENERIC_CLOCKEVENTS_BUILD
 	default y
 	depends on GENERIC_CLOCKEVENTS || GENERIC_CLOCKEVENTS_MIGR
 

Quoted 6 lines

kernel/time/Kconfig +4 -0

+# Selectable by architectures which want to reuse the clocksource as
+# sched_clock
+config HAVE_CLKSRC_SCHED_CLOCK
+       bool

From what I can tell this isn't used so I'm guessing we can drop it?

Many embedded platforms use the clocksource which is used for timekeeping as their sched_clock as well. Though they need to take care about the wraparounds as sched_clock() wants to be monotonic increasing nsescs. The timekeeping code and the NOHZ core take care of the timekeeping clocksource wraparound so we can simply use the timekeeping clocksource if possible.

clocksources which can be used by the clksrc sched_clock must have the flag CLOCK_SOURCE_SCHED_CLOCK set. This works with the default (weak) sched_clock implementation but the clksrc based function is also provided for architectures where multi platform kernels might override it. If there is no CLOCK_SOURCE_SCHED_CLOCK available, then the function returns the jiffies based sched clock.

Signed-off-by Thomas Gleixner <[email protected]>

---
 include/linux/clocksource.h |    1
 include/linux/sched.h       |    1
 kernel/sched_clock.c        |   12 ----
 kernel/time/Kconfig         |    4 +
 kernel/time/timekeeping.c   |  125
++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 131 insertions(+), 12 deletions(-)

Many embedded platforms use the clocksource which is used for timekeeping as their sched_clock as well. Though they need to take care about the wraparounds as sched_clock() wants to be monotonic increasing nsescs. The timekeeping code and the NOHZ core take care of the timekeeping clocksource wraparound so we can simply use the timekeeping clocksource if possible.

clocksources which can be used by the clksrc sched_clock must have the flag CLOCK_SOURCE_SCHED_CLOCK set. This works with the default (weak) sched_clock implementation but the clksrc based function is also provided for architectures where multi platform kernels might override it. If there is no CLOCK_SOURCE_SCHED_CLOCK available, then the function returns the jiffies based sched clock.

Signed-off-by Thomas Gleixner <[email protected]>

Review

Quoted 7 lines · Thomas Gleixner

struct clocksource time_clocksource = {

(quoted diff) +1 -1

 	.name		= "s5p_clocksource_timer",
 	.rating		= 250,
 	.read		= s5p_timer_read,
 	.mask		= CLOCKSOURCE_MASK(32),
-	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
+	.flags		= CLOCK_SOURCE_IS_CONTINUOUS | CLOCKS_SOURCE_SCHED_CLOCK,

Drop the extra 'S'. CLOCK_SOURCE_SCHED_CLOCK.

Signed-off-by Thomas Gleixner <[email protected]>

struct clocksource time_clocksource = {

(quoted diff) +1 -1

 	.name		= "s5p_clocksource_timer",
 	.rating		= 250,
 	.read		= s5p_timer_read,
 	.mask		= CLOCKSOURCE_MASK(32),
-	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
+	.flags		= CLOCK_SOURCE_IS_CONTINUOUS |

CLOCKS_SOURCE_SCHED_CLOCK,

(quoted diff) +1 -5

-static cycle_t read_cycles(struct clocksource *cs)
+static cycle_t notrace read_cycles(struct clocksource *cs)
 {
 	struct timer_s *t = &timers[TID_CLOCKSOURCE];
 
 	return (cycles_t)timer32_read(t);
 }
 
-/*
- * Kernel assumes that sched_clock can be called early but may not have
- * things ready yet.
- */

Signed-off-by Thomas Gleixner <[email protected]>

---
 arch/arm/mach-omap1/time.c               |   62 +--------------------------
 arch/arm/mach-omap2/timer-gp.c           |   17 -------
 arch/arm/plat-omap/counter_32k.c         |   71 ++-----------------------------
 arch/arm/plat-omap/dmtimer.c             |    4 -
 arch/arm/plat-omap/include/plat/common.h |    1 
 5 files changed, 13 insertions(+), 142 deletions(-)

Quoted 8 lines · Thomas Gleixner

Signed-off-by Thomas Gleixner <[email protected]>

---
 arch/arm/mach-omap1/time.c               |   62 +--------------------------
 arch/arm/mach-omap2/timer-gp.c           |   17 -------
 arch/arm/plat-omap/counter_32k.c         |   71 ++-----------------------------
 arch/arm/plat-omap/dmtimer.c             |    4 -
 arch/arm/plat-omap/include/plat/common.h |    1 
 5 files changed, 13 insertions(+), 142 deletions(-)

This one needs the following patch to compile. Also, this fails with following, probably because the clocks are not set up at that point.

Unable to handle kernel NULL pointer dereference at virtual address 00000000
pgd = c0004000
[00000000] *pgd=00000000
Internal error: Oops: 80000005 [#1] SMP
last sysfs file: 
Modules linked in:
CPU: 0    Not tainted  (2.6.39-rc5-00081-gc3dec8c #166)

PC is at 0x0
LR is at read_persistent_clock+0x24/0xb4

The generic code should probably just return 0 until the clocks are initialized. Or do you have some other solution for that in mind?

The generic sched_clock implementation returns a jiffies based value as long as there is no clocksource registered. But the above does not fail in sched_clock() it fails in read_persistent_clock() which does

        last_cycles = cycles;
        cycles = clocksource_32k.read(&clocksource_32k);

So the following change causes this:

- .read = omap_32k_read_dummy,

Which leads to the question why you have a read_persistent_clock() at all if it always reads 0 via omap_32k_read_dummy ? Or is this meant just for the resume case? Then the above and the removal of omap_32k_read_dummy() needs to be undone.

Quoted 11 lines · Thomas Gleixner

The generic sched_clock implementation returns a jiffies based value as long as there is no clocksource registered. But the above does not fail in sched_clock() it fails in read_persistent_clock() which does

        last_cycles = cycles;
        cycles = clocksource_32k.read(&clocksource_32k);

So the following change causes this:

- .read = omap_32k_read_dummy,

Ah right.

Quoted 4 lines

Which leads to the question why you have a read_persistent_clock() at all if it always reads 0 via omap_32k_read_dummy ? Or is this meant just for the resume case? Then the above and the removal of omap_32k_read_dummy() needs to be undone.

The omap_32k_read_dummy needs to return 0 until the clocks are enabled and the right read function is selected. The .read gets then set in omap_init_clocksource_32k.

It would be nice to handle that in a generic way though, I would assume the same issue exists for other platforms too.

Quoted 24 lines · Tony Lindgren

Quoted 11 lines · Thomas Gleixner

The generic sched_clock implementation returns a jiffies based value as long as there is no clocksource registered. But the above does not fail in sched_clock() it fails in read_persistent_clock() which does

        last_cycles = cycles;
        cycles = clocksource_32k.read(&clocksource_32k);

So the following change causes this:

- .read = omap_32k_read_dummy,

Ah right.

Quoted 4 lines

Which leads to the question why you have a read_persistent_clock() at all if it always reads 0 via omap_32k_read_dummy ? Or is this meant just for the resume case? Then the above and the removal of omap_32k_read_dummy() needs to be undone.

The omap_32k_read_dummy needs to return 0 until the clocks are enabled and the right read function is selected. The .read gets then set in omap_init_clocksource_32k.

Quoted 2 lines

It would be nice to handle that in a generic way though, I would assume the same issue exists for other platforms too.

static inline cycles_t clocksource_read_safe(struct clocksource *c)

{
	return c->read ? c->read(c) : 0;
}

For the core timekeeping code that's a non issue as nothing should register a clocksource with an invalid read function pointer :)

Signed-off-by Thomas Gleixner <[email protected]>

---
arch/arm/mach-pxa/time.c | 31 +++----------------------------

1 file changed, 3 insertions(+), 28 deletions(-)

Index: linux-2.6-tip/arch/arm/mach-pxa/time.c
===================================================================

arch/arm/mach-pxa/time.c +3 -28

@@ -21,31 +21,8 @@
 #include <asm/div64.h>
 #include <asm/mach/irq.h>
 #include <asm/mach/time.h>
-#include <asm/sched_clock.h>
-#include <mach/regs-ost.h>
-
-/*
- * This is PXA's sched_clock implementation. This has a resolution
- * of at least 308 ns and a maximum value of 208 days.
- *
- * The return value is guaranteed to be monotonic in that range as
- * long as there is always less than 582 seconds between successive
- * calls to sched_clock() which should always be the case in practice.
- */
-static DEFINE_CLOCK_DATA(cd);
-
-unsigned long long notrace sched_clock(void)
-{
-	u32 cyc = OSCR;
-	return cyc_to_sched_clock(&cd, cyc, (u32)~0);
-}
-
-static void notrace pxa_update_sched_clock(void)
-{
-	u32 cyc = OSCR;
-	update_sched_clock(&cd, cyc, (u32)~0);
-}
 
+#include <mach/regs-ost.h>
 
 #define MIN_OSCR_DELTA 16
 

@@ -105,7 +82,7 @@ static struct clock_event_device ckevt_p
 	.set_mode	= pxa_osmr0_set_mode,
 };
 
-static cycle_t pxa_read_oscr(struct clocksource *cs)
+static cycle_t notrace pxa_read_oscr(struct clocksource *cs)
 {
 	return OSCR;
 }

@@ -115,7 +92,7 @@ static struct clocksource cksrc_pxa_oscr
 	.rating         = 200,
 	.read           = pxa_read_oscr,
 	.mask           = CLOCKSOURCE_MASK(32),
-	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
+	.flags		= CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SCHED_CLOCK,
 };
 
 static struct irqaction pxa_ost0_irq = {

@@ -132,8 +109,6 @@ static void __init pxa_timer_init(void)
 	OIER = 0;
 	OSSR = OSSR_M0 | OSSR_M1 | OSSR_M2 | OSSR_M3;
 
-	init_sched_clock(&cd, pxa_update_sched_clock, 32, clock_tick_rate);
-
 	clocksource_calc_mult_shift(&cksrc_pxa_oscr0, clock_tick_rate, 4);
 	clockevents_calc_mult_shift(&ckevt_pxa_osmr0, clock_tick_rate, 4);
 	ckevt_pxa_osmr0.max_delta_ns =

Quoted 3 lines · Thomas Gleixner

Signed-off-by Thomas Gleixner <[email protected]>

---
arch/arm/mach-pxa/time.c | 31 +++----------------------------

I think we need to remove HAVE_SCHED_CLOCK from ARCH_PXA in arch/arm/Kconfig as well.

Didn't look into other sub-arch, but should be applicable to those selecting HAVE_SCHED_CLOCK.

Eric,

Quoted 8 lines · Eric

Quoted 3 lines · Thomas Gleixner

Signed-off-by Thomas Gleixner <[email protected]>

---
arch/arm/mach-pxa/time.c | ? 31 +++----------------------------

Quoted 5 lines

I think we need to remove HAVE_SCHED_CLOCK from ARCH_PXA in arch/arm/Kconfig as well.

Didn't look into other sub-arch, but should be applicable to those selecting HAVE_SCHED_CLOCK.

Thomas already did this. See 16/16

Sascha

--

Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? | ? ? ? ? ? ? ? ? ? ? ? ? ? ? |
Industrial Linux Solutions ? ? ? ? ? ? ? ? | http://www.pengutronix.de/ ?|
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 ? ?|
Amtsgericht Hildesheim, HRA 2686 ? ? ? ? ? | Fax: ? +49-5121-206917-5555 |

Signed-off-by Thomas Gleixner <[email protected]>

---
?arch/arm/mach-u300/timer.c | ? 29 ++--------------------------- ?1 file changed, 2 insertions(+), 27 deletions(-)

Signed-off-by Thomas Gleixner <[email protected]>

---
 arch/arm/plat-mxc/time.c |   28 +++-------------------------
 1 file changed, 3 insertions(+), 25 deletions(-)

Index: linux-2.6-tip/arch/arm/plat-mxc/time.c
===================================================================

arch/arm/plat-mxc/time.c +3 -25

@@ -27,7 +27,6 @@
 #include <linux/clk.h>
 
 #include <mach/hardware.h>
-#include <asm/sched_clock.h>
 #include <asm/mach/time.h>
 #include <mach/common.h>
 

@@ -106,17 +105,12 @@ static void gpt_irq_acknowledge(void)
 		__raw_writel(V2_TSTAT_OF1, timer_base + V2_TSTAT);
 }
 
-static cycle_t dummy_get_cycles(struct clocksource *cs)
-{
-	return 0;
-}
-
-static cycle_t mx1_2_get_cycles(struct clocksource *cs)
+static cycle_t notrace mx1_2_get_cycles(struct clocksource *cs)
 {
 	return __raw_readl(timer_base + MX1_2_TCN);
 }
 
-static cycle_t v2_get_cycles(struct clocksource *cs)
+static cycle_t notrace v2_get_cycles(struct clocksource *cs)
 {
 	return __raw_readl(timer_base + V2_TCN);
 }

@@ -124,25 +118,10 @@ static cycle_t v2_get_cycles(struct cloc
 static struct clocksource clocksource_mxc = {
 	.name 		= "mxc_timer1",
 	.rating		= 200,
-	.read		= dummy_get_cycles,
 	.mask		= CLOCKSOURCE_MASK(32),
-	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
+	.flags		= CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SCHED_CLOCK,
 };
 
-static DEFINE_CLOCK_DATA(cd);
-unsigned long long notrace sched_clock(void)
-{
-	cycle_t cyc = clocksource_mxc.read(&clocksource_mxc);
-
-	return cyc_to_sched_clock(&cd, cyc, (u32)~0);
-}
-
-static void notrace mxc_update_sched_clock(void)
-{
-	cycle_t cyc = clocksource_mxc.read(&clocksource_mxc);
-	update_sched_clock(&cd, cyc, (u32)~0);
-}
-
 static int __init mxc_clocksource_init(struct clk *timer_clk)
 {
 	unsigned int c = clk_get_rate(timer_clk);

@@ -152,7 +131,6 @@ static int __init mxc_clocksource_init(s
 	else
 		clocksource_mxc.read = mx1_2_get_cycles;
 
-	init_sched_clock(&cd, mxc_update_sched_clock, 32, c);
 	clocksource_register_hz(&clocksource_mxc, c);
 
 	return 0;

Signed-off-by Thomas Gleixner <[email protected]>

---
?arch/arm/plat-nomadik/Kconfig | ? ?1 ?arch/arm/plat-nomadik/timer.c | ? 45 ++---------------------------------------- ?2 files changed, 3 insertions(+), 43 deletions(-)

All users converted to use the clocksource based sched_clock.

Signed-off-by Thomas Gleixner <[email protected]>

---
 arch/arm/Kconfig                   |   15 ----
 arch/arm/include/asm/sched_clock.h |  120

---
 arch/arm/kernel/Makefile           |    1
 arch/arm/kernel/sched_clock.c      |   74 ----------------------
 arch/arm/kernel/time.c             |    4 -
 5 files changed, 214 deletions(-)

Most of ARM sched_clocks is using the timekeeping clocksource as the sched_clock. Only versatile uses a different clock for it, but there is no real reason why it can't use the clocksource as well.

Another 650 lines gone :)

2011/4/23 Thomas Gleixner <[email protected]>:

Quoted 3 lines

Most of ARM sched_clocks is using the timekeeping clocksource as the sched_clock. Only versatile uses a different clock for it, but there is no real reason why it can't use the clocksource as well.

Excellent patch set!

Quoted 1 line

Another 650 lines gone :)

Any reason why we can't simply move *all* of these to drivers/clocksource/* and proceed with consolidation there?

It must be a pain having to grep through the tree to find all of these (even though I bet you know them all by heart now).

Quoted 2 lines

Any reason why we can't simply move *all* of these to drivers/clocksource/* and proceed with consolidation there?

This gives me some pause, as having the clocksources in the platform code makes it easier to know what platform needs to be tested when a change is made. The rule so far has been if more then one arch can make use of it, it should be moved to the drivers/clocksource dir. Otherwise, leave it close to any arch specific code that manages the hardware.

2011/4/25 john stultz <[email protected]>:

Quoted 9 lines · Linus Walleij

Quoted 2 lines

Any reason why we can't simply move *all* of these to drivers/clocksource/* and proceed with consolidation there?

This gives me some pause, as having the clocksources in the platform code makes it easier to know what platform needs to be tested when a change is made. The rule so far has been if more then one arch can make use of it, it should be moved to the drivers/clocksource dir. Otherwise, leave it close to any arch specific code that manages the hardware.

That means that by definition anything in a plat-* directory, say e.g. plat-nomadik/timer.c and plat-omap/dmtimer.c should go into drivers/clocksource I believe?

2011/4/23 Thomas Gleixner <[email protected]>:

Quoted 3 lines

Most of ARM sched_clocks is using the timekeeping clocksource as the sched_clock. Only versatile uses a different clock for it, but there is no real reason why it can't use the clocksource as well.

Excellent patch set!

Quoted 1 line

Another 650 lines gone :)

Any reason why we can't simply move *all* of these to drivers/clocksource/* and proceed with consolidation there?

It must be a pain having to grep through the tree to find all of these (even though I bet you know them all by heart now).

Most of ARM sched_clocks is using the timekeeping clocksource as the sched_clock. Only versatile uses a different clock for it, but there is no real reason why it can't use the clocksource as well.

Quoted 3 lines · Thomas Gleixner

Most of ARM sched_clocks is using the timekeeping clocksource as the sched_clock. Only versatile uses a different clock for it, but there is no real reason why it can't use the clocksource as well.

sched_clock really needs to be setup earlier than clocksources are setup, before sched_init() is called in init/main.c. This is where the scheduler reads the initial value of sched_clock() to start its time measurements. This is well before clocksources are initialized, or even the irq subsystem.

So I don't believe bolting it directly into the clocksource code in this way is correct. As things are currently setup with my sched_clock() implementation, we can (and in any cases do) get the sched_clock() up and running really early which is what's required.

Quoted 10 lines · Russell King - ARM Linux

Quoted 3 lines · Thomas Gleixner

Most of ARM sched_clocks is using the timekeeping clocksource as the sched_clock. Only versatile uses a different clock for it, but there is no real reason why it can't use the clocksource as well.

sched_clock really needs to be setup earlier than clocksources are setup, before sched_init() is called in init/main.c. This is where the scheduler reads the initial value of sched_clock() to start its time measurements. This is well before clocksources are initialized, or even the irq subsystem.

Quoted 4 lines

So I don't believe bolting it directly into the clocksource code in this way is correct. As things are currently setup with my sched_clock() implementation, we can (and in any cases do) get the sched_clock() up and running really early which is what's required.

Not really. There is no requirement to have a high resolution sched clock in the early boot process. The only requirement is that sched_clock is monotonic increasing nsecs. So the code utilizes jiffies up to the point where the clocksource is installed and then switches over keeping the clock monotonic. The scheduler does not care whether it gets coarse grained time information in the early boot stage and then fine grained information at some point later.

The only drawback is that you don't get precise timestamps for tracing up to the point where the clock source is installed. But that's really not a big deal to worry about.

Quoted 25 lines · Thomas Gleixner

Quoted 10 lines · Russell King - ARM Linux

Quoted 3 lines · Thomas Gleixner

Most of ARM sched_clocks is using the timekeeping clocksource as the sched_clock. Only versatile uses a different clock for it, but there is no real reason why it can't use the clocksource as well.

sched_clock really needs to be setup earlier than clocksources are setup, before sched_init() is called in init/main.c. This is where the scheduler reads the initial value of sched_clock() to start its time measurements. This is well before clocksources are initialized, or even the irq subsystem.

Quoted 4 lines

So I don't believe bolting it directly into the clocksource code in this way is correct. As things are currently setup with my sched_clock() implementation, we can (and in any cases do) get the sched_clock() up and running really early which is what's required.

Not really. There is no requirement to have a high resolution sched clock in the early boot process. The only requirement is that sched_clock is monotonic increasing nsecs. So the code utilizes jiffies up to the point where the clocksource is installed and then switches over keeping the clock monotonic. The scheduler does not care whether it gets coarse grained time information in the early boot stage and then fine grained information at some point later.

Quoted 3 lines

The only drawback is that you don't get precise timestamps for tracing up to the point where the clock source is installed. But that's really not a big deal to worry about.

What about PRINTK_TIME? I think that uses sched_clock right from the start.

Quoted 2 lines

What about PRINTK_TIME? I think that uses sched_clock right from the start.

Right, but where is the problem ? A few time stamps with jiffies resolution and then the finegrained ones.

2011/4/29 Thomas Gleixner <[email protected]>:

Quoted 6 lines · Tony Lindgren

Quoted 2 lines

What about PRINTK_TIME? I think that uses sched_clock right from the start.

Right, but where is the problem ? A few time stamps with jiffies resolution and then the finegrained ones.

Jonas ?berg has this pending patch that we call "boot time recorder". What it does is basically start a free running timer before the kernel is booted, e.g. in U-boot or even in the ROM of your machine if you can.

If we need more finegrained sched_clock() before the clocksource is up, a good approach would be to utilize something like that for sched_clock up to that point, plus it has the upside that [0.000000] is as close to electrical power-on as possible.

Quoted 13 lines · Linus Walleij

2011/4/29 Thomas Gleixner <[email protected]>:

Quoted 6 lines · Tony Lindgren

Quoted 2 lines

What about PRINTK_TIME? I think that uses sched_clock right from the start.

Right, but where is the problem ? A few time stamps with jiffies resolution and then the finegrained ones.

Jonas ?berg has this pending patch that we call "boot time recorder". What it does is basically start a free running timer before the kernel is booted, e.g. in U-boot or even in the ROM of your machine if you can.

Quoted 5 lines

If we need more finegrained sched_clock() before the clocksource is up, a good approach would be to utilize something like that for sched_clock up to that point, plus it has the upside that [0.000000] is as close to electrical power-on as possible.

Well, you can read out that value at the point the clocksource gets installed so you have the time spent up to that point without adding another facility, level of indirection ...

Quoted 11 lines · Linus Walleij

Jonas ?berg has this pending patch that we call "boot time recorder". What it does is basically start a free running timer before the kernel is booted, e.g. in U-boot or even in the ROM of your machine if you can.

If we need more finegrained sched_clock() before the clocksource is up, a good approach would be to utilize something like that for sched_clock up to that point, plus it has the upside that [0.000000] is as close to electrical power-on as possible.

Well, you can read out that value at the point the clocksource gets installed so you have the time spent up to that point without adding another facility, level of indirection ...

Most of ARM sched_clocks is using the timekeeping clocksource as the sched_clock. Only versatile uses a different clock for it, but there is no real reason why it can't use the clocksource as well.

The core timekeeping code makes already sure that we don't wrap the timekeeper and we can base a sched_clock easily on that.

It uses the read function of the clocksource and keeps track of time independent of the timekeeping code as the ARM sched_clock implementation does. It also uses it's own sequence counter so sched_clock is not dependent on xtime_lock. The update sequence is just adding offsets once in a while and runs from do_timer() which also makes sure that only one core tries to update it.

Only lightly tested on orion. Please give it a go.

Another 650 lines gone :)