Re: [PATCH v2 3/9] clocksource/cadence_ttc: Store timer frequencyin driver data

From: Daniel Lezcano
Date: Thu Dec 19 2013 - 15:53:19 EST

Next message: Davidlohr Bueso: "Re: [PATCH v4 3/4] futex: Document ordering guarantees"
Previous message: Randy Dunlap: "Re: [PATCH v4 3/4] futex: Document ordering guarantees"
In reply to: SÃren Brinkmann: "Re: [PATCH v2 3/9] clocksource/cadence_ttc: Store timer frequency indriver data"
Next in thread: SÃren Brinkmann: "Re: [PATCH v2 3/9] clocksource/cadence_ttc: Store timer frequency indriver data"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 12/19/2013 07:32 PM, SÃren Brinkmann wrote:

Hi Daniel,

On Wed, Dec 18, 2013 at 10:58:26PM +0100, Daniel Lezcano wrote:
On 12/18/2013 05:47 PM, SÃren Brinkmann wrote:
Hi Daniel,

On Wed, Dec 18, 2013 at 03:53:51PM +0100, Daniel Lezcano wrote:
On 12/17/2013 08:21 PM, SÃren Brinkmann wrote:
Hi Daniel,

On Tue, Nov 26, 2013 at 05:04:50PM -0800, Soren Brinkmann wrote:
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.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xxxxxxxxxx>
Acked-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>

I doubt that we'll resolve all issues with this series before the
holidays or even the next merge window. Could you take this patch into
your tree for 3.14? It is not directly related to the cpufreq work and
fixes an actual issue that triggers a kernel WARN under some condition
(I missed preserving the details and the trace). That would take the
easy stuff out of the way and we can focus on the more controversial
changes.

You are asking to take it for 3.14 but shouldn't it go as a 3.13 fix ?

That's also an option. As I remember, the patch fixes a kernel WARN. The
system still seemed operational though. Up to you whether this is
considered severe enough for the 3.13 series. I'm happy either way.

I was not able to reproduce the WARN with my board.

Please, could you give the WARN or give the procedure to reproduce it ?

I can't either... I thought I saw the WARN on a vanilla kernel during
boot (IIRC, when cpuidle started). Is there any chance the timer core
calls the timer's set_mode() from interrupt context?
Anyway, let's drop it for now. I'll make sure to record more information
in case it reappears.

Finally I was able to reproduce it with the highres timers disabled, the periodic tick system and the locks debug.

Indeed, we are in an interrupt context (IPI) and we are calling clk_get_rate in the the set_mode function which in turn ends up by getting a mutex... Even if that does not hang, it is a potential kernel crash so I will apply the patch with an updated changelog.

------------[ cut here ]------------
WARNING: CPU: 1 PID: 0 at /home/dlezcano/Work/src/cpuidle-next/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)
---[ end trace 8185ad1c7a65f6e7 ]---

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

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

Next message: Davidlohr Bueso: "Re: [PATCH v4 3/4] futex: Document ordering guarantees"
Previous message: Randy Dunlap: "Re: [PATCH v4 3/4] futex: Document ordering guarantees"
In reply to: SÃren Brinkmann: "Re: [PATCH v2 3/9] clocksource/cadence_ttc: Store timer frequency indriver data"
Next in thread: SÃren Brinkmann: "Re: [PATCH v2 3/9] clocksource/cadence_ttc: Store timer frequency indriver data"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]