2013-07-18 23:21:36

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v4 00/17] 64-bit friendly generic sched_clock()

This patchset adds support for 64 bit counters in the generic
sched_clock code and converts drivers over to use it. Based
on v3.11-rc1.

Changes since v3:
* Move to use seqcount to fix issues with 64-bit cyc counters
* Move to hrtimer to fix underflow/overflow errors in wraparound
calculation
* Use of 1 hour in clocks_calc_mult_shift
* Converted over drivers in drivers/clocksource

Stephen Boyd (17):
clocksource: Extract max nsec calculation into separate function
sched_clock: Use seqcount instead of rolling our own
sched_clock: Use an hrtimer instead of timer
sched_clock: Add support for >32 bit sched_clock
arch_timer: Move to generic sched_clock framework
sched_clock: Remove sched_clock_func() hook
clocksource: bcm2835: Switch to sched_clock_register()
ocksource: dbx500-prcmu: Switch to sched_clock_register()
clocksource: dw_apb_timer_of: Switch to sched_clock_register()
clocksource: mxs_timer: Switch to sched_clock_register()
clocksource: nomadik: Switch to sched_clock_register()
clocksource: samsung_pwm_timer: Switch to sched_clock_register()
clocksource: tegra: Switch to sched_clock_register()
clocksource: time-armada-370-xp: Switch to sched_clock_register()
clocksource: sirf: Switch to sched_clock_register() and use 64 bits
clocksource: vf_pit_timer: Switch to sched_clock_register()
sched_clock: Deprecate setup_sched_clock()

arch/arm/kernel/arch_timer.c | 14 ----
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/time.c | 10 ---
drivers/clocksource/arm_arch_timer.c | 4 ++
drivers/clocksource/bcm2835_timer.c | 4 +-
drivers/clocksource/clksrc-dbx500-prcmu.c | 5 +-
drivers/clocksource/dw_apb_timer_of.c | 4 +-
drivers/clocksource/mxs_timer.c | 4 +-
drivers/clocksource/nomadik-mtu.c | 4 +-
drivers/clocksource/samsung_pwm_timer.c | 4 +-
drivers/clocksource/tegra20_timer.c | 4 +-
drivers/clocksource/time-armada-370-xp.c | 4 +-
drivers/clocksource/timer-prima2.c | 6 +-
drivers/clocksource/vf_pit_timer.c | 4 +-
include/linux/clocksource.h | 2 +
include/linux/sched_clock.h | 7 +-
kernel/time/clocksource.c | 45 ++++++++-----
kernel/time/sched_clock.c | 105 +++++++++++++++---------------
18 files changed, 116 insertions(+), 115 deletions(-)

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


2013-07-18 23:21:44

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v4 09/17] clocksource: dw_apb_timer_of: Switch to sched_clock_register()

The 32 bit sched_clock interface now supports 64 bits. Upgrade to
the 64 bit function to allow us to remove the 32 bit registration
interface.

Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/clocksource/dw_apb_timer_of.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c
index 4cbae4f..003b230 100644
--- a/drivers/clocksource/dw_apb_timer_of.c
+++ b/drivers/clocksource/dw_apb_timer_of.c
@@ -106,7 +106,7 @@ static void add_clocksource(struct device_node *source_timer)
sched_rate = rate;
}

-static u32 read_sched_clock(void)
+static u64 read_sched_clock(void)
{
return __raw_readl(sched_io_base);
}
@@ -128,7 +128,7 @@ static void init_sched_clock(void)
of_node_put(sched_timer);
}

- setup_sched_clock(read_sched_clock, 32, sched_rate);
+ sched_clock_register(read_sched_clock, 32, sched_rate);
}

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

2013-07-18 23:21:47

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v4 08/17] ocksource: dbx500-prcmu: Switch to sched_clock_register()

The 32 bit sched_clock interface now supports 64 bits. Upgrade to
the 64 bit function to allow us to remove the 32 bit registration
interface.

Cc: Srinidhi Kasagar <[email protected]>
Cc: Linus Walleij <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/clocksource/clksrc-dbx500-prcmu.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/clksrc-dbx500-prcmu.c b/drivers/clocksource/clksrc-dbx500-prcmu.c
index a9fd4ad..b375106 100644
--- a/drivers/clocksource/clksrc-dbx500-prcmu.c
+++ b/drivers/clocksource/clksrc-dbx500-prcmu.c
@@ -53,7 +53,7 @@ static struct clocksource clocksource_dbx500_prcmu = {

#ifdef CONFIG_CLKSRC_DBX500_PRCMU_SCHED_CLOCK

-static u32 notrace dbx500_prcmu_sched_clock_read(void)
+static u64 notrace dbx500_prcmu_sched_clock_read(void)
{
if (unlikely(!clksrc_dbx500_timer_base))
return 0;
@@ -81,8 +81,7 @@ void __init clksrc_dbx500_prcmu_init(void __iomem *base)
clksrc_dbx500_timer_base + PRCMU_TIMER_REF);
}
#ifdef CONFIG_CLKSRC_DBX500_PRCMU_SCHED_CLOCK
- setup_sched_clock(dbx500_prcmu_sched_clock_read,
- 32, RATE_32K);
+ sched_clock_register(dbx500_prcmu_sched_clock_read, 32, RATE_32K);
#endif
clocksource_register_hz(&clocksource_dbx500_prcmu, RATE_32K);
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-07-18 23:21:49

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v4 14/17] clocksource: time-armada-370-xp: Switch to sched_clock_register()

The 32 bit sched_clock interface now supports 64 bits. Upgrade to
the 64 bit function to allow us to remove the 32 bit registration
interface.

Cc: Gregory CLEMENT <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/clocksource/time-armada-370-xp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
index efdca32..2bec8dc 100644
--- a/drivers/clocksource/time-armada-370-xp.c
+++ b/drivers/clocksource/time-armada-370-xp.c
@@ -71,7 +71,7 @@ static u32 ticks_per_jiffy;

static struct clock_event_device __percpu **percpu_armada_370_xp_evt;

-static u32 notrace armada_370_xp_read_sched_clock(void)
+static u64 notrace armada_370_xp_read_sched_clock(void)
{
return ~readl(timer_base + TIMER0_VAL_OFF);
}
@@ -258,7 +258,7 @@ void __init armada_370_xp_timer_init(void)
/*
* Set scale and timer for sched_clock.
*/
- setup_sched_clock(armada_370_xp_read_sched_clock, 32, timer_clk);
+ sched_clock_register(armada_370_xp_read_sched_clock, 32, timer_clk);

/*
* Setup free-running clocksource timer (interrupts
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-07-18 23:21:46

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v4 10/17] clocksource: mxs_timer: Switch to sched_clock_register()

The 32 bit sched_clock interface now supports 64 bits. Upgrade to
the 64 bit function to allow us to remove the 32 bit registration
interface.

Cc: Shawn Guo <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/clocksource/mxs_timer.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/mxs_timer.c b/drivers/clocksource/mxs_timer.c
index 0f5e65f..445b68a 100644
--- a/drivers/clocksource/mxs_timer.c
+++ b/drivers/clocksource/mxs_timer.c
@@ -222,7 +222,7 @@ static struct clocksource clocksource_mxs = {
.flags = CLOCK_SOURCE_IS_CONTINUOUS,
};

-static u32 notrace mxs_read_sched_clock_v2(void)
+static u64 notrace mxs_read_sched_clock_v2(void)
{
return ~readl_relaxed(mxs_timrot_base + HW_TIMROT_RUNNING_COUNTn(1));
}
@@ -236,7 +236,7 @@ static int __init mxs_clocksource_init(struct clk *timer_clk)
else {
clocksource_mmio_init(mxs_timrot_base + HW_TIMROT_RUNNING_COUNTn(1),
"mxs_timer", c, 200, 32, clocksource_mmio_readl_down);
- setup_sched_clock(mxs_read_sched_clock_v2, 32, c);
+ sched_clock_register(mxs_read_sched_clock_v2, 32, c);
}

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

2013-07-18 23:21:43

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v4 06/17] sched_clock: Remove sched_clock_func() hook

Nobody is using sched_clock_func() anymore now that sched_clock
supports up to 64 bits. Remove the hook so that new code only
uses sched_clock_register().

Signed-off-by: Stephen Boyd <[email protected]>
---
include/linux/sched_clock.h | 2 --
kernel/time/sched_clock.c | 9 +--------
2 files changed, 1 insertion(+), 10 deletions(-)

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

-extern unsigned long long (*sched_clock_func)(void);
-
#endif
diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
index 3aa83ba..659c59a 100644
--- a/kernel/time/sched_clock.c
+++ b/kernel/time/sched_clock.c
@@ -63,7 +63,7 @@ static inline u64 notrace cyc_to_ns(u64 cyc, u32 mult, u32 shift)
return (cyc * mult) >> shift;
}

-static unsigned long long notrace sched_clock_32(void)
+unsigned long long notrace sched_clock(void)
{
u64 epoch_ns;
u64 epoch_cyc;
@@ -176,13 +176,6 @@ void __init setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate)
sched_clock_register(read_sched_clock_32_wrapper, bits, rate);
}

-unsigned long long __read_mostly (*sched_clock_func)(void) = sched_clock_32;
-
-unsigned long long notrace sched_clock(void)
-{
- return sched_clock_func();
-}
-
void __init sched_clock_postinit(void)
{
/*
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-07-18 23:21:42

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v4 03/17] sched_clock: Use an hrtimer instead of timer

In the next patch we're going to increase the number of bits that
the generic sched_clock can handle to be greater than 32. With
more than 32 bits the wraparound time can be larger than what can
fit into the units that msecs_to_jiffies takes (unsigned int).
Luckily, the wraparound is initially calculated in nanoseconds
which we can easily use with hrtimers, so switch to using an
hrtimer.

Cc: Russell King <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
kernel/time/sched_clock.c | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
index 396f7b9..a269890b 100644
--- a/kernel/time/sched_clock.c
+++ b/kernel/time/sched_clock.c
@@ -8,15 +8,17 @@
#include <linux/clocksource.h>
#include <linux/init.h>
#include <linux/jiffies.h>
+#include <linux/ktime.h>
#include <linux/kernel.h>
#include <linux/moduleparam.h>
#include <linux/sched.h>
#include <linux/syscore_ops.h>
-#include <linux/timer.h>
+#include <linux/hrtimer.h>
#include <linux/sched_clock.h>
#include <linux/seqlock.h>

struct clock_data {
+ ktime_t wrap_kt;
u64 epoch_ns;
u32 epoch_cyc;
seqcount_t seq;
@@ -26,8 +28,7 @@ struct clock_data {
bool suspended;
};

-static void sched_clock_poll(unsigned long wrap_ticks);
-static DEFINE_TIMER(sched_clock_timer, sched_clock_poll, 0, 0);
+static struct hrtimer sched_clock_timer;
static int irqtime = -1;

core_param(irqtime, irqtime, int, 0400);
@@ -93,15 +94,16 @@ static void notrace update_sched_clock(void)
raw_local_irq_restore(flags);
}

-static void sched_clock_poll(unsigned long wrap_ticks)
+static enum hrtimer_restart sched_clock_poll(struct hrtimer *hrt)
{
- mod_timer(&sched_clock_timer, round_jiffies(jiffies + wrap_ticks));
update_sched_clock();
+ hrtimer_forward_now(hrt, cd.wrap_kt);
+ return HRTIMER_RESTART;
}

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

@@ -129,19 +131,19 @@ 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;
+ cd.wrap_kt = ns_to_ktime(wrap - (wrap >> 3));

/* 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 %lluns\n",
+ bits, r, r_unit, res, wrap);

/*
* Start the timer to keep sched_clock() properly updated and
* sets the initial epoch.
*/
- sched_clock_timer.data = msecs_to_jiffies(w - (w / 10));
+ hrtimer_init(&sched_clock_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ sched_clock_timer.function = sched_clock_poll;
update_sched_clock();

/*
@@ -172,12 +174,13 @@ void __init sched_clock_postinit(void)
if (read_sched_clock == jiffy_sched_clock_read)
setup_sched_clock(jiffy_sched_clock_read, 32, HZ);

- sched_clock_poll(sched_clock_timer.data);
+ update_sched_clock();
+ hrtimer_start(&sched_clock_timer, cd.wrap_kt, HRTIMER_MODE_REL);
}

static int sched_clock_suspend(void)
{
- sched_clock_poll(sched_clock_timer.data);
+ sched_clock_poll(&sched_clock_timer);
cd.suspended = true;
return 0;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-07-18 23:23:01

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v4 16/17] clocksource: vf_pit_timer: Switch to sched_clock_register()

The 32 bit sched_clock interface now supports 64 bits. Upgrade to
the 64 bit function to allow us to remove the 32 bit registration
interface.

Cc: Jingchang Lu <[email protected]>
Cc: Fabio Estevam <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/clocksource/vf_pit_timer.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/vf_pit_timer.c b/drivers/clocksource/vf_pit_timer.c
index 587e020..02821b0 100644
--- a/drivers/clocksource/vf_pit_timer.c
+++ b/drivers/clocksource/vf_pit_timer.c
@@ -52,7 +52,7 @@ static inline void pit_irq_acknowledge(void)
__raw_writel(PITTFLG_TIF, clkevt_base + PITTFLG);
}

-static unsigned int pit_read_sched_clock(void)
+static u64 pit_read_sched_clock(void)
{
return __raw_readl(clksrc_base + PITCVAL);
}
@@ -64,7 +64,7 @@ static int __init pit_clocksource_init(unsigned long rate)
__raw_writel(~0UL, clksrc_base + PITLDVAL);
__raw_writel(PITTCTRL_TEN, clksrc_base + PITTCTRL);

- setup_sched_clock(pit_read_sched_clock, 32, rate);
+ sched_clock_register(pit_read_sched_clock, 32, rate);
return clocksource_mmio_init(clksrc_base + PITCVAL, "vf-pit", rate,
300, 32, clocksource_mmio_readl_down);
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-07-18 23:22:59

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v4 17/17] sched_clock: Deprecate setup_sched_clock()

Users of the generic sched_clock framework should use
sched_clock_register() instead of setup_sched_clock(). Mark the
function as deprecated.

Signed-off-by: Stephen Boyd <[email protected]>
---
include/linux/sched_clock.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/sched_clock.h b/include/linux/sched_clock.h
index cddf0c2..d3da3e2 100644
--- a/include/linux/sched_clock.h
+++ b/include/linux/sched_clock.h
@@ -14,7 +14,8 @@ extern void sched_clock_postinit(void);
static inline void sched_clock_postinit(void) { }
#endif

-extern void setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate);
+/* Use sched_clock_register() instead */
+extern void __deprecated setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate);
extern void sched_clock_register(u64 (*read)(void), int bits,
unsigned long rate);

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

2013-07-18 23:23:39

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v4 15/17] clocksource: sirf: Switch to sched_clock_register() and use 64 bits

The 32 bit sched_clock interface now supports 64 bits. Upgrade to
the 64 bit function to allow us to remove the 32 bit registration
interface and use all 64 bits of this timer.

Cc: Barry Song <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/clocksource/timer-prima2.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/timer-prima2.c b/drivers/clocksource/timer-prima2.c
index ef3cfb2..8a492d3 100644
--- a/drivers/clocksource/timer-prima2.c
+++ b/drivers/clocksource/timer-prima2.c
@@ -165,9 +165,9 @@ static struct irqaction sirfsoc_timer_irq = {
};

/* Overwrite weak default sched_clock with more precise one */
-static u32 notrace sirfsoc_read_sched_clock(void)
+static u64 notrace sirfsoc_read_sched_clock(void)
{
- return (u32)(sirfsoc_timer_read(NULL) & 0xffffffff);
+ return sirfsoc_timer_read(NULL);
}

static void __init sirfsoc_clockevent_init(void)
@@ -206,7 +206,7 @@ static void __init sirfsoc_prima2_timer_init(struct device_node *np)

BUG_ON(clocksource_register_hz(&sirfsoc_clocksource, CLOCK_TICK_RATE));

- setup_sched_clock(sirfsoc_read_sched_clock, 32, CLOCK_TICK_RATE);
+ sched_clock_register(sirfsoc_read_sched_clock, 64, CLOCK_TICK_RATE);

BUG_ON(setup_irq(sirfsoc_timer_irq.irq, &sirfsoc_timer_irq));

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

2013-07-18 23:23:55

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v4 13/17] clocksource: tegra: Switch to sched_clock_register()

The 32 bit sched_clock interface now supports 64 bits. Upgrade to
the 64 bit function to allow us to remove the 32 bit registration
interface.

Cc: Stephen Warren <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/clocksource/tegra20_timer.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/tegra20_timer.c b/drivers/clocksource/tegra20_timer.c
index 9396170..5cff616 100644
--- a/drivers/clocksource/tegra20_timer.c
+++ b/drivers/clocksource/tegra20_timer.c
@@ -98,7 +98,7 @@ static struct clock_event_device tegra_clockevent = {
.set_mode = tegra_timer_set_mode,
};

-static u32 notrace tegra_read_sched_clock(void)
+static u64 notrace tegra_read_sched_clock(void)
{
return timer_readl(TIMERUS_CNTR_1US);
}
@@ -200,7 +200,7 @@ static void __init tegra20_init_timer(struct device_node *np)
WARN(1, "Unknown clock rate");
}

- setup_sched_clock(tegra_read_sched_clock, 32, 1000000);
+ sched_clock_register(tegra_read_sched_clock, 32, 1000000);

if (clocksource_mmio_init(timer_reg_base + TIMERUS_CNTR_1US,
"timer_us", 1000000, 300, 32, clocksource_mmio_readl_up)) {
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-07-18 23:24:12

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v4 12/17] clocksource: samsung_pwm_timer: Switch to sched_clock_register()

The 32 bit sched_clock interface now supports 64 bits. Upgrade to
the 64 bit function to allow us to remove the 32 bit registration
interface.

Cc: Tomasz Figa <[email protected]>
Cc: Kyungmin Park <[email protected]>
Cc: Kukjin Kim <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/clocksource/samsung_pwm_timer.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/samsung_pwm_timer.c b/drivers/clocksource/samsung_pwm_timer.c
index 584b547..09e8bc7 100644
--- a/drivers/clocksource/samsung_pwm_timer.c
+++ b/drivers/clocksource/samsung_pwm_timer.c
@@ -310,7 +310,7 @@ static void __iomem *samsung_timer_reg(void)
* this wraps around for now, since it is just a relative time
* stamp. (Inspired by U300 implementation.)
*/
-static u32 notrace samsung_read_sched_clock(void)
+static u64 notrace samsung_read_sched_clock(void)
{
void __iomem *reg = samsung_timer_reg();

@@ -337,7 +337,7 @@ static void __init samsung_clocksource_init(void)
samsung_time_setup(pwm.source_id, pwm.tcnt_max);
samsung_time_start(pwm.source_id, true);

- setup_sched_clock(samsung_read_sched_clock,
+ sched_clock_register(samsung_read_sched_clock,
pwm.variant.bits, clock_rate);

ret = clocksource_mmio_init(reg, "samsung_clocksource_timer",
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-07-18 23:21:39

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v4 02/17] sched_clock: Use seqcount instead of rolling our own

We're going to increase the cyc value to 64 bits in the near
future. Doing that is going to break the custom seqcount
implementation in the sched_clock code because 64 bit numbers
aren't guaranteed to be atomic. Replace the cyc_copy with a
seqcount to avoid this problem.

Cc: Russell King <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
kernel/time/sched_clock.c | 27 ++++++++-------------------
1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
index a326f27..396f7b9 100644
--- a/kernel/time/sched_clock.c
+++ b/kernel/time/sched_clock.c
@@ -14,11 +14,12 @@
#include <linux/syscore_ops.h>
#include <linux/timer.h>
#include <linux/sched_clock.h>
+#include <linux/seqlock.h>

struct clock_data {
u64 epoch_ns;
u32 epoch_cyc;
- u32 epoch_cyc_copy;
+ seqcount_t seq;
unsigned long rate;
u32 mult;
u32 shift;
@@ -54,23 +55,16 @@ static unsigned long long notrace sched_clock_32(void)
u64 epoch_ns;
u32 epoch_cyc;
u32 cyc;
+ unsigned long seq;

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
- * epoch_cyc_copy in strict order, and read them in strict order.
- * If epoch_cyc and epoch_cyc_copy are not equal, then we're in
- * the middle of an update, and we should repeat the load.
- */
do {
+ seq = read_seqcount_begin(&cd.seq);
epoch_cyc = cd.epoch_cyc;
- smp_rmb();
epoch_ns = cd.epoch_ns;
- smp_rmb();
- } while (epoch_cyc != cd.epoch_cyc_copy);
+ } while (read_seqcount_retry(&cd.seq, seq));

cyc = read_sched_clock();
cyc = (cyc - epoch_cyc) & sched_clock_mask;
@@ -90,16 +84,12 @@ static void notrace update_sched_clock(void)
ns = cd.epoch_ns +
cyc_to_ns((cyc - cd.epoch_cyc) & sched_clock_mask,
cd.mult, cd.shift);
- /*
- * Write epoch_cyc and epoch_ns in a way that the update is
- * detectable in cyc_to_fixed_sched_clock().
- */
+
raw_local_irq_save(flags);
- cd.epoch_cyc_copy = cyc;
- smp_wmb();
+ write_seqcount_begin(&cd.seq);
cd.epoch_ns = ns;
- smp_wmb();
cd.epoch_cyc = cyc;
+ write_seqcount_end(&cd.seq);
raw_local_irq_restore(flags);
}

@@ -195,7 +185,6 @@ static int sched_clock_suspend(void)
static void sched_clock_resume(void)
{
cd.epoch_cyc = read_sched_clock();
- cd.epoch_cyc_copy = cd.epoch_cyc;
cd.suspended = false;
}

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

2013-07-18 23:21:37

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v4 01/17] clocksource: Extract max nsec calculation into separate function

We need to calculate the same number in the clocksource code and
the sched_clock code, so extract this code into its own function.
We also drop the min_t and just use min() because the two types
are the same.

Signed-off-by: Stephen Boyd <[email protected]>
---
include/linux/clocksource.h | 2 ++
kernel/time/clocksource.c | 45 ++++++++++++++++++++++++++++++---------------
2 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index dbbf8aa..67301a4 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -292,6 +292,8 @@ extern void clocksource_resume(void);
extern struct clocksource * __init __weak clocksource_default_clock(void);
extern void clocksource_mark_unstable(struct clocksource *cs);

+extern u64
+clocks_calc_max_nsecs(u32 mult, u32 shift, u32 maxadj, u64 mask);
extern void
clocks_calc_mult_shift(u32 *mult, u32 *shift, u32 from, u32 to, u32 minsec);

diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index 50a8736..637a14a 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -537,40 +537,55 @@ static u32 clocksource_max_adjustment(struct clocksource *cs)
}

/**
- * clocksource_max_deferment - Returns max time the clocksource can be deferred
- * @cs: Pointer to clocksource
- *
+ * clocks_calc_max_nsecs - Returns maximum nanoseconds that can be converted
+ * @mult: cycle to nanosecond multiplier
+ * @shift: cycle to nanosecond divisor (power of two)
+ * @maxadj: maximum adjustment value to mult (~11%)
+ * @mask: bitmask for two's complement subtraction of non 64 bit counters
*/
-static u64 clocksource_max_deferment(struct clocksource *cs)
+u64 clocks_calc_max_nsecs(u32 mult, u32 shift, u32 maxadj, u64 mask)
{
u64 max_nsecs, max_cycles;

/*
* Calculate the maximum number of cycles that we can pass to the
* cyc2ns function without overflowing a 64-bit signed result. The
- * maximum number of cycles is equal to ULLONG_MAX/(cs->mult+cs->maxadj)
+ * maximum number of cycles is equal to ULLONG_MAX/(mult+maxadj)
* which is equivalent to the below.
- * max_cycles < (2^63)/(cs->mult + cs->maxadj)
- * max_cycles < 2^(log2((2^63)/(cs->mult + cs->maxadj)))
- * max_cycles < 2^(log2(2^63) - log2(cs->mult + cs->maxadj))
- * max_cycles < 2^(63 - log2(cs->mult + cs->maxadj))
- * max_cycles < 1 << (63 - log2(cs->mult + cs->maxadj))
+ * max_cycles < (2^63)/(mult + maxadj)
+ * max_cycles < 2^(log2((2^63)/(mult + maxadj)))
+ * max_cycles < 2^(log2(2^63) - log2(mult + maxadj))
+ * max_cycles < 2^(63 - log2(mult + maxadj))
+ * max_cycles < 1 << (63 - log2(mult + maxadj))
* Please note that we add 1 to the result of the log2 to account for
* any rounding errors, ensure the above inequality is satisfied and
* no overflow will occur.
*/
- max_cycles = 1ULL << (63 - (ilog2(cs->mult + cs->maxadj) + 1));
+ max_cycles = 1ULL << (63 - (ilog2(mult + maxadj) + 1));

/*
* The actual maximum number of cycles we can defer the clocksource is
- * determined by the minimum of max_cycles and cs->mask.
+ * determined by the minimum of max_cycles and mask.
* Note: Here we subtract the maxadj to make sure we don't sleep for
* too long if there's a large negative adjustment.
*/
- max_cycles = min_t(u64, max_cycles, (u64) cs->mask);
- max_nsecs = clocksource_cyc2ns(max_cycles, cs->mult - cs->maxadj,
- cs->shift);
+ max_cycles = min(max_cycles, mask);
+ max_nsecs = clocksource_cyc2ns(max_cycles, mult - maxadj, shift);
+
+ return max_nsecs;
+}
+
+/**
+ * clocksource_max_deferment - Returns max time the clocksource can be deferred
+ * @cs: Pointer to clocksource
+ *
+ */
+static u64 clocksource_max_deferment(struct clocksource *cs)
+{
+ u64 max_nsecs;

+ max_nsecs = clocks_calc_max_nsecs(cs->mult, cs->shift, cs->maxadj,
+ cs->mask);
/*
* To ensure that the clocksource does not wrap whilst we are idle,
* limit the time the clocksource can be deferred by 12.5%. Please
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-07-18 23:24:42

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v4 11/17] clocksource: nomadik: Switch to sched_clock_register()

The 32 bit sched_clock interface now supports 64 bits. Upgrade to
the 64 bit function to allow us to remove the 32 bit registration
interface.

Cc: Linus Walleij <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/clocksource/nomadik-mtu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/nomadik-mtu.c b/drivers/clocksource/nomadik-mtu.c
index 7d2c2c5..2242cd3 100644
--- a/drivers/clocksource/nomadik-mtu.c
+++ b/drivers/clocksource/nomadik-mtu.c
@@ -76,7 +76,7 @@ static struct delay_timer mtu_delay_timer;
* local implementation which uses the clocksource to get some
* better resolution when scheduling the kernel.
*/
-static u32 notrace nomadik_read_sched_clock(void)
+static u64 notrace nomadik_read_sched_clock(void)
{
if (unlikely(!mtu_base))
return 0;
@@ -230,7 +230,7 @@ static void __init __nmdk_timer_init(void __iomem *base, int irq,
"mtu_0");

#ifdef CONFIG_CLKSRC_NOMADIK_MTU_SCHED_CLOCK
- setup_sched_clock(nomadik_read_sched_clock, 32, rate);
+ sched_clock_register(nomadik_read_sched_clock, 32, rate);
#endif

/* Timer 1 is used for events, register irq and clockevents */
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-07-18 23:25:17

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v4 07/17] clocksource: bcm2835: Switch to sched_clock_register()

The 32 bit sched_clock interface now supports 64 bits. Upgrade to
the 64 bit function to allow us to remove the 32 bit registration
interface.

Cc: Stephen Warren <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/clocksource/bcm2835_timer.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/bcm2835_timer.c b/drivers/clocksource/bcm2835_timer.c
index 07ea7ce..26ed331 100644
--- a/drivers/clocksource/bcm2835_timer.c
+++ b/drivers/clocksource/bcm2835_timer.c
@@ -49,7 +49,7 @@ struct bcm2835_timer {

static void __iomem *system_clock __read_mostly;

-static u32 notrace bcm2835_sched_read(void)
+static u64 notrace bcm2835_sched_read(void)
{
return readl_relaxed(system_clock);
}
@@ -110,7 +110,7 @@ static void __init bcm2835_timer_init(struct device_node *node)
panic("Can't read clock-frequency");

system_clock = base + REG_COUNTER_LO;
- setup_sched_clock(bcm2835_sched_read, 32, freq);
+ sched_clock_register(bcm2835_sched_read, 32, freq);

clocksource_mmio_init(base + REG_COUNTER_LO, node->name,
freq, 300, 32, clocksource_mmio_readl_up);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-07-18 23:25:40

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v4 05/17] arch_timer: Move to generic sched_clock framework

Register with the generic 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. Finally, it
allows us to move the sched_clock setup into drivers clocksource
out of the arch ports.

Cc: Christopher Covington <[email protected]>
Cc: Catalin Marinas <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
arch/arm/kernel/arch_timer.c | 14 --------------
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/time.c | 10 ----------
drivers/clocksource/arm_arch_timer.c | 4 ++++
4 files changed, 5 insertions(+), 24 deletions(-)

diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
index 221f07b..1791f12 100644
--- a/arch/arm/kernel/arch_timer.c
+++ b/arch/arm/kernel/arch_timer.c
@@ -11,7 +11,6 @@
#include <linux/init.h>
#include <linux/types.h>
#include <linux/errno.h>
-#include <linux/sched_clock.h>

#include <asm/delay.h>

@@ -22,13 +21,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 +40,5 @@ 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);
-
return 0;
}
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 9737e97..e32b471 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -14,6 +14,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 03dc371..29c39d5 100644
--- a/arch/arm64/kernel/time.c
+++ b/arch/arm64/kernel/time.c
@@ -61,13 +61,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;
-}
-
void __init time_init(void)
{
u32 arch_timer_rate;
@@ -78,9 +71,6 @@ 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;
-
/* Calibrate the delay loop directly */
lpj_fine = arch_timer_rate / HZ;
}
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 053d846..2facf14 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -17,6 +17,7 @@
#include <linux/interrupt.h>
#include <linux/of_irq.h>
#include <linux/io.h>
+#include <linux/sched_clock.h>

#include <asm/arch_timer.h>
#include <asm/virt.h>
@@ -281,6 +282,9 @@ static int __init arch_timer_register(void)
timecounter_init(&timecounter, &cyclecounter,
arch_counter_get_cntvct());

+ /* 56 bits minimum, so we assume worst case rollover */
+ sched_clock_register(arch_timer_read_counter, 56, arch_timer_rate);
+
if (arch_timer_use_virtual) {
ppi = arch_timer_ppi[VIRT_PPI];
err = request_percpu_irq(ppi, arch_timer_handler_virt,
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-07-18 23:25:54

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v4 04/17] sched_clock: Add support for >32 bit sched_clock

The ARM architected system counter has at least 56 usable bits.
Add support for counters with more than 32 bits to the generic
sched_clock implementation so we can increase the time between
wakeups due to 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. On my
system using 56 bits over 32 bits changes the wraparound time
from a few minutes to an hour. For faster running counters (GHz
range) this is even more important because we may not be able to
execute the timer in time to deal with the wraparound if only 32
bits are used.

We choose a maxsec value of 3600 seconds because we assume no
system will go idle for more than an hour. In the future we may
need to increase this value.

Note: All users should switch over to the 64-bit read function so
we can remove setup_sched_clock() in favor of sched_clock_register().

Cc: Russell King <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
include/linux/sched_clock.h | 2 ++
kernel/time/sched_clock.c | 46 +++++++++++++++++++++++++++++++--------------
2 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/include/linux/sched_clock.h b/include/linux/sched_clock.h
index fa7922c..eca7abe 100644
--- a/include/linux/sched_clock.h
+++ b/include/linux/sched_clock.h
@@ -15,6 +15,8 @@ static inline void sched_clock_postinit(void) { }
#endif

extern void setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate);
+extern void sched_clock_register(u64 (*read)(void), int bits,
+ unsigned long rate);

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

diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
index a269890b..3aa83ba 100644
--- a/kernel/time/sched_clock.c
+++ b/kernel/time/sched_clock.c
@@ -16,11 +16,12 @@
#include <linux/hrtimer.h>
#include <linux/sched_clock.h>
#include <linux/seqlock.h>
+#include <linux/bitops.h>

struct clock_data {
ktime_t wrap_kt;
u64 epoch_ns;
- u32 epoch_cyc;
+ u64 epoch_cyc;
seqcount_t seq;
unsigned long rate;
u32 mult;
@@ -37,14 +38,25 @@ static struct clock_data cd = {
.mult = NSEC_PER_SEC / HZ,
};

-static u32 __read_mostly sched_clock_mask = 0xffffffff;
+static u64 __read_mostly sched_clock_mask;

-static u32 notrace jiffy_sched_clock_read(void)
+static u64 notrace jiffy_sched_clock_read(void)
{
- return (u32)(jiffies - INITIAL_JIFFIES);
+ /*
+ * We don't need to use get_jiffies_64 on 32-bit arches here
+ * because we register with BITS_PER_LONG
+ */
+ return (u64)(jiffies - INITIAL_JIFFIES);
+}
+
+static u32 __read_mostly (*read_sched_clock_32)(void);
+
+static u64 notrace read_sched_clock_32_wrapper(void)
+{
+ return read_sched_clock_32();
}

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

static inline u64 notrace cyc_to_ns(u64 cyc, u32 mult, u32 shift)
{
@@ -54,8 +66,8 @@ static inline u64 notrace cyc_to_ns(u64 cyc, u32 mult, u32 shift)
static unsigned long long notrace sched_clock_32(void)
{
u64 epoch_ns;
- u32 epoch_cyc;
- u32 cyc;
+ u64 epoch_cyc;
+ u64 cyc;
unsigned long seq;

if (cd.suspended)
@@ -78,7 +90,7 @@ static unsigned long long notrace sched_clock_32(void)
static void notrace update_sched_clock(void)
{
unsigned long flags;
- u32 cyc;
+ u64 cyc;
u64 ns;

cyc = read_sched_clock();
@@ -101,7 +113,8 @@ static enum hrtimer_restart sched_clock_poll(struct hrtimer *hrt)
return HRTIMER_RESTART;
}

-void __init setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate)
+void __init sched_clock_register(u64 (*read)(void), int bits,
+ unsigned long rate)
{
unsigned long r;
u64 res, wrap;
@@ -110,14 +123,13 @@ void __init setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate)
if (cd.rate > rate)
return;

- BUG_ON(bits > 32);
WARN_ON(!irqs_disabled());
read_sched_clock = read;
- sched_clock_mask = (1 << bits) - 1;
+ sched_clock_mask = CLOCKSOURCE_MASK(bits);
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);
+ clocks_calc_mult_shift(&cd.mult, &cd.shift, rate, NSEC_PER_SEC, 3600);

r = rate;
if (r >= 4000000) {
@@ -130,7 +142,7 @@ void __init setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate)
r_unit = ' ';

/* calculate how many ns until we wrap */
- wrap = cyc_to_ns((1ULL << bits) - 1, cd.mult, cd.shift);
+ wrap = clocks_calc_max_nsecs(cd.mult, cd.shift, 0, sched_clock_mask);
cd.wrap_kt = ns_to_ktime(wrap - (wrap >> 3));

/* calculate the ns resolution of this counter */
@@ -158,6 +170,12 @@ void __init setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate)
pr_debug("Registered %pF as sched_clock source\n", read);
}

+void __init setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate)
+{
+ read_sched_clock_32 = read;
+ sched_clock_register(read_sched_clock_32_wrapper, bits, rate);
+}
+
unsigned long long __read_mostly (*sched_clock_func)(void) = sched_clock_32;

unsigned long long notrace sched_clock(void)
@@ -172,7 +190,7 @@ void __init sched_clock_postinit(void)
* make it the final one one.
*/
if (read_sched_clock == jiffy_sched_clock_read)
- setup_sched_clock(jiffy_sched_clock_read, 32, HZ);
+ sched_clock_register(jiffy_sched_clock_read, BITS_PER_LONG, HZ);

update_sched_clock();
hrtimer_start(&sched_clock_timer, cd.wrap_kt, HRTIMER_MODE_REL);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2013-07-18 23:59:53

by John Stultz

[permalink] [raw]
Subject: Re: [PATCH v4 00/17] 64-bit friendly generic sched_clock()

On 07/18/2013 04:21 PM, Stephen Boyd wrote:
> This patchset adds support for 64 bit counters in the generic
> sched_clock code and converts drivers over to use it. Based
> on v3.11-rc1.
>
> Changes since v3:
> * Move to use seqcount to fix issues with 64-bit cyc counters
> * Move to hrtimer to fix underflow/overflow errors in wraparound
> calculation
> * Use of 1 hour in clocks_calc_mult_shift
> * Converted over drivers in drivers/clocksource

I've not been able to take a deep review yet, but this looks pretty much
like what we discussed last week, so I'm happy with it so far. Has this
gotten much testing (on both 32 and 64bit systems?)

One detail: Most of this is likely to go in via tip/timers/core, but the
5/17 "arch_timer: Move to generic sched_clock" will need some
synchronization with Catalin to make sure its ok to go in via tip. Not
sure what other arm64 changes are pending that would depend or collide
with that change.

thanks
-john

2013-07-19 00:18:32

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 08/17] ocksource: dbx500-prcmu: Switch to sched_clock_register()

On 07/18, Stephen Boyd wrote:
> The 32 bit sched_clock interface now supports 64 bits. Upgrade to
> the 64 bit function to allow us to remove the 32 bit registration
> interface.
>
> Cc: Srinidhi Kasagar <[email protected]>
> Cc: Linus Walleij <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---

Hmph, the subject got messed up. If I don't resend please correct
to clocksource.

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

2013-07-19 00:23:18

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 00/17] 64-bit friendly generic sched_clock()

On 07/18, John Stultz wrote:
> On 07/18/2013 04:21 PM, Stephen Boyd wrote:
> >This patchset adds support for 64 bit counters in the generic
> >sched_clock code and converts drivers over to use it. Based
> >on v3.11-rc1.
> >
> >Changes since v3:
> > * Move to use seqcount to fix issues with 64-bit cyc counters
> > * Move to hrtimer to fix underflow/overflow errors in wraparound
> > calculation
> > * Use of 1 hour in clocks_calc_mult_shift
> > * Converted over drivers in drivers/clocksource
>
> I've not been able to take a deep review yet, but this looks pretty
> much like what we discussed last week, so I'm happy with it so far.
> Has this gotten much testing (on both 32 and 64bit systems?)

I've tested it on a couple 32 bit systems. I'll ask around for
some 64 bit system testing.

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

2013-07-19 09:04:34

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v4 02/17] sched_clock: Use seqcount instead of rolling our own

On Fri, Jul 19, 2013 at 12:21:15AM +0100, Stephen Boyd wrote:
> We're going to increase the cyc value to 64 bits in the near
> future. Doing that is going to break the custom seqcount
> implementation in the sched_clock code because 64 bit numbers
> aren't guaranteed to be atomic. Replace the cyc_copy with a
> seqcount to avoid this problem.
>
> Cc: Russell King <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> kernel/time/sched_clock.c | 27 ++++++++-------------------
> 1 file changed, 8 insertions(+), 19 deletions(-)

Looks good to me. The current scheme would be very fiddly to extend to
64-bit values on 32-bit architectures without cheap atomic doubleword
accesses.

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

Will

2013-07-19 09:23:29

by Baruch Siach

[permalink] [raw]
Subject: Re: [PATCH v4 04/17] sched_clock: Add support for >32 bit sched_clock

Hi Stephen,

On Thu, Jul 18, 2013 at 04:21:17PM -0700, Stephen Boyd wrote:
> The ARM architected system counter has at least 56 usable bits.
> Add support for counters with more than 32 bits to the generic
> sched_clock implementation so we can increase the time between
> wakeups due to 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. On my
> system using 56 bits over 32 bits changes the wraparound time
> from a few minutes to an hour. For faster running counters (GHz
> range) this is even more important because we may not be able to
> execute the timer in time to deal with the wraparound if only 32
> bits are used.
>
> We choose a maxsec value of 3600 seconds because we assume no
> system will go idle for more than an hour. In the future we may
> need to increase this value.
>
> Note: All users should switch over to the 64-bit read function so
> we can remove setup_sched_clock() in favor of sched_clock_register().
>
> Cc: Russell King <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---

[..]

> @@ -110,14 +123,13 @@ void __init setup_sched_clock(u32 (*read)(void), int
> bits, unsigned long rate)
> if (cd.rate > rate)
> return;
>
> - BUG_ON(bits > 32);
> WARN_ON(!irqs_disabled());
> read_sched_clock = read;
> - sched_clock_mask = (1 << bits) - 1;
> + sched_clock_mask = CLOCKSOURCE_MASK(bits);

Note that this conflicts with my integer overflow fix
(http://article.gmane.org/gmane.linux.ports.arm.kernel/252498) that I hope
will get merged before 3.11 is out.

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-07-19 14:20:27

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH v4 02/17] sched_clock: Use seqcount instead of rolling our own

On Fri, 19 Jul 2013, Will Deacon wrote:

> Looks good to me. The current scheme would be very fiddly to extend to
> 64-bit values on 32-bit architectures without cheap atomic doubleword
> accesses.

You should have a look at include/linux/cnt32_to_63.h.
This could be applied to pure software counters if the low part is
atomically increased.


Nicolas

2013-07-19 14:27:38

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH v4 02/17] sched_clock: Use seqcount instead of rolling our own

On Fri, Jul 19, 2013 at 10:20:19AM -0400, Nicolas Pitre wrote:
> On Fri, 19 Jul 2013, Will Deacon wrote:
>
> > Looks good to me. The current scheme would be very fiddly to extend to
> > 64-bit values on 32-bit architectures without cheap atomic doubleword
> > accesses.
>
> You should have a look at include/linux/cnt32_to_63.h.
> This could be applied to pure software counters if the low part is
> atomically increased.

But this can't be used for sched_clock(). That's exactly why I originally
had to rewrite that thing in the first place.

2013-07-19 16:32:21

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 04/17] sched_clock: Add support for >32 bit sched_clock

On 07/19, Baruch Siach wrote:
> On Thu, Jul 18, 2013 at 04:21:17PM -0700, Stephen Boyd wrote:
> > @@ -110,14 +123,13 @@ void __init setup_sched_clock(u32 (*read)(void), int
> > bits, unsigned long rate)
> > if (cd.rate > rate)
> > return;
> >
> > - BUG_ON(bits > 32);
> > WARN_ON(!irqs_disabled());
> > read_sched_clock = read;
> > - sched_clock_mask = (1 << bits) - 1;
> > + sched_clock_mask = CLOCKSOURCE_MASK(bits);
>
> Note that this conflicts with my integer overflow fix
> (http://article.gmane.org/gmane.linux.ports.arm.kernel/252498) that I hope
> will get merged before 3.11 is out.
>

Thanks for the heads up. The conflict looks minor and I'll rebase
if necessary.

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

2013-07-19 19:34:24

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH v4 13/17] clocksource: tegra: Switch to sched_clock_register()

On 07/18/2013 05:21 PM, Stephen Boyd wrote:
> The 32 bit sched_clock interface now supports 64 bits. Upgrade to
> the 64 bit function to allow us to remove the 32 bit registration
> interface.

Acked-by: Stephen Warren <[email protected]>
Tested-by: Stephen Warren <[email protected]>

2013-07-19 19:35:02

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH v4 07/17] clocksource: bcm2835: Switch to sched_clock_register()

On 07/18/2013 05:21 PM, Stephen Boyd wrote:
> The 32 bit sched_clock interface now supports 64 bits. Upgrade to
> the 64 bit function to allow us to remove the 32 bit registration
> interface.

Acked-by: Stephen Warren <[email protected]>

2013-07-20 20:51:12

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v4 00/17] 64-bit friendly generic sched_clock()

On Fri, Jul 19, 2013 at 1:21 AM, Stephen Boyd <[email protected]> wrote:

> This patchset adds support for 64 bit counters in the generic
> sched_clock code and converts drivers over to use it. Based
> on v3.11-rc1.
>
> Changes since v3:

> ocksource: dbx500-prcmu: Switch to sched_clock_register()

Fix subject.

> clocksource: nomadik: Switch to sched_clock_register()

Apart from that Acked-by: Linus Walleij <[email protected]>
for these two drivers.

BTW: why are you not patching arch/arm/mach-u300/timer.c?
Please make sure to grep for all setup_sched_clock() if you're
going to do this.

Yours,
Linus Walleij

2013-07-22 08:10:05

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH v4 10/17] clocksource: mxs_timer: Switch to sched_clock_register()

On Thu, Jul 18, 2013 at 04:21:23PM -0700, Stephen Boyd wrote:
> The 32 bit sched_clock interface now supports 64 bits. Upgrade to
> the 64 bit function to allow us to remove the 32 bit registration
> interface.
>
> Cc: Shawn Guo <[email protected]>

Acked-by: Shawn Guo <[email protected]>

BTW, will the series break setup_sched_clock() users that are not
converted, like arch/arm/mach-imx/time.c? I can understand that's the
consequence of not moving things into drivers/ folder :)

Shawn

> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> drivers/clocksource/mxs_timer.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clocksource/mxs_timer.c b/drivers/clocksource/mxs_timer.c
> index 0f5e65f..445b68a 100644
> --- a/drivers/clocksource/mxs_timer.c
> +++ b/drivers/clocksource/mxs_timer.c
> @@ -222,7 +222,7 @@ static struct clocksource clocksource_mxs = {
> .flags = CLOCK_SOURCE_IS_CONTINUOUS,
> };
>
> -static u32 notrace mxs_read_sched_clock_v2(void)
> +static u64 notrace mxs_read_sched_clock_v2(void)
> {
> return ~readl_relaxed(mxs_timrot_base + HW_TIMROT_RUNNING_COUNTn(1));
> }
> @@ -236,7 +236,7 @@ static int __init mxs_clocksource_init(struct clk *timer_clk)
> else {
> clocksource_mmio_init(mxs_timrot_base + HW_TIMROT_RUNNING_COUNTn(1),
> "mxs_timer", c, 200, 32, clocksource_mmio_readl_down);
> - setup_sched_clock(mxs_read_sched_clock_v2, 32, c);
> + sched_clock_register(mxs_read_sched_clock_v2, 32, c);
> }
>
> return 0;
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
>

2013-07-22 16:23:51

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 10/17] clocksource: mxs_timer: Switch to sched_clock_register()

On 07/22, Shawn Guo wrote:
> On Thu, Jul 18, 2013 at 04:21:23PM -0700, Stephen Boyd wrote:
> > The 32 bit sched_clock interface now supports 64 bits. Upgrade to
> > the 64 bit function to allow us to remove the 32 bit registration
> > interface.
> >
> > Cc: Shawn Guo <[email protected]>
>
> Acked-by: Shawn Guo <[email protected]>
>
> BTW, will the series break setup_sched_clock() users that are not
> converted, like arch/arm/mach-imx/time.c? I can understand that's the
> consequence of not moving things into drivers/ folder :)

Thanks.

I will convert all arch/arm users when this series is pulled into
the tip tree. The arm patches will have to go through the arm-soc
branch and so we'll need some stable branch in tip for that.

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

2013-07-22 16:24:43

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 00/17] 64-bit friendly generic sched_clock()

On 07/20, Linus Walleij wrote:
> On Fri, Jul 19, 2013 at 1:21 AM, Stephen Boyd <[email protected]> wrote:
>
> > This patchset adds support for 64 bit counters in the generic
> > sched_clock code and converts drivers over to use it. Based
> > on v3.11-rc1.
> >
> > Changes since v3:
>
> > ocksource: dbx500-prcmu: Switch to sched_clock_register()
>
> Fix subject.
>
> > clocksource: nomadik: Switch to sched_clock_register()
>
> Apart from that Acked-by: Linus Walleij <[email protected]>
> for these two drivers.
>
> BTW: why are you not patching arch/arm/mach-u300/timer.c?
> Please make sure to grep for all setup_sched_clock() if you're
> going to do this.

Thanks.

I'll convert the arch/arm users once this series is accepted into
the tip tree. Then send off the rest through the arm-soc tree.


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

2013-07-22 17:07:18

by John Stultz

[permalink] [raw]
Subject: Re: [PATCH v4 00/17] 64-bit friendly generic sched_clock()

On 07/18/2013 04:21 PM, Stephen Boyd wrote:
> This patchset adds support for 64 bit counters in the generic
> sched_clock code and converts drivers over to use it. Based
> on v3.11-rc1.
>
> Changes since v3:
> * Move to use seqcount to fix issues with 64-bit cyc counters
> * Move to hrtimer to fix underflow/overflow errors in wraparound
> calculation
> * Use of 1 hour in clocks_calc_mult_shift
> * Converted over drivers in drivers/clocksource
>
> Stephen Boyd (17):
> clocksource: Extract max nsec calculation into separate function
> sched_clock: Use seqcount instead of rolling our own
> sched_clock: Use an hrtimer instead of timer
> sched_clock: Add support for >32 bit sched_clock
> arch_timer: Move to generic sched_clock framework
> sched_clock: Remove sched_clock_func() hook
> clocksource: bcm2835: Switch to sched_clock_register()
> ocksource: dbx500-prcmu: Switch to sched_clock_register()
> clocksource: dw_apb_timer_of: Switch to sched_clock_register()
> clocksource: mxs_timer: Switch to sched_clock_register()
> clocksource: nomadik: Switch to sched_clock_register()
> clocksource: samsung_pwm_timer: Switch to sched_clock_register()
> clocksource: tegra: Switch to sched_clock_register()
> clocksource: time-armada-370-xp: Switch to sched_clock_register()
> clocksource: sirf: Switch to sched_clock_register() and use 64 bits
> clocksource: vf_pit_timer: Switch to sched_clock_register()
> sched_clock: Deprecate setup_sched_clock()

Ok, so if there's no major objections with this set, here's my plan:

1) I'm going to queue 1-4 in a fortglx/3.12/sched-clock64 branch, I'll
send git pull requests to both Thomas and Olof, so they can both merge
the common base for the conversion code in both drivers/clocksource and
arch/arm.

2) If I can get Catalin's ack on 5/17, I'll queue it and 6/17 for
tip/timers/core.

3) I'll also queue the rest and send to Thomas or Daniel for
tip/timers/core.

That sound ok to everyone?

thanks
-john

2013-07-22 18:21:24

by John Stultz

[permalink] [raw]
Subject: Re: [PATCH v4 03/17] sched_clock: Use an hrtimer instead of timer

On 07/18/2013 04:21 PM, Stephen Boyd wrote:
> In the next patch we're going to increase the number of bits that
> the generic sched_clock can handle to be greater than 32. With
> more than 32 bits the wraparound time can be larger than what can
> fit into the units that msecs_to_jiffies takes (unsigned int).
> Luckily, the wraparound is initially calculated in nanoseconds
> which we can easily use with hrtimers, so switch to using an
> hrtimer.
>
> Cc: Russell King <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>


Hrmm. So in my testing (under qemu), this patch causes bootup to hang.

qemu-system-arm -kernel zImage-arm -M vexpress-a9 -cpu cortex-a9
-nographic -m 1024 -append 'root=/dev/mmcblk0p2 rw mem=1024M
raid=noautodetect console=ttyAMA0,38400n8 rootwait vmalloc=256MB
devtmpfs.mount=0' -sd test-arm.img -redir tcp:4300::22

Config file attached.

I haven't gotten a chance to look very closely, but it seems the
folowing patch resolves the issue. I'm not sure if we're seeing callers
to setup_sched_clock happen after sched_clock_postinit or what, but it
probably needs another look over.

thanks
-john


diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
index a269890b..c018ffc 100644
--- a/kernel/time/sched_clock.c
+++ b/kernel/time/sched_clock.c
@@ -138,12 +138,6 @@ void __init setup_sched_clock(u32 (*read)(void), int bits, unsigned long rate)
pr_info("sched_clock: %u bits at %lu%cHz, resolution %lluns, wraps every %lluns\n",
bits, r, r_unit, res, wrap);

- /*
- * Start the timer to keep sched_clock() properly updated and
- * sets the initial epoch.
- */
- hrtimer_init(&sched_clock_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
- sched_clock_timer.function = sched_clock_poll;
update_sched_clock();

/*
@@ -175,6 +169,13 @@ void __init sched_clock_postinit(void)
setup_sched_clock(jiffy_sched_clock_read, 32, HZ);

update_sched_clock();
+
+ /*
+ * Start the timer to keep sched_clock() properly updated and
+ * sets the initial epoch.
+ */
+ hrtimer_init(&sched_clock_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ sched_clock_timer.function = sched_clock_poll;
hrtimer_start(&sched_clock_timer, cd.wrap_kt, HRTIMER_MODE_REL);
}



Attachments:
config.vexpress-qemu.gz (19.52 kB)

2013-07-22 18:45:45

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 03/17] sched_clock: Use an hrtimer instead of timer

On 07/22/13 11:21, John Stultz wrote:
> On 07/18/2013 04:21 PM, Stephen Boyd wrote:
>> In the next patch we're going to increase the number of bits that
>> the generic sched_clock can handle to be greater than 32. With
>> more than 32 bits the wraparound time can be larger than what can
>> fit into the units that msecs_to_jiffies takes (unsigned int).
>> Luckily, the wraparound is initially calculated in nanoseconds
>> which we can easily use with hrtimers, so switch to using an
>> hrtimer.
>>
>> Cc: Russell King <[email protected]>
>> Signed-off-by: Stephen Boyd <[email protected]>
>
>
> Hrmm. So in my testing (under qemu), this patch causes bootup to hang.
>
> qemu-system-arm -kernel zImage-arm -M vexpress-a9 -cpu cortex-a9
> -nographic -m 1024 -append 'root=/dev/mmcblk0p2 rw mem=1024M
> raid=noautodetect console=ttyAMA0,38400n8 rootwait vmalloc=256MB
> devtmpfs.mount=0' -sd test-arm.img -redir tcp:4300::22
>
> Config file attached.
>
> I haven't gotten a chance to look very closely, but it seems the
> folowing patch resolves the issue. I'm not sure if we're seeing
> callers to setup_sched_clock happen after sched_clock_postinit or
> what, but it probably needs another look over.
>
> thanks
> -john
>
>
> diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
> index a269890b..c018ffc 100644
> --- a/kernel/time/sched_clock.c
> +++ b/kernel/time/sched_clock.c
> @@ -138,12 +138,6 @@ void __init setup_sched_clock(u32 (*read)(void),
> int bits, unsigned long rate)
> pr_info("sched_clock: %u bits at %lu%cHz, resolution %lluns,
> wraps every %lluns\n",
> bits, r, r_unit, res, wrap);
>
> - /*
> - * Start the timer to keep sched_clock() properly updated and
> - * sets the initial epoch.
> - */
> - hrtimer_init(&sched_clock_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> - sched_clock_timer.function = sched_clock_poll;
> update_sched_clock();
>
> /*
> @@ -175,6 +169,13 @@ void __init sched_clock_postinit(void)
> setup_sched_clock(jiffy_sched_clock_read, 32, HZ);
>
> update_sched_clock();
> +
> + /*
> + * Start the timer to keep sched_clock() properly updated and
> + * sets the initial epoch.
> + */
> + hrtimer_init(&sched_clock_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> + sched_clock_timer.function = sched_clock_poll;
> hrtimer_start(&sched_clock_timer, cd.wrap_kt, HRTIMER_MODE_REL);
> }
>
>

Hmm. Is it too early to use hrtimers? Moving the hrtimer_start() into
sched_clock_register() also causes the same crash.

[ 0.000000] Unable to handle kernel NULL pointer dereference at
virtual address 00000000
[ 0.000000] pgd = c0004000
[ 0.000000] [00000000] *pgd=00000000
[ 0.000000] Internal error: Oops: 5 [#1] SMP ARM
[ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
3.11.0-rc1-00017-gecfb8ad-dirty #69
[ 0.000000] task: c063b600 ti: c0630000 task.ti: c0630000
[ 0.000000] PC is at _raw_spin_lock_irqsave+0xc/0x48
[ 0.000000] LR is at lock_hrtimer_base+0x20/0x4c
[ 0.000000] pc : [<c043b7f8>] lr : [<c005edb8>] psr: a00001d3
[ 0.000000] sp : c0631f40 ip : 00000001 fp : 00000000
[ 0.000000] r10: 00000000 r9 : 410fc090 r8 : c06e44c0
[ 0.000000] r7 : c063c508 r6 : c0631f7c r5 : c06e44c0 r4 : c062e538
[ 0.000000] r3 : 00000024 r2 : 00000000 r1 : c0631f7c r0 : a00001d3
[ 0.000000] Flags: NzCv IRQs off FIQs off Mode SVC_32 ISA ARM
Segment kernel
[ 0.000000] Control: 10c5387d Table: 6000406a DAC: 00000015
[ 0.000000] Process swapper/0 (pid: 0, stack limit = 0xc0630238)
[ 0.000000] Stack: (0xc0631f40 to 0xc0632000)
[ 0.000000] 1f40: c06be640 c06150a0 c0f05940 c005f344 00000000
00989680 755552dc 00000024
[ 0.000000] 1f60: c063cd80 c0441354 755552dc 00000024 00000000
00000000 c063cd80 00000002
[ 0.000000] 1f80: c0441354 c06be640 c06150a0 c0f05940 c063c508
6000406a 410fc090 00000000
[ 0.000000] 1fa0: 00000000 c005f74c 00000000 00000001 00000001
00000000 00000000 c05e5984
[ 0.000000] 1fc0: 00000001 c05c2e38 00000001 c05c06e0 ffffffff
ffffffff c05c02ec 00000000
[ 0.000000] 1fe0: 00000000 c06150a0 10c5387d c0638418 c061509c
60008074 00000000 00000000
[ 0.000000] [<c043b7f8>] (_raw_spin_lock_irqsave+0xc/0x48) from
[<c005edb8>] (lock_hrtimer_base+0x20/0x4c)
[ 0.000000] [<c005edb8>] (lock_hrtimer_base+0x20/0x4c) from
[<c005f344>] (__hrtimer_start_range_ns+0x1c/0x3d8)
[ 0.000000] [<c005f344>] (__hrtimer_start_range_ns+0x1c/0x3d8) from
[<c005f74c>] (hrtimer_start+0x20/0x28)
[ 0.000000] [<c005f74c>] (hrtimer_start+0x20/0x28) from [<c05e5984>]
(sched_clock_postinit+0x44/0x5c)
[ 0.000000] [<c05e5984>] (sched_clock_postinit+0x44/0x5c) from
[<c05c06e0>] (start_kernel+0x1d0/0x31c)
[ 0.000000] [<c05c06e0>] (start_kernel+0x1d0/0x31c) from [<60008074>]
(0x60008074)
[ 0.000000] Code: e12fff1e e1a02000 e10f0000 f10c0080 (e1923f9f)

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

2013-07-22 18:58:50

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 03/17] sched_clock: Use an hrtimer instead of timer

On 07/22/13 11:45, Stephen Boyd wrote:
> On 07/22/13 11:21, John Stultz wrote:
>> On 07/18/2013 04:21 PM, Stephen Boyd wrote:
>>> In the next patch we're going to increase the number of bits that
>>> the generic sched_clock can handle to be greater than 32. With
>>> more than 32 bits the wraparound time can be larger than what can
>>> fit into the units that msecs_to_jiffies takes (unsigned int).
>>> Luckily, the wraparound is initially calculated in nanoseconds
>>> which we can easily use with hrtimers, so switch to using an
>>> hrtimer.
>>>
>>> Cc: Russell King <[email protected]>
>>> Signed-off-by: Stephen Boyd <[email protected]>
>>
>> Hrmm. So in my testing (under qemu), this patch causes bootup to hang.
>>
>> qemu-system-arm -kernel zImage-arm -M vexpress-a9 -cpu cortex-a9
>> -nographic -m 1024 -append 'root=/dev/mmcblk0p2 rw mem=1024M
>> raid=noautodetect console=ttyAMA0,38400n8 rootwait vmalloc=256MB
>> devtmpfs.mount=0' -sd test-arm.img -redir tcp:4300::22
>>
>> Config file attached.
>>
>> I haven't gotten a chance to look very closely, but it seems the
>> folowing patch resolves the issue. I'm not sure if we're seeing
>> callers to setup_sched_clock happen after sched_clock_postinit or
>> what, but it probably needs another look over.
>>
>> thanks
>> -john
>>
>>
>> diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
>> index a269890b..c018ffc 100644
>> --- a/kernel/time/sched_clock.c
>> +++ b/kernel/time/sched_clock.c
>> @@ -138,12 +138,6 @@ void __init setup_sched_clock(u32 (*read)(void),
>> int bits, unsigned long rate)
>> pr_info("sched_clock: %u bits at %lu%cHz, resolution %lluns,
>> wraps every %lluns\n",
>> bits, r, r_unit, res, wrap);
>>
>> - /*
>> - * Start the timer to keep sched_clock() properly updated and
>> - * sets the initial epoch.
>> - */
>> - hrtimer_init(&sched_clock_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>> - sched_clock_timer.function = sched_clock_poll;
>> update_sched_clock();
>>
>> /*
>> @@ -175,6 +169,13 @@ void __init sched_clock_postinit(void)
>> setup_sched_clock(jiffy_sched_clock_read, 32, HZ);
>>
>> update_sched_clock();
>> +
>> + /*
>> + * Start the timer to keep sched_clock() properly updated and
>> + * sets the initial epoch.
>> + */
>> + hrtimer_init(&sched_clock_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>> + sched_clock_timer.function = sched_clock_poll;
>> hrtimer_start(&sched_clock_timer, cd.wrap_kt, HRTIMER_MODE_REL);
>> }
>>
>>
> Hmm. Is it too early to use hrtimers? Moving the hrtimer_start() into
> sched_clock_register() also causes the same crash.

Yes that seems to be the problem. The vexpress board is setting up the
sched_clock in setup_arch() (via v2m_init_early) which runs before
hrtimers_init(). I've only tested this on boards that setup the timer in
the time_init() callback which runs after hrtimers_init(). Your patch
should be fine, although it would be nice if we didn't have callers
setting up the sched_clock so early.

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

2013-07-22 19:07:33

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH v4 03/17] sched_clock: Use an hrtimer instead of timer

On Mon, Jul 22, 2013 at 11:58:47AM -0700, Stephen Boyd wrote:
> On 07/22/13 11:45, Stephen Boyd wrote:
> > Hmm. Is it too early to use hrtimers? Moving the hrtimer_start() into
> > sched_clock_register() also causes the same crash.
>
> Yes that seems to be the problem. The vexpress board is setting up the
> sched_clock in setup_arch() (via v2m_init_early) which runs before
> hrtimers_init(). I've only tested this on boards that setup the timer in
> the time_init() callback which runs after hrtimers_init(). Your patch
> should be fine, although it would be nice if we didn't have callers
> setting up the sched_clock so early.

However, it _is_ the correct place to do it, as I've repeatedly stated.
The reason for this is that the scheduler will have already read from
sched_clock() by the time you get to timer_init() to seed its idea of
the start time for PID0 - again, as I've explained multiple times.

2013-07-22 20:48:25

by John Stultz

[permalink] [raw]
Subject: Re: [PATCH v4 03/17] sched_clock: Use an hrtimer instead of timer

On 07/22/2013 11:58 AM, Stephen Boyd wrote:
> On 07/22/13 11:45, Stephen Boyd wrote:
>> On 07/22/13 11:21, John Stultz wrote:
>>> On 07/18/2013 04:21 PM, Stephen Boyd wrote:
>>>> In the next patch we're going to increase the number of bits that
>>>> the generic sched_clock can handle to be greater than 32. With
>>>> more than 32 bits the wraparound time can be larger than what can
>>>> fit into the units that msecs_to_jiffies takes (unsigned int).
>>>> Luckily, the wraparound is initially calculated in nanoseconds
>>>> which we can easily use with hrtimers, so switch to using an
>>>> hrtimer.
>>>>
>>>> Cc: Russell King <[email protected]>
>>>> Signed-off-by: Stephen Boyd <[email protected]>
>>> Hrmm. So in my testing (under qemu), this patch causes bootup to hang.
>>>
>>> qemu-system-arm -kernel zImage-arm -M vexpress-a9 -cpu cortex-a9
>>> -nographic -m 1024 -append 'root=/dev/mmcblk0p2 rw mem=1024M
>>> raid=noautodetect console=ttyAMA0,38400n8 rootwait vmalloc=256MB
>>> devtmpfs.mount=0' -sd test-arm.img -redir tcp:4300::22
>>>
>>> Config file attached.
>>>
>>> I haven't gotten a chance to look very closely, but it seems the
>>> folowing patch resolves the issue. I'm not sure if we're seeing
>>> callers to setup_sched_clock happen after sched_clock_postinit or
>>> what, but it probably needs another look over.
>>>
>>> thanks
>>> -john
>>>
>>>
>>> diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
>>> index a269890b..c018ffc 100644
>>> --- a/kernel/time/sched_clock.c
>>> +++ b/kernel/time/sched_clock.c
>>> @@ -138,12 +138,6 @@ void __init setup_sched_clock(u32 (*read)(void),
>>> int bits, unsigned long rate)
>>> pr_info("sched_clock: %u bits at %lu%cHz, resolution %lluns,
>>> wraps every %lluns\n",
>>> bits, r, r_unit, res, wrap);
>>>
>>> - /*
>>> - * Start the timer to keep sched_clock() properly updated and
>>> - * sets the initial epoch.
>>> - */
>>> - hrtimer_init(&sched_clock_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>>> - sched_clock_timer.function = sched_clock_poll;
>>> update_sched_clock();
>>>
>>> /*
>>> @@ -175,6 +169,13 @@ void __init sched_clock_postinit(void)
>>> setup_sched_clock(jiffy_sched_clock_read, 32, HZ);
>>>
>>> update_sched_clock();
>>> +
>>> + /*
>>> + * Start the timer to keep sched_clock() properly updated and
>>> + * sets the initial epoch.
>>> + */
>>> + hrtimer_init(&sched_clock_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>>> + sched_clock_timer.function = sched_clock_poll;
>>> hrtimer_start(&sched_clock_timer, cd.wrap_kt, HRTIMER_MODE_REL);
>>> }
>>>
>>>
>> Hmm. Is it too early to use hrtimers? Moving the hrtimer_start() into
>> sched_clock_register() also causes the same crash.
> Yes that seems to be the problem. The vexpress board is setting up the
> sched_clock in setup_arch() (via v2m_init_early) which runs before
> hrtimers_init(). I've only tested this on boards that setup the timer in
> the time_init() callback which runs after hrtimers_init(). Your patch
> should be fine, although it would be nice if we didn't have callers
> setting up the sched_clock so early.

Although as Russell pointed out, setting up sched_clock later isn't
really a good option (although I'd like it best if we could handle
switching sched_clocks dynamically as needed so there were less
constraints on when it has to be registered).

So I'll probably fold in my change into the patch if you're ok with that?

thanks
-john

2013-07-22 20:50:57

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 03/17] sched_clock: Use an hrtimer instead of timer

On 07/22/13 13:48, John Stultz wrote:
> On 07/22/2013 11:58 AM, Stephen Boyd wrote:
>> On 07/22/13 11:45, Stephen Boyd wrote:
>>> On 07/22/13 11:21, John Stultz wrote:
>>>> On 07/18/2013 04:21 PM, Stephen Boyd wrote:
>>>>> In the next patch we're going to increase the number of bits that
>>>>> the generic sched_clock can handle to be greater than 32. With
>>>>> more than 32 bits the wraparound time can be larger than what can
>>>>> fit into the units that msecs_to_jiffies takes (unsigned int).
>>>>> Luckily, the wraparound is initially calculated in nanoseconds
>>>>> which we can easily use with hrtimers, so switch to using an
>>>>> hrtimer.
>>>>>
>>>>> Cc: Russell King <[email protected]>
>>>>> Signed-off-by: Stephen Boyd <[email protected]>
>>>> Hrmm. So in my testing (under qemu), this patch causes bootup to hang.
>>>>
>>>> qemu-system-arm -kernel zImage-arm -M vexpress-a9 -cpu cortex-a9
>>>> -nographic -m 1024 -append 'root=/dev/mmcblk0p2 rw mem=1024M
>>>> raid=noautodetect console=ttyAMA0,38400n8 rootwait vmalloc=256MB
>>>> devtmpfs.mount=0' -sd test-arm.img -redir tcp:4300::22
>>>>
>>>> Config file attached.
>>>>
>>>> I haven't gotten a chance to look very closely, but it seems the
>>>> folowing patch resolves the issue. I'm not sure if we're seeing
>>>> callers to setup_sched_clock happen after sched_clock_postinit or
>>>> what, but it probably needs another look over.
>>>>
>>>> thanks
>>>> -john
>>>>
>>>>
>>>> diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
>>>> index a269890b..c018ffc 100644
>>>> --- a/kernel/time/sched_clock.c
>>>> +++ b/kernel/time/sched_clock.c
>>>> @@ -138,12 +138,6 @@ void __init setup_sched_clock(u32 (*read)(void),
>>>> int bits, unsigned long rate)
>>>> pr_info("sched_clock: %u bits at %lu%cHz, resolution %lluns,
>>>> wraps every %lluns\n",
>>>> bits, r, r_unit, res, wrap);
>>>> - /*
>>>> - * Start the timer to keep sched_clock() properly updated and
>>>> - * sets the initial epoch.
>>>> - */
>>>> - hrtimer_init(&sched_clock_timer, CLOCK_MONOTONIC,
>>>> HRTIMER_MODE_REL);
>>>> - sched_clock_timer.function = sched_clock_poll;
>>>> update_sched_clock();
>>>> /*
>>>> @@ -175,6 +169,13 @@ void __init sched_clock_postinit(void)
>>>> setup_sched_clock(jiffy_sched_clock_read, 32, HZ);
>>>> update_sched_clock();
>>>> +
>>>> + /*
>>>> + * Start the timer to keep sched_clock() properly updated and
>>>> + * sets the initial epoch.
>>>> + */
>>>> + hrtimer_init(&sched_clock_timer, CLOCK_MONOTONIC,
>>>> HRTIMER_MODE_REL);
>>>> + sched_clock_timer.function = sched_clock_poll;
>>>> hrtimer_start(&sched_clock_timer, cd.wrap_kt, HRTIMER_MODE_REL);
>>>> }
>>>>
>>> Hmm. Is it too early to use hrtimers? Moving the hrtimer_start() into
>>> sched_clock_register() also causes the same crash.
>> Yes that seems to be the problem. The vexpress board is setting up the
>> sched_clock in setup_arch() (via v2m_init_early) which runs before
>> hrtimers_init(). I've only tested this on boards that setup the timer in
>> the time_init() callback which runs after hrtimers_init(). Your patch
>> should be fine, although it would be nice if we didn't have callers
>> setting up the sched_clock so early.
>
> Although as Russell pointed out, setting up sched_clock later isn't
> really a good option (although I'd like it best if we could handle
> switching sched_clocks dynamically as needed so there were less
> constraints on when it has to be registered).
>
> So I'll probably fold in my change into the patch if you're ok with that?
>

Yes that's fine to fold it in.

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

2013-07-24 14:44:27

by Christopher Covington

[permalink] [raw]
Subject: Re: [PATCH v4 00/17] 64-bit friendly generic sched_clock()

On 07/18/2013 07:21 PM, Stephen Boyd wrote:
> This patchset adds support for 64 bit counters in the generic
> sched_clock code and converts drivers over to use it. Based
> on v3.11-rc1.
>
> Changes since v3:
> * Move to use seqcount to fix issues with 64-bit cyc counters
> * Move to hrtimer to fix underflow/overflow errors in wraparound
> calculation
> * Use of 1 hour in clocks_calc_mult_shift
> * Converted over drivers in drivers/clocksource
>
> Stephen Boyd (17):
> clocksource: Extract max nsec calculation into separate function
> sched_clock: Use seqcount instead of rolling our own
> sched_clock: Use an hrtimer instead of timer
> sched_clock: Add support for >32 bit sched_clock
> arch_timer: Move to generic sched_clock framework
> sched_clock: Remove sched_clock_func() hook
> clocksource: bcm2835: Switch to sched_clock_register()
> ocksource: dbx500-prcmu: Switch to sched_clock_register()
> clocksource: dw_apb_timer_of: Switch to sched_clock_register()
> clocksource: mxs_timer: Switch to sched_clock_register()
> clocksource: nomadik: Switch to sched_clock_register()
> clocksource: samsung_pwm_timer: Switch to sched_clock_register()
> clocksource: tegra: Switch to sched_clock_register()
> clocksource: time-armada-370-xp: Switch to sched_clock_register()
> clocksource: sirf: Switch to sched_clock_register() and use 64 bits
> clocksource: vf_pit_timer: Switch to sched_clock_register()
> sched_clock: Deprecate setup_sched_clock()
>
> arch/arm/kernel/arch_timer.c | 14 ----
> arch/arm64/Kconfig | 1 +
> arch/arm64/kernel/time.c | 10 ---
> drivers/clocksource/arm_arch_timer.c | 4 ++
> drivers/clocksource/bcm2835_timer.c | 4 +-
> drivers/clocksource/clksrc-dbx500-prcmu.c | 5 +-
> drivers/clocksource/dw_apb_timer_of.c | 4 +-
> drivers/clocksource/mxs_timer.c | 4 +-
> drivers/clocksource/nomadik-mtu.c | 4 +-
> drivers/clocksource/samsung_pwm_timer.c | 4 +-
> drivers/clocksource/tegra20_timer.c | 4 +-
> drivers/clocksource/time-armada-370-xp.c | 4 +-
> drivers/clocksource/timer-prima2.c | 6 +-
> drivers/clocksource/vf_pit_timer.c | 4 +-
> include/linux/clocksource.h | 2 +
> include/linux/sched_clock.h | 7 +-
> kernel/time/clocksource.c | 45 ++++++++-----
> kernel/time/sched_clock.c | 105 +++++++++++++++---------------
> 18 files changed, 116 insertions(+), 115 deletions(-)
>

I ran 32-bit and 64-bit kernels with these patches on the Versatile Express
software model and things looked fine, so for the applicable patches,

Tested-by: Christopher Covington <[email protected]>

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

2013-07-30 10:05:08

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH v4 07/17] clocksource: bcm2835: Switch to sched_clock_register()

On 07/19/2013 01:21 AM, Stephen Boyd wrote:
> The 32 bit sched_clock interface now supports 64 bits. Upgrade to
> the 64 bit function to allow us to remove the 32 bit registration
> interface.
>
> Cc: Stephen Warren <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---

Hi Stephen,

you sent a series with tick and clocksource changes.

John is recipient of part of them. I replaced him to maintain the
clocksource drivers.

Is the series you sent for clocksource drivers supposed to be taken by
me or by Russell ?

In the future if there are no dependencies, it would be preferable to
group the clocksource drivers changes into a series and send them to me.

Thanks
-- Daniel

> drivers/clocksource/bcm2835_timer.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clocksource/bcm2835_timer.c b/drivers/clocksource/bcm2835_timer.c
> index 07ea7ce..26ed331 100644
> --- a/drivers/clocksource/bcm2835_timer.c
> +++ b/drivers/clocksource/bcm2835_timer.c
> @@ -49,7 +49,7 @@ struct bcm2835_timer {
>
> static void __iomem *system_clock __read_mostly;
>
> -static u32 notrace bcm2835_sched_read(void)
> +static u64 notrace bcm2835_sched_read(void)
> {
> return readl_relaxed(system_clock);
> }
> @@ -110,7 +110,7 @@ static void __init bcm2835_timer_init(struct device_node *node)
> panic("Can't read clock-frequency");
>
> system_clock = base + REG_COUNTER_LO;
> - setup_sched_clock(bcm2835_sched_read, 32, freq);
> + sched_clock_register(bcm2835_sched_read, 32, freq);
>
> clocksource_mmio_init(base + REG_COUNTER_LO, node->name,
> freq, 300, 32, clocksource_mmio_readl_up);
>


--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

2013-07-30 16:12:29

by John Stultz

[permalink] [raw]
Subject: Re: [PATCH v4 07/17] clocksource: bcm2835: Switch to sched_clock_register()

On 07/30/2013 03:04 AM, Daniel Lezcano wrote:
> On 07/19/2013 01:21 AM, Stephen Boyd wrote:
>> The 32 bit sched_clock interface now supports 64 bits. Upgrade to
>> the 64 bit function to allow us to remove the 32 bit registration
>> interface.
>>
>> Cc: Stephen Warren <[email protected]>
>> Signed-off-by: Stephen Boyd <[email protected]>
>> ---
> Hi Stephen,
>
> you sent a series with tick and clocksource changes.
>
> John is recipient of part of them. I replaced him to maintain the
> clocksource drivers.
>
> Is the series you sent for clocksource drivers supposed to be taken by
> me or by Russell ?
>
> In the future if there are no dependencies, it would be preferable to
> group the clocksource drivers changes into a series and send them to me.


I believe there are some dependencies on the generic bits.

I'm planning on queing the generic parts in one branch (to send to
thomas), queuing the drivers/clocksource bits to send to you, but was
waiting on an Ack from Catalin.

I got busy with some other work items, so I haven't pushed these out
yet, but I'll review things again and push them out today.

thanks
-john


2013-08-06 09:04:51

by Gregory CLEMENT

[permalink] [raw]
Subject: Re: [PATCH v4 14/17] clocksource: time-armada-370-xp: Switch to sched_clock_register()

On 19/07/2013 01:21, Stephen Boyd wrote:
> The 32 bit sched_clock interface now supports 64 bits. Upgrade to
> the 64 bit function to allow us to remove the 32 bit registration
> interface.

Acked-by: Gregory CLEMENT <[email protected]>

>
> Cc: Gregory CLEMENT <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> drivers/clocksource/time-armada-370-xp.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
> index efdca32..2bec8dc 100644
> --- a/drivers/clocksource/time-armada-370-xp.c
> +++ b/drivers/clocksource/time-armada-370-xp.c
> @@ -71,7 +71,7 @@ static u32 ticks_per_jiffy;
>
> static struct clock_event_device __percpu **percpu_armada_370_xp_evt;
>
> -static u32 notrace armada_370_xp_read_sched_clock(void)
> +static u64 notrace armada_370_xp_read_sched_clock(void)
> {
> return ~readl(timer_base + TIMER0_VAL_OFF);
> }
> @@ -258,7 +258,7 @@ void __init armada_370_xp_timer_init(void)
> /*
> * Set scale and timer for sched_clock.
> */
> - setup_sched_clock(armada_370_xp_read_sched_clock, 32, timer_clk);
> + sched_clock_register(armada_370_xp_read_sched_clock, 32, timer_clk);
>
> /*
> * Setup free-running clocksource timer (interrupts
>


--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

2013-10-02 17:45:27

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v4 05/17] arch_timer: Move to generic sched_clock framework

Hi Stephen,

On Fri, Jul 19, 2013 at 12:21:18AM +0100, Stephen Boyd wrote:
> Register with the generic 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. Finally, it
> allows us to move the sched_clock setup into drivers clocksource
> out of the arch ports.

Sorry, this one slipped through the cracks.

> diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
> index 221f07b..1791f12 100644
> --- a/arch/arm/kernel/arch_timer.c
> +++ b/arch/arm/kernel/arch_timer.c
> @@ -11,7 +11,6 @@
> #include <linux/init.h>
> #include <linux/types.h>
> #include <linux/errno.h>
> -#include <linux/sched_clock.h>
>
> #include <asm/delay.h>
>
> @@ -22,13 +21,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 +40,5 @@ 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);
> -
> return 0;
> }
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 9737e97..e32b471 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -14,6 +14,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 03dc371..29c39d5 100644
> --- a/arch/arm64/kernel/time.c
> +++ b/arch/arm64/kernel/time.c
> @@ -61,13 +61,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;
> -}
> -
> void __init time_init(void)
> {
> u32 arch_timer_rate;
> @@ -78,9 +71,6 @@ 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;
> -
> /* Calibrate the delay loop directly */
> lpj_fine = arch_timer_rate / HZ;
> }
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 053d846..2facf14 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -17,6 +17,7 @@
> #include <linux/interrupt.h>
> #include <linux/of_irq.h>
> #include <linux/io.h>
> +#include <linux/sched_clock.h>
>
> #include <asm/arch_timer.h>
> #include <asm/virt.h>
> @@ -281,6 +282,9 @@ static int __init arch_timer_register(void)
> timecounter_init(&timecounter, &cyclecounter,
> arch_counter_get_cntvct());
>
> + /* 56 bits minimum, so we assume worst case rollover */
> + sched_clock_register(arch_timer_read_counter, 56, arch_timer_rate);

We've got the same assumption elsewhere in the kernel (e.g. vdso) but at
some point we should probably deal with >56 bits in case somebody makes a
high-frequency timer with more significant bits.

However, I think this patch is going in the right direction:

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

Will

2013-10-02 17:47:14

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v4 00/17] 64-bit friendly generic sched_clock()

On Fri, Jul 19, 2013 at 12:59:44AM +0100, John Stultz wrote:
> On 07/18/2013 04:21 PM, Stephen Boyd wrote:
> > This patchset adds support for 64 bit counters in the generic
> > sched_clock code and converts drivers over to use it. Based
> > on v3.11-rc1.
> >
> > Changes since v3:
> > * Move to use seqcount to fix issues with 64-bit cyc counters
> > * Move to hrtimer to fix underflow/overflow errors in wraparound
> > calculation
> > * Use of 1 hour in clocks_calc_mult_shift
> > * Converted over drivers in drivers/clocksource
>
> I've not been able to take a deep review yet, but this looks pretty much
> like what we discussed last week, so I'm happy with it so far. Has this
> gotten much testing (on both 32 and 64bit systems?)
>
> One detail: Most of this is likely to go in via tip/timers/core, but the
> 5/17 "arch_timer: Move to generic sched_clock" will need some
> synchronization with Catalin to make sure its ok to go in via tip. Not
> sure what other arm64 changes are pending that would depend or collide
> with that change.

I wouldn't expect anything more than a trivial Kconfig clash with the arm64
tree, if that.

Will

2013-10-02 18:03:06

by John Stultz

[permalink] [raw]
Subject: Re: [PATCH v4 00/17] 64-bit friendly generic sched_clock()

On 10/02/2013 10:47 AM, Will Deacon wrote:
> On Fri, Jul 19, 2013 at 12:59:44AM +0100, John Stultz wrote:
>> On 07/18/2013 04:21 PM, Stephen Boyd wrote:
>>> This patchset adds support for 64 bit counters in the generic
>>> sched_clock code and converts drivers over to use it. Based
>>> on v3.11-rc1.
>>>
>>> Changes since v3:
>>> * Move to use seqcount to fix issues with 64-bit cyc counters
>>> * Move to hrtimer to fix underflow/overflow errors in wraparound
>>> calculation
>>> * Use of 1 hour in clocks_calc_mult_shift
>>> * Converted over drivers in drivers/clocksource
>> I've not been able to take a deep review yet, but this looks pretty much
>> like what we discussed last week, so I'm happy with it so far. Has this
>> gotten much testing (on both 32 and 64bit systems?)
>>
>> One detail: Most of this is likely to go in via tip/timers/core, but the
>> 5/17 "arch_timer: Move to generic sched_clock" will need some
>> synchronization with Catalin to make sure its ok to go in via tip. Not
>> sure what other arm64 changes are pending that would depend or collide
>> with that change.
> I wouldn't expect anything more than a trivial Kconfig clash with the arm64
> tree, if that.

So I also have a branch with these changes based on a branch with only
the prereqs that are already merged, so I can provide a pull request
that can go in via the aarch64 tree and won't collide with tip. Would
that be preferrable?

thanks
-john

2013-10-02 18:13:55

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v4 00/17] 64-bit friendly generic sched_clock()

On Wed, Oct 02, 2013 at 07:02:58PM +0100, John Stultz wrote:
> On 10/02/2013 10:47 AM, Will Deacon wrote:
> > On Fri, Jul 19, 2013 at 12:59:44AM +0100, John Stultz wrote:
> >> On 07/18/2013 04:21 PM, Stephen Boyd wrote:
> >>> This patchset adds support for 64 bit counters in the generic
> >>> sched_clock code and converts drivers over to use it. Based
> >>> on v3.11-rc1.
> >>>
> >>> Changes since v3:
> >>> * Move to use seqcount to fix issues with 64-bit cyc counters
> >>> * Move to hrtimer to fix underflow/overflow errors in wraparound
> >>> calculation
> >>> * Use of 1 hour in clocks_calc_mult_shift
> >>> * Converted over drivers in drivers/clocksource
> >> I've not been able to take a deep review yet, but this looks pretty much
> >> like what we discussed last week, so I'm happy with it so far. Has this
> >> gotten much testing (on both 32 and 64bit systems?)
> >>
> >> One detail: Most of this is likely to go in via tip/timers/core, but the
> >> 5/17 "arch_timer: Move to generic sched_clock" will need some
> >> synchronization with Catalin to make sure its ok to go in via tip. Not
> >> sure what other arm64 changes are pending that would depend or collide
> >> with that change.
> > I wouldn't expect anything more than a trivial Kconfig clash with the arm64
> > tree, if that.
>
> So I also have a branch with these changes based on a branch with only
> the prereqs that are already merged, so I can provide a pull request
> that can go in via the aarch64 tree and won't collide with tip. Would
> that be preferrable?

I think it's simpler for us if you just take the lot via your tree, unless
you have any objections.

Cheers,

Will

2013-10-14 18:44:46

by Kevin Hilman

[permalink] [raw]
Subject: Re: [PATCH v4 05/17] arch_timer: Move to generic sched_clock framework

Stephen Boyd <[email protected]> writes:

> Register with the generic 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. Finally, it
> allows us to move the sched_clock setup into drivers clocksource
> out of the arch ports.
>
> Cc: Christopher Covington <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>

A boot failure on Exynos5/Arndale showed up in next-20131014[1], and a
subsequent bisect has fingered this patch as the culprit.

I haven't had a chance to debug this any further, but wanted to share in
case someone else can debug.

The console log is below, but don't think there is much useful there as
it shows nothing after the 'Starting kernel ...' from u-boot.

Kevin

[1] http://lists.linaro.org/pipermail/kernel-build-reports/2013-October/000651.html

Connected to arndale console [channel connected] (~$quit to exit)
(user:khilman) is already connected

~$hardreset

Command(arndale console)> hardreset
(user:khilman) Reboot arndale
Reboot: arndale ; phidget 2 0 : off, sleep, on


U-Boot 2013.01.-rc1-dirty (Jun 28 2013 - 07:14:48) for ARNDALE5250

CPU: Exynos5250@1000MHz

Board: for ARNDALE5250
I2C: ready
DRAM: 2 GiB
WARNING: Caches not enabled

Checking Boot Mode ... SDMMC
MMC: EXYNOS DWMMC: 0, EXYNOS DWMMC: 1, EXYNOS DWMMC: 2
In: serial
Out: serial
Err: serial
Net: No ethernet found.
(Re)start USB...
USB0: USB EHCI 1.00
scanning bus 0 for devices... 4 USB Device(s) found
scanning usb for storage devices... 0 Storage Device(s) found
scanning usb for ethernet devices... 1 Ethernet Device(s) found
Hit any key to stop autoboot:
3  0
ARNDALE5250 # version
version

U-Boot 2013.01.-rc1-dirty (Jun 28 2013 - 07:14:48) for ARNDALE5250
arm-linux-gnueabi-gcc (Ubuntu/Linaro 4.7.2-1ubuntu1) 4.7.2
GNU ld (GNU Binutils for Ubuntu) 2.22.90.20120919
ARNDALE5250 # setenv bootargs console=tty0 console=ttySAC2,115200n8 rw root=/dev/mmcblk1p3 rootwait rootfstype=ext4
setenv bootargs console=tty0 console=ttySAC2,115200n8 rw root=/dev/mmcblk1p3 rootwait rootfstype=ext4
ARNDALE5250 # setenv netargs 'setenv bootargs ${bootargs} ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}::::192.168.1.254:none'
setenv netargs 'setenv bootargs ${bootargs} ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}::::192.168.1.254:none'
ARNDALE5250 # if test -n ${initenv}; then run initenv; fi
if test -n ${initenv}; then run initenv; fi
ARNDALE5250 # if test -n ${preboot}; then run preboot; fi
if test -n ${preboot}; then run preboot; fi
(Re)start USB...
USB0: USB EHCI 1.00
scanning bus 0 for devices... 4 USB Device(s) found
scanning usb for storage devices... 0 Storage Device(s) found
scanning usb for ethernet devices... 1 Ethernet Device(s) found
ARNDALE5250 #setenv autoload no; setenv autoboot no
setenv autoload no; setenv autoboot no
ARNDALE5250 # dhcp
dhcp
Waiting for Ethernet connection... done.
BOOTP broadcast 1
DHCP client bound to address 192.168.1.171
ARNDALE5250 # setenv serverip 192.168.1.2
setenv serverip 192.168.1.2
ARNDALE5250 # if test -n ${netargs}; then run netargs; fi
if test -n ${netargs}; then run netargs; fi
ARNDALE5250 # tftp 0x40800000 tmp/arndale-lNb7OG/zImage
tftp 0x40800000 tmp/arndale-lNb7OG/zImage
Waiting for Ethernet connection... done.
Using asx0 device
TFTP from server 192.168.1.2; our IP address is 192.168.1.171
Filename 'tmp/arndale-lNb7OG/zImage'.
Load address: 0x40800000
Loading: *#################################################################
#################################################################
#######################################################
done
Bytes transferred = 2709912 (295998 hex)
ARNDALE5250 # tftp 0x407c0000 tmp/arndale-lNb7OG/exynos5250-arndale.dtb
tftp 0x407c0000 tmp/arndale-lNb7OG/exynos5250-arndale.dtb
Waiting for Ethernet connection... done.
Using asx0 device
TFTP from server 192.168.1.2; our IP address is 192.168.1.171
Filename 'tmp/arndale-lNb7OG/exynos5250-arndale.dtb'.
Load address: 0x407c0000
Loading: *###
done
Bytes transferred = 32949 (80b5 hex)
ARNDALE5250 # printenv bootargs
printenv bootargs
bootargs=console=tty0 console=ttySAC2,115200n8 rw root=/dev/mmcblk1p3 rootwait rootfstype=ext4 ip=192.168.1.171:192.168.1.2:192.168.1.254:255.255.255.0::::192.168.1.254:none
ARNDALE5250 # bootz 0x40800000 - 0x407c0000
bootz 0x40800000 - 0x407c0000
## Flattened Device Tree blob at 407c0000
Booting using the fdt blob at 0x407c0000
Using Device Tree in place at 407c0000, end 407cb0b4

Starting kernel ...

~$off
# PYBOOT: Exception: kernel: ERROR: did not start booting.
# PYBOOT: Time: 33.50 seconds.
# PYBOOT: Result: FAIL

2013-10-14 18:55:47

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 05/17] arch_timer: Move to generic sched_clock framework

On 10/14/13 11:44, Kevin Hilman wrote:
> Stephen Boyd <[email protected]> writes:
>
>> Register with the generic 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. Finally, it
>> allows us to move the sched_clock setup into drivers clocksource
>> out of the arch ports.
>>
>> Cc: Christopher Covington <[email protected]>
>> Cc: Catalin Marinas <[email protected]>
>> Signed-off-by: Stephen Boyd <[email protected]>
> A boot failure on Exynos5/Arndale showed up in next-20131014[1], and a
> subsequent bisect has fingered this patch as the culprit.
>
> I haven't had a chance to debug this any further, but wanted to share in
> case someone else can debug.
>
> The console log is below, but don't think there is much useful there as
> it shows nothing after the 'Starting kernel ...' from u-boot.

debug_ll output would be nice. Anyway, that patch looks "weird". It is
definitely not what I sent out. Most notably, this hunk jumps out

@@ -471,6 +472,15 @@ static int __init arch_timer_register(void)
goto out;
}

+ clocksource_register_hz(&clocksource_counter, arch_timer_rate);
+ cyclecounter.mult = clocksource_counter.mult;
+ cyclecounter.shift = clocksource_counter.shift;
+ timecounter_init(&timecounter, &cyclecounter,
+ arch_counter_get_cntvct());
+
+ /* 56 bits minimum, so we assume worst case rollover */
+ sched_clock_register(arch_timer_read_counter, 56, arch_timer_rate);
+
if (arch_timer_use_virtual) {
ppi = arch_timer_ppi[VIRT_PPI];
err = request_percpu_irq(ppi, arch_timer_handler_virt,


Which is adding the clocksource_register_hz() and timecounter_init()
call twice. It should only be adding the sched_clock_register() call and
the sched_clock_register() call should be in arch_counter_register().
Can you try this patch?

----8<----

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 5d52789..865ecd8 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -420,6 +420,9 @@ static void __init arch_counter_register(unsigned type)
cyclecounter.mult = clocksource_counter.mult;
cyclecounter.shift = clocksource_counter.shift;
timecounter_init(&timecounter, &cyclecounter, start_count);
+
+ /* 56 bits minimum, so we assume worst case rollover */
+ sched_clock_register(arch_timer_read_counter, 56, arch_timer_rate);
}

static void arch_timer_stop(struct clock_event_device *clk)
@@ -472,15 +475,6 @@ static int __init arch_timer_register(void)
goto out;
}

- clocksource_register_hz(&clocksource_counter, arch_timer_rate);
- cyclecounter.mult = clocksource_counter.mult;
- cyclecounter.shift = clocksource_counter.shift;
- timecounter_init(&timecounter, &cyclecounter,
- arch_counter_get_cntvct());
-
- /* 56 bits minimum, so we assume worst case rollover */
- sched_clock_register(arch_timer_read_counter, 56, arch_timer_rate);
-
if (arch_timer_use_virtual) {
ppi = arch_timer_ppi[VIRT_PPI];
err = request_percpu_irq(ppi, arch_timer_handler_virt,

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

2013-10-14 20:15:00

by John Stultz

[permalink] [raw]
Subject: Re: [PATCH v4 05/17] arch_timer: Move to generic sched_clock framework

On Mon, Oct 14, 2013 at 11:55 AM, Stephen Boyd <[email protected]> wrote:
> On 10/14/13 11:44, Kevin Hilman wrote:
>> Stephen Boyd <[email protected]> writes:
>>
>>> Register with the generic 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. Finally, it
>>> allows us to move the sched_clock setup into drivers clocksource
>>> out of the arch ports.
>>>
>>> Cc: Christopher Covington <[email protected]>
>>> Cc: Catalin Marinas <[email protected]>
>>> Signed-off-by: Stephen Boyd <[email protected]>
>> A boot failure on Exynos5/Arndale showed up in next-20131014[1], and a
>> subsequent bisect has fingered this patch as the culprit.
>>
>> I haven't had a chance to debug this any further, but wanted to share in
>> case someone else can debug.
>>
>> The console log is below, but don't think there is much useful there as
>> it shows nothing after the 'Starting kernel ...' from u-boot.
>
> debug_ll output would be nice. Anyway, that patch looks "weird". It is
> definitely not what I sent out. Most notably, this hunk jumps out
>
> @@ -471,6 +472,15 @@ static int __init arch_timer_register(void)
> goto out;
> }
>
> + clocksource_register_hz(&clocksource_counter, arch_timer_rate);
> + cyclecounter.mult = clocksource_counter.mult;
> + cyclecounter.shift = clocksource_counter.shift;
> + timecounter_init(&timecounter, &cyclecounter,
> + arch_counter_get_cntvct());
> +
> + /* 56 bits minimum, so we assume worst case rollover */
> + sched_clock_register(arch_timer_read_counter, 56, arch_timer_rate);
> +
> if (arch_timer_use_virtual) {
> ppi = arch_timer_ppi[VIRT_PPI];
> err = request_percpu_irq(ppi, arch_timer_handler_virt,
>
>
> Which is adding the clocksource_register_hz() and timecounter_init()
> call twice. It should only be adding the sched_clock_register() call and
> the sched_clock_register() call should be in arch_counter_register().

Sorry, that's my fault. That bit collided when I applied the patch
onto my queue.

thanks
-john

2013-10-14 20:19:35

by John Stultz

[permalink] [raw]
Subject: Re: [PATCH v4 05/17] arch_timer: Move to generic sched_clock framework

On Mon, Oct 14, 2013 at 1:14 PM, Kevin Hilman <[email protected]> wrote:
> Stephen Boyd <[email protected]> writes:
>
>> Can you try this patch?
>
> Yup, your patch gets it booting again.
>
> John, how do you want to proceed on this? The version of $SUBJECT patch
> that made it to -next doesn't match the one in sent in this thread.

We'll need to get the fix applied to -tip, where my mis-merge landed.

thanks
-john