2014-02-13 03:11:28

by Tarek Dakhran

[permalink] [raw]
Subject: [PATCH v1 0/2] exynos_mct driver: fix irq allocation and cleanup

exynos4_local_timer_setup called on the secondary cpu before
irqs are enabled. request_irq can sleep, which produces next warning:

on boot:
[ 0.370000] CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
[ 0.370000] Setting up static identity map for 0x403b5700 - 0x403b5758
[ 0.395000] CPU1: Booted secondary processor
[ 0.395000] ------------[ cut here ]------------
[ 0.395000] WARNING: CPU: 1 PID: 0 at kernel/locking/lockdep.c:2742 lockdep_trace_alloc+0xe0/0xfc()
[ 0.395000] DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags))
[ 0.395000] Modules linked in:
[ 0.395000] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 3.14.0-rc2-00004-g0db95f4 #128
[ 0.395000] [<c0014308>] (unwind_backtrace) from [<c0011690>] (show_stack+0x10/0x14)
[ 0.395000] [<c0011690>] (show_stack) from [<c03ae7d0>] (dump_stack+0x6c/0xb8)
[ 0.395000] [<c03ae7d0>] (dump_stack) from [<c001d504>] (warn_slowpath_common+0x68/0x8c)
[ 0.395000] [<c001d504>] (warn_slowpath_common) from [<c001d5bc>] (warn_slowpath_fmt+0x30/0x40)
[ 0.395000] [<c001d5bc>] (warn_slowpath_fmt) from [<c0059824>] (lockdep_trace_alloc+0xe0/0xfc)
[ 0.395000] [<c0059824>] (lockdep_trace_alloc) from [<c00bee24>] (kmem_cache_alloc+0x24/0x160)
[ 0.395000] [<c00bee24>] (kmem_cache_alloc) from [<c0068174>] (request_threaded_irq+0x64/0x130)
[ 0.395000] [<c0068174>] (request_threaded_irq) from [<c02efaf8>] (exynos4_local_timer_setup+0xd0/0x124)
[ 0.395000] [<c02efaf8>] (exynos4_local_timer_setup) from [<c02efc34>] (exynos4_mct_cpu_notify+0x78/0xf0)
[ 0.395000] [<c02efc34>] (exynos4_mct_cpu_notify) from [<c003d318>] (notifier_call_chain+0x44/0x84)
[ 0.395000] [<c003d318>] (notifier_call_chain) from [<c001d61c>] (__cpu_notify+0x24/0x40)
[ 0.395000] [<c001d61c>] (__cpu_notify) from [<c0013314>] (secondary_start_kernel+0xe4/0x134)
[ 0.395000] [<c0013314>] (secondary_start_kernel) from [<40008624>] (0x40008624)
[ 0.395000] ---[ end trace 347890460e745f50 ]---
[ 0.420000] CPU1: update cpu_power 1024
[ 0.420000] CPU1: thread -1, cpu 1, socket 0, mpidr 80000001

on hotplug:
[ 108.040000] CPU3: Booted secondary processor
[ 108.040000] BUG: sleeping function called from invalid context at mm/slub.c:965
[ 108.040000] in_atomic(): 1, irqs_disabled(): 128, pid: 0, name: swapper/3
[ 108.040000] INFO: lockdep is turned off.
[ 108.040000] irq event stamp: 0
[ 108.040000] hardirqs last enabled at (0): [< (null)>] (null)
[ 108.040000] hardirqs last disabled at (0): [<c001b768>] copy_process.part.2+0x2a4/0x12f4
[ 108.040000] softirqs last enabled at (0): [<c001b768>] copy_process.part.2+0x2a4/0x12f4
[ 108.040000] softirqs last disabled at (0): [< (null)>] (null)
[ 108.040000] CPU: 3 PID: 0 Comm: swapper/3 Tainted: G W 3.14.0-rc2-00004-g0db95f4 #128
[ 108.040000] [<c0014308>] (unwind_backtrace) from [<c0011690>] (show_stack+0x10/0x14)
[ 108.040000] [<c0011690>] (show_stack) from [<c03ae7d0>] (dump_stack+0x6c/0xb8)
[ 108.040000] [<c03ae7d0>] (dump_stack) from [<c00beed4>] (kmem_cache_alloc+0xd4/0x160)
[ 108.040000] [<c00beed4>] (kmem_cache_alloc) from [<c0068174>] (request_threaded_irq+0x64/0x130)
[ 108.040000] [<c0068174>] (request_threaded_irq) from [<c02efaf8>] (exynos4_local_timer_setup+0xd0/0x124)
[ 108.040000] [<c02efaf8>] (exynos4_local_timer_setup) from [<c02efc34>] (exynos4_mct_cpu_notify+0x78/0xf0)
[ 108.040000] [<c02efc34>] (exynos4_mct_cpu_notify) from [<c003d318>] (notifier_call_chain+0x44/0x84)
[ 108.040000] [<c003d318>] (notifier_call_chain) from [<c001d61c>] (__cpu_notify+0x24/0x40)
[ 108.040000] [<c001d61c>] (__cpu_notify) from [<c0013314>] (secondary_start_kernel+0xe4/0x134)
[ 108.040000] [<c0013314>] (secondary_start_kernel) from [<40008624>] (0x40008624)

First patch fixes this problem by removing request_irq from exynos4_local_timer_setup
Second removes non-dt stuff.

Tested on linux kernel v3.14-rc2.
Comments and additions would be appreciated.
Thank you.
Tarek.

Tarek Dakhran (2):
clocksource: mct: remove request_irq from exynos4_local_timer_setup
clocksource: mct: cleanup, remove non-dt stuff from mct

arch/arm/mach-exynos/common.h | 2 --
drivers/clocksource/exynos_mct.c | 55 +++++++++++++++++++++-----------------
2 files changed, 31 insertions(+), 26 deletions(-)

--
1.7.10.4


2014-02-13 03:11:30

by Tarek Dakhran

[permalink] [raw]
Subject: [PATCH v1 1/2] clocksource: mct: remove request_irq from exynos4_local_timer_setup

exynos4_local_timer_setup called on the secondary cpu before
irqs are enabled. request_irq can sleep, which produces next warning:

BUG: sleeping function called from invalid context at mm/slub.c:965
in_atomic(): 1, irqs_disabled(): 128, pid: 0, name: swapper/3

Call setup_irq for each local timer in exynos4_timer_resources,
and only call enable_irq during percpu timer setup.

Signed-off-by: Tarek Dakhran <[email protected]>
---
drivers/clocksource/exynos_mct.c | 38 ++++++++++++++++++++++++++++++--------
1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 48f76bc..1cde3de 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -82,6 +82,7 @@ static void __iomem *reg_base;
static unsigned long clk_rate;
static unsigned int mct_int_type;
static int mct_irqs[MCT_NR_IRQS];
+static struct irqaction __percpu *mct_LX_irqaction;

struct mct_clock_event_device {
struct clock_event_device evt;
@@ -402,6 +403,25 @@ static irqreturn_t exynos4_mct_tick_isr(int irq, void *dev_id)
return IRQ_HANDLED;
}

+static void exynos4_setup_irqaction_spi(unsigned int cpu)
+{
+ struct irqaction *pcpu_irqaction = per_cpu_ptr(mct_LX_irqaction, cpu);
+ unsigned int irq = mct_irqs[MCT_L0_IRQ + cpu];
+ int err;
+
+ pcpu_irqaction->name = per_cpu(percpu_mct_tick, cpu).name;
+ pcpu_irqaction->flags = IRQF_TIMER | IRQF_NOBALANCING;
+ pcpu_irqaction->handler = exynos4_mct_tick_isr;
+ pcpu_irqaction->dev_id = &per_cpu(percpu_mct_tick, cpu);
+
+ err = setup_irq(irq, pcpu_irqaction);
+ if (err) {
+ pr_err("MCT: can't setup IRQ %d (%d)\n", irq, err);
+ return;
+ }
+ disable_irq(irq);
+}
+
static int exynos4_local_timer_setup(struct clock_event_device *evt)
{
struct mct_clock_event_device *mevt;
@@ -425,13 +445,7 @@ static int exynos4_local_timer_setup(struct clock_event_device *evt)

if (mct_int_type == MCT_INT_SPI) {
evt->irq = mct_irqs[MCT_L0_IRQ + cpu];
- if (request_irq(evt->irq, exynos4_mct_tick_isr,
- IRQF_TIMER | IRQF_NOBALANCING,
- evt->name, mevt)) {
- pr_err("exynos-mct: cannot register IRQ %d\n",
- evt->irq);
- return -EIO;
- }
+ enable_irq(evt->irq);
} else {
enable_percpu_irq(mct_irqs[MCT_L0_IRQ], 0);
}
@@ -443,7 +457,7 @@ static void exynos4_local_timer_stop(struct clock_event_device *evt)
{
evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
if (mct_int_type == MCT_INT_SPI)
- free_irq(evt->irq, this_cpu_ptr(&percpu_mct_tick));
+ disable_irq(evt->irq);
else
disable_percpu_irq(mct_irqs[MCT_L0_IRQ]);
}
@@ -485,9 +499,12 @@ static struct notifier_block exynos4_mct_cpu_nb = {
static void __init exynos4_timer_resources(struct device_node *np, void __iomem *base)
{
int err;
+ u32 i, cpu, nr_irqs;
struct mct_clock_event_device *mevt = this_cpu_ptr(&percpu_mct_tick);
struct clk *mct_clk, *tick_clk;

+ nr_irqs = of_irq_count(np);
+
tick_clk = np ? of_clk_get_by_name(np, "fin_pll") :
clk_get(NULL, "fin_pll");
if (IS_ERR(tick_clk))
@@ -511,6 +528,11 @@ static void __init exynos4_timer_resources(struct device_node *np, void __iomem
WARN(err, "MCT: can't request IRQ %d (%d)\n",
mct_irqs[MCT_L0_IRQ], err);
} else {
+ mct_LX_irqaction = alloc_percpu(struct irqaction);
+ BUG_ON(!mct_LX_irqaction);
+
+ for (i = MCT_L0_IRQ, cpu = 0; i < nr_irqs; i++, cpu++)
+ exynos4_setup_irqaction_spi(cpu);
irq_set_affinity(mct_irqs[MCT_L0_IRQ], cpumask_of(0));
}

--
1.7.10.4

2014-02-13 03:11:40

by Tarek Dakhran

[permalink] [raw]
Subject: [PATCH v1 2/2] clocksource: mct: cleanup, remove non-dt stuff from mct

mct_init not used anywhere, remove this non-dt stuff.
also remove declaration of mct_init
in arch/arm/mach-exynos/common.h

Signed-off-by: Tarek Dakhran <[email protected]>
---
arch/arm/mach-exynos/common.h | 2 --
drivers/clocksource/exynos_mct.c | 17 +----------------
2 files changed, 1 insertion(+), 18 deletions(-)

diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index f76967b..8945170 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -15,8 +15,6 @@
#include <linux/reboot.h>
#include <linux/of.h>

-void mct_init(void __iomem *base, int irq_g0, int irq_l0, int irq_l1);
-
struct map_desc;
void exynos_init_io(void);
void exynos4_restart(enum reboot_mode mode, const char *cmd);
diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 1cde3de..a94a908 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -548,18 +548,6 @@ out_irq:
free_percpu_irq(mct_irqs[MCT_L0_IRQ], &percpu_mct_tick);
}

-void __init mct_init(void __iomem *base, int irq_g0, int irq_l0, int irq_l1)
-{
- mct_irqs[MCT_G0_IRQ] = irq_g0;
- mct_irqs[MCT_L0_IRQ] = irq_l0;
- mct_irqs[MCT_L1_IRQ] = irq_l1;
- mct_int_type = MCT_INT_SPI;
-
- exynos4_timer_resources(NULL, base);
- exynos4_clocksource_init();
- exynos4_clockevent_init();
-}
-
static void __init mct_init_dt(struct device_node *np, unsigned int int_type)
{
u32 nr_irqs, i;
@@ -574,11 +562,8 @@ static void __init mct_init_dt(struct device_node *np, unsigned int int_type)
* timer irqs are specified after the four global timer
* irqs are specified.
*/
-#ifdef CONFIG_OF
nr_irqs = of_irq_count(np);
-#else
- nr_irqs = 0;
-#endif
+
for (i = MCT_L0_IRQ; i < nr_irqs; i++)
mct_irqs[i] = irq_of_parse_and_map(np, i);

--
1.7.10.4

2014-02-13 12:36:26

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] exynos_mct driver: fix irq allocation and cleanup

Hi Tarek,

On 13.02.2014 04:08, Tarek Dakhran wrote:
> exynos4_local_timer_setup called on the secondary cpu before
> irqs are enabled. request_irq can sleep, which produces next warning:
>
> on boot:
> [ 0.370000] CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
> [ 0.370000] Setting up static identity map for 0x403b5700 - 0x403b5758
> [ 0.395000] CPU1: Booted secondary processor
> [ 0.395000] ------------[ cut here ]------------
> [ 0.395000] WARNING: CPU: 1 PID: 0 at kernel/locking/lockdep.c:2742 lockdep_trace_alloc+0xe0/0xfc()
> [ 0.395000] DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags))
> [ 0.395000] Modules linked in:
> [ 0.395000] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 3.14.0-rc2-00004-g0db95f4 #128
> [ 0.395000] [<c0014308>] (unwind_backtrace) from [<c0011690>] (show_stack+0x10/0x14)
> [ 0.395000] [<c0011690>] (show_stack) from [<c03ae7d0>] (dump_stack+0x6c/0xb8)
> [ 0.395000] [<c03ae7d0>] (dump_stack) from [<c001d504>] (warn_slowpath_common+0x68/0x8c)
> [ 0.395000] [<c001d504>] (warn_slowpath_common) from [<c001d5bc>] (warn_slowpath_fmt+0x30/0x40)
> [ 0.395000] [<c001d5bc>] (warn_slowpath_fmt) from [<c0059824>] (lockdep_trace_alloc+0xe0/0xfc)
> [ 0.395000] [<c0059824>] (lockdep_trace_alloc) from [<c00bee24>] (kmem_cache_alloc+0x24/0x160)
> [ 0.395000] [<c00bee24>] (kmem_cache_alloc) from [<c0068174>] (request_threaded_irq+0x64/0x130)
> [ 0.395000] [<c0068174>] (request_threaded_irq) from [<c02efaf8>] (exynos4_local_timer_setup+0xd0/0x124)
> [ 0.395000] [<c02efaf8>] (exynos4_local_timer_setup) from [<c02efc34>] (exynos4_mct_cpu_notify+0x78/0xf0)
> [ 0.395000] [<c02efc34>] (exynos4_mct_cpu_notify) from [<c003d318>] (notifier_call_chain+0x44/0x84)
> [ 0.395000] [<c003d318>] (notifier_call_chain) from [<c001d61c>] (__cpu_notify+0x24/0x40)
> [ 0.395000] [<c001d61c>] (__cpu_notify) from [<c0013314>] (secondary_start_kernel+0xe4/0x134)
> [ 0.395000] [<c0013314>] (secondary_start_kernel) from [<40008624>] (0x40008624)
> [ 0.395000] ---[ end trace 347890460e745f50 ]---
> [ 0.420000] CPU1: update cpu_power 1024
> [ 0.420000] CPU1: thread -1, cpu 1, socket 0, mpidr 80000001
>
> on hotplug:
> [ 108.040000] CPU3: Booted secondary processor
> [ 108.040000] BUG: sleeping function called from invalid context at mm/slub.c:965
> [ 108.040000] in_atomic(): 1, irqs_disabled(): 128, pid: 0, name: swapper/3
> [ 108.040000] INFO: lockdep is turned off.
> [ 108.040000] irq event stamp: 0
> [ 108.040000] hardirqs last enabled at (0): [< (null)>] (null)
> [ 108.040000] hardirqs last disabled at (0): [<c001b768>] copy_process.part.2+0x2a4/0x12f4
> [ 108.040000] softirqs last enabled at (0): [<c001b768>] copy_process.part.2+0x2a4/0x12f4
> [ 108.040000] softirqs last disabled at (0): [< (null)>] (null)
> [ 108.040000] CPU: 3 PID: 0 Comm: swapper/3 Tainted: G W 3.14.0-rc2-00004-g0db95f4 #128
> [ 108.040000] [<c0014308>] (unwind_backtrace) from [<c0011690>] (show_stack+0x10/0x14)
> [ 108.040000] [<c0011690>] (show_stack) from [<c03ae7d0>] (dump_stack+0x6c/0xb8)
> [ 108.040000] [<c03ae7d0>] (dump_stack) from [<c00beed4>] (kmem_cache_alloc+0xd4/0x160)
> [ 108.040000] [<c00beed4>] (kmem_cache_alloc) from [<c0068174>] (request_threaded_irq+0x64/0x130)
> [ 108.040000] [<c0068174>] (request_threaded_irq) from [<c02efaf8>] (exynos4_local_timer_setup+0xd0/0x124)
> [ 108.040000] [<c02efaf8>] (exynos4_local_timer_setup) from [<c02efc34>] (exynos4_mct_cpu_notify+0x78/0xf0)
> [ 108.040000] [<c02efc34>] (exynos4_mct_cpu_notify) from [<c003d318>] (notifier_call_chain+0x44/0x84)
> [ 108.040000] [<c003d318>] (notifier_call_chain) from [<c001d61c>] (__cpu_notify+0x24/0x40)
> [ 108.040000] [<c001d61c>] (__cpu_notify) from [<c0013314>] (secondary_start_kernel+0xe4/0x134)
> [ 108.040000] [<c0013314>] (secondary_start_kernel) from [<40008624>] (0x40008624)
>
> First patch fixes this problem by removing request_irq from exynos4_local_timer_setup
> Second removes non-dt stuff.
>
> Tested on linux kernel v3.14-rc2.

It would be nice to say on which boards it has been tested. Let me check
this on our boards anyway.

Best regards,
Tomasz

2014-02-15 04:12:18

by Tarek Dakhran

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] exynos_mct driver: fix irq allocation and cleanup

On Thu, Feb 13, 2014 at 10:31 PM, Tarek Dakhran <[email protected]> wrote:
> Hi Tomasz,
>
> On Thursday, February 13, 2014, Tomasz Figa <[email protected]> wrote:
>>
>> Hi Tarek,
>>
>> On 13.02.2014 04:08, Tarek Dakhran wrote:
>>>
>>> exynos4_local_timer_setup called on the secondary cpu before
>>> irqs are enabled. request_irq can sleep, which produces next warning:
>>>
>>> on boot:
>>> [ 0.370000] CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
>>> [ 0.370000] Setting up static identity map for 0x403b5700 - 0x403b5758
>>> [ 0.395000] CPU1: Booted secondary processor
>>> [ 0.395000] ------------[ cut here ]------------
>>> [ 0.395000] WARNING: CPU: 1 PID: 0 at kernel/locking/lockdep.c:2742
>>> lockdep_trace_alloc+0xe0/0xfc()
>>> [ 0.395000] DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags))
>>> [ 0.395000] Modules linked in:
>>> [ 0.395000] CPU: 1 PID: 0 Comm: swapper/1 Not tainted
>>> 3.14.0-rc2-00004-g0db95f4 #128
>>> [ 0.395000] [<c0014308>] (unwind_backtrace) from [<c0011690>]
>>> (show_stack+0x10/0x14)
>>> [ 0.395000] [<c0011690>] (show_stack) from [<c03ae7d0>]
>>> (dump_stack+0x6c/0xb8)
>>> [ 0.395000] [<c03ae7d0>] (dump_stack) from [<c001d504>]
>>> (warn_slowpath_common+0x68/0x8c)
>>> [ 0.395000] [<c001d504>] (warn_slowpath_common) from [<c001d5bc>]
>>> (warn_slowpath_fmt+0x30/0x40)
>>> [ 0.395000] [<c001d5bc>] (warn_slowpath_fmt) from [<c0059824>]
>>> (lockdep_trace_alloc+0xe0/0xfc)
>>> [ 0.395000] [<c0059824>] (lockdep_trace_alloc) from [<c00bee24>]
>>> (kmem_cache_alloc+0x24/0x160)
>>> [ 0.395000] [<c00bee24>] (kmem_cache_alloc) from [<c0068174>]
>>> (request_threaded_irq+0x64/0x130)
>>> [ 0.395000] [<c0068174>] (request_threaded_irq) from [<c02efaf8>]
>>> (exynos4_local_timer_setup+0xd0/0x124)
>>> [ 0.395000] [<c02efaf8>] (exynos4_local_timer_setup) from [<c02efc34>]
>>> (exynos4_mct_cpu_notify+0x78/0xf0)
>>> [ 0.395000] [<c02efc34>] (exynos4_mct_cpu_notify) from [<c003d318>]
>>> (notifier_call_chain+0x44/0x84)
>>> [ 0.395000] [<c003d318>] (notifier_call_chain) from [<c001d61c>]
>>> (__cpu_notify+0x24/0x40)
>>> [ 0.395000] [<c001d61c>] (__cpu_notify) from [<c0013314>]
>>> (secondary_start_kernel+0xe4/0x134)
>>> [ 0.395000] [<c0013314>] (secondary_start_kernel) from [<40008624>]
>>> (0x40008624)
>>> [ 0.395000] ---[ end trace 347890460e745f50 ]---
>>> [ 0.420000] CPU1: update cpu_power 1024
>>> [ 0.420000] CPU1: thread -1, cpu 1, socket 0, mpidr 80000001
>>>
>>> on hotplug:
>>> [ 108.040000] CPU3: Booted secondary processor
>>> [ 108.040000] BUG: sleeping function called from invalid context at
>>> mm/slub.c:965
>>> [ 108.040000] in_atomic(): 1, irqs_disabled(): 128, pid: 0, name:
>>> swapper/3
>>> [ 108.040000] INFO: lockdep is turned off.
>>> [ 108.040000] irq event stamp: 0
>>> [ 108.040000] hardirqs last enabled at (0): [< (null)>] (null)
>>> [ 108.040000] hardirqs last disabled at (0): [<c001b768>]
>>> copy_process.part.2+0x2a4/0x12f4
>>> [ 108.040000] softirqs last enabled at (0): [<c001b768>]
>>> copy_process.part.2+0x2a4/0x12f4
>>> [ 108.040000] softirqs last disabled at (0): [< (null)>] (null)
>>> [ 108.040000] CPU: 3 PID: 0 Comm: swapper/3 Tainted: G W
>>> 3.14.0-rc2-00004-g0db95f4 #128
>>> [ 108.040000] [<c0014308>] (unwind_backtrace) from [<c0011690>]
>>> (show_stack+0x10/0x14)
>>> [ 108.040000] [<c0011690>] (show_stack) from [<c03ae7d0>]
>>> (dump_stack+0x6c/0xb8)
>>> [ 108.040000] [<c03ae7d0>] (dump_stack) from [<c00beed4>]
>>> (kmem_cache_alloc+0xd4/0x160)
>>> [ 108.040000] [<c00beed4>] (kmem_cache_alloc) from [<c0068174>]
>>> (request_threaded_irq+0x64/0x130)
>>> [ 108.040000] [<c0068174>] (request_threaded_irq) from [<c02efaf8>]
>>> (exynos4_local_timer_setup+0xd0/0x124)
>>> [ 108.040000] [<c02efaf8>] (exynos4_local_timer_setup) from [<c02efc34>]
>>> (exynos4_mct_cpu_notify+0x78/0xf0)
>>> [ 108.040000] [<c02efc34>] (exynos4_mct_cpu_notify) from [<c003d318>]
>>> (notifier_call_chain+0x44/0x84)
>>> [ 108.040000] [<c003d318>] (notifier_call_chain) from [<c001d61c>]
>>> (__cpu_notify+0x24/0x40)
>>> [ 108.040000] [<c001d61c>] (__cpu_notify) from [<c0013314>]
>>> (secondary_start_kernel+0xe4/0x134)
>>> [ 108.040000] [<c0013314>] (secondary_start_kernel) from [<40008624>]
>>> (0x40008624)
>>>
>>> First patch fixes this problem by removing request_irq from
>>> exynos4_local_timer_setup
>>> Second removes non-dt stuff.
>>>
>>> Tested on linux kernel v3.14-rc2.
>>
>>
>> It would be nice to say on which boards it has been tested. Let me check
>> this on our boards anyway.
>>
>> Best regards,
>> Tomasz
>
>
> Tested on smdk5410 reference board with lockdep enabled.
> I think the problem will present on all boards due to call sleeping function
> in irq_disabled context.
>
> Best Regards,
> Tarek
>
Has anyone else tested this patches?
if yes, what is the result?


Best regards,
. Tarek

2014-02-15 12:42:56

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] clocksource: mct: remove request_irq from exynos4_local_timer_setup

On Thu, 13 Feb 2014, Tarek Dakhran wrote:

> exynos4_local_timer_setup called on the secondary cpu before
> irqs are enabled. request_irq can sleep, which produces next warning:
>
> BUG: sleeping function called from invalid context at mm/slub.c:965
> in_atomic(): 1, irqs_disabled(): 128, pid: 0, name: swapper/3
>
> Call setup_irq for each local timer in exynos4_timer_resources,
> and only call enable_irq during percpu timer setup.
>
> Signed-off-by: Tarek Dakhran <[email protected]>
> ---
> drivers/clocksource/exynos_mct.c | 38 ++++++++++++++++++++++++++++++--------
> 1 file changed, 30 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
> index 48f76bc..1cde3de 100644
> --- a/drivers/clocksource/exynos_mct.c
> +++ b/drivers/clocksource/exynos_mct.c
> @@ -82,6 +82,7 @@ static void __iomem *reg_base;
> static unsigned long clk_rate;
> static unsigned int mct_int_type;
> static int mct_irqs[MCT_NR_IRQS];
> +static struct irqaction __percpu *mct_LX_irqaction;
>
> struct mct_clock_event_device {
> struct clock_event_device evt;
> @@ -402,6 +403,25 @@ static irqreturn_t exynos4_mct_tick_isr(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> +static void exynos4_setup_irqaction_spi(unsigned int cpu)
> +{
> + struct irqaction *pcpu_irqaction = per_cpu_ptr(mct_LX_irqaction, cpu);
> + unsigned int irq = mct_irqs[MCT_L0_IRQ + cpu];
> + int err;
> +
> + pcpu_irqaction->name = per_cpu(percpu_mct_tick, cpu).name;
> + pcpu_irqaction->flags = IRQF_TIMER | IRQF_NOBALANCING;
> + pcpu_irqaction->handler = exynos4_mct_tick_isr;
> + pcpu_irqaction->dev_id = &per_cpu(percpu_mct_tick, cpu);
> +
> + err = setup_irq(irq, pcpu_irqaction);

This is just crap. Why don't you use request_irq()?

> + if (err) {
> + pr_err("MCT: can't setup IRQ %d (%d)\n", irq, err);
> + return;
> + }
> + disable_irq(irq);

This is completely backwards. We have flags which tell the core not to
enable an interrupt at setup time. Also what's wrong with enabling the
interrupt in the first place? When the timer is stopped then no
interrupts happen and it does not matter a bit whether the irq line is
enabled or not.

Thanks,

tglx