linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL] clockevents/clocksources: 3.13 fixes
@ 2013-12-30 10:43 Daniel Lezcano
  2013-12-30 10:44 ` [PATCH] clocksource: cadence_ttc: Fix mutex taken inside interrupt context Daniel Lezcano
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Daniel Lezcano @ 2013-12-30 10:43 UTC (permalink / raw)
  To: linux-arm-kernel


Hi Thomas and Ingo,

here is a pull request for a single fix for 3.13. It is based on the 
latest timers/urgent update.

  * Soren Brinkmann fixed the cadence_ttc driver where a call to 
clk_get_rate happens in an interrupt context. More precisely in an IPI 
when the broadcast timer is initialized for each cpu in the cpuidle driver

Thanks
   -- Daniel

The following changes since commit b0031f227e47919797dc0e1c1990f3ef151ff0cc:

   Merge tag 's2mps11-build' of 
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator 
(2013-12-17 12:57:36 -0800)

are available in the git repository at:


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

for you to fetch changes up to c1dcc927dae01dfd4904ee82ce2c00b50eab6dc3:

   clocksource: cadence_ttc: Fix mutex taken inside interrupt context 
(2013-12-30 11:32:24 +0100)

----------------------------------------------------------------
Soren Brinkmann (1):
       clocksource: cadence_ttc: Fix mutex taken inside interrupt context

  drivers/clocksource/cadence_ttc_timer.c |   21 +++++++++++++--------
  1 file changed, 13 insertions(+), 8 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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] clocksource: cadence_ttc: Fix mutex taken inside interrupt context
  2013-12-30 10:43 [GIT PULL] clockevents/clocksources: 3.13 fixes Daniel Lezcano
@ 2013-12-30 10:44 ` Daniel Lezcano
  2014-01-14 15:01 ` [GIT PULL] clockevents/clocksources: 3.13 fixes Daniel Lezcano
  2014-01-15  6:41 ` Ingo Molnar
  2 siblings, 0 replies; 4+ messages in thread
From: Daniel Lezcano @ 2013-12-30 10:44 UTC (permalink / raw)
  To: linux-arm-kernel

From: Soren Brinkmann <soren.brinkmann@xilinx.com>

When the kernel is compiled with:
CONFIG_HIGH_RES_TIMERS=no
CONFIG_HZ_PERIODIC=yes
CONFIG_DEBUG_ATOMIC_SLEEP=yes

The following WARN appears:

WARNING: CPU: 1 PID: 0 at linux/kernel/mutex.c:856 mutex_trylock+0x70/0x1fc()
DEBUG_LOCKS_WARN_ON(in_interrupt())
Modules linked in:
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 3.12.0-xilinx-dirty #93
[<c0014a78>] (unwind_backtrace+0x0/0x11c) from [<c0011b6c>] (show_stack+0x10/0x14)
[<c0011b6c>] (show_stack+0x10/0x14) from [<c039120c>] (dump_stack+0x7c/0xc0)
[<c039120c>] (dump_stack+0x7c/0xc0) from [<c001fda4>] (warn_slowpath_common+0x60/0x84)
[<c001fda4>] (warn_slowpath_common+0x60/0x84) from [<c001fe48>] (warn_slowpath_fmt+0x2c/0x3c)
[<c001fe48>] (warn_slowpath_fmt+0x2c/0x3c) from [<c0392658>] (mutex_trylock+0x70/0x1fc)
[<c0392658>] (mutex_trylock+0x70/0x1fc) from [<c02dfc08>] (clk_prepare_lock+0xc/0xe4)
[<c02dfc08>] (clk_prepare_lock+0xc/0xe4) from [<c02e099c>] (clk_get_rate+0xc/0x44)
[<c02e099c>] (clk_get_rate+0xc/0x44) from [<c02d0394>] (ttc_set_mode+0x34/0x78)
[<c02d0394>] (ttc_set_mode+0x34/0x78) from [<c005f794>] (clockevents_set_mode+0x28/0x5c)
[<c005f794>] (clockevents_set_mode+0x28/0x5c) from [<c00607fc>] (tick_broadcast_on_off+0x190/0x1c0)
[<c00607fc>] (tick_broadcast_on_off+0x190/0x1c0) from [<c005f168>] (clockevents_notify+0x58/0x1ac)
[<c005f168>] (clockevents_notify+0x58/0x1ac) from [<c02b99dc>] (cpuidle_setup_broadcast_timer+0x20/0x24)
[<c02b99dc>] (cpuidle_setup_broadcast_timer+0x20/0x24) from [<c006cd04>] (generic_smp_call_function_single_interrupt+0)
[<c006cd04>] (generic_smp_call_function_single_interrupt+0xe0/0x130) from [<c00138c8>] (handle_IPI+0x88/0x118)
[<c00138c8>] (handle_IPI+0x88/0x118) from [<c0008504>] (gic_handle_irq+0x58/0x60)
[<c0008504>] (gic_handle_irq+0x58/0x60) from [<c0012644>] (__irq_svc+0x44/0x78)
Exception stack(0xef099fa0 to 0xef099fe8)
9fa0: 00000001 ef092100 00000000 ef092100 ef098000 00000015 c0399f2c c0579d74
9fc0: 0000406a 413fc090 00000000 00000000 00000000 ef099fe8 c00666ec c000f46c
9fe0: 20000113 ffffffff
[<c0012644>] (__irq_svc+0x44/0x78) from [<c000f46c>] (arch_cpu_idle+0x34/0x3c)
[<c000f46c>] (arch_cpu_idle+0x34/0x3c) from [<c0053980>] (cpu_startup_entry+0xa8/0x10c)
[<c0053980>] (cpu_startup_entry+0xa8/0x10c) from [<000085a4>] (0x85a4)

We are in an interrupt context (IPI) and we are calling clk_get_rate in the
set_mode function which in turn ends up by getting a mutex... Even if that
does not hang, it is a potential kernel deadlock.

It is not allowed to call clk_get_rate() from interrupt context. To
avoid such calls the timer input frequency is stored in the driver's
data struct which makes it accessible to the driver in any context.

[dlezcano] completed the changelog with the WARN trace and added a more
detailed description. Tested on zync zc702.

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Tested-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/cadence_ttc_timer.c |   21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/clocksource/cadence_ttc_timer.c b/drivers/clocksource/cadence_ttc_timer.c
index b2bb3a4b..a92350b 100644
--- a/drivers/clocksource/cadence_ttc_timer.c
+++ b/drivers/clocksource/cadence_ttc_timer.c
@@ -67,11 +67,13 @@
  * struct ttc_timer - This definition defines local timer structure
  *
  * @base_addr:	Base address of timer
+ * @freq:	Timer input clock frequency
  * @clk:	Associated clock source
  * @clk_rate_change_nb	Notifier block for clock rate changes
  */
 struct ttc_timer {
 	void __iomem *base_addr;
+	unsigned long freq;
 	struct clk *clk;
 	struct notifier_block clk_rate_change_nb;
 };
@@ -196,9 +198,8 @@ static void ttc_set_mode(enum clock_event_mode mode,
 
 	switch (mode) {
 	case CLOCK_EVT_MODE_PERIODIC:
-		ttc_set_interval(timer,
-				DIV_ROUND_CLOSEST(clk_get_rate(ttce->ttc.clk),
-					PRESCALE * HZ));
+		ttc_set_interval(timer, DIV_ROUND_CLOSEST(ttce->ttc.freq,
+						PRESCALE * HZ));
 		break;
 	case CLOCK_EVT_MODE_ONESHOT:
 	case CLOCK_EVT_MODE_UNUSED:
@@ -273,6 +274,8 @@ static void __init ttc_setup_clocksource(struct clk *clk, void __iomem *base)
 		return;
 	}
 
+	ttccs->ttc.freq = clk_get_rate(ttccs->ttc.clk);
+
 	ttccs->ttc.clk_rate_change_nb.notifier_call =
 		ttc_rate_change_clocksource_cb;
 	ttccs->ttc.clk_rate_change_nb.next = NULL;
@@ -298,16 +301,14 @@ static void __init ttc_setup_clocksource(struct clk *clk, void __iomem *base)
 	__raw_writel(CNT_CNTRL_RESET,
 		     ttccs->ttc.base_addr + TTC_CNT_CNTRL_OFFSET);
 
-	err = clocksource_register_hz(&ttccs->cs,
-			clk_get_rate(ttccs->ttc.clk) / PRESCALE);
+	err = clocksource_register_hz(&ttccs->cs, ttccs->ttc.freq / PRESCALE);
 	if (WARN_ON(err)) {
 		kfree(ttccs);
 		return;
 	}
 
 	ttc_sched_clock_val_reg = base + TTC_COUNT_VAL_OFFSET;
-	setup_sched_clock(ttc_sched_clock_read, 16,
-			clk_get_rate(ttccs->ttc.clk) / PRESCALE);
+	setup_sched_clock(ttc_sched_clock_read, 16, ttccs->ttc.freq / PRESCALE);
 }
 
 static int ttc_rate_change_clockevent_cb(struct notifier_block *nb,
@@ -334,6 +335,9 @@ static int ttc_rate_change_clockevent_cb(struct notifier_block *nb,
 				ndata->new_rate / PRESCALE);
 		local_irq_restore(flags);
 
+		/* update cached frequency */
+		ttc->freq = ndata->new_rate;
+
 		/* fall through */
 	}
 	case PRE_RATE_CHANGE:
@@ -367,6 +371,7 @@ static void __init ttc_setup_clockevent(struct clk *clk,
 	if (clk_notifier_register(ttcce->ttc.clk,
 				&ttcce->ttc.clk_rate_change_nb))
 		pr_warn("Unable to register clock notifier.\n");
+	ttcce->ttc.freq = clk_get_rate(ttcce->ttc.clk);
 
 	ttcce->ttc.base_addr = base;
 	ttcce->ce.name = "ttc_clockevent";
@@ -396,7 +401,7 @@ static void __init ttc_setup_clockevent(struct clk *clk,
 	}
 
 	clockevents_config_and_register(&ttcce->ce,
-			clk_get_rate(ttcce->ttc.clk) / PRESCALE, 1, 0xfffe);
+			ttcce->ttc.freq / PRESCALE, 1, 0xfffe);
 }
 
 /**
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [GIT PULL] clockevents/clocksources: 3.13 fixes
  2013-12-30 10:43 [GIT PULL] clockevents/clocksources: 3.13 fixes Daniel Lezcano
  2013-12-30 10:44 ` [PATCH] clocksource: cadence_ttc: Fix mutex taken inside interrupt context Daniel Lezcano
@ 2014-01-14 15:01 ` Daniel Lezcano
  2014-01-15  6:41 ` Ingo Molnar
  2 siblings, 0 replies; 4+ messages in thread
From: Daniel Lezcano @ 2014-01-14 15:01 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/30/2013 11:43 AM, Daniel Lezcano wrote:
>
> Hi Thomas and Ingo,
>
> here is a pull request for a single fix for 3.13. It is based on the
> latest timers/urgent update.
>
>   * Soren Brinkmann fixed the cadence_ttc driver where a call to
> clk_get_rate happens in an interrupt context. More precisely in an IPI
> when the broadcast timer is initialized for each cpu in the cpuidle driver

Hi Thomas and Ingo,

just a head up for this PR. It has been sent right before the PR for 
3.14, so I wanted to me sure you did not missed it.

Thanks
   -- Daniel

>
> Thanks
>    -- Daniel
>
> The following changes since commit
> b0031f227e47919797dc0e1c1990f3ef151ff0cc:
>
>    Merge tag 's2mps11-build' of
> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator
> (2013-12-17 12:57:36 -0800)
>
> are available in the git repository at:
>
>
>    git://git.linaro.org/people/daniel.lezcano/linux.git
> clockevents/3.13-fixes
>
> for you to fetch changes up to c1dcc927dae01dfd4904ee82ce2c00b50eab6dc3:
>
>    clocksource: cadence_ttc: Fix mutex taken inside interrupt context
> (2013-12-30 11:32:24 +0100)
>
> ----------------------------------------------------------------
> Soren Brinkmann (1):
>        clocksource: cadence_ttc: Fix mutex taken inside interrupt context
>
>   drivers/clocksource/cadence_ttc_timer.c |   21 +++++++++++++--------
>   1 file changed, 13 insertions(+), 8 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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [GIT PULL] clockevents/clocksources: 3.13 fixes
  2013-12-30 10:43 [GIT PULL] clockevents/clocksources: 3.13 fixes Daniel Lezcano
  2013-12-30 10:44 ` [PATCH] clocksource: cadence_ttc: Fix mutex taken inside interrupt context Daniel Lezcano
  2014-01-14 15:01 ` [GIT PULL] clockevents/clocksources: 3.13 fixes Daniel Lezcano
@ 2014-01-15  6:41 ` Ingo Molnar
  2 siblings, 0 replies; 4+ messages in thread
From: Ingo Molnar @ 2014-01-15  6:41 UTC (permalink / raw)
  To: linux-arm-kernel


* Daniel Lezcano <daniel.lezcano@linaro.org> wrote:

> 
> Hi Thomas and Ingo,
> 
> here is a pull request for a single fix for 3.13. It is based on the
> latest timers/urgent update.
> 
>  * Soren Brinkmann fixed the cadence_ttc driver where a call to
> clk_get_rate happens in an interrupt context. More precisely in an
> IPI when the broadcast timer is initialized for each cpu in the
> cpuidle driver
> 
> Thanks
>   -- Daniel
> 
> The following changes since commit b0031f227e47919797dc0e1c1990f3ef151ff0cc:
> 
>   Merge tag 's2mps11-build' of
> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator
> (2013-12-17 12:57:36 -0800)
> 
> are available in the git repository at:
> 
> 
>   git://git.linaro.org/people/daniel.lezcano/linux.git
> clockevents/3.13-fixes
> 
> for you to fetch changes up to c1dcc927dae01dfd4904ee82ce2c00b50eab6dc3:
> 
>   clocksource: cadence_ttc: Fix mutex taken inside interrupt context
> (2013-12-30 11:32:24 +0100)
> 
> ----------------------------------------------------------------
> Soren Brinkmann (1):
>       clocksource: cadence_ttc: Fix mutex taken inside interrupt context
> 
>  drivers/clocksource/cadence_ttc_timer.c |   21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)

Pulled into tip:timers/urgent, thanks Daniel!

	Ingo

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-01-15  6:41 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-30 10:43 [GIT PULL] clockevents/clocksources: 3.13 fixes Daniel Lezcano
2013-12-30 10:44 ` [PATCH] clocksource: cadence_ttc: Fix mutex taken inside interrupt context Daniel Lezcano
2014-01-14 15:01 ` [GIT PULL] clockevents/clocksources: 3.13 fixes Daniel Lezcano
2014-01-15  6:41 ` Ingo Molnar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).