2013-06-02 06:39:55

by Stephen Boyd

[permalink] [raw]
Subject: [PATCHv2 0/6] Make ARM's sched_clock generic + 64 bit friendly

This is mostly a resend of a patch series I sent a little over a month
ago. I've reordered the patches so that John can pick up the first three
and get a generic sched_clock layer without having to take the 64 bit
patches. The last three patches add 64 bit support and move the architected
timers on ARM64 and ARM to use it.

Stephen Boyd (6):
ARM: sched_clock: Remove unused needs_suspend member
ARM: sched_clock: Return suspended count earlier
sched_clock: Make ARM's sched_clock generic for all architectures
sched_clock: Add support for >32 bit sched_clock
ARM: arch_timer: Move to setup_sched_clock_64()
arm64: Move to generic sched_clock infrastructure

arch/arm/Kconfig | 1 +
arch/arm/common/timer-sp.c | 2 +-
arch/arm/kernel/Makefile | 2 +-
arch/arm/kernel/arch_timer.c | 16 +--
arch/arm/kernel/time.c | 4 +-
arch/arm/mach-davinci/time.c | 2 +-
arch/arm/mach-imx/time.c | 2 +-
arch/arm/mach-integrator/integrator_ap.c | 2 +-
arch/arm/mach-ixp4xx/common.c | 2 +-
arch/arm/mach-mmp/time.c | 2 +-
arch/arm/mach-msm/timer.c | 2 +-
arch/arm/mach-omap1/time.c | 2 +-
arch/arm/mach-omap2/timer.c | 2 +-
arch/arm/mach-pxa/time.c | 2 +-
arch/arm/mach-sa1100/time.c | 2 +-
arch/arm/mach-u300/timer.c | 2 +-
arch/arm/plat-iop/time.c | 2 +-
arch/arm/plat-omap/counter_32k.c | 2 +-
arch/arm/plat-orion/time.c | 2 +-
arch/arm/plat-samsung/samsung-time.c | 2 +-
arch/arm/plat-versatile/sched-clock.c | 2 +-
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/time.c | 11 +-
drivers/clocksource/bcm2835_timer.c | 2 +-
drivers/clocksource/clksrc-dbx500-prcmu.c | 3 +-
drivers/clocksource/dw_apb_timer_of.c | 2 +-
drivers/clocksource/mxs_timer.c | 2 +-
drivers/clocksource/nomadik-mtu.c | 2 +-
drivers/clocksource/samsung_pwm_timer.c | 2 +-
drivers/clocksource/tegra20_timer.c | 2 +-
drivers/clocksource/time-armada-370-xp.c | 2 +-
drivers/clocksource/timer-marco.c | 2 +-
drivers/clocksource/timer-prima2.c | 2 +-
.../include/asm => include/linux}/sched_clock.h | 14 ++-
init/Kconfig | 3 +
init/main.c | 2 +
kernel/time/Makefile | 1 +
{arch/arm/kernel => kernel/time}/sched_clock.c | 114 +++++++++++++++------
38 files changed, 132 insertions(+), 92 deletions(-)
rename {arch/arm/include/asm => include/linux}/sched_clock.h (62%)
rename {arch/arm/kernel => kernel/time}/sched_clock.c (74%)

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation


2013-06-02 06:40:06

by Stephen Boyd

[permalink] [raw]
Subject: [PATCHv2 1/6] ARM: sched_clock: Remove unused needs_suspend member

The needs_suspend member is unused now that we always do the
suspend/resume handling (see 6a4dae5 (ARM: 7565/1: sched: stop
sched_clock() during suspend, 2012-10-23)).

Signed-off-by: Stephen Boyd <[email protected]>
---
arch/arm/kernel/sched_clock.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/arm/kernel/sched_clock.c b/arch/arm/kernel/sched_clock.c
index e8edcaa..45efe86 100644
--- a/arch/arm/kernel/sched_clock.c
+++ b/arch/arm/kernel/sched_clock.c
@@ -24,7 +24,6 @@ struct clock_data {
u32 mult;
u32 shift;
bool suspended;
- bool needs_suspend;
};

static void sched_clock_poll(unsigned long wrap_ticks);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-06-02 06:40:11

by Stephen Boyd

[permalink] [raw]
Subject: [PATCHv2 2/6] ARM: sched_clock: Return suspended count earlier

If we're suspended and sched_clock() is called we're going to
read the hardware one more time and throw away that value and
return back the cached value we saved during the suspend
callback. This is wasteful. Let's short circuit all that and
return the cached value as early as possible if we're suspended.

Signed-off-by: Stephen Boyd <[email protected]>
---
arch/arm/kernel/sched_clock.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/kernel/sched_clock.c b/arch/arm/kernel/sched_clock.c
index 45efe86..a781c59 100644
--- a/arch/arm/kernel/sched_clock.c
+++ b/arch/arm/kernel/sched_clock.c
@@ -55,9 +55,6 @@ static unsigned long long notrace cyc_to_sched_clock(u32 cyc, u32 mask)
u64 epoch_ns;
u32 epoch_cyc;

- if (cd.suspended)
- return cd.epoch_ns;
-
/*
* Load the epoch_cyc and epoch_ns atomically. We do this by
* ensuring that we always write epoch_cyc, epoch_ns and
@@ -174,6 +171,9 @@ unsigned long long __read_mostly (*sched_clock_func)(void) = sched_clock_32;

unsigned long long notrace sched_clock(void)
{
+ if (cd.suspended)
+ return cd.epoch_ns;
+
return sched_clock_func();
}

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-06-02 06:40:16

by Stephen Boyd

[permalink] [raw]
Subject: [PATCHv2 3/6] sched_clock: Make ARM's sched_clock generic for all architectures

Nothing about the sched_clock implementation in the ARM port is
specific to the architecture. Generalize the code so that other
architectures can use it by selecting GENERIC_SCHED_CLOCK.

Signed-off-by: Stephen Boyd <[email protected]>
---
arch/arm/Kconfig | 1 +
arch/arm/common/timer-sp.c | 2 +-
arch/arm/kernel/Makefile | 2 +-
arch/arm/kernel/arch_timer.c | 2 +-
arch/arm/kernel/time.c | 4 +---
arch/arm/mach-davinci/time.c | 2 +-
arch/arm/mach-imx/time.c | 2 +-
arch/arm/mach-integrator/integrator_ap.c | 2 +-
arch/arm/mach-ixp4xx/common.c | 2 +-
arch/arm/mach-mmp/time.c | 2 +-
arch/arm/mach-msm/timer.c | 2 +-
arch/arm/mach-omap1/time.c | 2 +-
arch/arm/mach-omap2/timer.c | 2 +-
arch/arm/mach-pxa/time.c | 2 +-
arch/arm/mach-sa1100/time.c | 2 +-
arch/arm/mach-u300/timer.c | 2 +-
arch/arm/plat-iop/time.c | 2 +-
arch/arm/plat-omap/counter_32k.c | 2 +-
arch/arm/plat-orion/time.c | 2 +-
arch/arm/plat-samsung/samsung-time.c | 2 +-
arch/arm/plat-versatile/sched-clock.c | 2 +-
drivers/clocksource/bcm2835_timer.c | 2 +-
drivers/clocksource/clksrc-dbx500-prcmu.c | 3 +--
drivers/clocksource/dw_apb_timer_of.c | 2 +-
drivers/clocksource/mxs_timer.c | 2 +-
drivers/clocksource/nomadik-mtu.c | 2 +-
drivers/clocksource/samsung_pwm_timer.c | 2 +-
drivers/clocksource/tegra20_timer.c | 2 +-
drivers/clocksource/time-armada-370-xp.c | 2 +-
drivers/clocksource/timer-marco.c | 2 +-
drivers/clocksource/timer-prima2.c | 2 +-
{arch/arm/include/asm => include/linux}/sched_clock.h | 9 +++++++--
init/Kconfig | 3 +++
init/main.c | 2 ++
kernel/time/Makefile | 1 +
{arch/arm/kernel => kernel/time}/sched_clock.c | 3 +--
36 files changed, 45 insertions(+), 37 deletions(-)
rename {arch/arm/include/asm => include/linux}/sched_clock.h (75%)
rename {arch/arm/kernel => kernel/time}/sched_clock.c (99%)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 49d993c..53d3a35 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -14,6 +14,7 @@ config ARM
select GENERIC_IRQ_PROBE
select GENERIC_IRQ_SHOW
select GENERIC_PCI_IOMAP
+ select GENERIC_SCHED_CLOCK
select GENERIC_SMP_IDLE_THREAD
select GENERIC_IDLE_POLL_SETUP
select GENERIC_STRNCPY_FROM_USER
diff --git a/arch/arm/common/timer-sp.c b/arch/arm/common/timer-sp.c
index ddc7407..023ee63 100644
--- a/arch/arm/common/timer-sp.c
+++ b/arch/arm/common/timer-sp.c
@@ -28,8 +28,8 @@
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_irq.h>
+#include <linux/sched_clock.h>

-#include <asm/sched_clock.h>
#include <asm/hardware/arm_timer.h>
#include <asm/hardware/timer-sp.h>

diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index 5f3338e..97cb057 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -16,7 +16,7 @@ CFLAGS_REMOVE_return_address.o = -pg
# Object file lists.

obj-y := elf.o entry-armv.o entry-common.o irq.o opcodes.o \
- process.o ptrace.o return_address.o sched_clock.o \
+ process.o ptrace.o return_address.o \
setup.o signal.o stacktrace.o sys_arm.o time.o traps.o

obj-$(CONFIG_ATAGS) += atags_parse.o
diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
index 59dcdce..221f07b 100644
--- a/arch/arm/kernel/arch_timer.c
+++ b/arch/arm/kernel/arch_timer.c
@@ -11,9 +11,9 @@
#include <linux/init.h>
#include <linux/types.h>
#include <linux/errno.h>
+#include <linux/sched_clock.h>

#include <asm/delay.h>
-#include <asm/sched_clock.h>

#include <clocksource/arm_arch_timer.h>

diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c
index abff4e9..98aee32 100644
--- a/arch/arm/kernel/time.c
+++ b/arch/arm/kernel/time.c
@@ -24,9 +24,9 @@
#include <linux/timer.h>
#include <linux/clocksource.h>
#include <linux/irq.h>
+#include <linux/sched_clock.h>

#include <asm/thread_info.h>
-#include <asm/sched_clock.h>
#include <asm/stacktrace.h>
#include <asm/mach/arch.h>
#include <asm/mach/time.h>
@@ -120,6 +120,4 @@ void __init time_init(void)
machine_desc->init_time();
else
clocksource_of_init();
-
- sched_clock_postinit();
}
diff --git a/arch/arm/mach-davinci/time.c b/arch/arm/mach-davinci/time.c
index bad361e..7a55b5c 100644
--- a/arch/arm/mach-davinci/time.c
+++ b/arch/arm/mach-davinci/time.c
@@ -18,8 +18,8 @@
#include <linux/clk.h>
#include <linux/err.h>
#include <linux/platform_device.h>
+#include <linux/sched_clock.h>

-#include <asm/sched_clock.h>
#include <asm/mach/irq.h>
#include <asm/mach/time.h>

diff --git a/arch/arm/mach-imx/time.c b/arch/arm/mach-imx/time.c
index fea9131..cd46529 100644
--- a/arch/arm/mach-imx/time.c
+++ b/arch/arm/mach-imx/time.c
@@ -26,8 +26,8 @@
#include <linux/clockchips.h>
#include <linux/clk.h>
#include <linux/err.h>
+#include <linux/sched_clock.h>

-#include <asm/sched_clock.h>
#include <asm/mach/time.h>

#include "common.h"
diff --git a/arch/arm/mach-integrator/integrator_ap.c b/arch/arm/mach-integrator/integrator_ap.c
index b23c8e4..aa43462 100644
--- a/arch/arm/mach-integrator/integrator_ap.c
+++ b/arch/arm/mach-integrator/integrator_ap.c
@@ -41,6 +41,7 @@
#include <linux/stat.h>
#include <linux/sys_soc.h>
#include <linux/termios.h>
+#include <linux/sched_clock.h>
#include <video/vga.h>

#include <mach/hardware.h>
@@ -49,7 +50,6 @@
#include <asm/setup.h>
#include <asm/param.h> /* HZ */
#include <asm/mach-types.h>
-#include <asm/sched_clock.h>

#include <mach/lm.h>
#include <mach/irqs.h>
diff --git a/arch/arm/mach-ixp4xx/common.c b/arch/arm/mach-ixp4xx/common.c
index 6600cff..58307cf 100644
--- a/arch/arm/mach-ixp4xx/common.c
+++ b/arch/arm/mach-ixp4xx/common.c
@@ -30,6 +30,7 @@
#include <linux/export.h>
#include <linux/gpio.h>
#include <linux/cpu.h>
+#include <linux/sched_clock.h>

#include <mach/udc.h>
#include <mach/hardware.h>
@@ -38,7 +39,6 @@
#include <asm/pgtable.h>
#include <asm/page.h>
#include <asm/irq.h>
-#include <asm/sched_clock.h>
#include <asm/system_misc.h>

#include <asm/mach/map.h>
diff --git a/arch/arm/mach-mmp/time.c b/arch/arm/mach-mmp/time.c
index 86a18b3..7ac41e8 100644
--- a/arch/arm/mach-mmp/time.c
+++ b/arch/arm/mach-mmp/time.c
@@ -28,8 +28,8 @@
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_irq.h>
+#include <linux/sched_clock.h>

-#include <asm/sched_clock.h>
#include <mach/addr-map.h>
#include <mach/regs-timers.h>
#include <mach/regs-apbc.h>
diff --git a/arch/arm/mach-msm/timer.c b/arch/arm/mach-msm/timer.c
index 284313f..b6418fd 100644
--- a/arch/arm/mach-msm/timer.c
+++ b/arch/arm/mach-msm/timer.c
@@ -23,10 +23,10 @@
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_irq.h>
+#include <linux/sched_clock.h>

#include <asm/mach/time.h>
#include <asm/localtimer.h>
-#include <asm/sched_clock.h>

#include "common.h"

diff --git a/arch/arm/mach-omap1/time.c b/arch/arm/mach-omap1/time.c
index 726ec23..80603d2 100644
--- a/arch/arm/mach-omap1/time.c
+++ b/arch/arm/mach-omap1/time.c
@@ -43,9 +43,9 @@
#include <linux/clocksource.h>
#include <linux/clockchips.h>
#include <linux/io.h>
+#include <linux/sched_clock.h>

#include <asm/irq.h>
-#include <asm/sched_clock.h>

#include <mach/hardware.h>
#include <asm/mach/irq.h>
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index f8b23b8..4c069b0 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -41,10 +41,10 @@
#include <linux/of_irq.h>
#include <linux/platform_device.h>
#include <linux/platform_data/dmtimer-omap.h>
+#include <linux/sched_clock.h>

#include <asm/mach/time.h>
#include <asm/smp_twd.h>
-#include <asm/sched_clock.h>

#include "omap_hwmod.h"
#include "omap_device.h"
diff --git a/arch/arm/mach-pxa/time.c b/arch/arm/mach-pxa/time.c
index 8f1ee92..9aa852a 100644
--- a/arch/arm/mach-pxa/time.c
+++ b/arch/arm/mach-pxa/time.c
@@ -16,11 +16,11 @@
#include <linux/init.h>
#include <linux/interrupt.h>
#include <linux/clockchips.h>
+#include <linux/sched_clock.h>

#include <asm/div64.h>
#include <asm/mach/irq.h>
#include <asm/mach/time.h>
-#include <asm/sched_clock.h>
#include <mach/regs-ost.h>
#include <mach/irqs.h>

diff --git a/arch/arm/mach-sa1100/time.c b/arch/arm/mach-sa1100/time.c
index a59a13a..713c86c 100644
--- a/arch/arm/mach-sa1100/time.c
+++ b/arch/arm/mach-sa1100/time.c
@@ -14,9 +14,9 @@
#include <linux/irq.h>
#include <linux/timex.h>
#include <linux/clockchips.h>
+#include <linux/sched_clock.h>

#include <asm/mach/time.h>
-#include <asm/sched_clock.h>
#include <mach/hardware.h>
#include <mach/irqs.h>

diff --git a/arch/arm/mach-u300/timer.c b/arch/arm/mach-u300/timer.c
index d9e7320..af771b7 100644
--- a/arch/arm/mach-u300/timer.c
+++ b/arch/arm/mach-u300/timer.c
@@ -18,12 +18,12 @@
#include <linux/clk.h>
#include <linux/err.h>
#include <linux/irq.h>
+#include <linux/sched_clock.h>

#include <mach/hardware.h>
#include <mach/irqs.h>

/* Generic stuff */
-#include <asm/sched_clock.h>
#include <asm/mach/map.h>
#include <asm/mach/time.h>

diff --git a/arch/arm/plat-iop/time.c b/arch/arm/plat-iop/time.c
index 837a2d5..29606bd7 100644
--- a/arch/arm/plat-iop/time.c
+++ b/arch/arm/plat-iop/time.c
@@ -22,9 +22,9 @@
#include <linux/clocksource.h>
#include <linux/clockchips.h>
#include <linux/export.h>
+#include <linux/sched_clock.h>
#include <mach/hardware.h>
#include <asm/irq.h>
-#include <asm/sched_clock.h>
#include <asm/uaccess.h>
#include <asm/mach/irq.h>
#include <asm/mach/time.h>
diff --git a/arch/arm/plat-omap/counter_32k.c b/arch/arm/plat-omap/counter_32k.c
index 5b0b86b..d9bc98e 100644
--- a/arch/arm/plat-omap/counter_32k.c
+++ b/arch/arm/plat-omap/counter_32k.c
@@ -18,9 +18,9 @@
#include <linux/err.h>
#include <linux/io.h>
#include <linux/clocksource.h>
+#include <linux/sched_clock.h>

#include <asm/mach/time.h>
-#include <asm/sched_clock.h>

#include <plat/counter-32k.h>

diff --git a/arch/arm/plat-orion/time.c b/arch/arm/plat-orion/time.c
index 5d5ac0f..9d2b2ac 100644
--- a/arch/arm/plat-orion/time.c
+++ b/arch/arm/plat-orion/time.c
@@ -16,7 +16,7 @@
#include <linux/clockchips.h>
#include <linux/interrupt.h>
#include <linux/irq.h>
-#include <asm/sched_clock.h>
+#include <linux/sched_clock.h>

/*
* MBus bridge block registers.
diff --git a/arch/arm/plat-samsung/samsung-time.c b/arch/arm/plat-samsung/samsung-time.c
index f899cbc..2957075 100644
--- a/arch/arm/plat-samsung/samsung-time.c
+++ b/arch/arm/plat-samsung/samsung-time.c
@@ -15,12 +15,12 @@
#include <linux/clk.h>
#include <linux/clockchips.h>
#include <linux/platform_device.h>
+#include <linux/sched_clock.h>

#include <asm/smp_twd.h>
#include <asm/mach/time.h>
#include <asm/mach/arch.h>
#include <asm/mach/map.h>
-#include <asm/sched_clock.h>

#include <mach/map.h>
#include <plat/devs.h>
diff --git a/arch/arm/plat-versatile/sched-clock.c b/arch/arm/plat-versatile/sched-clock.c
index b33b74c..51b109e 100644
--- a/arch/arm/plat-versatile/sched-clock.c
+++ b/arch/arm/plat-versatile/sched-clock.c
@@ -20,8 +20,8 @@
*/
#include <linux/kernel.h>
#include <linux/io.h>
+#include <linux/sched_clock.h>

-#include <asm/sched_clock.h>
#include <plat/sched_clock.h>

static void __iomem *ctr;
diff --git a/drivers/clocksource/bcm2835_timer.c b/drivers/clocksource/bcm2835_timer.c
index 766611d..07ea7ce 100644
--- a/drivers/clocksource/bcm2835_timer.c
+++ b/drivers/clocksource/bcm2835_timer.c
@@ -28,8 +28,8 @@
#include <linux/of_platform.h>
#include <linux/slab.h>
#include <linux/string.h>
+#include <linux/sched_clock.h>

-#include <asm/sched_clock.h>
#include <asm/irq.h>

#define REG_CONTROL 0x00
diff --git a/drivers/clocksource/clksrc-dbx500-prcmu.c b/drivers/clocksource/clksrc-dbx500-prcmu.c
index 54f3d11..0a7fb24 100644
--- a/drivers/clocksource/clksrc-dbx500-prcmu.c
+++ b/drivers/clocksource/clksrc-dbx500-prcmu.c
@@ -14,8 +14,7 @@
*/
#include <linux/clockchips.h>
#include <linux/clksrc-dbx500-prcmu.h>
-
-#include <asm/sched_clock.h>
+#include <linux/sched_clock.h>

#define RATE_32K 32768

diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c
index ab09ed3..f417e50 100644
--- a/drivers/clocksource/dw_apb_timer_of.c
+++ b/drivers/clocksource/dw_apb_timer_of.c
@@ -20,9 +20,9 @@
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_irq.h>
+#include <linux/sched_clock.h>

#include <asm/mach/time.h>
-#include <asm/sched_clock.h>

static void timer_get_base_and_rate(struct device_node *np,
void __iomem **base, u32 *rate)
diff --git a/drivers/clocksource/mxs_timer.c b/drivers/clocksource/mxs_timer.c
index 02af420..0f5e65f 100644
--- a/drivers/clocksource/mxs_timer.c
+++ b/drivers/clocksource/mxs_timer.c
@@ -29,9 +29,9 @@
#include <linux/of_address.h>
#include <linux/of_irq.h>
#include <linux/stmp_device.h>
+#include <linux/sched_clock.h>

#include <asm/mach/time.h>
-#include <asm/sched_clock.h>

/*
* There are 2 versions of the timrot on Freescale MXS-based SoCs.
diff --git a/drivers/clocksource/nomadik-mtu.c b/drivers/clocksource/nomadik-mtu.c
index e405531..8864c17 100644
--- a/drivers/clocksource/nomadik-mtu.c
+++ b/drivers/clocksource/nomadik-mtu.c
@@ -18,8 +18,8 @@
#include <linux/delay.h>
#include <linux/err.h>
#include <linux/platform_data/clocksource-nomadik-mtu.h>
+#include <linux/sched_clock.h>
#include <asm/mach/time.h>
-#include <asm/sched_clock.h>

/*
* The MTU device hosts four different counters, with 4 set of
diff --git a/drivers/clocksource/samsung_pwm_timer.c b/drivers/clocksource/samsung_pwm_timer.c
index 0234c8d..584b547 100644
--- a/drivers/clocksource/samsung_pwm_timer.c
+++ b/drivers/clocksource/samsung_pwm_timer.c
@@ -21,10 +21,10 @@
#include <linux/of_irq.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
+#include <linux/sched_clock.h>

#include <clocksource/samsung_pwm.h>

-#include <asm/sched_clock.h>

/*
* Clocksource driver
diff --git a/drivers/clocksource/tegra20_timer.c b/drivers/clocksource/tegra20_timer.c
index ae877b0..9396170 100644
--- a/drivers/clocksource/tegra20_timer.c
+++ b/drivers/clocksource/tegra20_timer.c
@@ -26,10 +26,10 @@
#include <linux/io.h>
#include <linux/of_address.h>
#include <linux/of_irq.h>
+#include <linux/sched_clock.h>

#include <asm/mach/time.h>
#include <asm/smp_twd.h>
-#include <asm/sched_clock.h>

#define RTC_SECONDS 0x08
#define RTC_SHADOW_SECONDS 0x0c
diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
index 47a6730..efdca32 100644
--- a/drivers/clocksource/time-armada-370-xp.c
+++ b/drivers/clocksource/time-armada-370-xp.c
@@ -27,8 +27,8 @@
#include <linux/of_address.h>
#include <linux/irq.h>
#include <linux/module.h>
+#include <linux/sched_clock.h>

-#include <asm/sched_clock.h>
#include <asm/localtimer.h>
#include <linux/percpu.h>
/*
diff --git a/drivers/clocksource/timer-marco.c b/drivers/clocksource/timer-marco.c
index 97738db..e5dc912 100644
--- a/drivers/clocksource/timer-marco.c
+++ b/drivers/clocksource/timer-marco.c
@@ -17,7 +17,7 @@
#include <linux/of.h>
#include <linux/of_irq.h>
#include <linux/of_address.h>
-#include <asm/sched_clock.h>
+#include <linux/sched_clock.h>
#include <asm/localtimer.h>
#include <asm/mach/time.h>

diff --git a/drivers/clocksource/timer-prima2.c b/drivers/clocksource/timer-prima2.c
index 7608826..ef3cfb2 100644
--- a/drivers/clocksource/timer-prima2.c
+++ b/drivers/clocksource/timer-prima2.c
@@ -18,7 +18,7 @@
#include <linux/of.h>
#include <linux/of_irq.h>
#include <linux/of_address.h>
-#include <asm/sched_clock.h>
+#include <linux/sched_clock.h>
#include <asm/mach/time.h>

#define SIRFSOC_TIMER_COUNTER_LO 0x0000
diff --git a/arch/arm/include/asm/sched_clock.h b/include/linux/sched_clock.h
similarity index 75%
rename from arch/arm/include/asm/sched_clock.h
rename to include/linux/sched_clock.h
index 3d520dd..fa7922c 100644
--- a/arch/arm/include/asm/sched_clock.h
+++ b/include/linux/sched_clock.h
@@ -5,10 +5,15 @@
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*/
-#ifndef ASM_SCHED_CLOCK
-#define ASM_SCHED_CLOCK
+#ifndef LINUX_SCHED_CLOCK
+#define LINUX_SCHED_CLOCK

+#ifdef CONFIG_GENERIC_SCHED_CLOCK
extern void sched_clock_postinit(void);
+#else
+static inline void sched_clock_postinit(void) { }
+#endif
+
extern void setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate);

extern unsigned long long (*sched_clock_func)(void);
diff --git a/init/Kconfig b/init/Kconfig
index 9d3a788..1a3f933 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -757,6 +757,9 @@ config LOG_BUF_SHIFT
config HAVE_UNSTABLE_SCHED_CLOCK
bool

+config GENERIC_SCHED_CLOCK
+ bool
+
#
# For architectures that want to enable the support for NUMA-affine scheduler
# balancing logic:
diff --git a/init/main.c b/init/main.c
index 9484f4b..bef4a6a 100644
--- a/init/main.c
+++ b/init/main.c
@@ -74,6 +74,7 @@
#include <linux/ptrace.h>
#include <linux/blkdev.h>
#include <linux/elevator.h>
+#include <linux/sched_clock.h>

#include <asm/io.h>
#include <asm/bugs.h>
@@ -555,6 +556,7 @@ asmlinkage void __init start_kernel(void)
softirq_init();
timekeeping_init();
time_init();
+ sched_clock_postinit();
profile_init();
call_function_init();
WARN(!irqs_disabled(), "Interrupts were enabled early\n");
diff --git a/kernel/time/Makefile b/kernel/time/Makefile
index ff7d9d2..6644658 100644
--- a/kernel/time/Makefile
+++ b/kernel/time/Makefile
@@ -4,6 +4,7 @@ obj-y += timeconv.o posix-clock.o alarmtimer.o
obj-$(CONFIG_GENERIC_CLOCKEVENTS_BUILD) += clockevents.o
obj-$(CONFIG_GENERIC_CLOCKEVENTS) += tick-common.o
obj-$(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST) += tick-broadcast.o
+obj-$(CONFIG_GENERIC_SCHED_CLOCK) += sched_clock.o
obj-$(CONFIG_TICK_ONESHOT) += tick-oneshot.o
obj-$(CONFIG_TICK_ONESHOT) += tick-sched.o
obj-$(CONFIG_TIMER_STATS) += timer_stats.o
diff --git a/arch/arm/kernel/sched_clock.c b/kernel/time/sched_clock.c
similarity index 99%
rename from arch/arm/kernel/sched_clock.c
rename to kernel/time/sched_clock.c
index a781c59..aad1ae6 100644
--- a/arch/arm/kernel/sched_clock.c
+++ b/kernel/time/sched_clock.c
@@ -13,8 +13,7 @@
#include <linux/sched.h>
#include <linux/syscore_ops.h>
#include <linux/timer.h>
-
-#include <asm/sched_clock.h>
+#include <linux/sched_clock.h>

struct clock_data {
u64 epoch_ns;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-06-02 06:40:24

by Stephen Boyd

[permalink] [raw]
Subject: [PATCHv2 4/6] sched_clock: Add support for >32 bit sched_clock

The ARM architected system counter has at least 56 useable bits.
Add support for counters with more than 32 bits to the generic
sched_clock implementation so we can avoid the complexity of
dealing with wrap-around on these devices while benefiting from
the irqtime accounting and suspend/resume handling that the
generic sched_clock code already has.

Signed-off-by: Stephen Boyd <[email protected]>
---
include/linux/sched_clock.h | 2 +
kernel/time/sched_clock.c | 101 ++++++++++++++++++++++++++++++++------------
2 files changed, 77 insertions(+), 26 deletions(-)

diff --git a/include/linux/sched_clock.h b/include/linux/sched_clock.h
index fa7922c..e732b39 100644
--- a/include/linux/sched_clock.h
+++ b/include/linux/sched_clock.h
@@ -18,4 +18,6 @@ extern void setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate);

extern unsigned long long (*sched_clock_func)(void);

+extern void setup_sched_clock_64(u64 (*read)(void), int bits,
+ unsigned long rate);
#endif
diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
index aad1ae6..482242c 100644
--- a/kernel/time/sched_clock.c
+++ b/kernel/time/sched_clock.c
@@ -43,6 +43,7 @@ static u32 notrace jiffy_sched_clock_read(void)
}

static u32 __read_mostly (*read_sched_clock)(void) = jiffy_sched_clock_read;
+static u64 __read_mostly (*read_sched_clock_64)(void);

static inline u64 notrace cyc_to_ns(u64 cyc, u32 mult, u32 shift)
{
@@ -103,24 +104,12 @@ static void sched_clock_poll(unsigned long wrap_ticks)
update_sched_clock();
}

-void __init setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate)
+static u64 __init sched_clock_calc_wrap(int bits, unsigned long rate)
{
- unsigned long r, w;
+ unsigned long r;
u64 res, wrap;
char r_unit;

- if (cd.rate > rate)
- return;
-
- BUG_ON(bits > 32);
- WARN_ON(!irqs_disabled());
- read_sched_clock = read;
- sched_clock_mask = (1 << bits) - 1;
- cd.rate = rate;
-
- /* calculate the mult/shift to convert counter ticks to ns. */
- clocks_calc_mult_shift(&cd.mult, &cd.shift, rate, NSEC_PER_SEC, 0);
-
r = rate;
if (r >= 4000000) {
r /= 1000000;
@@ -134,12 +123,39 @@ void __init setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate)
/* calculate how many ns until we wrap */
wrap = cyc_to_ns((1ULL << bits) - 1, cd.mult, cd.shift);
do_div(wrap, NSEC_PER_MSEC);
- w = wrap;

/* calculate the ns resolution of this counter */
res = cyc_to_ns(1ULL, cd.mult, cd.shift);
- pr_info("sched_clock: %u bits at %lu%cHz, resolution %lluns, wraps every %lums\n",
- bits, r, r_unit, res, w);
+ pr_info("sched_clock: %u bits at %lu%cHz, resolution %lluns, wraps every %llums\n",
+ bits, r, r_unit, res, wrap);
+
+ return wrap;
+}
+
+static void __init try_to_enable_irqtime(unsigned long rate)
+{
+ /* Enable IRQ time accounting if we have a fast enough sched_clock */
+ if (irqtime > 0 || (irqtime == -1 && rate >= 1000000))
+ enable_sched_clock_irqtime();
+}
+
+void __init setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate)
+{
+ unsigned long w;
+
+ if (cd.rate > rate)
+ return;
+
+ BUG_ON(bits > 32);
+ WARN_ON(!irqs_disabled());
+ read_sched_clock = read;
+ sched_clock_mask = (1 << bits) - 1;
+ cd.rate = rate;
+
+ /* calculate the mult/shift to convert counter ticks to ns. */
+ clocks_calc_mult_shift(&cd.mult, &cd.shift, rate, NSEC_PER_SEC, 0);
+
+ w = sched_clock_calc_wrap(bits, rate);

/*
* Start the timer to keep sched_clock() properly updated and
@@ -153,9 +169,7 @@ void __init setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate)
*/
cd.epoch_ns = 0;

- /* Enable IRQ time accounting if we have a fast enough sched_clock */
- if (irqtime > 0 || (irqtime == -1 && rate >= 1000000))
- enable_sched_clock_irqtime();
+ try_to_enable_irqtime(rate);

pr_debug("Registered %pF as sched_clock source\n", read);
}
@@ -168,6 +182,32 @@ static unsigned long long notrace sched_clock_32(void)

unsigned long long __read_mostly (*sched_clock_func)(void) = sched_clock_32;

+static unsigned long long notrace sched_clock_64(void)
+{
+ u64 cyc = read_sched_clock_64() - cd.epoch_ns;
+ return cyc * cd.mult;
+}
+
+void __init
+setup_sched_clock_64(u64 (*read)(void), int bits, unsigned long rate)
+{
+ if (cd.rate > rate)
+ return;
+
+ BUG_ON(bits <= 32);
+ WARN_ON(!irqs_disabled());
+ read_sched_clock_64 = read;
+ sched_clock_func = sched_clock_64;
+ cd.rate = rate;
+ cd.mult = NSEC_PER_SEC / rate;
+ cd.epoch_ns = read_sched_clock_64();
+
+ sched_clock_calc_wrap(bits, rate);
+
+ try_to_enable_irqtime(rate);
+ pr_debug("Registered %pF as %u bit sched_clock source\n", read, bits);
+}
+
unsigned long long notrace sched_clock(void)
{
if (cd.suspended)
@@ -180,25 +220,34 @@ void __init sched_clock_postinit(void)
{
/*
* If no sched_clock function has been provided at that point,
- * make it the final one one.
+ * make it the final one.
*/
- if (read_sched_clock == jiffy_sched_clock_read)
+ if (read_sched_clock == jiffy_sched_clock_read && !read_sched_clock_64)
setup_sched_clock(jiffy_sched_clock_read, 32, HZ);

- sched_clock_poll(sched_clock_timer.data);
+ if (sched_clock_func == sched_clock_32)
+ sched_clock_poll(sched_clock_timer.data);
}

static int sched_clock_suspend(void)
{
- sched_clock_poll(sched_clock_timer.data);
+ if (sched_clock_func == sched_clock_32)
+ sched_clock_poll(sched_clock_timer.data);
+ else
+ cd.epoch_ns = read_sched_clock_64();
+
cd.suspended = true;
return 0;
}

static void sched_clock_resume(void)
{
- cd.epoch_cyc = read_sched_clock();
- cd.epoch_cyc_copy = cd.epoch_cyc;
+ if (sched_clock_func == sched_clock_32) {
+ cd.epoch_cyc = read_sched_clock();
+ cd.epoch_cyc_copy = cd.epoch_cyc;
+ } else {
+ cd.epoch_ns += read_sched_clock_64() - cd.epoch_ns;
+ }
cd.suspended = false;
}

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-06-02 06:40:59

by Stephen Boyd

[permalink] [raw]
Subject: [PATCHv2 6/6] arm64: Move to generic sched_clock infrastructure

Use the generic sched_clock infrastructure instead of rolling our
own. This has the added benefit of fixing suspend/resume as
outlined in 6a4dae5 (ARM: 7565/1: sched: stop sched_clock()
during suspend, 2012-10-23) and correcting the timestamps when
the hardware returns a value instead of 0 upon the first read.

Tested-by: Christopher Covington <[email protected]>
Acked-by: Catalin Marinas <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/time.c | 11 ++---------
2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 56b3f6d..f9c6e92 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -13,6 +13,7 @@ config ARM64
select GENERIC_IOMAP
select GENERIC_IRQ_PROBE
select GENERIC_IRQ_SHOW
+ select GENERIC_SCHED_CLOCK
select GENERIC_SMP_IDLE_THREAD
select GENERIC_TIME_VSYSCALL
select HARDIRQS_SW_RESEND
diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c
index a551f88..fd07ef9 100644
--- a/arch/arm64/kernel/time.c
+++ b/arch/arm64/kernel/time.c
@@ -33,6 +33,7 @@
#include <linux/irq.h>
#include <linux/delay.h>
#include <linux/clocksource.h>
+#include <linux/sched_clock.h>

#include <clocksource/arm_arch_timer.h>

@@ -61,13 +62,6 @@ unsigned long profile_pc(struct pt_regs *regs)
EXPORT_SYMBOL(profile_pc);
#endif

-static u64 sched_clock_mult __read_mostly;
-
-unsigned long long notrace sched_clock(void)
-{
- return arch_timer_read_counter() * sched_clock_mult;
-}
-
int read_current_timer(unsigned long *timer_value)
{
*timer_value = arch_timer_read_counter();
@@ -84,8 +78,7 @@ void __init time_init(void)
if (!arch_timer_rate)
panic("Unable to initialise architected timer.\n");

- /* Cache the sched_clock multiplier to save a divide in the hot path. */
- sched_clock_mult = NSEC_PER_SEC / arch_timer_rate;
+ setup_sched_clock_64(arch_timer_read_counter, 56, arch_timer_rate);

/* Calibrate the delay loop directly */
lpj_fine = arch_timer_rate / HZ;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-06-02 06:41:09

by Stephen Boyd

[permalink] [raw]
Subject: [PATCHv2 5/6] ARM: arch_timer: Move to setup_sched_clock_64()

Register with the ARM sched_clock framework now that it supports
64 bits. This fixes two problems with the current sched_clock
support for machines using the architected timers. First off, we
don't subtract the start value from subsequent sched_clock calls
so we can potentially start off with sched_clock returning
gigantic numbers. Second, there is no support for suspend/resume
handling so problems such as discussed in 6a4dae5 (ARM: 7565/1:
sched: stop sched_clock() during suspend, 2012-10-23) can happen
without this patch.

Signed-off-by: Stephen Boyd <[email protected]>
---
arch/arm/kernel/arch_timer.c | 14 ++------------
include/linux/sched_clock.h | 3 ---
kernel/time/sched_clock.c | 3 ++-
3 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
index 221f07b..e1ea2bf 100644
--- a/arch/arm/kernel/arch_timer.c
+++ b/arch/arm/kernel/arch_timer.c
@@ -22,13 +22,6 @@ static unsigned long arch_timer_read_counter_long(void)
return arch_timer_read_counter();
}

-static u32 sched_clock_mult __read_mostly;
-
-static unsigned long long notrace arch_timer_sched_clock(void)
-{
- return arch_timer_read_counter() * sched_clock_mult;
-}
-
static struct delay_timer arch_delay_timer;

static void __init arch_timer_delay_timer_register(void)
@@ -48,11 +41,8 @@ int __init arch_timer_arch_init(void)

arch_timer_delay_timer_register();

- /* Cache the sched_clock multiplier to save a divide in the hot path. */
- sched_clock_mult = NSEC_PER_SEC / arch_timer_rate;
- sched_clock_func = arch_timer_sched_clock;
- pr_info("sched_clock: ARM arch timer >56 bits at %ukHz, resolution %uns\n",
- arch_timer_rate / 1000, sched_clock_mult);
+ /* 56 bits minimum, so we assume worst case rollover */
+ setup_sched_clock_64(arch_timer_read_counter, 56, arch_timer_rate);

return 0;
}
diff --git a/include/linux/sched_clock.h b/include/linux/sched_clock.h
index e732b39..495d4c6 100644
--- a/include/linux/sched_clock.h
+++ b/include/linux/sched_clock.h
@@ -15,9 +15,6 @@ static inline void sched_clock_postinit(void) { }
#endif

extern void setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate);
-
-extern unsigned long long (*sched_clock_func)(void);
-
extern void setup_sched_clock_64(u64 (*read)(void), int bits,
unsigned long rate);
#endif
diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
index 482242c..617a65e 100644
--- a/kernel/time/sched_clock.c
+++ b/kernel/time/sched_clock.c
@@ -180,7 +180,8 @@ static unsigned long long notrace sched_clock_32(void)
return cyc_to_sched_clock(cyc, sched_clock_mask);
}

-unsigned long long __read_mostly (*sched_clock_func)(void) = sched_clock_32;
+static unsigned long long __read_mostly
+(*sched_clock_func)(void) = sched_clock_32;

static unsigned long long notrace sched_clock_64(void)
{
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-06-03 07:12:47

by Baruch Siach

[permalink] [raw]
Subject: Re: [PATCHv2 3/6] sched_clock: Make ARM's sched_clock generic for all architectures

Hi Stephen,

On Sat, Jun 01, 2013 at 11:39:40PM -0700, Stephen Boyd wrote:
> Nothing about the sched_clock implementation in the ARM port is
> specific to the architecture. Generalize the code so that other
> architectures can use it by selecting GENERIC_SCHED_CLOCK.
>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> arch/arm/Kconfig | 1 +
> arch/arm/common/timer-sp.c | 2 +-
> arch/arm/kernel/Makefile | 2 +-
> arch/arm/kernel/arch_timer.c | 2 +-
> arch/arm/kernel/time.c | 4 +---
> arch/arm/mach-davinci/time.c | 2 +-
> arch/arm/mach-imx/time.c | 2 +-
> arch/arm/mach-integrator/integrator_ap.c | 2 +-
> arch/arm/mach-ixp4xx/common.c | 2 +-
> arch/arm/mach-mmp/time.c | 2 +-
> arch/arm/mach-msm/timer.c | 2 +-
> arch/arm/mach-omap1/time.c | 2 +-
> arch/arm/mach-omap2/timer.c | 2 +-
> arch/arm/mach-pxa/time.c | 2 +-
> arch/arm/mach-sa1100/time.c | 2 +-
> arch/arm/mach-u300/timer.c | 2 +-
> arch/arm/plat-iop/time.c | 2 +-
> arch/arm/plat-omap/counter_32k.c | 2 +-
> arch/arm/plat-orion/time.c | 2 +-
> arch/arm/plat-samsung/samsung-time.c | 2 +-
> arch/arm/plat-versatile/sched-clock.c | 2 +-
> drivers/clocksource/bcm2835_timer.c | 2 +-
> drivers/clocksource/clksrc-dbx500-prcmu.c | 3 +--
> drivers/clocksource/dw_apb_timer_of.c | 2 +-
> drivers/clocksource/mxs_timer.c | 2 +-
> drivers/clocksource/nomadik-mtu.c | 2 +-
> drivers/clocksource/samsung_pwm_timer.c | 2 +-
> drivers/clocksource/tegra20_timer.c | 2 +-
> drivers/clocksource/time-armada-370-xp.c | 2 +-
> drivers/clocksource/timer-marco.c | 2 +-
> drivers/clocksource/timer-prima2.c | 2 +-
> {arch/arm/include/asm => include/linux}/sched_clock.h | 9 +++++++--

Shouldn't we just merge this header into the existing linux/sched.h?

baruch

> init/Kconfig | 3 +++
> init/main.c | 2 ++
> kernel/time/Makefile | 1 +
> {arch/arm/kernel => kernel/time}/sched_clock.c | 3 +--
> 36 files changed, 45 insertions(+), 37 deletions(-)
> rename {arch/arm/include/asm => include/linux}/sched_clock.h (75%)
> rename {arch/arm/kernel => kernel/time}/sched_clock.c (99%)

--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- [email protected] - tel: +972.2.679.5364, http://www.tkos.co.il -

2013-06-03 08:53:00

by Baruch Siach

[permalink] [raw]
Subject: Re: [PATCHv2 0/6] Make ARM's sched_clock generic + 64 bit friendly

Hi Stephen,

On Sat, Jun 01, 2013 at 11:39:37PM -0700, Stephen Boyd wrote:
> This is mostly a resend of a patch series I sent a little over a month
> ago. I've reordered the patches so that John can pick up the first three
> and get a generic sched_clock layer without having to take the 64 bit
> patches. The last three patches add 64 bit support and move the architected
> timers on ARM64 and ARM to use it.

You can have my

Build-tested-by: Baruch Siach <[email protected]>

for xtensa target, with the dw_apb_timer driver (no real hardware yet).

Thanks for pushing this series.

baruch

> Stephen Boyd (6):
> ARM: sched_clock: Remove unused needs_suspend member
> ARM: sched_clock: Return suspended count earlier
> sched_clock: Make ARM's sched_clock generic for all architectures
> sched_clock: Add support for >32 bit sched_clock
> ARM: arch_timer: Move to setup_sched_clock_64()
> arm64: Move to generic sched_clock infrastructure
>
> arch/arm/Kconfig | 1 +
> arch/arm/common/timer-sp.c | 2 +-
> arch/arm/kernel/Makefile | 2 +-
> arch/arm/kernel/arch_timer.c | 16 +--
> arch/arm/kernel/time.c | 4 +-
> arch/arm/mach-davinci/time.c | 2 +-
> arch/arm/mach-imx/time.c | 2 +-
> arch/arm/mach-integrator/integrator_ap.c | 2 +-
> arch/arm/mach-ixp4xx/common.c | 2 +-
> arch/arm/mach-mmp/time.c | 2 +-
> arch/arm/mach-msm/timer.c | 2 +-
> arch/arm/mach-omap1/time.c | 2 +-
> arch/arm/mach-omap2/timer.c | 2 +-
> arch/arm/mach-pxa/time.c | 2 +-
> arch/arm/mach-sa1100/time.c | 2 +-
> arch/arm/mach-u300/timer.c | 2 +-
> arch/arm/plat-iop/time.c | 2 +-
> arch/arm/plat-omap/counter_32k.c | 2 +-
> arch/arm/plat-orion/time.c | 2 +-
> arch/arm/plat-samsung/samsung-time.c | 2 +-
> arch/arm/plat-versatile/sched-clock.c | 2 +-
> arch/arm64/Kconfig | 1 +
> arch/arm64/kernel/time.c | 11 +-
> drivers/clocksource/bcm2835_timer.c | 2 +-
> drivers/clocksource/clksrc-dbx500-prcmu.c | 3 +-
> drivers/clocksource/dw_apb_timer_of.c | 2 +-
> drivers/clocksource/mxs_timer.c | 2 +-
> drivers/clocksource/nomadik-mtu.c | 2 +-
> drivers/clocksource/samsung_pwm_timer.c | 2 +-
> drivers/clocksource/tegra20_timer.c | 2 +-
> drivers/clocksource/time-armada-370-xp.c | 2 +-
> drivers/clocksource/timer-marco.c | 2 +-
> drivers/clocksource/timer-prima2.c | 2 +-
> .../include/asm => include/linux}/sched_clock.h | 14 ++-
> init/Kconfig | 3 +
> init/main.c | 2 +
> kernel/time/Makefile | 1 +
> {arch/arm/kernel => kernel/time}/sched_clock.c | 114 +++++++++++++++------
> 38 files changed, 132 insertions(+), 92 deletions(-)
> rename {arch/arm/include/asm => include/linux}/sched_clock.h (62%)
> rename {arch/arm/kernel => kernel/time}/sched_clock.c (74%)
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- [email protected] - tel: +972.2.679.5364, http://www.tkos.co.il -

2013-06-03 09:39:57

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCHv2 4/6] sched_clock: Add support for >32 bit sched_clock

On Sat, Jun 01, 2013 at 11:39:41PM -0700, Stephen Boyd wrote:
> The ARM architected system counter has at least 56 useable bits.
> Add support for counters with more than 32 bits to the generic
> sched_clock implementation so we can avoid the complexity of
> dealing with wrap-around on these devices while benefiting from
> the irqtime accounting and suspend/resume handling that the
> generic sched_clock code already has.

This looks like a horrid hack to me.

> +static unsigned long long notrace sched_clock_64(void)
> +{
> + u64 cyc = read_sched_clock_64() - cd.epoch_ns;
> + return cyc * cd.mult;

So, the use of cd.mult implies that the return value from
read_sched_clock_64() is not nanoseconds but something else. But then
we subtract it from the nanoseconds epoch - which has to be nanoseconds
because you simply return that when suspended.

> +}
> +
> +void __init
> +setup_sched_clock_64(u64 (*read)(void), int bits, unsigned long rate)
> +{
> + if (cd.rate > rate)
> + return;
> +
> + BUG_ON(bits <= 32);
> + WARN_ON(!irqs_disabled());
> + read_sched_clock_64 = read;
> + sched_clock_func = sched_clock_64;
> + cd.rate = rate;
> + cd.mult = NSEC_PER_SEC / rate;

Here, you don't check that the (2^bits) * mult results in a wrap of the
resulting 64-bit number, which is a _basic_ requirement for sched_clock
(hence all the code for <=32bit clocks, otherwise we wouldn't need this
complexity in the first place.)

So, I think this whole approach is broken.

2013-06-03 19:50:16

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCHv2 3/6] sched_clock: Make ARM's sched_clock generic for all architectures

On 06/03/13 00:12, Baruch Siach wrote:
> Hi Stephen,
>
> On Sat, Jun 01, 2013 at 11:39:40PM -0700, Stephen Boyd wrote:
>> Nothing about the sched_clock implementation in the ARM port is
>> specific to the architecture. Generalize the code so that other
>> architectures can use it by selecting GENERIC_SCHED_CLOCK.
>>
>> Signed-off-by: Stephen Boyd <[email protected]>
>> ---
>> arch/arm/Kconfig | 1 +
>> arch/arm/common/timer-sp.c | 2 +-
>> arch/arm/kernel/Makefile | 2 +-
>> arch/arm/kernel/arch_timer.c | 2 +-
>> arch/arm/kernel/time.c | 4 +---
>> arch/arm/mach-davinci/time.c | 2 +-
>> arch/arm/mach-imx/time.c | 2 +-
>> arch/arm/mach-integrator/integrator_ap.c | 2 +-
>> arch/arm/mach-ixp4xx/common.c | 2 +-
>> arch/arm/mach-mmp/time.c | 2 +-
>> arch/arm/mach-msm/timer.c | 2 +-
>> arch/arm/mach-omap1/time.c | 2 +-
>> arch/arm/mach-omap2/timer.c | 2 +-
>> arch/arm/mach-pxa/time.c | 2 +-
>> arch/arm/mach-sa1100/time.c | 2 +-
>> arch/arm/mach-u300/timer.c | 2 +-
>> arch/arm/plat-iop/time.c | 2 +-
>> arch/arm/plat-omap/counter_32k.c | 2 +-
>> arch/arm/plat-orion/time.c | 2 +-
>> arch/arm/plat-samsung/samsung-time.c | 2 +-
>> arch/arm/plat-versatile/sched-clock.c | 2 +-
>> drivers/clocksource/bcm2835_timer.c | 2 +-
>> drivers/clocksource/clksrc-dbx500-prcmu.c | 3 +--
>> drivers/clocksource/dw_apb_timer_of.c | 2 +-
>> drivers/clocksource/mxs_timer.c | 2 +-
>> drivers/clocksource/nomadik-mtu.c | 2 +-
>> drivers/clocksource/samsung_pwm_timer.c | 2 +-
>> drivers/clocksource/tegra20_timer.c | 2 +-
>> drivers/clocksource/time-armada-370-xp.c | 2 +-
>> drivers/clocksource/timer-marco.c | 2 +-
>> drivers/clocksource/timer-prima2.c | 2 +-
>> {arch/arm/include/asm => include/linux}/sched_clock.h | 9 +++++++--
> Shouldn't we just merge this header into the existing linux/sched.h?

I don't know. John/Thomas, any thoughts? One benefit with it this way is
that we don't have to recompile all the timer drivers if we change
sched.h for other reasons.

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2013-06-03 21:12:04

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCHv2 4/6] sched_clock: Add support for >32 bit sched_clock

On 06/03/13 02:39, Russell King - ARM Linux wrote:
> On Sat, Jun 01, 2013 at 11:39:41PM -0700, Stephen Boyd wrote:
>> +static unsigned long long notrace sched_clock_64(void)
>> +{
>> + u64 cyc = read_sched_clock_64() - cd.epoch_ns;
>> + return cyc * cd.mult;
> So, the use of cd.mult implies that the return value from
> read_sched_clock_64() is not nanoseconds but something else. But then
> we subtract it from the nanoseconds epoch - which has to be nanoseconds
> because you simply return that when suspended.

You're right, it is confusing and broken. I was thinking we may need a
union for epoch_ns but I will try to make it always nanoseconds and see
if that makes the code clearer.

>
>> +}
>> +
>> +void __init
>> +setup_sched_clock_64(u64 (*read)(void), int bits, unsigned long rate)
>> +{
>> + if (cd.rate > rate)
>> + return;
>> +
>> + BUG_ON(bits <= 32);
>> + WARN_ON(!irqs_disabled());
>> + read_sched_clock_64 = read;
>> + sched_clock_func = sched_clock_64;
>> + cd.rate = rate;
>> + cd.mult = NSEC_PER_SEC / rate;
> Here, you don't check that the (2^bits) * mult results in a wrap of the
> resulting 64-bit number, which is a _basic_ requirement for sched_clock
> (hence all the code for <=32bit clocks, otherwise we wouldn't need this
> complexity in the first place.)

Ok I will use clocks_calc_mult_shift() here.

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2013-06-03 22:15:20

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCHv2 4/6] sched_clock: Add support for >32 bit sched_clock

On Mon, Jun 03, 2013 at 02:11:59PM -0700, Stephen Boyd wrote:
> On 06/03/13 02:39, Russell King - ARM Linux wrote:
> > On Sat, Jun 01, 2013 at 11:39:41PM -0700, Stephen Boyd wrote:
> >> +}
> >> +
> >> +void __init
> >> +setup_sched_clock_64(u64 (*read)(void), int bits, unsigned long rate)
> >> +{
> >> + if (cd.rate > rate)
> >> + return;
> >> +
> >> + BUG_ON(bits <= 32);
> >> + WARN_ON(!irqs_disabled());
> >> + read_sched_clock_64 = read;
> >> + sched_clock_func = sched_clock_64;
> >> + cd.rate = rate;
> >> + cd.mult = NSEC_PER_SEC / rate;
> > Here, you don't check that the (2^bits) * mult results in a wrap of the
> > resulting 64-bit number, which is a _basic_ requirement for sched_clock
> > (hence all the code for <=32bit clocks, otherwise we wouldn't need this
> > complexity in the first place.)
>
> Ok I will use clocks_calc_mult_shift() here.

No, that's not the problem.

If you have a 56-bit clock which ticks at a period of 1ns, then
cd.rate = 1, and your sched_clock() values will be truncated to 56-bits.
The scheduler always _requires_ 64-bits from sched_clock. That's why we
have the complicated code to extend the 32-bits-or-less to a _full_
64-bit value.

Let me make this clearer: sched_clock() return values _must_ without
exception monotonically increment from zero to 2^64-1 and then wrap
back to zero. No other behaviour is acceptable for sched_clock().

2013-06-04 00:19:55

by John Stultz

[permalink] [raw]
Subject: Re: [PATCHv2 0/6] Make ARM's sched_clock generic + 64 bit friendly

On 06/01/2013 11:39 PM, Stephen Boyd wrote:
> This is mostly a resend of a patch series I sent a little over a month
> ago. I've reordered the patches so that John can pick up the first three
> and get a generic sched_clock layer without having to take the 64 bit
> patches. The last three patches add 64 bit support and move the architected
> timers on ARM64 and ARM to use it.


Yea, so from the initial look over it, I think I'm happy with queuing
the first three patches, although I'd like to get some acks from arm
folks on at least the first two, just so no one is surprised with it
going through the -tip tree.

It looks like the arm64 bit stuff needs a few more iterations w/
Russell, so I'll not touch those.

We still have a ways to go to consolidate x86 into this (need to handle
pv hooks), but it should provide a base to get some of the simpler
arches sharing the same infrastructure.

Also I suspect we can maybe further combine this with the
kernel/sched/clock.c code, but I think this is a good start for now.

thanks
-john

2013-06-04 01:52:04

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCHv2 4/6] sched_clock: Add support for >32 bit sched_clock

On 06/03/13 15:12, Russell King - ARM Linux wrote:
> On Mon, Jun 03, 2013 at 02:11:59PM -0700, Stephen Boyd wrote:
>> On 06/03/13 02:39, Russell King - ARM Linux wrote:
>>> On Sat, Jun 01, 2013 at 11:39:41PM -0700, Stephen Boyd wrote:
>>>> +}
>>>> +
>>>> +void __init
>>>> +setup_sched_clock_64(u64 (*read)(void), int bits, unsigned long rate)
>>>> +{
>>>> + if (cd.rate > rate)
>>>> + return;
>>>> +
>>>> + BUG_ON(bits <= 32);
>>>> + WARN_ON(!irqs_disabled());
>>>> + read_sched_clock_64 = read;
>>>> + sched_clock_func = sched_clock_64;
>>>> + cd.rate = rate;
>>>> + cd.mult = NSEC_PER_SEC / rate;
>>> Here, you don't check that the (2^bits) * mult results in a wrap of the
>>> resulting 64-bit number, which is a _basic_ requirement for sched_clock
>>> (hence all the code for <=32bit clocks, otherwise we wouldn't need this
>>> complexity in the first place.)
>> Ok I will use clocks_calc_mult_shift() here.
> No, that's not the problem.
>
> If you have a 56-bit clock which ticks at a period of 1ns, then
> cd.rate = 1, and your sched_clock() values will be truncated to 56-bits.
> The scheduler always _requires_ 64-bits from sched_clock. That's why we
> have the complicated code to extend the 32-bits-or-less to a _full_
> 64-bit value.
>
> Let me make this clearer: sched_clock() return values _must_ without
> exception monotonically increment from zero to 2^64-1 and then wrap
> back to zero. No other behaviour is acceptable for sched_clock().

Ok so you're saying if we have less than 64 bits of useable information
we _must_ do something to find where the wraparound will occur and
adjust for it so that epoch_ns is always incrementing until 2^64-1. Fair
enough. I was trying to avoid more work because on arm architected timer
platforms it takes many years for that to happen.

I'll see what I can do.

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2013-06-04 10:21:36

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCHv2 4/6] sched_clock: Add support for >32 bit sched_clock

On Mon, Jun 03, 2013 at 06:51:59PM -0700, Stephen Boyd wrote:
> On 06/03/13 15:12, Russell King - ARM Linux wrote:
> > If you have a 56-bit clock which ticks at a period of 1ns, then
> > cd.rate = 1, and your sched_clock() values will be truncated to 56-bits.
> > The scheduler always _requires_ 64-bits from sched_clock. That's why we
> > have the complicated code to extend the 32-bits-or-less to a _full_
> > 64-bit value.
> >
> > Let me make this clearer: sched_clock() return values _must_ without
> > exception monotonically increment from zero to 2^64-1 and then wrap
> > back to zero. No other behaviour is acceptable for sched_clock().
>
> Ok so you're saying if we have less than 64 bits of useable information
> we _must_ do something to find where the wraparound will occur and
> adjust for it so that epoch_ns is always incrementing until 2^64-1. Fair
> enough. I was trying to avoid more work because on arm architected timer
> platforms it takes many years for that to happen.
>
> I'll see what I can do.

Well, 56 bits at 1ns intervals is 833 days (2^56 / (1000000000*60*60*24)).
We used to say that 497 days was enough several years ago, and that got
fixed. We used to say 640K was enough memory for anything, and that
got fixed.

Whenever there's a limit, that limit will always be exceeded. 833 days
uptime has already been exceeded by ARM machines - I have one at the
moment:

11:17:58 up 1082 days, 11:53, 14 users, load average: 1.20, 1.28, 1.32

and I would not be surprised if there were others around.

2013-06-04 16:10:24

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCHv2 0/6] Make ARM's sched_clock generic + 64 bit friendly

On Tue, Jun 04, 2013 at 01:19:48AM +0100, John Stultz wrote:
> On 06/01/2013 11:39 PM, Stephen Boyd wrote:
> > This is mostly a resend of a patch series I sent a little over a month
> > ago. I've reordered the patches so that John can pick up the first three
> > and get a generic sched_clock layer without having to take the 64 bit
> > patches. The last three patches add 64 bit support and move the architected
> > timers on ARM64 and ARM to use it.
>
>
> Yea, so from the initial look over it, I think I'm happy with queuing
> the first three patches, although I'd like to get some acks from arm
> folks on at least the first two, just so no one is surprised with it
> going through the -tip tree.

The first two patches are pretty simple so, FWIW, you can add my ack on those:

Acked-by: Will Deacon <[email protected]>

Will

2013-06-04 17:53:11

by John Stultz

[permalink] [raw]
Subject: Re: [PATCHv2 0/6] Make ARM's sched_clock generic + 64 bit friendly

On 06/04/2013 09:09 AM, Will Deacon wrote:
> On Tue, Jun 04, 2013 at 01:19:48AM +0100, John Stultz wrote:
>> On 06/01/2013 11:39 PM, Stephen Boyd wrote:
>>> This is mostly a resend of a patch series I sent a little over a month
>>> ago. I've reordered the patches so that John can pick up the first three
>>> and get a generic sched_clock layer without having to take the 64 bit
>>> patches. The last three patches add 64 bit support and move the architected
>>> timers on ARM64 and ARM to use it.
>>
>> Yea, so from the initial look over it, I think I'm happy with queuing
>> the first three patches, although I'd like to get some acks from arm
>> folks on at least the first two, just so no one is surprised with it
>> going through the -tip tree.
> The first two patches are pretty simple so, FWIW, you can add my ack on those:
>
> Acked-by: Will Deacon <[email protected]>
Thanks! Added to the commits.

-john

2013-06-04 17:56:06

by John Stultz

[permalink] [raw]
Subject: Re: [PATCHv2 3/6] sched_clock: Make ARM's sched_clock generic for all architectures

On 06/03/2013 12:50 PM, Stephen Boyd wrote:
> On 06/03/13 00:12, Baruch Siach wrote:
>> Hi Stephen,
>>
>> On Sat, Jun 01, 2013 at 11:39:40PM -0700, Stephen Boyd wrote:
>>> {arch/arm/include/asm => include/linux}/sched_clock.h | 9 +++++++--
>> Shouldn't we just merge this header into the existing linux/sched.h?
> I don't know. John/Thomas, any thoughts? One benefit with it this way is
> that we don't have to recompile all the timer drivers if we change
> sched.h for other reasons.

Yea, I'm fine keeping it separate for now. We can merge them together if
we see fit later.

But if anyone feels particularly strongly, let me know.

thanks
-john

2013-06-04 18:16:30

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCHv2 3/6] sched_clock: Make ARM's sched_clock generic for all architectures

On Tue, Jun 04, 2013 at 10:56:00AM -0700, John Stultz wrote:
> On 06/03/2013 12:50 PM, Stephen Boyd wrote:
>> On 06/03/13 00:12, Baruch Siach wrote:
>>> Hi Stephen,
>>>
>>> On Sat, Jun 01, 2013 at 11:39:40PM -0700, Stephen Boyd wrote:
>>>> {arch/arm/include/asm => include/linux}/sched_clock.h | 9 +++++++--
>>> Shouldn't we just merge this header into the existing linux/sched.h?
>> I don't know. John/Thomas, any thoughts? One benefit with it this way is
>> that we don't have to recompile all the timer drivers if we change
>> sched.h for other reasons.
>
> Yea, I'm fine keeping it separate for now. We can merge them together if
> we see fit later.
>
> But if anyone feels particularly strongly, let me know.

I'd suggest keeping it separate. linux/sched.h is already a big source of
rebuilds because it's included by virtually the entire kernel. Having
linux/sched.h carved up into smaller chunks (maybe moving the definition
of task_struct and associated bits out of it) would probably be a good idea.

2013-06-10 04:12:15

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCHv2 4/6] sched_clock: Add support for >32 bit sched_clock

On 06/04/2013 05:21 AM, Russell King - ARM Linux wrote:
> On Mon, Jun 03, 2013 at 06:51:59PM -0700, Stephen Boyd wrote:
>> On 06/03/13 15:12, Russell King - ARM Linux wrote:
>>> If you have a 56-bit clock which ticks at a period of 1ns, then
>>> cd.rate = 1, and your sched_clock() values will be truncated to 56-bits.
>>> The scheduler always _requires_ 64-bits from sched_clock. That's why we
>>> have the complicated code to extend the 32-bits-or-less to a _full_
>>> 64-bit value.
>>>
>>> Let me make this clearer: sched_clock() return values _must_ without
>>> exception monotonically increment from zero to 2^64-1 and then wrap
>>> back to zero. No other behaviour is acceptable for sched_clock().
>>
>> Ok so you're saying if we have less than 64 bits of useable information
>> we _must_ do something to find where the wraparound will occur and
>> adjust for it so that epoch_ns is always incrementing until 2^64-1. Fair
>> enough. I was trying to avoid more work because on arm architected timer
>> platforms it takes many years for that to happen.
>>
>> I'll see what I can do.
>
> Well, 56 bits at 1ns intervals is 833 days (2^56 / (1000000000*60*60*24)).
> We used to say that 497 days was enough several years ago, and that got
> fixed. We used to say 640K was enough memory for anything, and that
> got fixed.

The ARM ARM states a minimum resolution of 40 years AND at least 56-bits
of resolution. So a 1Gz counter would have to have more that 56 bits.

Rob

2013-06-10 15:16:39

by anish singh

[permalink] [raw]
Subject: Re: [PATCHv2 4/6] sched_clock: Add support for >32 bit sched_clock

On Tue, Jun 4, 2013 at 3:42 AM, Russell King - ARM Linux
<[email protected]> wrote:
> On Mon, Jun 03, 2013 at 02:11:59PM -0700, Stephen Boyd wrote:
>> On 06/03/13 02:39, Russell King - ARM Linux wrote:
>> > On Sat, Jun 01, 2013 at 11:39:41PM -0700, Stephen Boyd wrote:
>> >> +}
>> >> +
>> >> +void __init
>> >> +setup_sched_clock_64(u64 (*read)(void), int bits, unsigned long rate)
>> >> +{
>> >> + if (cd.rate > rate)
>> >> + return;
>> >> +
>> >> + BUG_ON(bits <= 32);
>> >> + WARN_ON(!irqs_disabled());
>> >> + read_sched_clock_64 = read;
>> >> + sched_clock_func = sched_clock_64;
>> >> + cd.rate = rate;
>> >> + cd.mult = NSEC_PER_SEC / rate;
>> > Here, you don't check that the (2^bits) * mult results in a wrap of the
>> > resulting 64-bit number, which is a _basic_ requirement for sched_clock
>> > (hence all the code for <=32bit clocks, otherwise we wouldn't need this
>> > complexity in the first place.)
>>
>> Ok I will use clocks_calc_mult_shift() here.
>
> No, that's not the problem.
>
> If you have a 56-bit clock which ticks at a period of 1ns, then
> cd.rate = 1, and your sched_clock() values will be truncated to 56-bits.
> The scheduler always _requires_ 64-bits from sched_clock. That's why we
> have the complicated code to extend the 32-bits-or-less to a _full_
> 64-bit value.
>
> Let me make this clearer: sched_clock() return values _must_ without
> exception monotonically increment from zero to 2^64-1 and then wrap
> back to zero. No other behaviour is acceptable for sched_clock().

Probably a trivial question.I was wondering why this particular requirement
exists in the first place.I looked into this commit 112f38a4a3 but couldn't
gather the reason.

> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2013-06-10 15:39:12

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCHv2 4/6] sched_clock: Add support for >32 bit sched_clock

On Mon, Jun 10, 2013 at 08:46:36PM +0530, anish singh wrote:
> Probably a trivial question.I was wondering why this particular requirement
> exists in the first place.I looked into this commit 112f38a4a3 but couldn't
> gather the reason.

You're looking at a commit introducing an implementation. The requirement
isn't driven by the implementation. It's driven by the code and the maths
in the core scheduler, and its been a requirement for years.

sched_clock() needs to be monotonic, and needs to wrap at 64-bit, because
calculations are done by comparing the difference of two 64-bit values
returned from this function.

Let's take a trivial example - if you have a 16 bit counter, and you have
a value of 0xc000 ns, and next time you read it, it has value 0x0001 ns,
then what value do you end up with when you calculate the time passed
using 64-bit maths.

That's 0x0000000000000001 - 0x000000000000c000. The answer is a very big
number which is not the correct 16385. This means that things like process
timeslice counting and scheduler fairness is compromised - I'd expect even
more so if you're running RT and this is being used to provide guarantees.

2013-06-10 16:01:25

by anish singh

[permalink] [raw]
Subject: Re: [PATCHv2 4/6] sched_clock: Add support for >32 bit sched_clock

On Mon, Jun 10, 2013 at 9:08 PM, Russell King - ARM Linux
<[email protected]> wrote:

Least I can do is to say "Thanks".
> On Mon, Jun 10, 2013 at 08:46:36PM +0530, anish singh wrote:
>> Probably a trivial question.I was wondering why this particular requirement
>> exists in the first place.I looked into this commit 112f38a4a3 but couldn't
>> gather the reason.
>
> You're looking at a commit introducing an implementation. The requirement
> isn't driven by the implementation. It's driven by the code and the maths
> in the core scheduler, and its been a requirement for years.
>
> sched_clock() needs to be monotonic, and needs to wrap at 64-bit, because
> calculations are done by comparing the difference of two 64-bit values
> returned from this function.

Yes, and this is the question.If it is 32 bit then also it can overflow but
it will happen relatively fast.So I guess that is the reason why we use 64 bit
and this will avoid recalculations for recalibration.
>
> Let's take a trivial example - if you have a 16 bit counter, and you have
> a value of 0xc000 ns, and next time you read it, it has value 0x0001 ns,
> then what value do you end up with when you calculate the time passed
> using 64-bit maths.
>
> That's 0x0000000000000001 - 0x000000000000c000. The answer is a very big
> number which is not the correct 16385. This means that things like process
> timeslice counting and scheduler fairness is compromised - I'd expect even

So you mean when counter overflows the scheduler doesn't handle it?
> more so if you're running RT and this is being used to provide guarantees.

2013-06-10 16:08:26

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCHv2 4/6] sched_clock: Add support for >32 bit sched_clock

On Mon, Jun 10, 2013 at 09:31:21PM +0530, anish singh wrote:
> On Mon, Jun 10, 2013 at 9:08 PM, Russell King - ARM Linux
> <[email protected]> wrote:
>
> Least I can do is to say "Thanks".
> > On Mon, Jun 10, 2013 at 08:46:36PM +0530, anish singh wrote:
> >> Probably a trivial question.I was wondering why this particular requirement
> >> exists in the first place.I looked into this commit 112f38a4a3 but couldn't
> >> gather the reason.
> >
> > You're looking at a commit introducing an implementation. The requirement
> > isn't driven by the implementation. It's driven by the code and the maths
> > in the core scheduler, and its been a requirement for years.
> >
> > sched_clock() needs to be monotonic, and needs to wrap at 64-bit, because
> > calculations are done by comparing the difference of two 64-bit values
> > returned from this function.
>
> Yes, and this is the question.If it is 32 bit then also it can overflow but
> it will happen relatively fast.So I guess that is the reason why we use 64 bit
> and this will avoid recalculations for recalibration.

And that's why 112f38a4a3 is there - to ensure that we extend a 32-bit
or smaller counter all the way up to the full 64-bits. This replaces
the previous generation code which only extended it to 63-bits. Problems
were reported!

> > Let's take a trivial example - if you have a 16 bit counter, and you have
> > a value of 0xc000 ns, and next time you read it, it has value 0x0001 ns,
> > then what value do you end up with when you calculate the time passed
> > using 64-bit maths.
> >
> > That's 0x0000000000000001 - 0x000000000000c000. The answer is a very big
> > number which is not the correct 16385. This means that things like process
> > timeslice counting and scheduler fairness is compromised - I'd expect even
>
> So you mean when counter overflows the scheduler doesn't handle it?

There is no handling of counter overflows at scheduler level because
the specification for sched_clock() is that this function _will_ return
a monotonically increasing 64-bit value from 0 to the maximum 64-bit
value.

The reason for this is that there are popular architectures around
which do this natively, so the powers that be do not want additional
useless code cluttering their architectures.

2013-06-14 17:18:11

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCHv2 4/6] sched_clock: Add support for >32 bit sched_clock

On Mon, Jun 10, 2013 at 05:12:08AM +0100, Rob Herring wrote:
> On 06/04/2013 05:21 AM, Russell King - ARM Linux wrote:
> > On Mon, Jun 03, 2013 at 06:51:59PM -0700, Stephen Boyd wrote:
> >> On 06/03/13 15:12, Russell King - ARM Linux wrote:
> >>> If you have a 56-bit clock which ticks at a period of 1ns, then
> >>> cd.rate = 1, and your sched_clock() values will be truncated to 56-bits.
> >>> The scheduler always _requires_ 64-bits from sched_clock. That's why we
> >>> have the complicated code to extend the 32-bits-or-less to a _full_
> >>> 64-bit value.
> >>>
> >>> Let me make this clearer: sched_clock() return values _must_ without
> >>> exception monotonically increment from zero to 2^64-1 and then wrap
> >>> back to zero. No other behaviour is acceptable for sched_clock().
> >>
> >> Ok so you're saying if we have less than 64 bits of useable information
> >> we _must_ do something to find where the wraparound will occur and
> >> adjust for it so that epoch_ns is always incrementing until 2^64-1. Fair
> >> enough. I was trying to avoid more work because on arm architected timer
> >> platforms it takes many years for that to happen.
> >>
> >> I'll see what I can do.
> >
> > Well, 56 bits at 1ns intervals is 833 days (2^56 / (1000000000*60*60*24)).
> > We used to say that 497 days was enough several years ago, and that got
> > fixed. We used to say 640K was enough memory for anything, and that
> > got fixed.
>
> The ARM ARM states a minimum resolution of 40 years AND at least 56-bits
> of resolution. So a 1Gz counter would have to have more that 56 bits.

At a quick calculation, with a full 64-bit counter and 40-year roll-over
we can have maximum 14.6GHz clock. So we shouldn't just mask the top
8-bit of the counter as the bottom 56 could roll over in much less time.

--
Catalin

2013-06-16 09:45:09

by Baruch Siach

[permalink] [raw]
Subject: Re: [PATCHv2 0/6] Make ARM's sched_clock generic + 64 bit friendly

Hi John,

On Tue, Jun 04, 2013 at 10:53:04AM -0700, John Stultz wrote:
> On 06/04/2013 09:09 AM, Will Deacon wrote:
> >On Tue, Jun 04, 2013 at 01:19:48AM +0100, John Stultz wrote:
> >>On 06/01/2013 11:39 PM, Stephen Boyd wrote:
> >>>This is mostly a resend of a patch series I sent a little over a month
> >>>ago. I've reordered the patches so that John can pick up the first three
> >>>and get a generic sched_clock layer without having to take the 64 bit
> >>>patches. The last three patches add 64 bit support and move the architected
> >>>timers on ARM64 and ARM to use it.
> >>
> >>Yea, so from the initial look over it, I think I'm happy with queuing
> >>the first three patches, although I'd like to get some acks from arm
> >>folks on at least the first two, just so no one is surprised with it
> >>going through the -tip tree.
> >The first two patches are pretty simple so, FWIW, you can add my ack on those:
> >
> > Acked-by: Will Deacon <[email protected]>
> Thanks! Added to the commits.

What is the status of this series then? Is there a chance of seeing it in
3.11? I want to base some xtensa architecture work on this, so I'd like to
know whether there is a stable base to start from.

baruch

--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- [email protected] - tel: +972.2.679.5364, http://www.tkos.co.il -

2013-06-17 16:23:06

by John Stultz

[permalink] [raw]
Subject: Re: [PATCHv2 0/6] Make ARM's sched_clock generic + 64 bit friendly

On 06/16/2013 02:45 AM, Baruch Siach wrote:
> Hi John,
>
> On Tue, Jun 04, 2013 at 10:53:04AM -0700, John Stultz wrote:
>> On 06/04/2013 09:09 AM, Will Deacon wrote:
>>> On Tue, Jun 04, 2013 at 01:19:48AM +0100, John Stultz wrote:
>>>> On 06/01/2013 11:39 PM, Stephen Boyd wrote:
>>>>> This is mostly a resend of a patch series I sent a little over a month
>>>>> ago. I've reordered the patches so that John can pick up the first three
>>>>> and get a generic sched_clock layer without having to take the 64 bit
>>>>> patches. The last three patches add 64 bit support and move the architected
>>>>> timers on ARM64 and ARM to use it.
>>>> Yea, so from the initial look over it, I think I'm happy with queuing
>>>> the first three patches, although I'd like to get some acks from arm
>>>> folks on at least the first two, just so no one is surprised with it
>>>> going through the -tip tree.
>>> The first two patches are pretty simple so, FWIW, you can add my ack on those:
>>>
>>> Acked-by: Will Deacon <[email protected]>
>> Thanks! Added to the commits.
> What is the status of this series then? Is there a chance of seeing it in
> 3.11? I want to base some xtensa architecture work on this, so I'd like to
> know whether there is a stable base to start from.
I've got the first three in my internal tree, and was going to send it
by Thomas for 3.11 this week.

Thanks for the ping!
-john

2013-06-17 18:02:24

by Baruch Siach

[permalink] [raw]
Subject: Re: [PATCHv2 0/6] Make ARM's sched_clock generic + 64 bit friendly

Hi John,

On Mon, Jun 17, 2013 at 09:23:00AM -0700, John Stultz wrote:
> On 06/16/2013 02:45 AM, Baruch Siach wrote:
> >On Tue, Jun 04, 2013 at 10:53:04AM -0700, John Stultz wrote:
> >>On 06/04/2013 09:09 AM, Will Deacon wrote:
> >>>On Tue, Jun 04, 2013 at 01:19:48AM +0100, John Stultz wrote:
> >>>>On 06/01/2013 11:39 PM, Stephen Boyd wrote:
> >>>>>This is mostly a resend of a patch series I sent a little over a month
> >>>>>ago. I've reordered the patches so that John can pick up the first three
> >>>>>and get a generic sched_clock layer without having to take the 64 bit
> >>>>>patches. The last three patches add 64 bit support and move the architected
> >>>>>timers on ARM64 and ARM to use it.
> >>>>Yea, so from the initial look over it, I think I'm happy with queuing
> >>>>the first three patches, although I'd like to get some acks from arm
> >>>>folks on at least the first two, just so no one is surprised with it
> >>>>going through the -tip tree.
> >>>The first two patches are pretty simple so, FWIW, you can add my ack on those:
> >>>
> >>> Acked-by: Will Deacon <[email protected]>
> >>Thanks! Added to the commits.
> >What is the status of this series then? Is there a chance of seeing it in
> >3.11? I want to base some xtensa architecture work on this, so I'd like to
> >know whether there is a stable base to start from.
> I've got the first three in my internal tree, and was going to send
> it by Thomas for 3.11 this week.

Good, thanks. This means that the xtensa ccount sched_clock patch
(http://lists.linux-xtensa.org/pipermail/linux-xtensa/Week-of-Mon-20130617/001077.html)
that depends on the third patch in this series, can go in for 3.11, if the
xtensa maintainer acks it (added to Cc). How should we handle the dependency
then?

baruch

--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- [email protected] - tel: +972.2.679.5364, http://www.tkos.co.il -

2013-06-17 18:14:13

by John Stultz

[permalink] [raw]
Subject: Re: [PATCHv2 0/6] Make ARM's sched_clock generic + 64 bit friendly

On 06/17/2013 11:02 AM, Baruch Siach wrote:
>> to send
>> >it by Thomas for 3.11 this week.
> Good, thanks. This means that the xtensa ccount sched_clock patch
> (http://lists.linux-xtensa.org/pipermail/linux-xtensa/Week-of-Mon-20130617/001077.html)
> that depends on the third patch in this series, can go in for 3.11, if the
> xtensa maintainer acks it (added to Cc). How should we handle the dependency
> then?
Well, Thomas hasn't picked them up yet, so he could still object, so no
promises yet :)

As for the dependency, either you can base your patches off of
tip/timers/core (once the patches land there) and send a pull request to
the maintainer (so he gets the dependencies in -tip), or we can see
about queuing your changes via tip/timers/core (assuming you get
maintainer acks).

thanks
-john

2013-06-21 15:46:44

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCHv2 3/6] sched_clock: Make ARM's sched_clock generic for all architectures

On Sunday 02 June 2013, Stephen Boyd wrote:
> Nothing about the sched_clock implementation in the ARM port is
> specific to the architecture. Generalize the code so that other
> architectures can use it by selecting GENERIC_SCHED_CLOCK.
>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> arch/arm/Kconfig | 1 +
> arch/arm/common/timer-sp.c | 2 +-
> arch/arm/kernel/Makefile | 2 +-
> arch/arm/kernel/arch_timer.c | 2 +-
> arch/arm/kernel/time.c | 4 +---
> arch/arm/mach-davinci/time.c | 2 +-
> arch/arm/mach-imx/time.c | 2 +-
> arch/arm/mach-integrator/integrator_ap.c | 2 +-
> arch/arm/mach-ixp4xx/common.c | 2 +-
> arch/arm/mach-mmp/time.c | 2 +-
> arch/arm/mach-msm/timer.c | 2 +-
> arch/arm/mach-omap1/time.c | 2 +-
> arch/arm/mach-omap2/timer.c | 2 +-
> arch/arm/mach-pxa/time.c | 2 +-
> arch/arm/mach-sa1100/time.c | 2 +-
> arch/arm/mach-u300/timer.c | 2 +-
> arch/arm/plat-iop/time.c | 2 +-
> arch/arm/plat-omap/counter_32k.c | 2 +-
> arch/arm/plat-orion/time.c | 2 +-
> arch/arm/plat-samsung/samsung-time.c | 2 +-
> arch/arm/plat-versatile/sched-clock.c | 2 +-
> drivers/clocksource/bcm2835_timer.c | 2 +-
> drivers/clocksource/clksrc-dbx500-prcmu.c | 3 +--
> drivers/clocksource/dw_apb_timer_of.c | 2 +-
> drivers/clocksource/mxs_timer.c | 2 +-
> drivers/clocksource/nomadik-mtu.c | 2 +-
> drivers/clocksource/samsung_pwm_timer.c | 2 +-
> drivers/clocksource/tegra20_timer.c | 2 +-
> drivers/clocksource/time-armada-370-xp.c | 2 +-
> drivers/clocksource/timer-marco.c | 2 +-
> drivers/clocksource/timer-prima2.c | 2 +-
> {arch/arm/include/asm => include/linux}/sched_clock.h | 9 +++++++--
> init/Kconfig | 3 +++
> init/main.c | 2 ++
> kernel/time/Makefile | 1 +
> {arch/arm/kernel => kernel/time}/sched_clock.c | 3 +--

This causes build failures when doing a simple merge with the arm-soc
tree:

==> build/clps711x_defconfig/faillog <==
/git/arm-soc/arch/arm/mach-clps711x/common.c:37:29: fatal error: asm/sched_clock.h: No such file or directory
#include <asm/sched_clock.h>
^
==> build/imx_v6_v7_defconfig/faillog <==
/git/arm-soc/drivers/clocksource/vf_pit_timer.c:15:29: fatal error: asm/sched_clock.h: No such file or directory
#include <asm/sched_clock.h>

How about adding back a temporary arch/arm/include/asm/sched_clock.h
that only contains "#include <linux/sched_clock.h>" so we can change
those two files after the merge and still get a bisectible history?

Arnd

2013-06-21 17:05:28

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCHv2 3/6] sched_clock: Make ARM's sched_clock generic for all architectures

On 06/21, Arnd Bergmann wrote:
>
> This causes build failures when doing a simple merge with the arm-soc
> tree:
>
> ==> build/clps711x_defconfig/faillog <==
> /git/arm-soc/arch/arm/mach-clps711x/common.c:37:29: fatal error: asm/sched_clock.h: No such file or directory
> #include <asm/sched_clock.h>
> ^
> ==> build/imx_v6_v7_defconfig/faillog <==
> /git/arm-soc/drivers/clocksource/vf_pit_timer.c:15:29: fatal error: asm/sched_clock.h: No such file or directory
> #include <asm/sched_clock.h>
>
> How about adding back a temporary arch/arm/include/asm/sched_clock.h
> that only contains "#include <linux/sched_clock.h>" so we can change
> those two files after the merge and still get a bisectible history?
>

Sounds fine. John can you add this patch on top?

----8<-----
Subject: [PATCH] sched_clock: Add temporary asm/sched_clock.h

Some new users of the ARM sched_clock framework are going through
the arm-soc tree. Before 38ff87f (sched_clock: Make ARM's
sched_clock generic for all architectures, 2013-06-01) the header
file was in asm, but now it's in linux. One solution would be to
do an evil merge of the arm-soc tree and fix up the asm users,
but it's easier to add a temporary asm header that we can remove
along with the few stragglers after the merge window is over.

Signed-off-by: Stephen Boyd <[email protected]>
---
arch/arm/include/asm/sched_clock.h | 4 ++++
1 file changed, 4 insertions(+)
create mode 100644 arch/arm/include/asm/sched_clock.h

diff --git a/arch/arm/include/asm/sched_clock.h b/arch/arm/include/asm/sched_clock.h
new file mode 100644
index 0000000..2389b71
--- /dev/null
+++ b/arch/arm/include/asm/sched_clock.h
@@ -0,0 +1,4 @@
+/* You shouldn't include this file. Use linux/sched_clock.h instead.
+ * Temporary file until all asm/sched_clock.h users are gone
+ */
+#include <linux/sched_clock.h>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2013-06-21 17:42:14

by John Stultz

[permalink] [raw]
Subject: Re: [PATCHv2 3/6] sched_clock: Make ARM's sched_clock generic for all architectures

On 06/21/2013 10:05 AM, Stephen Boyd wrote:
> On 06/21, Arnd Bergmann wrote:
>> This causes build failures when doing a simple merge with the arm-soc
>> tree:
>>
>> ==> build/clps711x_defconfig/faillog <==
>> /git/arm-soc/arch/arm/mach-clps711x/common.c:37:29: fatal error: asm/sched_clock.h: No such file or directory
>> #include <asm/sched_clock.h>
>> ^
>> ==> build/imx_v6_v7_defconfig/faillog <==
>> /git/arm-soc/drivers/clocksource/vf_pit_timer.c:15:29: fatal error: asm/sched_clock.h: No such file or directory
>> #include <asm/sched_clock.h>
>>
>> How about adding back a temporary arch/arm/include/asm/sched_clock.h
>> that only contains "#include <linux/sched_clock.h>" so we can change
>> those two files after the merge and still get a bisectible history?
>>
> Sounds fine. John can you add this patch on top?

Thanks. I've queued this and will send it on to Thomas here shortly.

thanks
-john

2013-06-24 22:45:55

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCHv2 3/6] sched_clock: Make ARM's sched_clock generic for all architectures

On 06/21/13 10:42, John Stultz wrote:
> On 06/21/2013 10:05 AM, Stephen Boyd wrote:
>> On 06/21, Arnd Bergmann wrote:
>>> This causes build failures when doing a simple merge with the arm-soc
>>> tree:
>>>
>>> ==> build/clps711x_defconfig/faillog <==
>>> /git/arm-soc/arch/arm/mach-clps711x/common.c:37:29: fatal error:
>>> asm/sched_clock.h: No such file or directory
>>> #include <asm/sched_clock.h>
>>> ^
>>> ==> build/imx_v6_v7_defconfig/faillog <==
>>> /git/arm-soc/drivers/clocksource/vf_pit_timer.c:15:29: fatal error:
>>> asm/sched_clock.h: No such file or directory
>>> #include <asm/sched_clock.h>
>>>
>>> How about adding back a temporary arch/arm/include/asm/sched_clock.h
>>> that only contains "#include <linux/sched_clock.h>" so we can change
>>> those two files after the merge and still get a bisectible history?
>>>
>> Sounds fine. John can you add this patch on top?
>
> Thanks. I've queued this and will send it on to Thomas here shortly.

Thanks John. Can you send this off to the tip tree? I think we'll need
to merge tip/timers into arm-soc so I can send the rest of my "kill ARM
local timer API" series through and this patch helps make that more
palatable.

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2013-06-24 22:54:56

by John Stultz

[permalink] [raw]
Subject: Re: [PATCHv2 3/6] sched_clock: Make ARM's sched_clock generic for all architectures

On 06/24/2013 03:45 PM, Stephen Boyd wrote:
> On 06/21/13 10:42, John Stultz wrote:
>> On 06/21/2013 10:05 AM, Stephen Boyd wrote:
>>> On 06/21, Arnd Bergmann wrote:
>>>> This causes build failures when doing a simple merge with the arm-soc
>>>> tree:
>>>>
>>>> ==> build/clps711x_defconfig/faillog <==
>>>> /git/arm-soc/arch/arm/mach-clps711x/common.c:37:29: fatal error:
>>>> asm/sched_clock.h: No such file or directory
>>>> #include <asm/sched_clock.h>
>>>> ^
>>>> ==> build/imx_v6_v7_defconfig/faillog <==
>>>> /git/arm-soc/drivers/clocksource/vf_pit_timer.c:15:29: fatal error:
>>>> asm/sched_clock.h: No such file or directory
>>>> #include <asm/sched_clock.h>
>>>>
>>>> How about adding back a temporary arch/arm/include/asm/sched_clock.h
>>>> that only contains "#include <linux/sched_clock.h>" so we can change
>>>> those two files after the merge and still get a bisectible history?
>>>>
>>> Sounds fine. John can you add this patch on top?
>> Thanks. I've queued this and will send it on to Thomas here shortly.
> Thanks John. Can you send this off to the tip tree? I think we'll need
> to merge tip/timers into arm-soc so I can send the rest of my "kill ARM
> local timer API" series through and this patch helps make that more
> palatable.

Thanks for the reminder! Just sent the pull request.

thanks
-john