Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753654AbbBBIYa (ORCPT ); Mon, 2 Feb 2015 03:24:30 -0500 Received: from mail-vc0-f182.google.com ([209.85.220.182]:45933 "EHLO mail-vc0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752315AbbBBIY1 (ORCPT ); Mon, 2 Feb 2015 03:24:27 -0500 MIME-Version: 1.0 In-Reply-To: <1422627691-10774-1-git-send-email-d.eppel@samsung.com> References: <1422627691-10774-1-git-send-email-d.eppel@samsung.com> Date: Mon, 2 Feb 2015 09:24:26 +0100 Message-ID: Subject: Re: [PATCH] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif. From: =?UTF-8?Q?Krzysztof_Koz=C5=82owski?= To: Damian Eppel Cc: daniel.lezcano@linaro.org, tglx@linutronix.de, kgene@kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, kyungmin.park@samsung.com, stable@vger.kernel.org, =?UTF-8?B?QmFydMWCb21pZWogxbtvxYJuaWVya2lld2ljeg==?= Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8064 Lines: 172 2015-01-30 15:21 GMT+01:00 Damian Eppel : > This is to fix an issue of sleeping in atomic context when processing > hotplug notifications in Exynos MCT(Multi-Core Timer). > The issue was reproducible on Exynos 3250 (Rinato board) and Exynos 5420 > (Arndale Octa board). > > Whilst testing cpu hotplug events on kernel configured with DEBUG_PREEMPT > and DEBUG_ATOMIC_SLEEP we get following BUG message, caused by calling > request_irq() and free_irq() in the context of hotplug notification > (which is in this case atomic context). > > root@target:~# echo 0 > /sys/devices/system/cpu/cpu1/online > > [ 25.157867] IRQ18 no longer affine to CPU1 > ... > [ 25.158445] CPU1: shutdown > > root@target:~# echo 1 > /sys/devices/system/cpu/cpu1/online > > [ 40.785859] CPU1: Software reset > [ 40.786660] BUG: sleeping function called from invalid context at mm/slub.c:1241 > [ 40.786668] in_atomic(): 1, irqs_disabled(): 128, pid: 0, name: swapper/1 > [ 40.786678] Preemption disabled at:[< (null)>] (null) > [ 40.786681] > [ 40.786692] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 3.19.0-rc4-00024-g7dca860 #36 > [ 40.786698] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) > [ 40.786728] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) > [ 40.786747] [] (show_stack) from [] (dump_stack+0x70/0xbc) > [ 40.786767] [] (dump_stack) from [] (kmem_cache_alloc+0xd8/0x170) > [ 40.786785] [] (kmem_cache_alloc) from [] (request_threaded_irq+0x64/0x128) > [ 40.786804] [] (request_threaded_irq) from [] (exynos4_local_timer_setup+0xc0/0x13c) > [ 40.786820] [] (exynos4_local_timer_setup) from [] (exynos4_mct_cpu_notify+0x30/0xa8) > [ 40.786838] [] (exynos4_mct_cpu_notify) from [] (notifier_call_chain+0x44/0x84) > [ 40.786857] [] (notifier_call_chain) from [] (__cpu_notify+0x28/0x44) > [ 40.786873] [] (__cpu_notify) from [] (secondary_start_kernel+0xec/0x150) > [ 40.786886] [] (secondary_start_kernel) from [<40008764>] (0x40008764) > > Solution: > Clockevent irqs cannot be requested/freed every time cpu is > hot-plugged/unplugged as CPU_STARTING/CPU_DYING notifications > that signals hotplug or unplug events are sent with both preemption > and local interrupts disabled. Since request_irq() may sleep it is > moved to the initialization stage and performed for all possible > cpus prior putting them online. Then, in the case of hotplug event > the irq asociated with the given cpu will simply be enabled. > Similarly on cpu unplug event the interrupt is not freed but just > disabled. > > Note that after successful request_irq() call for a clockevent device > associated to given cpu the requested irq is disabled (via disable_irq()). > That is to make things symmetric as we expect hotplug event as a next > thing (which will enable irq again). This should not pose any problems > because at the time of requesting irq the clockevent device is not > fully initialized yet, therefore should not produce interrupts at that point. > > For disabling an irq at cpu unplug notification disable_irq_nosync() is > chosen which is a non-blocking function. This again shouldn't be a problem as > prior sending CPU_DYING notification interrupts are migrated away > from the cpu. > > Fixes: 7114cd749a12 ("clocksource: exynos_mct: use (request/free)_irq calls for local timer registration") > Signed-off-by: Damian Eppel > Cc: > Reported-by: Krzysztof Kozlowski > --- > drivers/clocksource/exynos_mct.c | 45 ++++++++++++++++++++++++++++------------ > 1 file changed, 32 insertions(+), 13 deletions(-) Hi, Look fine from my point of view. Reviewed-by: Krzysztof Kozlowski Tested on Arndale Octa Board. Fixes the sleep-in-atomic. Tested-by: Krzysztof Kozlowski Best regards, Krzysztof > > diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c > index 83564c9..9beca58 100644 > --- a/drivers/clocksource/exynos_mct.c > +++ b/drivers/clocksource/exynos_mct.c > @@ -466,15 +466,12 @@ static int exynos4_local_timer_setup(struct clock_event_device *evt) > exynos4_mct_write(TICK_BASE_CNT, mevt->base + MCT_L_TCNTB_OFFSET); > > 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); > + > + if (evt->irq == -1) > return -EIO; > - } > - irq_force_affinity(mct_irqs[MCT_L0_IRQ + cpu], cpumask_of(cpu)); > + > + irq_force_affinity(evt->irq, cpumask_of(cpu)); > + enable_irq(evt->irq); > } else { > enable_percpu_irq(mct_irqs[MCT_L0_IRQ], 0); > } > @@ -487,10 +484,12 @@ static int exynos4_local_timer_setup(struct clock_event_device *evt) > 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)); > - else > + if (mct_int_type == MCT_INT_SPI) { > + if (evt->irq != -1) > + disable_irq_nosync(evt->irq); > + } else { > disable_percpu_irq(mct_irqs[MCT_L0_IRQ]); > + } > } > > static int exynos4_mct_cpu_notify(struct notifier_block *self, > @@ -522,7 +521,7 @@ static struct notifier_block exynos4_mct_cpu_nb = { > > static void __init exynos4_timer_resources(struct device_node *np, void __iomem *base) > { > - int err; > + int err, cpu; > struct mct_clock_event_device *mevt = this_cpu_ptr(&percpu_mct_tick); > struct clk *mct_clk, *tick_clk; > > @@ -549,7 +548,27 @@ 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 { > - irq_set_affinity(mct_irqs[MCT_L0_IRQ], cpumask_of(0)); > + for_each_possible_cpu(cpu) { > + int mct_irq = mct_irqs[MCT_L0_IRQ + cpu]; > + struct mct_clock_event_device *pcpu_mevt = > + per_cpu_ptr(&percpu_mct_tick, cpu); > + > + pcpu_mevt->evt.irq = -1; > + > + if (request_irq(mct_irq, > + exynos4_mct_tick_isr, > + IRQF_TIMER | IRQF_NOBALANCING, > + pcpu_mevt->name, pcpu_mevt)) { > + pr_err("exynos-mct: cannot register IRQ (cpu%d)\n", > + cpu); > + > + continue; > + } > + pcpu_mevt->evt.irq = mct_irq; > + irq_force_affinity(mct_irq, cpumask_of(cpu)); > + > + disable_irq(mct_irq); > + } > } > > err = register_cpu_notifier(&exynos4_mct_cpu_nb); > -- > 1.9.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/