All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
@ 2015-03-12  9:11 ` Damian Eppel
  0 siblings, 0 replies; 29+ messages in thread
From: Damian Eppel @ 2015-03-12  9:11 UTC (permalink / raw)
  To: daniel.lezcano
  Cc: tglx, kgene, linux-kernel, linux-arm-kernel, linux-samsung-soc,
	kyungmin.park, k.kozlowski, m.jabrzyk, Damian Eppel, stable

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] [<c0014a00>] (unwind_backtrace) from [<c0011980>] (show_stack+0x10/0x14)
[   40.786747] [<c0011980>] (show_stack) from [<c0449ba0>] (dump_stack+0x70/0xbc)
[   40.786767] [<c0449ba0>] (dump_stack) from [<c00c6124>] (kmem_cache_alloc+0xd8/0x170)
[   40.786785] [<c00c6124>] (kmem_cache_alloc) from [<c005d6f8>] (request_threaded_irq+0x64/0x128)
[   40.786804] [<c005d6f8>] (request_threaded_irq) from [<c0350b8c>] (exynos4_local_timer_setup+0xc0/0x13c)
[   40.786820] [<c0350b8c>] (exynos4_local_timer_setup) from [<c0350ca8>] (exynos4_mct_cpu_notify+0x30/0xa8)
[   40.786838] [<c0350ca8>] (exynos4_mct_cpu_notify) from [<c003b330>] (notifier_call_chain+0x44/0x84)
[   40.786857] [<c003b330>] (notifier_call_chain) from [<c0022fd4>] (__cpu_notify+0x28/0x44)
[   40.786873] [<c0022fd4>] (__cpu_notify) from [<c0013714>] (secondary_start_kernel+0xec/0x150)
[   40.786886] [<c0013714>] (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 <d.eppel@samsung.com>
Cc: <stable@vger.kernel.org>
Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
(Tested on Arndale Octa Board)
Tested-by: Marcin Jabrzyk <m.jabrzyk@samsung.com>
(Tested on Rinato B2 (Exynos 3250) board)
---

Notes:
    Changes since v1:
            o added Krzysztof's and Marcin's Reviewed-by / Tested-by
              with information about the test HW.

 drivers/clocksource/exynos_mct.c | 45 ++++++++++++++++++++++++++++------------
 1 file changed, 32 insertions(+), 13 deletions(-)

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


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

* [PATCH v2] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
@ 2015-03-12  9:11 ` Damian Eppel
  0 siblings, 0 replies; 29+ messages in thread
From: Damian Eppel @ 2015-03-12  9:11 UTC (permalink / raw)
  To: linux-arm-kernel

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 at target:~# echo 0 > /sys/devices/system/cpu/cpu1/online

[   25.157867] IRQ18 no longer affine to CPU1
...
[   25.158445] CPU1: shutdown

root at 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] [<c0014a00>] (unwind_backtrace) from [<c0011980>] (show_stack+0x10/0x14)
[   40.786747] [<c0011980>] (show_stack) from [<c0449ba0>] (dump_stack+0x70/0xbc)
[   40.786767] [<c0449ba0>] (dump_stack) from [<c00c6124>] (kmem_cache_alloc+0xd8/0x170)
[   40.786785] [<c00c6124>] (kmem_cache_alloc) from [<c005d6f8>] (request_threaded_irq+0x64/0x128)
[   40.786804] [<c005d6f8>] (request_threaded_irq) from [<c0350b8c>] (exynos4_local_timer_setup+0xc0/0x13c)
[   40.786820] [<c0350b8c>] (exynos4_local_timer_setup) from [<c0350ca8>] (exynos4_mct_cpu_notify+0x30/0xa8)
[   40.786838] [<c0350ca8>] (exynos4_mct_cpu_notify) from [<c003b330>] (notifier_call_chain+0x44/0x84)
[   40.786857] [<c003b330>] (notifier_call_chain) from [<c0022fd4>] (__cpu_notify+0x28/0x44)
[   40.786873] [<c0022fd4>] (__cpu_notify) from [<c0013714>] (secondary_start_kernel+0xec/0x150)
[   40.786886] [<c0013714>] (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 <d.eppel@samsung.com>
Cc: <stable@vger.kernel.org>
Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
(Tested on Arndale Octa Board)
Tested-by: Marcin Jabrzyk <m.jabrzyk@samsung.com>
(Tested on Rinato B2 (Exynos 3250) board)
---

Notes:
    Changes since v1:
            o added Krzysztof's and Marcin's Reviewed-by / Tested-by
              with information about the test HW.

 drivers/clocksource/exynos_mct.c | 45 ++++++++++++++++++++++++++++------------
 1 file changed, 32 insertions(+), 13 deletions(-)

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

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

* Re: [PATCH v2] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
  2015-03-12  9:11 ` Damian Eppel
@ 2015-05-11  1:33   ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 29+ messages in thread
From: Krzysztof Kozlowski @ 2015-05-11  1:33 UTC (permalink / raw)
  To: daniel.lezcano, tglx
  Cc: linux-samsung-soc, linux-kernel, stable, kyungmin.park, kgene,
	Damian Eppel, m.jabrzyk, linux-arm-kernel

2015-03-12 18:11 GMT+09:00 Damian Eppel <d.eppel@samsung.com>:
> 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] [<c0014a00>] (unwind_backtrace) from [<c0011980>] (show_stack+0x10/0x14)
> [   40.786747] [<c0011980>] (show_stack) from [<c0449ba0>] (dump_stack+0x70/0xbc)
> [   40.786767] [<c0449ba0>] (dump_stack) from [<c00c6124>] (kmem_cache_alloc+0xd8/0x170)
> [   40.786785] [<c00c6124>] (kmem_cache_alloc) from [<c005d6f8>] (request_threaded_irq+0x64/0x128)
> [   40.786804] [<c005d6f8>] (request_threaded_irq) from [<c0350b8c>] (exynos4_local_timer_setup+0xc0/0x13c)
> [   40.786820] [<c0350b8c>] (exynos4_local_timer_setup) from [<c0350ca8>] (exynos4_mct_cpu_notify+0x30/0xa8)
> [   40.786838] [<c0350ca8>] (exynos4_mct_cpu_notify) from [<c003b330>] (notifier_call_chain+0x44/0x84)
> [   40.786857] [<c003b330>] (notifier_call_chain) from [<c0022fd4>] (__cpu_notify+0x28/0x44)
> [   40.786873] [<c0022fd4>] (__cpu_notify) from [<c0013714>] (secondary_start_kernel+0xec/0x150)
> [   40.786886] [<c0013714>] (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 <d.eppel@samsung.com>
> Cc: <stable@vger.kernel.org>
> Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> (Tested on Arndale Octa Board)
> Tested-by: Marcin Jabrzyk <m.jabrzyk@samsung.com>
> (Tested on Rinato B2 (Exynos 3250) board)

Hi Daniel and Thomas,

Do you have any comments on this patch? Could you pick it up?

Best regards,
Krzysztof

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

* [PATCH v2] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
@ 2015-05-11  1:33   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 29+ messages in thread
From: Krzysztof Kozlowski @ 2015-05-11  1:33 UTC (permalink / raw)
  To: linux-arm-kernel

2015-03-12 18:11 GMT+09:00 Damian Eppel <d.eppel@samsung.com>:
> 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 at target:~# echo 0 > /sys/devices/system/cpu/cpu1/online
>
> [   25.157867] IRQ18 no longer affine to CPU1
> ...
> [   25.158445] CPU1: shutdown
>
> root at 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] [<c0014a00>] (unwind_backtrace) from [<c0011980>] (show_stack+0x10/0x14)
> [   40.786747] [<c0011980>] (show_stack) from [<c0449ba0>] (dump_stack+0x70/0xbc)
> [   40.786767] [<c0449ba0>] (dump_stack) from [<c00c6124>] (kmem_cache_alloc+0xd8/0x170)
> [   40.786785] [<c00c6124>] (kmem_cache_alloc) from [<c005d6f8>] (request_threaded_irq+0x64/0x128)
> [   40.786804] [<c005d6f8>] (request_threaded_irq) from [<c0350b8c>] (exynos4_local_timer_setup+0xc0/0x13c)
> [   40.786820] [<c0350b8c>] (exynos4_local_timer_setup) from [<c0350ca8>] (exynos4_mct_cpu_notify+0x30/0xa8)
> [   40.786838] [<c0350ca8>] (exynos4_mct_cpu_notify) from [<c003b330>] (notifier_call_chain+0x44/0x84)
> [   40.786857] [<c003b330>] (notifier_call_chain) from [<c0022fd4>] (__cpu_notify+0x28/0x44)
> [   40.786873] [<c0022fd4>] (__cpu_notify) from [<c0013714>] (secondary_start_kernel+0xec/0x150)
> [   40.786886] [<c0013714>] (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 <d.eppel@samsung.com>
> Cc: <stable@vger.kernel.org>
> Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> (Tested on Arndale Octa Board)
> Tested-by: Marcin Jabrzyk <m.jabrzyk@samsung.com>
> (Tested on Rinato B2 (Exynos 3250) board)

Hi Daniel and Thomas,

Do you have any comments on this patch? Could you pick it up?

Best regards,
Krzysztof

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

* Re: [PATCH v2] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
  2015-05-11  1:33   ` Krzysztof Kozlowski
@ 2015-05-11  7:30     ` Daniel Lezcano
  -1 siblings, 0 replies; 29+ messages in thread
From: Daniel Lezcano @ 2015-05-11  7:30 UTC (permalink / raw)
  To: Krzysztof Kozlowski, tglx
  Cc: linux-samsung-soc, linux-kernel, stable, kyungmin.park, kgene,
	Damian Eppel, m.jabrzyk, linux-arm-kernel

On 05/11/2015 03:33 AM, Krzysztof Kozlowski wrote:
> 2015-03-12 18:11 GMT+09:00 Damian Eppel <d.eppel@samsung.com>:
>> 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] [<c0014a00>] (unwind_backtrace) from [<c0011980>] (show_stack+0x10/0x14)
>> [   40.786747] [<c0011980>] (show_stack) from [<c0449ba0>] (dump_stack+0x70/0xbc)
>> [   40.786767] [<c0449ba0>] (dump_stack) from [<c00c6124>] (kmem_cache_alloc+0xd8/0x170)
>> [   40.786785] [<c00c6124>] (kmem_cache_alloc) from [<c005d6f8>] (request_threaded_irq+0x64/0x128)
>> [   40.786804] [<c005d6f8>] (request_threaded_irq) from [<c0350b8c>] (exynos4_local_timer_setup+0xc0/0x13c)
>> [   40.786820] [<c0350b8c>] (exynos4_local_timer_setup) from [<c0350ca8>] (exynos4_mct_cpu_notify+0x30/0xa8)
>> [   40.786838] [<c0350ca8>] (exynos4_mct_cpu_notify) from [<c003b330>] (notifier_call_chain+0x44/0x84)
>> [   40.786857] [<c003b330>] (notifier_call_chain) from [<c0022fd4>] (__cpu_notify+0x28/0x44)
>> [   40.786873] [<c0022fd4>] (__cpu_notify) from [<c0013714>] (secondary_start_kernel+0xec/0x150)
>> [   40.786886] [<c0013714>] (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 <d.eppel@samsung.com>
>> Cc: <stable@vger.kernel.org>
>> Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> (Tested on Arndale Octa Board)
>> Tested-by: Marcin Jabrzyk <m.jabrzyk@samsung.com>
>> (Tested on Rinato B2 (Exynos 3250) board)
>
> Hi Daniel and Thomas,
>
> Do you have any comments on this patch? Could you pick it up?

Not yet.


-- 
  <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] 29+ messages in thread

* [PATCH v2] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
@ 2015-05-11  7:30     ` Daniel Lezcano
  0 siblings, 0 replies; 29+ messages in thread
From: Daniel Lezcano @ 2015-05-11  7:30 UTC (permalink / raw)
  To: linux-arm-kernel

On 05/11/2015 03:33 AM, Krzysztof Kozlowski wrote:
> 2015-03-12 18:11 GMT+09:00 Damian Eppel <d.eppel@samsung.com>:
>> 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 at target:~# echo 0 > /sys/devices/system/cpu/cpu1/online
>>
>> [   25.157867] IRQ18 no longer affine to CPU1
>> ...
>> [   25.158445] CPU1: shutdown
>>
>> root at 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] [<c0014a00>] (unwind_backtrace) from [<c0011980>] (show_stack+0x10/0x14)
>> [   40.786747] [<c0011980>] (show_stack) from [<c0449ba0>] (dump_stack+0x70/0xbc)
>> [   40.786767] [<c0449ba0>] (dump_stack) from [<c00c6124>] (kmem_cache_alloc+0xd8/0x170)
>> [   40.786785] [<c00c6124>] (kmem_cache_alloc) from [<c005d6f8>] (request_threaded_irq+0x64/0x128)
>> [   40.786804] [<c005d6f8>] (request_threaded_irq) from [<c0350b8c>] (exynos4_local_timer_setup+0xc0/0x13c)
>> [   40.786820] [<c0350b8c>] (exynos4_local_timer_setup) from [<c0350ca8>] (exynos4_mct_cpu_notify+0x30/0xa8)
>> [   40.786838] [<c0350ca8>] (exynos4_mct_cpu_notify) from [<c003b330>] (notifier_call_chain+0x44/0x84)
>> [   40.786857] [<c003b330>] (notifier_call_chain) from [<c0022fd4>] (__cpu_notify+0x28/0x44)
>> [   40.786873] [<c0022fd4>] (__cpu_notify) from [<c0013714>] (secondary_start_kernel+0xec/0x150)
>> [   40.786886] [<c0013714>] (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 <d.eppel@samsung.com>
>> Cc: <stable@vger.kernel.org>
>> Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> (Tested on Arndale Octa Board)
>> Tested-by: Marcin Jabrzyk <m.jabrzyk@samsung.com>
>> (Tested on Rinato B2 (Exynos 3250) board)
>
> Hi Daniel and Thomas,
>
> Do you have any comments on this patch? Could you pick it up?

Not yet.


-- 
  <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] 29+ messages in thread

* Re: [PATCH v2] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
  2015-03-12  9:11 ` Damian Eppel
@ 2015-05-11 11:18   ` Daniel Lezcano
  -1 siblings, 0 replies; 29+ messages in thread
From: Daniel Lezcano @ 2015-05-11 11:18 UTC (permalink / raw)
  To: Damian Eppel
  Cc: tglx, kgene, linux-kernel, linux-arm-kernel, linux-samsung-soc,
	kyungmin.park, k.kozlowski, m.jabrzyk, stable

On 03/12/2015 10:11 AM, Damian Eppel wrote:
> 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] [<c0014a00>] (unwind_backtrace) from [<c0011980>] (show_stack+0x10/0x14)
> [   40.786747] [<c0011980>] (show_stack) from [<c0449ba0>] (dump_stack+0x70/0xbc)
> [   40.786767] [<c0449ba0>] (dump_stack) from [<c00c6124>] (kmem_cache_alloc+0xd8/0x170)
> [   40.786785] [<c00c6124>] (kmem_cache_alloc) from [<c005d6f8>] (request_threaded_irq+0x64/0x128)
> [   40.786804] [<c005d6f8>] (request_threaded_irq) from [<c0350b8c>] (exynos4_local_timer_setup+0xc0/0x13c)
> [   40.786820] [<c0350b8c>] (exynos4_local_timer_setup) from [<c0350ca8>] (exynos4_mct_cpu_notify+0x30/0xa8)
> [   40.786838] [<c0350ca8>] (exynos4_mct_cpu_notify) from [<c003b330>] (notifier_call_chain+0x44/0x84)
> [   40.786857] [<c003b330>] (notifier_call_chain) from [<c0022fd4>] (__cpu_notify+0x28/0x44)
> [   40.786873] [<c0022fd4>] (__cpu_notify) from [<c0013714>] (secondary_start_kernel+0xec/0x150)
> [   40.786886] [<c0013714>] (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.

The code sounds very complex for what it is supposed to do.

Perhaps I am missing something but you have more or less the same 
functionality than the smp_twd timers and these ones don't look so complex.

Could you please look at the smp_twd.c implementation ?


> Fixes: 7114cd749a12 ("clocksource: exynos_mct: use (request/free)_irq calls for local timer registration")
> Signed-off-by: Damian Eppel <d.eppel@samsung.com>
> Cc: <stable@vger.kernel.org>
> Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> (Tested on Arndale Octa Board)
> Tested-by: Marcin Jabrzyk <m.jabrzyk@samsung.com>
> (Tested on Rinato B2 (Exynos 3250) board)
> ---
>
> Notes:
>      Changes since v1:
>              o added Krzysztof's and Marcin's Reviewed-by / Tested-by
>                with information about the test HW.
>
>   drivers/clocksource/exynos_mct.c | 45 ++++++++++++++++++++++++++++------------
>   1 file changed, 32 insertions(+), 13 deletions(-)
>
> 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);
>


-- 
  <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] 29+ messages in thread

* [PATCH v2] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
@ 2015-05-11 11:18   ` Daniel Lezcano
  0 siblings, 0 replies; 29+ messages in thread
From: Daniel Lezcano @ 2015-05-11 11:18 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/12/2015 10:11 AM, Damian Eppel wrote:
> 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 at target:~# echo 0 > /sys/devices/system/cpu/cpu1/online
>
> [   25.157867] IRQ18 no longer affine to CPU1
> ...
> [   25.158445] CPU1: shutdown
>
> root at 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] [<c0014a00>] (unwind_backtrace) from [<c0011980>] (show_stack+0x10/0x14)
> [   40.786747] [<c0011980>] (show_stack) from [<c0449ba0>] (dump_stack+0x70/0xbc)
> [   40.786767] [<c0449ba0>] (dump_stack) from [<c00c6124>] (kmem_cache_alloc+0xd8/0x170)
> [   40.786785] [<c00c6124>] (kmem_cache_alloc) from [<c005d6f8>] (request_threaded_irq+0x64/0x128)
> [   40.786804] [<c005d6f8>] (request_threaded_irq) from [<c0350b8c>] (exynos4_local_timer_setup+0xc0/0x13c)
> [   40.786820] [<c0350b8c>] (exynos4_local_timer_setup) from [<c0350ca8>] (exynos4_mct_cpu_notify+0x30/0xa8)
> [   40.786838] [<c0350ca8>] (exynos4_mct_cpu_notify) from [<c003b330>] (notifier_call_chain+0x44/0x84)
> [   40.786857] [<c003b330>] (notifier_call_chain) from [<c0022fd4>] (__cpu_notify+0x28/0x44)
> [   40.786873] [<c0022fd4>] (__cpu_notify) from [<c0013714>] (secondary_start_kernel+0xec/0x150)
> [   40.786886] [<c0013714>] (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.

The code sounds very complex for what it is supposed to do.

Perhaps I am missing something but you have more or less the same 
functionality than the smp_twd timers and these ones don't look so complex.

Could you please look at the smp_twd.c implementation ?


> Fixes: 7114cd749a12 ("clocksource: exynos_mct: use (request/free)_irq calls for local timer registration")
> Signed-off-by: Damian Eppel <d.eppel@samsung.com>
> Cc: <stable@vger.kernel.org>
> Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> (Tested on Arndale Octa Board)
> Tested-by: Marcin Jabrzyk <m.jabrzyk@samsung.com>
> (Tested on Rinato B2 (Exynos 3250) board)
> ---
>
> Notes:
>      Changes since v1:
>              o added Krzysztof's and Marcin's Reviewed-by / Tested-by
>                with information about the test HW.
>
>   drivers/clocksource/exynos_mct.c | 45 ++++++++++++++++++++++++++++------------
>   1 file changed, 32 insertions(+), 13 deletions(-)
>
> 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);
>


-- 
  <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] 29+ messages in thread

* Re: [PATCH v2] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
  2015-05-11 11:18   ` Daniel Lezcano
@ 2015-05-25 15:24     ` Damian Eppel
  -1 siblings, 0 replies; 29+ messages in thread
From: Damian Eppel @ 2015-05-25 15:24 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: tglx, kgene, linux-kernel, linux-arm-kernel, linux-samsung-soc,
	kyungmin.park, k.kozlowski, m.jabrzyk, d.eppel, stable

On Mon, 2015-05-11 at 13:18 +0200, Daniel Lezcano wrote:
> On 03/12/2015 10:11 AM, Damian Eppel wrote:
> > 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] [<c0014a00>] (unwind_backtrace) from [<c0011980>] (show_stack+0x10/0x14)
> > [   40.786747] [<c0011980>] (show_stack) from [<c0449ba0>] (dump_stack+0x70/0xbc)
> > [   40.786767] [<c0449ba0>] (dump_stack) from [<c00c6124>] (kmem_cache_alloc+0xd8/0x170)
> > [   40.786785] [<c00c6124>] (kmem_cache_alloc) from [<c005d6f8>] (request_threaded_irq+0x64/0x128)
> > [   40.786804] [<c005d6f8>] (request_threaded_irq) from [<c0350b8c>] (exynos4_local_timer_setup+0xc0/0x13c)
> > [   40.786820] [<c0350b8c>] (exynos4_local_timer_setup) from [<c0350ca8>] (exynos4_mct_cpu_notify+0x30/0xa8)
> > [   40.786838] [<c0350ca8>] (exynos4_mct_cpu_notify) from [<c003b330>] (notifier_call_chain+0x44/0x84)
> > [   40.786857] [<c003b330>] (notifier_call_chain) from [<c0022fd4>] (__cpu_notify+0x28/0x44)
> > [   40.786873] [<c0022fd4>] (__cpu_notify) from [<c0013714>] (secondary_start_kernel+0xec/0x150)
> > [   40.786886] [<c0013714>] (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.
> 
> The code sounds very complex for what it is supposed to do.
> 
> Perhaps I am missing something but you have more or less the same 
> functionality than the smp_twd timers and these ones don't look so complex.
> 
> Could you please look at the smp_twd.c implementation ?

Hi Daniel,

exynos_mct.c driver looks more complex as it supports two types of timer
interrupts - private and shared peripheral interrupts (for exynos4412
and exynos4210 accordingly). In smp_twd.c driver I can see only PPI type
of irqs supported. SPI and PPI irqs differs slightly in setup - thus two
different code paths appears in the driver in initialization and
handling of CPU notifications. The fix is addressing issue that appears
only for hardware using SPI irqs so it is hard to compare it to
smp_twd.c.
BTW, If we remove support for SPI irqs in exynos_mct.c it would look
almost the same as smp_twd.c. 

Best Regards,
Damian
> 
> 
> > Fixes: 7114cd749a12 ("clocksource: exynos_mct: use (request/free)_irq calls for local timer registration")
> > Signed-off-by: Damian Eppel <d.eppel@samsung.com>
> > Cc: <stable@vger.kernel.org>
> > Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > (Tested on Arndale Octa Board)
> > Tested-by: Marcin Jabrzyk <m.jabrzyk@samsung.com>
> > (Tested on Rinato B2 (Exynos 3250) board)
> > ---
> >
> > Notes:
> >      Changes since v1:
> >              o added Krzysztof's and Marcin's Reviewed-by / Tested-by
> >                with information about the test HW.
> >
> >   drivers/clocksource/exynos_mct.c | 45 ++++++++++++++++++++++++++++------------
> >   1 file changed, 32 insertions(+), 13 deletions(-)
> >
> > 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);
> >
> 
> 



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

* [PATCH v2] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
@ 2015-05-25 15:24     ` Damian Eppel
  0 siblings, 0 replies; 29+ messages in thread
From: Damian Eppel @ 2015-05-25 15:24 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 2015-05-11 at 13:18 +0200, Daniel Lezcano wrote:
> On 03/12/2015 10:11 AM, Damian Eppel wrote:
> > 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 at target:~# echo 0 > /sys/devices/system/cpu/cpu1/online
> >
> > [   25.157867] IRQ18 no longer affine to CPU1
> > ...
> > [   25.158445] CPU1: shutdown
> >
> > root at 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] [<c0014a00>] (unwind_backtrace) from [<c0011980>] (show_stack+0x10/0x14)
> > [   40.786747] [<c0011980>] (show_stack) from [<c0449ba0>] (dump_stack+0x70/0xbc)
> > [   40.786767] [<c0449ba0>] (dump_stack) from [<c00c6124>] (kmem_cache_alloc+0xd8/0x170)
> > [   40.786785] [<c00c6124>] (kmem_cache_alloc) from [<c005d6f8>] (request_threaded_irq+0x64/0x128)
> > [   40.786804] [<c005d6f8>] (request_threaded_irq) from [<c0350b8c>] (exynos4_local_timer_setup+0xc0/0x13c)
> > [   40.786820] [<c0350b8c>] (exynos4_local_timer_setup) from [<c0350ca8>] (exynos4_mct_cpu_notify+0x30/0xa8)
> > [   40.786838] [<c0350ca8>] (exynos4_mct_cpu_notify) from [<c003b330>] (notifier_call_chain+0x44/0x84)
> > [   40.786857] [<c003b330>] (notifier_call_chain) from [<c0022fd4>] (__cpu_notify+0x28/0x44)
> > [   40.786873] [<c0022fd4>] (__cpu_notify) from [<c0013714>] (secondary_start_kernel+0xec/0x150)
> > [   40.786886] [<c0013714>] (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.
> 
> The code sounds very complex for what it is supposed to do.
> 
> Perhaps I am missing something but you have more or less the same 
> functionality than the smp_twd timers and these ones don't look so complex.
> 
> Could you please look at the smp_twd.c implementation ?

Hi Daniel,

exynos_mct.c driver looks more complex as it supports two types of timer
interrupts - private and shared peripheral interrupts (for exynos4412
and exynos4210 accordingly). In smp_twd.c driver I can see only PPI type
of irqs supported. SPI and PPI irqs differs slightly in setup - thus two
different code paths appears in the driver in initialization and
handling of CPU notifications. The fix is addressing issue that appears
only for hardware using SPI irqs so it is hard to compare it to
smp_twd.c.
BTW, If we remove support for SPI irqs in exynos_mct.c it would look
almost the same as smp_twd.c. 

Best Regards,
Damian
> 
> 
> > Fixes: 7114cd749a12 ("clocksource: exynos_mct: use (request/free)_irq calls for local timer registration")
> > Signed-off-by: Damian Eppel <d.eppel@samsung.com>
> > Cc: <stable@vger.kernel.org>
> > Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > (Tested on Arndale Octa Board)
> > Tested-by: Marcin Jabrzyk <m.jabrzyk@samsung.com>
> > (Tested on Rinato B2 (Exynos 3250) board)
> > ---
> >
> > Notes:
> >      Changes since v1:
> >              o added Krzysztof's and Marcin's Reviewed-by / Tested-by
> >                with information about the test HW.
> >
> >   drivers/clocksource/exynos_mct.c | 45 ++++++++++++++++++++++++++++------------
> >   1 file changed, 32 insertions(+), 13 deletions(-)
> >
> > 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);
> >
> 
> 

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

* Re: [PATCH v2] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
  2015-05-25 15:24     ` Damian Eppel
@ 2015-05-27 12:43       ` Daniel Lezcano
  -1 siblings, 0 replies; 29+ messages in thread
From: Daniel Lezcano @ 2015-05-27 12:43 UTC (permalink / raw)
  To: d.eppel
  Cc: tglx, kgene, linux-kernel, linux-arm-kernel, linux-samsung-soc,
	kyungmin.park, k.kozlowski, m.jabrzyk, stable

On 05/25/2015 05:24 PM, Damian Eppel wrote:
> On Mon, 2015-05-11 at 13:18 +0200, Daniel Lezcano wrote:

[ ... ]

>> The code sounds very complex for what it is supposed to do.
>>
>> Perhaps I am missing something but you have more or less the same
>> functionality than the smp_twd timers and these ones don't look so complex.
>>
>> Could you please look at the smp_twd.c implementation ?
>
> Hi Daniel,
>
> exynos_mct.c driver looks more complex as it supports two types of timer
> interrupts - private and shared peripheral interrupts (for exynos4412
> and exynos4210 accordingly). In smp_twd.c driver I can see only PPI type
> of irqs supported. SPI and PPI irqs differs slightly in setup - thus two
> different code paths appears in the driver in initialization and
> handling of CPU notifications. The fix is addressing issue that appears
> only for hardware using SPI irqs so it is hard to compare it to
> smp_twd.c.
> BTW, If we remove support for SPI irqs in exynos_mct.c it would look
> almost the same as smp_twd.c.

Ok, thanks.

I will have a deeper look this afternoon.

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


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

* [PATCH v2] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
@ 2015-05-27 12:43       ` Daniel Lezcano
  0 siblings, 0 replies; 29+ messages in thread
From: Daniel Lezcano @ 2015-05-27 12:43 UTC (permalink / raw)
  To: linux-arm-kernel

On 05/25/2015 05:24 PM, Damian Eppel wrote:
> On Mon, 2015-05-11 at 13:18 +0200, Daniel Lezcano wrote:

[ ... ]

>> The code sounds very complex for what it is supposed to do.
>>
>> Perhaps I am missing something but you have more or less the same
>> functionality than the smp_twd timers and these ones don't look so complex.
>>
>> Could you please look at the smp_twd.c implementation ?
>
> Hi Daniel,
>
> exynos_mct.c driver looks more complex as it supports two types of timer
> interrupts - private and shared peripheral interrupts (for exynos4412
> and exynos4210 accordingly). In smp_twd.c driver I can see only PPI type
> of irqs supported. SPI and PPI irqs differs slightly in setup - thus two
> different code paths appears in the driver in initialization and
> handling of CPU notifications. The fix is addressing issue that appears
> only for hardware using SPI irqs so it is hard to compare it to
> smp_twd.c.
> BTW, If we remove support for SPI irqs in exynos_mct.c it would look
> almost the same as smp_twd.c.

Ok, thanks.

I will have a deeper look this afternoon.

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

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

* Re: [PATCH v2] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
  2015-03-12  9:11 ` Damian Eppel
@ 2015-05-28  9:47   ` Marek Szyprowski
  -1 siblings, 0 replies; 29+ messages in thread
From: Marek Szyprowski @ 2015-05-28  9:47 UTC (permalink / raw)
  To: Damian Eppel, daniel.lezcano
  Cc: tglx, kgene, linux-kernel, linux-arm-kernel, linux-samsung-soc,
	kyungmin.park, k.kozlowski, m.jabrzyk, stable

Hello,

On 2015-03-12 10:11, Damian Eppel wrote:
> 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] [<c0014a00>] (unwind_backtrace) from [<c0011980>] (show_stack+0x10/0x14)
> [   40.786747] [<c0011980>] (show_stack) from [<c0449ba0>] (dump_stack+0x70/0xbc)
> [   40.786767] [<c0449ba0>] (dump_stack) from [<c00c6124>] (kmem_cache_alloc+0xd8/0x170)
> [   40.786785] [<c00c6124>] (kmem_cache_alloc) from [<c005d6f8>] (request_threaded_irq+0x64/0x128)
> [   40.786804] [<c005d6f8>] (request_threaded_irq) from [<c0350b8c>] (exynos4_local_timer_setup+0xc0/0x13c)
> [   40.786820] [<c0350b8c>] (exynos4_local_timer_setup) from [<c0350ca8>] (exynos4_mct_cpu_notify+0x30/0xa8)
> [   40.786838] [<c0350ca8>] (exynos4_mct_cpu_notify) from [<c003b330>] (notifier_call_chain+0x44/0x84)
> [   40.786857] [<c003b330>] (notifier_call_chain) from [<c0022fd4>] (__cpu_notify+0x28/0x44)
> [   40.786873] [<c0022fd4>] (__cpu_notify) from [<c0013714>] (secondary_start_kernel+0xec/0x150)
> [   40.786886] [<c0013714>] (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 <d.eppel@samsung.com>
> Cc: <stable@vger.kernel.org>
> Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> (Tested on Arndale Octa Board)
> Tested-by: Marcin Jabrzyk <m.jabrzyk@samsung.com>
> (Tested on Rinato B2 (Exynos 3250) board)
> ---
>
> Notes:
>      Changes since v1:
>              o added Krzysztof's and Marcin's Reviewed-by / Tested-by
>                with information about the test HW.
>
>   drivers/clocksource/exynos_mct.c | 45 ++++++++++++++++++++++++++++------------
>   1 file changed, 32 insertions(+), 13 deletions(-)
>
> 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);

Maybe it will be better to simply request this irq in disabled state? 
Just call
irq_set_status_flags(mct_irq, IRQ_NOAUTOEN) before request_irq().


> +		}
>   	}
>   
>   	err = register_cpu_notifier(&exynos4_mct_cpu_nb);

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


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

* [PATCH v2] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
@ 2015-05-28  9:47   ` Marek Szyprowski
  0 siblings, 0 replies; 29+ messages in thread
From: Marek Szyprowski @ 2015-05-28  9:47 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On 2015-03-12 10:11, Damian Eppel wrote:
> 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 at target:~# echo 0 > /sys/devices/system/cpu/cpu1/online
>
> [   25.157867] IRQ18 no longer affine to CPU1
> ...
> [   25.158445] CPU1: shutdown
>
> root at 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] [<c0014a00>] (unwind_backtrace) from [<c0011980>] (show_stack+0x10/0x14)
> [   40.786747] [<c0011980>] (show_stack) from [<c0449ba0>] (dump_stack+0x70/0xbc)
> [   40.786767] [<c0449ba0>] (dump_stack) from [<c00c6124>] (kmem_cache_alloc+0xd8/0x170)
> [   40.786785] [<c00c6124>] (kmem_cache_alloc) from [<c005d6f8>] (request_threaded_irq+0x64/0x128)
> [   40.786804] [<c005d6f8>] (request_threaded_irq) from [<c0350b8c>] (exynos4_local_timer_setup+0xc0/0x13c)
> [   40.786820] [<c0350b8c>] (exynos4_local_timer_setup) from [<c0350ca8>] (exynos4_mct_cpu_notify+0x30/0xa8)
> [   40.786838] [<c0350ca8>] (exynos4_mct_cpu_notify) from [<c003b330>] (notifier_call_chain+0x44/0x84)
> [   40.786857] [<c003b330>] (notifier_call_chain) from [<c0022fd4>] (__cpu_notify+0x28/0x44)
> [   40.786873] [<c0022fd4>] (__cpu_notify) from [<c0013714>] (secondary_start_kernel+0xec/0x150)
> [   40.786886] [<c0013714>] (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 <d.eppel@samsung.com>
> Cc: <stable@vger.kernel.org>
> Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> (Tested on Arndale Octa Board)
> Tested-by: Marcin Jabrzyk <m.jabrzyk@samsung.com>
> (Tested on Rinato B2 (Exynos 3250) board)
> ---
>
> Notes:
>      Changes since v1:
>              o added Krzysztof's and Marcin's Reviewed-by / Tested-by
>                with information about the test HW.
>
>   drivers/clocksource/exynos_mct.c | 45 ++++++++++++++++++++++++++++------------
>   1 file changed, 32 insertions(+), 13 deletions(-)
>
> 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);

Maybe it will be better to simply request this irq in disabled state? 
Just call
irq_set_status_flags(mct_irq, IRQ_NOAUTOEN) before request_irq().


> +		}
>   	}
>   
>   	err = register_cpu_notifier(&exynos4_mct_cpu_nb);

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

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

* Re: [PATCH v2] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
  2015-05-28  9:47   ` Marek Szyprowski
@ 2015-05-28 15:37     ` Damian Eppel
  -1 siblings, 0 replies; 29+ messages in thread
From: Damian Eppel @ 2015-05-28 15:37 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: daniel.lezcano, tglx, kgene, linux-kernel, linux-arm-kernel,
	linux-samsung-soc, kyungmin.park, k.kozlowski, m.jabrzyk,
	d.eppel, stable

On Thu, 2015-05-28 at 11:47 +0200, Marek Szyprowski wrote:
> Hello,
> 
> On 2015-03-12 10:11, Damian Eppel wrote:
> > 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] [<c0014a00>] (unwind_backtrace) from [<c0011980>] (show_stack+0x10/0x14)
> > [   40.786747] [<c0011980>] (show_stack) from [<c0449ba0>] (dump_stack+0x70/0xbc)
> > [   40.786767] [<c0449ba0>] (dump_stack) from [<c00c6124>] (kmem_cache_alloc+0xd8/0x170)
> > [   40.786785] [<c00c6124>] (kmem_cache_alloc) from [<c005d6f8>] (request_threaded_irq+0x64/0x128)
> > [   40.786804] [<c005d6f8>] (request_threaded_irq) from [<c0350b8c>] (exynos4_local_timer_setup+0xc0/0x13c)
> > [   40.786820] [<c0350b8c>] (exynos4_local_timer_setup) from [<c0350ca8>] (exynos4_mct_cpu_notify+0x30/0xa8)
> > [   40.786838] [<c0350ca8>] (exynos4_mct_cpu_notify) from [<c003b330>] (notifier_call_chain+0x44/0x84)
> > [   40.786857] [<c003b330>] (notifier_call_chain) from [<c0022fd4>] (__cpu_notify+0x28/0x44)
> > [   40.786873] [<c0022fd4>] (__cpu_notify) from [<c0013714>] (secondary_start_kernel+0xec/0x150)
> > [   40.786886] [<c0013714>] (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 <d.eppel@samsung.com>
> > Cc: <stable@vger.kernel.org>
> > Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > (Tested on Arndale Octa Board)
> > Tested-by: Marcin Jabrzyk <m.jabrzyk@samsung.com>
> > (Tested on Rinato B2 (Exynos 3250) board)
> > ---
> >
> > Notes:
> >      Changes since v1:
> >              o added Krzysztof's and Marcin's Reviewed-by / Tested-by
> >                with information about the test HW.
> >
> >   drivers/clocksource/exynos_mct.c | 45 ++++++++++++++++++++++++++++------------
> >   1 file changed, 32 insertions(+), 13 deletions(-)
> >
> > 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);
> 
> Maybe it will be better to simply request this irq in disabled state? 
> Just call
> irq_set_status_flags(mct_irq, IRQ_NOAUTOEN) before request_irq().
> 

Good point, thanks. I will correct that in patch V3.

Best Regards,
Damian

> 
> > +		}
> >   	}
> >   
> >   	err = register_cpu_notifier(&exynos4_mct_cpu_nb);
> 
> Best regards



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

* [PATCH v2] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
@ 2015-05-28 15:37     ` Damian Eppel
  0 siblings, 0 replies; 29+ messages in thread
From: Damian Eppel @ 2015-05-28 15:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 2015-05-28 at 11:47 +0200, Marek Szyprowski wrote:
> Hello,
> 
> On 2015-03-12 10:11, Damian Eppel wrote:
> > 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 at target:~# echo 0 > /sys/devices/system/cpu/cpu1/online
> >
> > [   25.157867] IRQ18 no longer affine to CPU1
> > ...
> > [   25.158445] CPU1: shutdown
> >
> > root at 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] [<c0014a00>] (unwind_backtrace) from [<c0011980>] (show_stack+0x10/0x14)
> > [   40.786747] [<c0011980>] (show_stack) from [<c0449ba0>] (dump_stack+0x70/0xbc)
> > [   40.786767] [<c0449ba0>] (dump_stack) from [<c00c6124>] (kmem_cache_alloc+0xd8/0x170)
> > [   40.786785] [<c00c6124>] (kmem_cache_alloc) from [<c005d6f8>] (request_threaded_irq+0x64/0x128)
> > [   40.786804] [<c005d6f8>] (request_threaded_irq) from [<c0350b8c>] (exynos4_local_timer_setup+0xc0/0x13c)
> > [   40.786820] [<c0350b8c>] (exynos4_local_timer_setup) from [<c0350ca8>] (exynos4_mct_cpu_notify+0x30/0xa8)
> > [   40.786838] [<c0350ca8>] (exynos4_mct_cpu_notify) from [<c003b330>] (notifier_call_chain+0x44/0x84)
> > [   40.786857] [<c003b330>] (notifier_call_chain) from [<c0022fd4>] (__cpu_notify+0x28/0x44)
> > [   40.786873] [<c0022fd4>] (__cpu_notify) from [<c0013714>] (secondary_start_kernel+0xec/0x150)
> > [   40.786886] [<c0013714>] (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 <d.eppel@samsung.com>
> > Cc: <stable@vger.kernel.org>
> > Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > (Tested on Arndale Octa Board)
> > Tested-by: Marcin Jabrzyk <m.jabrzyk@samsung.com>
> > (Tested on Rinato B2 (Exynos 3250) board)
> > ---
> >
> > Notes:
> >      Changes since v1:
> >              o added Krzysztof's and Marcin's Reviewed-by / Tested-by
> >                with information about the test HW.
> >
> >   drivers/clocksource/exynos_mct.c | 45 ++++++++++++++++++++++++++++------------
> >   1 file changed, 32 insertions(+), 13 deletions(-)
> >
> > 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);
> 
> Maybe it will be better to simply request this irq in disabled state? 
> Just call
> irq_set_status_flags(mct_irq, IRQ_NOAUTOEN) before request_irq().
> 

Good point, thanks. I will correct that in patch V3.

Best Regards,
Damian

> 
> > +		}
> >   	}
> >   
> >   	err = register_cpu_notifier(&exynos4_mct_cpu_nb);
> 
> Best regards

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

* [PATCH v3] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
  2015-03-12  9:11 ` Damian Eppel
@ 2015-06-02 11:11   ` Damian Eppel
  -1 siblings, 0 replies; 29+ messages in thread
From: Damian Eppel @ 2015-06-02 11:11 UTC (permalink / raw)
  To: daniel.lezcano, tglx, kgene, k.kozlowski, linux-kernel,
	linux-arm-kernel, linux-samsung-soc
  Cc: m.szyprowski, kyungmin.park, m.jabrzyk, d.eppel, stable

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] [<c0014a00>] (unwind_backtrace) from [<c0011980>] (show_stack+0x10/0x14)
[   40.786747] [<c0011980>] (show_stack) from [<c0449ba0>] (dump_stack+0x70/0xbc)
[   40.786767] [<c0449ba0>] (dump_stack) from [<c00c6124>] (kmem_cache_alloc+0xd8/0x170)
[   40.786785] [<c00c6124>] (kmem_cache_alloc) from [<c005d6f8>] (request_threaded_irq+0x64/0x128)
[   40.786804] [<c005d6f8>] (request_threaded_irq) from [<c0350b8c>] (exynos4_local_timer_setup+0xc0/0x13c)
[   40.786820] [<c0350b8c>] (exynos4_local_timer_setup) from [<c0350ca8>] (exynos4_mct_cpu_notify+0x30/0xa8)
[   40.786838] [<c0350ca8>] (exynos4_mct_cpu_notify) from [<c003b330>] (notifier_call_chain+0x44/0x84)
[   40.786857] [<c003b330>] (notifier_call_chain) from [<c0022fd4>] (__cpu_notify+0x28/0x44)
[   40.786873] [<c0022fd4>] (__cpu_notify) from [<c0013714>] (secondary_start_kernel+0xec/0x150)
[   40.786886] [<c0013714>] (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 <d.eppel@samsung.com>
Cc: <stable@vger.kernel.org>
Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
(Tested on Arndale Octa Board)
Tested-by: Marcin Jabrzyk <m.jabrzyk@samsung.com>
(Tested on Rinato B2 (Exynos 3250) board)
---

Notes:
    Changes since v1:
    	o added Krzysztof's and Marcin's Reviewed-by / Tested-by
    	  with information about the test HW.
    
    Changes since v2:
    	o requesting mct_irq already in disabled state instead of
    	  calling disable_irq() right after request_irq().

 drivers/clocksource/exynos_mct.c | 43 ++++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 83564c9..c844616 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,25 @@ 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;
+
+			irq_set_status_flags(mct_irq, IRQ_NOAUTOEN);
+			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;
+		}
 	}
 
 	err = register_cpu_notifier(&exynos4_mct_cpu_nb);
-- 
1.9.1


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

* [PATCH v3] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
@ 2015-06-02 11:11   ` Damian Eppel
  0 siblings, 0 replies; 29+ messages in thread
From: Damian Eppel @ 2015-06-02 11:11 UTC (permalink / raw)
  To: linux-arm-kernel

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 at target:~# echo 0 > /sys/devices/system/cpu/cpu1/online

[   25.157867] IRQ18 no longer affine to CPU1
...
[   25.158445] CPU1: shutdown

root at 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] [<c0014a00>] (unwind_backtrace) from [<c0011980>] (show_stack+0x10/0x14)
[   40.786747] [<c0011980>] (show_stack) from [<c0449ba0>] (dump_stack+0x70/0xbc)
[   40.786767] [<c0449ba0>] (dump_stack) from [<c00c6124>] (kmem_cache_alloc+0xd8/0x170)
[   40.786785] [<c00c6124>] (kmem_cache_alloc) from [<c005d6f8>] (request_threaded_irq+0x64/0x128)
[   40.786804] [<c005d6f8>] (request_threaded_irq) from [<c0350b8c>] (exynos4_local_timer_setup+0xc0/0x13c)
[   40.786820] [<c0350b8c>] (exynos4_local_timer_setup) from [<c0350ca8>] (exynos4_mct_cpu_notify+0x30/0xa8)
[   40.786838] [<c0350ca8>] (exynos4_mct_cpu_notify) from [<c003b330>] (notifier_call_chain+0x44/0x84)
[   40.786857] [<c003b330>] (notifier_call_chain) from [<c0022fd4>] (__cpu_notify+0x28/0x44)
[   40.786873] [<c0022fd4>] (__cpu_notify) from [<c0013714>] (secondary_start_kernel+0xec/0x150)
[   40.786886] [<c0013714>] (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 <d.eppel@samsung.com>
Cc: <stable@vger.kernel.org>
Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
(Tested on Arndale Octa Board)
Tested-by: Marcin Jabrzyk <m.jabrzyk@samsung.com>
(Tested on Rinato B2 (Exynos 3250) board)
---

Notes:
    Changes since v1:
    	o added Krzysztof's and Marcin's Reviewed-by / Tested-by
    	  with information about the test HW.
    
    Changes since v2:
    	o requesting mct_irq already in disabled state instead of
    	  calling disable_irq() right after request_irq().

 drivers/clocksource/exynos_mct.c | 43 ++++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 83564c9..c844616 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,25 @@ 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;
+
+			irq_set_status_flags(mct_irq, IRQ_NOAUTOEN);
+			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;
+		}
 	}
 
 	err = register_cpu_notifier(&exynos4_mct_cpu_nb);
-- 
1.9.1

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

* [RESEND PATCH v3] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
  2015-03-12  9:11 ` Damian Eppel
@ 2015-06-26 13:23   ` Damian Eppel
  -1 siblings, 0 replies; 29+ messages in thread
From: Damian Eppel @ 2015-06-26 13:23 UTC (permalink / raw)
  To: daniel.lezcano, tglx, kgene, k.kozlowski, linux-kernel,
	linux-arm-kernel, linux-samsung-soc
  Cc: m.szyprowski, kyungmin.park, m.jabrzyk, d.eppel, stable

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] [<c0014a00>] (unwind_backtrace) from [<c0011980>] (show_stack+0x10/0x14)
[   40.786747] [<c0011980>] (show_stack) from [<c0449ba0>] (dump_stack+0x70/0xbc)
[   40.786767] [<c0449ba0>] (dump_stack) from [<c00c6124>] (kmem_cache_alloc+0xd8/0x170)
[   40.786785] [<c00c6124>] (kmem_cache_alloc) from [<c005d6f8>] (request_threaded_irq+0x64/0x128)
[   40.786804] [<c005d6f8>] (request_threaded_irq) from [<c0350b8c>] (exynos4_local_timer_setup+0xc0/0x13c)
[   40.786820] [<c0350b8c>] (exynos4_local_timer_setup) from [<c0350ca8>] (exynos4_mct_cpu_notify+0x30/0xa8)
[   40.786838] [<c0350ca8>] (exynos4_mct_cpu_notify) from [<c003b330>] (notifier_call_chain+0x44/0x84)
[   40.786857] [<c003b330>] (notifier_call_chain) from [<c0022fd4>] (__cpu_notify+0x28/0x44)
[   40.786873] [<c0022fd4>] (__cpu_notify) from [<c0013714>] (secondary_start_kernel+0xec/0x150)
[   40.786886] [<c0013714>] (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 <d.eppel@samsung.com>
Cc: <stable@vger.kernel.org>
Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
(Tested on Arndale Octa Board)
Tested-by: Marcin Jabrzyk <m.jabrzyk@samsung.com>
(Tested on Rinato B2 (Exynos 3250) board)
---

Notes:
    Changes since v1:
    	o added Krzysztof's and Marcin's Reviewed-by / Tested-by
    	  with information about the test HW.
    
    Changes since v2:
    	o requesting mct_irq already in disabled state instead of
    	  calling disable_irq() right after request_irq().

 drivers/clocksource/exynos_mct.c | 43 ++++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 935b059..9064ff7 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -462,15 +462,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);
 	}
@@ -483,10 +480,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,
@@ -518,7 +517,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;
 
@@ -545,7 +544,25 @@ 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;
+
+			irq_set_status_flags(mct_irq, IRQ_NOAUTOEN);
+			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;
+		}
 	}
 
 	err = register_cpu_notifier(&exynos4_mct_cpu_nb);
-- 
1.9.1


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

* [RESEND PATCH v3] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
@ 2015-06-26 13:23   ` Damian Eppel
  0 siblings, 0 replies; 29+ messages in thread
From: Damian Eppel @ 2015-06-26 13:23 UTC (permalink / raw)
  To: linux-arm-kernel

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 at target:~# echo 0 > /sys/devices/system/cpu/cpu1/online

[   25.157867] IRQ18 no longer affine to CPU1
...
[   25.158445] CPU1: shutdown

root at 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] [<c0014a00>] (unwind_backtrace) from [<c0011980>] (show_stack+0x10/0x14)
[   40.786747] [<c0011980>] (show_stack) from [<c0449ba0>] (dump_stack+0x70/0xbc)
[   40.786767] [<c0449ba0>] (dump_stack) from [<c00c6124>] (kmem_cache_alloc+0xd8/0x170)
[   40.786785] [<c00c6124>] (kmem_cache_alloc) from [<c005d6f8>] (request_threaded_irq+0x64/0x128)
[   40.786804] [<c005d6f8>] (request_threaded_irq) from [<c0350b8c>] (exynos4_local_timer_setup+0xc0/0x13c)
[   40.786820] [<c0350b8c>] (exynos4_local_timer_setup) from [<c0350ca8>] (exynos4_mct_cpu_notify+0x30/0xa8)
[   40.786838] [<c0350ca8>] (exynos4_mct_cpu_notify) from [<c003b330>] (notifier_call_chain+0x44/0x84)
[   40.786857] [<c003b330>] (notifier_call_chain) from [<c0022fd4>] (__cpu_notify+0x28/0x44)
[   40.786873] [<c0022fd4>] (__cpu_notify) from [<c0013714>] (secondary_start_kernel+0xec/0x150)
[   40.786886] [<c0013714>] (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 <d.eppel@samsung.com>
Cc: <stable@vger.kernel.org>
Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
(Tested on Arndale Octa Board)
Tested-by: Marcin Jabrzyk <m.jabrzyk@samsung.com>
(Tested on Rinato B2 (Exynos 3250) board)
---

Notes:
    Changes since v1:
    	o added Krzysztof's and Marcin's Reviewed-by / Tested-by
    	  with information about the test HW.
    
    Changes since v2:
    	o requesting mct_irq already in disabled state instead of
    	  calling disable_irq() right after request_irq().

 drivers/clocksource/exynos_mct.c | 43 ++++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 935b059..9064ff7 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -462,15 +462,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);
 	}
@@ -483,10 +480,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,
@@ -518,7 +517,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;
 
@@ -545,7 +544,25 @@ 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;
+
+			irq_set_status_flags(mct_irq, IRQ_NOAUTOEN);
+			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;
+		}
 	}
 
 	err = register_cpu_notifier(&exynos4_mct_cpu_nb);
-- 
1.9.1

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

* [tip:timers/urgent] clocksource: exynos_mct: Avoid blocking calls in the cpu hotplug notifier
  2015-06-26 13:23   ` Damian Eppel
  (?)
@ 2015-06-26 19:54   ` tip-bot for Damian Eppel
  -1 siblings, 0 replies; 29+ messages in thread
From: tip-bot for Damian Eppel @ 2015-06-26 19:54 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, k.kozlowski, hpa, m.jabrzyk, linux-kernel, d.eppel, mingo, stable

Commit-ID:  56a94f13919c0db5958611b388e1581b4852f3c9
Gitweb:     http://git.kernel.org/tip/56a94f13919c0db5958611b388e1581b4852f3c9
Author:     Damian Eppel <d.eppel@samsung.com>
AuthorDate: Fri, 26 Jun 2015 15:23:04 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 26 Jun 2015 21:53:01 +0200

clocksource: exynos_mct: Avoid blocking calls in the cpu hotplug notifier

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

[   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] [<c0014a00>] (unwind_backtrace) from [<c0011980>] (show_stack+0x10/0x14)
[   40.786747] [<c0011980>] (show_stack) from [<c0449ba0>] (dump_stack+0x70/0xbc)
[   40.786767] [<c0449ba0>] (dump_stack) from [<c00c6124>] (kmem_cache_alloc+0xd8/0x170)
[   40.786785] [<c00c6124>] (kmem_cache_alloc) from [<c005d6f8>] (request_threaded_irq+0x64/0x128)
[   40.786804] [<c005d6f8>] (request_threaded_irq) from [<c0350b8c>] (exynos4_local_timer_setup+0xc0/0x13c)
[   40.786820] [<c0350b8c>] (exynos4_local_timer_setup) from [<c0350ca8>] (exynos4_mct_cpu_notify+0x30/0xa8)
[   40.786838] [<c0350ca8>] (exynos4_mct_cpu_notify) from [<c003b330>] (notifier_call_chain+0x44/0x84)
[   40.786857] [<c003b330>] (notifier_call_chain) from [<c0022fd4>] (__cpu_notify+0x28/0x44)
[   40.786873] [<c0022fd4>] (__cpu_notify) from [<c0013714>] (secondary_start_kernel+0xec/0x150)
[   40.786886] [<c0013714>] (secondary_start_kernel) from [<40008764>] (0x40008764)

Interrupts cannot be requested/freed in the CPU_STARTING/CPU_DYING
notifications which run on the hotplugged cpu with interrupts and
preemption disabled.

To avoid the issue, request the interrupts for all possible cpus in
the boot code. The interrupts are marked NO_AUTOENABLE to avoid a racy
request_irq/disable_irq() sequence. The flag prevents the
request_irq() code from enabling the interrupt immediately.

The interrupt is then enabled in the CPU_STARTING notifier of the
hotplugged cpu and again disabled with disable_irq_nosync() in the
CPU_DYING notifier.

[ tglx: Massaged changelog to match the patch ]

Fixes: 7114cd749a12 ("clocksource: exynos_mct: use (request/free)_irq calls for local timer registration")
Reported-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Tested-by: Marcin Jabrzyk <m.jabrzyk@samsung.com>
Signed-off-by: Damian Eppel <d.eppel@samsung.com>
Cc: m.szyprowski@samsung.com
Cc: kyungmin.park@samsung.com
Cc: daniel.lezcano@linaro.org
Cc: kgene@kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/1435324984-7328-1-git-send-email-d.eppel@samsung.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: <stable@vger.kernel.org>
---
 drivers/clocksource/exynos_mct.c | 43 ++++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 935b059..9064ff7 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -462,15 +462,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);
 	}
@@ -483,10 +480,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,
@@ -518,7 +517,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;
 
@@ -545,7 +544,25 @@ 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;
+
+			irq_set_status_flags(mct_irq, IRQ_NOAUTOEN);
+			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;
+		}
 	}
 
 	err = register_cpu_notifier(&exynos4_mct_cpu_nb);

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

* Re: [RESEND PATCH v3] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
  2015-06-26 13:23   ` Damian Eppel
@ 2015-06-29  9:19     ` Daniel Lezcano
  -1 siblings, 0 replies; 29+ messages in thread
From: Daniel Lezcano @ 2015-06-29  9:19 UTC (permalink / raw)
  To: Damian Eppel, tglx, kgene, k.kozlowski, linux-kernel,
	linux-arm-kernel, linux-samsung-soc
  Cc: m.szyprowski, kyungmin.park, m.jabrzyk, stable

On 06/26/2015 03:23 PM, Damian Eppel wrote:
> 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).

Applied as a 4.2 fix.

Should it be tagged stable@ ?



-- 
  <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] 29+ messages in thread

* [RESEND PATCH v3] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
@ 2015-06-29  9:19     ` Daniel Lezcano
  0 siblings, 0 replies; 29+ messages in thread
From: Daniel Lezcano @ 2015-06-29  9:19 UTC (permalink / raw)
  To: linux-arm-kernel

On 06/26/2015 03:23 PM, Damian Eppel wrote:
> 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).

Applied as a 4.2 fix.

Should it be tagged stable@ ?



-- 
  <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] 29+ messages in thread

* Re: [RESEND PATCH v3] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
  2015-06-29  9:19     ` Daniel Lezcano
@ 2015-06-29  9:24       ` Thomas Gleixner
  -1 siblings, 0 replies; 29+ messages in thread
From: Thomas Gleixner @ 2015-06-29  9:24 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Damian Eppel, kgene, k.kozlowski, linux-kernel, linux-arm-kernel,
	linux-samsung-soc, m.szyprowski, kyungmin.park, m.jabrzyk,
	stable

On Mon, 29 Jun 2015, Daniel Lezcano wrote:

> On 06/26/2015 03:23 PM, Damian Eppel wrote:
> > 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).
> 
> Applied as a 4.2 fix.
> 
> Should it be tagged stable@ ?

I have that one queued in tip/timers/urgent already. Please check my
branches before picking up stuff.

Thanks,

	tglx


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

* [RESEND PATCH v3] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
@ 2015-06-29  9:24       ` Thomas Gleixner
  0 siblings, 0 replies; 29+ messages in thread
From: Thomas Gleixner @ 2015-06-29  9:24 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 29 Jun 2015, Daniel Lezcano wrote:

> On 06/26/2015 03:23 PM, Damian Eppel wrote:
> > 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).
> 
> Applied as a 4.2 fix.
> 
> Should it be tagged stable@ ?

I have that one queued in tip/timers/urgent already. Please check my
branches before picking up stuff.

Thanks,

	tglx

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

* Re: [RESEND PATCH v3] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
  2015-06-29  9:24       ` Thomas Gleixner
@ 2015-06-29 10:14         ` Daniel Lezcano
  -1 siblings, 0 replies; 29+ messages in thread
From: Daniel Lezcano @ 2015-06-29 10:14 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Damian Eppel, kgene, k.kozlowski, linux-kernel, linux-arm-kernel,
	linux-samsung-soc, m.szyprowski, kyungmin.park, m.jabrzyk,
	stable

On 06/29/2015 11:24 AM, Thomas Gleixner wrote:
> On Mon, 29 Jun 2015, Daniel Lezcano wrote:
>
>> On 06/26/2015 03:23 PM, Damian Eppel wrote:
>>> 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).
>>
>> Applied as a 4.2 fix.
>>
>> Should it be tagged stable@ ?
>
> I have that one queued in tip/timers/urgent already. Please check my
> branches before picking up stuff.

Mmmh, yep. I thought I refreshed my branch. Now I see it.

Thanks for the heads up.

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


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

* [RESEND PATCH v3] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
@ 2015-06-29 10:14         ` Daniel Lezcano
  0 siblings, 0 replies; 29+ messages in thread
From: Daniel Lezcano @ 2015-06-29 10:14 UTC (permalink / raw)
  To: linux-arm-kernel

On 06/29/2015 11:24 AM, Thomas Gleixner wrote:
> On Mon, 29 Jun 2015, Daniel Lezcano wrote:
>
>> On 06/26/2015 03:23 PM, Damian Eppel wrote:
>>> 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).
>>
>> Applied as a 4.2 fix.
>>
>> Should it be tagged stable@ ?
>
> I have that one queued in tip/timers/urgent already. Please check my
> branches before picking up stuff.

Mmmh, yep. I thought I refreshed my branch. Now I see it.

Thanks for the heads up.

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

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

* Re: [RESEND PATCH v3] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
  2015-06-29  9:19     ` Daniel Lezcano
@ 2015-06-29 11:50       ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 29+ messages in thread
From: Krzysztof Kozlowski @ 2015-06-29 11:50 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Damian Eppel, tglx, kgene, Kozik, linux-kernel, linux-arm-kernel,
	linux-samsung-soc, kyungmin.park, m.jabrzyk, stable,
	Marek Szyprowski

2015-06-29 18:19 GMT+09:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
> On 06/26/2015 03:23 PM, Damian Eppel wrote:
>>
>> 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).
>
>
> Applied as a 4.2 fix.
>
> Should it be tagged stable@ ?

The patch and commit in tip have cc-stable and fixes tags, so
everything is fine.

Thanks!
Krzysztof

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

* [RESEND PATCH v3] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif.
@ 2015-06-29 11:50       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 29+ messages in thread
From: Krzysztof Kozlowski @ 2015-06-29 11:50 UTC (permalink / raw)
  To: linux-arm-kernel

2015-06-29 18:19 GMT+09:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
> On 06/26/2015 03:23 PM, Damian Eppel wrote:
>>
>> 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).
>
>
> Applied as a 4.2 fix.
>
> Should it be tagged stable@ ?

The patch and commit in tip have cc-stable and fixes tags, so
everything is fine.

Thanks!
Krzysztof

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

end of thread, other threads:[~2015-06-29 11:50 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-12  9:11 [PATCH v2] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif Damian Eppel
2015-03-12  9:11 ` Damian Eppel
2015-05-11  1:33 ` Krzysztof Kozlowski
2015-05-11  1:33   ` Krzysztof Kozlowski
2015-05-11  7:30   ` Daniel Lezcano
2015-05-11  7:30     ` Daniel Lezcano
2015-05-11 11:18 ` Daniel Lezcano
2015-05-11 11:18   ` Daniel Lezcano
2015-05-25 15:24   ` Damian Eppel
2015-05-25 15:24     ` Damian Eppel
2015-05-27 12:43     ` Daniel Lezcano
2015-05-27 12:43       ` Daniel Lezcano
2015-05-28  9:47 ` Marek Szyprowski
2015-05-28  9:47   ` Marek Szyprowski
2015-05-28 15:37   ` Damian Eppel
2015-05-28 15:37     ` Damian Eppel
2015-06-02 11:11 ` [PATCH v3] " Damian Eppel
2015-06-02 11:11   ` Damian Eppel
2015-06-26 13:23 ` [RESEND PATCH " Damian Eppel
2015-06-26 13:23   ` Damian Eppel
2015-06-26 19:54   ` [tip:timers/urgent] clocksource: exynos_mct: Avoid blocking calls in the cpu hotplug notifier tip-bot for Damian Eppel
2015-06-29  9:19   ` [RESEND PATCH v3] clocksource: exynos_mct: fix for sleeping in atomic ctx handling cpu hotplug notif Daniel Lezcano
2015-06-29  9:19     ` Daniel Lezcano
2015-06-29  9:24     ` Thomas Gleixner
2015-06-29  9:24       ` Thomas Gleixner
2015-06-29 10:14       ` Daniel Lezcano
2015-06-29 10:14         ` Daniel Lezcano
2015-06-29 11:50     ` Krzysztof Kozlowski
2015-06-29 11:50       ` Krzysztof Kozlowski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.