[PULL] clockevents: fixes for 4.3-rc7

[PULL] clockevents: fixes for 4.3-rc7

[Daniel Lezcano]

Hi Thomas,

this pull request contains a set of fixes for tip/timers/urgent:

  - Prevent ftrace recursion by adding the 'notrace' attribute to the 
sched_clock read callback (Jisheng Zhang).

  - Fix multiple shutdown call issue (Magnus Damm).

Beside that, the commit fc686d0037d782c994e338ecb01bfef8bbafff9f:

"clockevents/drivers/mtk: Fix spurious interrupt leading to crash"

is merged in tip/timers/core but it is needed also as a fix for 4.3. 
This pull request does not contain this fix in order to prevent a 
conflict when merging tip/timers/urgent into tip/timers/core.

How shall I backport this fix from 'core' to 'urgent' without bringing a 
conflict when you will merge both branches ?

Thanks !

   -- Daniel

The following changes since commit 56fd16cabac9cd8f15e2902898a9d0cc96e2fa70:

   timekeeping: Increment clock_was_set_seq in timekeeping_init() 
(2015-10-16 15:50:22 +0200)

are available in the git repository at:

   http://git.linaro.org/people/daniel.lezcano/linux.git 
clockevents/4.3-fixes

for you to fetch changes up to b52e0b08c33a1c58830e3a2350b632dbf8fc1688:

   clocksource/drivers/sh_mtu2: Fix multiple shutdown call issue 
(2015-10-28 10:19:32 +0100)

----------------------------------------------------------------
Jisheng Zhang (7):
       clocksource/drivers/arm_global_timer: Prevent ftrace recursion
       clocksource/drivers/pistachio: Prevent ftrace recursion
       clocksource/drivers/samsung_pwm_timer: Prevent ftrace recursion
       clocksource/drivers/prima2: Prevent ftrace recursion
       clocksource/drivers/vf_pit_timer: Prevent ftrace recursion
       clocksource/drivers/fsl_ftm_timer: Prevent ftrace recursion
       clocksource/drivers/digicolor: Prevent ftrace recursion

Magnus Damm (1):
       clocksource/drivers/sh_mtu2: Fix multiple shutdown call issue

  drivers/clocksource/arm_global_timer.c  | 9 +++++++--
  drivers/clocksource/fsl_ftm_timer.c     | 2 +-
  drivers/clocksource/samsung_pwm_timer.c | 2 +-
  drivers/clocksource/sh_mtu2.c           | 4 +++-
  drivers/clocksource/time-pistachio.c    | 3 ++-
  drivers/clocksource/timer-digicolor.c   | 2 +-
  drivers/clocksource/timer-prima2.c      | 2 +-
  drivers/clocksource/vf_pit_timer.c      | 2 +-
  8 files changed, 17 insertions(+), 9 deletions(-)

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

[PATCH 1/8] clocksource/drivers/arm_global_timer: Prevent ftrace recursion

[Daniel Lezcano]

From: Jisheng Zhang <jszhang@marvell.com>

Currently arm_global_timer can be used as a scheduler clock. We properly
marked gt_sched_clock_read() as notrace but we then call another function
gt_counter_read() that _wasn't_ notrace.

Having a traceable function in the sched_clock() path leads to a recursion
within ftrace and a kernel crash.

Fix this by adding an extra notrace function to keep other users of
gt_counter_read() traceable.

Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/arm_global_timer.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c
index 29ea50a..a2cb6fa 100644
--- a/drivers/clocksource/arm_global_timer.c
+++ b/drivers/clocksource/arm_global_timer.c
@@ -60,7 +60,7 @@ static struct clock_event_device __percpu *gt_evt;
  *  different to the 32-bit upper value read previously, go back to step 2.
  *  Otherwise the 64-bit timer counter value is correct.
  */
-static u64 gt_counter_read(void)
+static u64 notrace _gt_counter_read(void)
 {
 	u64 counter;
 	u32 lower;
@@ -79,6 +79,11 @@ static u64 gt_counter_read(void)
 	return counter;
 }
 
+static u64 gt_counter_read(void)
+{
+	return _gt_counter_read();
+}
+
 /**
  * To ensure that updates to comparator value register do not set the
  * Interrupt Status Register proceed as follows:
@@ -201,7 +206,7 @@ static struct clocksource gt_clocksource = {
 #ifdef CONFIG_CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
 static u64 notrace gt_sched_clock_read(void)
 {
-	return gt_counter_read();
+	return _gt_counter_read();
 }
 #endif
 
-- 
1.9.1

[PATCH 2/8] clocksource/drivers/pistachio: Prevent ftrace recursion

[Daniel Lezcano]

From: Jisheng Zhang <jszhang@marvell.com>

Currently pistachio can be used as a scheduler clock. We properly marked
pistachio_read_sched_clock() as notrace but we then call another function
pistachio_clocksource_read_cycles() that _wasn't_ notrace.

Having a traceable function in the sched_clock() path leads to a recursion
within ftrace and a kernel crash.

Fix this by adding notrace attribute to the pistachio_clocksource_read_cycles()
function.

Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/time-pistachio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/time-pistachio.c b/drivers/clocksource/time-pistachio.c
index 18d4266..bba6799 100644
--- a/drivers/clocksource/time-pistachio.c
+++ b/drivers/clocksource/time-pistachio.c
@@ -67,7 +67,8 @@ static inline void gpt_writel(void __iomem *base, u32 value, u32 offset,
 	writel(value, base + 0x20 * gpt_id + offset);
 }
 
-static cycle_t pistachio_clocksource_read_cycles(struct clocksource *cs)
+static cycle_t notrace
+pistachio_clocksource_read_cycles(struct clocksource *cs)
 {
 	struct pistachio_clocksource *pcs = to_pistachio_clocksource(cs);
 	u32 counter, overflw;
-- 
1.9.1

[PATCH 3/8] clocksource/drivers/samsung_pwm_timer: Prevent ftrace recursion

[Daniel Lezcano]

From: Jisheng Zhang <jszhang@marvell.com>

Currently samsung_pwm_timer can be used as a scheduler clock. We properly
marked samsung_read_sched_clock() as notrace but we then call another
function samsung_clocksource_read() that _wasn't_ notrace.

Having a traceable function in the sched_clock() path leads to a recursion
within ftrace and a kernel crash.

Fix this by adding notrace attribute to the samsung_clocksource_read()
function.

Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/samsung_pwm_timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/samsung_pwm_timer.c b/drivers/clocksource/samsung_pwm_timer.c
index bc90e13..9502bc4 100644
--- a/drivers/clocksource/samsung_pwm_timer.c
+++ b/drivers/clocksource/samsung_pwm_timer.c
@@ -307,7 +307,7 @@ static void samsung_clocksource_resume(struct clocksource *cs)
 	samsung_time_start(pwm.source_id, true);
 }
 
-static cycle_t samsung_clocksource_read(struct clocksource *c)
+static cycle_t notrace samsung_clocksource_read(struct clocksource *c)
 {
 	return ~readl_relaxed(pwm.source_reg);
 }
-- 
1.9.1

[PATCH 4/8] clocksource/drivers/prima2: Prevent ftrace recursion

[Daniel Lezcano]

From: Jisheng Zhang <jszhang@marvell.com>

Currently prima2 timer can be used as a scheduler clock. We properly
marked sirfsoc_read_sched_clock() as notrace but we then call another
function sirfsoc_timer_read() that _wasn't_ notrace.

Having a traceable function in the sched_clock() path leads to a recursion
within ftrace and a kernel crash.

Fix this by adding notrace attribute to the sirfsoc_timer_read() function.

Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/timer-prima2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-prima2.c b/drivers/clocksource/timer-prima2.c
index 78de982..2854c66 100644
--- a/drivers/clocksource/timer-prima2.c
+++ b/drivers/clocksource/timer-prima2.c
@@ -73,7 +73,7 @@ static irqreturn_t sirfsoc_timer_interrupt(int irq, void *dev_id)
 }
 
 /* read 64-bit timer counter */
-static cycle_t sirfsoc_timer_read(struct clocksource *cs)
+static cycle_t notrace sirfsoc_timer_read(struct clocksource *cs)
 {
 	u64 cycles;
 
-- 
1.9.1

[PATCH 5/8] clocksource/drivers/vf_pit_timer: Prevent ftrace recursion

[Daniel Lezcano]

From: Jisheng Zhang <jszhang@marvell.com>

Having a traceable function in the sched_clock() path leads to a recursion
within ftrace and a kernel crash.

We should not trace the pit_read_sched_clock() function. Fix this by adding a
notrace attribute to this function.

Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/vf_pit_timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/vf_pit_timer.c b/drivers/clocksource/vf_pit_timer.c
index f07ba99..a0e6c68 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 u64 pit_read_sched_clock(void)
+static u64 notrace pit_read_sched_clock(void)
 {
 	return ~__raw_readl(clksrc_base + PITCVAL);
 }
-- 
1.9.1

[PATCH 6/8] clocksource/drivers/fsl_ftm_timer: Prevent ftrace recursion

[Daniel Lezcano]

From: Jisheng Zhang <jszhang@marvell.com>

Having a traceable function in the sched_clock() path leads to a recursion
within ftrace and a kernel crash.

We should not trace the ftm_read_sched_clock() function.

Fix this by adding the notrace attribute to this function.

Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/fsl_ftm_timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/fsl_ftm_timer.c b/drivers/clocksource/fsl_ftm_timer.c
index ef43469..10202f1 100644
--- a/drivers/clocksource/fsl_ftm_timer.c
+++ b/drivers/clocksource/fsl_ftm_timer.c
@@ -118,7 +118,7 @@ static inline void ftm_reset_counter(void __iomem *base)
 	ftm_writel(0x00, base + FTM_CNT);
 }
 
-static u64 ftm_read_sched_clock(void)
+static u64 notrace ftm_read_sched_clock(void)
 {
 	return ftm_readl(priv->clksrc_base + FTM_CNT);
 }
-- 
1.9.1

[PATCH 7/8] clocksource/drivers/digicolor: Prevent ftrace recursion

[Daniel Lezcano]

From: Jisheng Zhang <jszhang@marvell.com>

Having a traceable function in the sched_clock() path leads to a recursion
within ftrace and a kernel crash.

We should not trace digicolor_timer_sched_read() function. Fix this by adding
the notrace attribute to this function.

Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
Acked-by: Baruch Siach <baruch@tkos.co.il>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/timer-digicolor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-digicolor.c b/drivers/clocksource/timer-digicolor.c
index e73947f0f..a536eeb 100644
--- a/drivers/clocksource/timer-digicolor.c
+++ b/drivers/clocksource/timer-digicolor.c
@@ -143,7 +143,7 @@ static irqreturn_t digicolor_timer_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static u64 digicolor_timer_sched_read(void)
+static u64 notrace digicolor_timer_sched_read(void)
 {
 	return ~readl(dc_timer_dev.base + COUNT(TIMER_B));
 }
-- 
1.9.1

[PATCH 8/8] clocksource/drivers/sh_mtu2: Fix multiple shutdown call issue

[Daniel Lezcano]

From: Magnus Damm <damm+renesas@opensource.se>

On the r7s72100 Genmai board the MTU2 driver currently triggers a common
clock framework WARN_ON(enable_count) when disabling the clock due to
the MTU2 driver after recent callback rework may call ->set_state_shutdown()
multiple times. A similar issue was spotted for the TMU driver and fixed in:
452b132 clocksource/drivers/sh_tmu: Fix traceback spotted in -next

On r7s72100 Genmai v4.3-rc7 built with shmobile_defconfig spits out the
following during boot:

sh_mtu2 fcff0000.timer: ch0: used for clock events
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at drivers/clk/clk.c:675 clk_core_disable+0x2c/0x6c()
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.3.0-rc7 #1
Hardware name: Generic R7S72100 (Flattened Device Tree)
Backtrace:
[<c00133d4>] (dump_backtrace) from [<c0013570>] (show_stack+0x18/0x1c)
[<c0013558>] (show_stack) from [<c01c7aac>] (dump_stack+0x74/0x90)
[<c01c7a38>] (dump_stack) from [<c00272fc>] (warn_slowpath_common+0x88/0xb4)
[<c0027274>] (warn_slowpath_common) from [<c0027400>] (warn_slowpath_null+0x24/0x2c)
[<c00273dc>] (warn_slowpath_null) from [<c03a9320>] (clk_core_disable+0x2c/0x6c)
[<c03a92f4>] (clk_core_disable) from [<c03aa0a0>] (clk_disable+0x40/0x4c)
[<c03aa060>] (clk_disable) from [<c0395d2c>] (sh_mtu2_disable+0x24/0x50)
[<c0395d08>] (sh_mtu2_disable) from [<c0395d6c>] (sh_mtu2_clock_event_shutdown+0x14/0x1c)
[<c0395d58>] (sh_mtu2_clock_event_shutdown) from [<c007d7d0>] (clockevents_switch_state+0xc8/0x114)
[<c007d708>] (clockevents_switch_state) from [<c007d834>] (clockevents_shutdown+0x18/0x28)
[<c007d81c>] (clockevents_shutdown) from [<c007dd58>] (clockevents_exchange_device+0x70/0x78)
[<c007dce8>] (clockevents_exchange_device) from [<c007e578>] (tick_check_new_device+0x88/0xe0)
[<c007e4f0>] (tick_check_new_device) from [<c007daf0>] (clockevents_register_device+0xac/0x120)
[<c007da44>] (clockevents_register_device) from [<c0395be8>] (sh_mtu2_probe+0x230/0x350)
[<c03959b8>] (sh_mtu2_probe) from [<c028b6f0>] (platform_drv_probe+0x50/0x98)

Reported-by: Chris Brandt <chris.brandt@renesas.com>
Fixes: 19a9ffb ("clockevents/drivers/sh_mtu2: Migrate to new 'set-state' interface")
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/clocksource/sh_mtu2.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/sh_mtu2.c b/drivers/clocksource/sh_mtu2.c
index f1985da..53aa7e9 100644
--- a/drivers/clocksource/sh_mtu2.c
+++ b/drivers/clocksource/sh_mtu2.c
@@ -280,7 +280,9 @@ static int sh_mtu2_clock_event_shutdown(struct clock_event_device *ced)
 {
 	struct sh_mtu2_channel *ch = ced_to_sh_mtu2(ced);
 
-	sh_mtu2_disable(ch);
+	if (clockevent_state_periodic(ced))
+		sh_mtu2_disable(ch);
+
 	return 0;
 }
 
-- 
1.9.1

Re: [PULL] clockevents: fixes for 4.3-rc7

[Thomas Gleixner]

On Wed, 28 Oct 2015, Daniel Lezcano wrote:
> this pull request contains a set of fixes for tip/timers/urgent:
> 
>  - Prevent ftrace recursion by adding the 'notrace' attribute to the
> sched_clock read callback (Jisheng Zhang).
> 
>  - Fix multiple shutdown call issue (Magnus Damm).
> 
> Beside that, the commit fc686d0037d782c994e338ecb01bfef8bbafff9f:
> 
> "clockevents/drivers/mtk: Fix spurious interrupt leading to crash"
> 
> is merged in tip/timers/core but it is needed also as a fix for 4.3. This pull
> request does not contain this fix in order to prevent a conflict when merging
> tip/timers/urgent into tip/timers/core.
> 
> How shall I backport this fix from 'core' to 'urgent' without bringing a
> conflict when you will merge both branches ?

The simplest way is to just do nothing and when that commit hits linus
tree during the merge window, ask the stable folks to pick it up.

Thanks,

	tglx

Re: [PULL] clockevents: fixes for 4.3-rc7

[Daniel Lezcano]

On 10/30/2015 10:43 AM, Thomas Gleixner wrote:
> On Wed, 28 Oct 2015, Daniel Lezcano wrote:
>> this pull request contains a set of fixes for tip/timers/urgent:
>>
>>   - Prevent ftrace recursion by adding the 'notrace' attribute to the
>> sched_clock read callback (Jisheng Zhang).
>>
>>   - Fix multiple shutdown call issue (Magnus Damm).
>>
>> Beside that, the commit fc686d0037d782c994e338ecb01bfef8bbafff9f:
>>
>> "clockevents/drivers/mtk: Fix spurious interrupt leading to crash"
>>
>> is merged in tip/timers/core but it is needed also as a fix for 4.3. This pull
>> request does not contain this fix in order to prevent a conflict when merging
>> tip/timers/urgent into tip/timers/core.
>>
>> How shall I backport this fix from 'core' to 'urgent' without bringing a
>> conflict when you will merge both branches ?
>
> The simplest way is to just do nothing and when that commit hits linus
> tree during the merge window, ask the stable folks to pick it up.

Ok, makes sense.

Thanks !

   -- Daniel


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