Fix cpu hotplug hang
RFC PATCH 1/1 Fix cpu hotplug hang
Hi Thomas, Ingo,
here is a patch that fixes a CPU hotplug hang I get on a Power6 box. It may
not be the only possible fix but it appears to be the cleanest I can think of at the moment.
Comments welcomed.
Thanks,
Sebastien.
Quoted 1 line
From
b3bf273f7a91a686db25112278fc554b47aa30c6Mon Sep 17 00:00:00 2001
From: Sebastien Dugue <[email protected]>
Date: Mon, 1 Dec 2008 12:22:06 +0100
Subject: [PATCH] Fix cpu hotplug hang
On architectures that support offlining all cpus (at least powerpc/pseries),
hot-unpluging the tick_do_timer_cpu can result in a system hang.
This comes from the fact that if the cpu going down happens to be the
cpu doing the tick, then as the tick_do_timer_cpu handover happens after the cpu is dead (via the CPU_DEAD notification), we're left without ticks, jiffies are frozen and any task relying on timers (msleep, ...) is stuck. That's particularly the case for the cpu looping in __cpu_die() waiting for the dying cpu to be dead.
This patch addresses this by having the tick_do_timer_cpu handover happen
earlier during the CPU_DYING notification. For this, a new clockevent notification type is introduced (CLOCK_EVT_NOTIFY_CPU_DYING) which is triggered in hrtimer_cpu_notify().
--- include/linux/clockchips.h | 1 + kernel/hrtimer.c | 4 ++++ kernel/time/tick-common.c | 26 +++++++++++++++++++------- 3 files changed, 24 insertions(+), 7 deletions(-)
include/linux/clockchips.h +1 -0
@@ -36,6 +36,7 @@ enum clock_event_nofitiers {CLOCK_EVT_NOTIFY_BROADCAST_EXIT ,CLOCK_EVT_NOTIFY_SUSPEND ,CLOCK_EVT_NOTIFY_RESUME , +CLOCK_EVT_NOTIFY_CPU_DYING , CLOCK_EVT_NOTIFY_CPU_DEAD, };
kernel/hrtimer.c +4 -0
@@ -1794,6 +1794,10 @@ static int __cpuinit hrtimer_cpu_notify(struct notifier_block *self,break ;#ifdef CONFIG_HOTPLUG_CPU +case CPU_DYING : +case CPU_DYING_FROZEN : +clockevents_notify (CLOCK_EVT_NOTIFY_CPU_DYING , &cpu); +break ;case CPU_DEAD :case CPU_DEAD_FROZEN :clockevents_notify (CLOCK_EVT_NOTIFY_CPU_DEAD , &cpu);
kernel/time/tick-common.c +19 -7
@@ -274,6 +274,21 @@ out_bc: }/* +* Transfer the do_timer job away from a dying cpu. +* +* Called with interrupts disabled. +*/ +static void tick_handover_do_timer (unsigned int *cpup) +{ +if (*cpup == tick_do_timer_cpu) { +int cpu =first_cpu (cpu_online_map); + + tick_do_timer_cpu = (cpu !=NR_CPUS ) ? cpu : +TICK_DO_TIMER_NONE ; + } +} + +/* * Shutdown an event device on a given cpu: * * This is called on a life CPU, when a CPU is dead. So we cannot
@@ -297,13 +312,6 @@ static void tick_shutdown(unsigned int *cpup)clockevents_exchange_device (dev,NULL ); td->evtdev =NULL ; } -/* Transfer the do_timer job away from this cpu */ -if (*cpup == tick_do_timer_cpu) { -int cpu =first_cpu (cpu_online_map); - - tick_do_timer_cpu = (cpu !=NR_CPUS ) ? cpu : -TICK_DO_TIMER_NONE ; - }spin_unlock_irqrestore (&tick_device_lock, flags); }
@@ -357,6 +365,10 @@ static int tick_notify(struct notifier_block *nb, unsigned long reason,tick_broadcast_oneshot_control (reason);break ; +case CLOCK_EVT_NOTIFY_CPU_DYING : +tick_handover_do_timer (dev); +break ; +case CLOCK_EVT_NOTIFY_CPU_DEAD :tick_shutdown_broadcast_oneshot (dev);tick_shutdown_broadcast (dev);
Signature · 1 line
1.6.0.1.308.gede4c
Quoted 8 lines · On Mon, 1 Dec 2008 14:09:07 +0100 Sebastien Dugue <[email protected]>…
Hi Thomas, Ingo,
here is a patch that fixes a CPU hotplug hang I get on a Power6 box. It may
not be the only possible fix but it appears to be the cleanest I can think of at the moment.Comments welcomed.
Ingo, Thomas, anybody care to comment on the hang and this possible fix?
Quoted 122 lines
Thanks,
Sebastien.
From
b3bf273f7a91a686db25112278fc554b47aa30c6Mon Sep 17 00:00:00 2001
From: Sebastien Dugue <[email protected]>
Date: Mon, 1 Dec 2008 12:22:06 +0100
Subject: [PATCH] Fix cpu hotplug hangOn architectures that support offlining all cpus (at least powerpc/pseries),
hot-unpluging thetick_do_timer_cpucan result in a system hang.This comes from the fact that if the cpu going down happens to be the
cpu doing the tick, then as thetick_do_timer_cpuhandover happens after the cpu is dead (via theCPU_DEADnotification), we're left without ticks, jiffies are frozen and any task relying on timers (msleep, ...) is stuck. That's particularly the case for the cpu looping in__cpu_die()waiting for the dying cpu to be dead.This patch addresses this by having the
tick_do_timer_cpuhandover happen
earlier during theCPU_DYINGnotification. For this, a new clockevent notification type is introduced (CLOCK_EVT_NOTIFY_CPU_DYING) which is triggered inhrtimer_cpu_notify().Signed-off-by Sebastien Dugue <[email protected]>Cc Thomas Gleixner <[email protected]>Cc Ingo Molnar <[email protected]>--- include/linux/clockchips.h | 1 + kernel/hrtimer.c | 4 ++++ kernel/time/tick-common.c | 26 +++++++++++++++++++------- 3 files changed, 24 insertions(+), 7 deletions(-)include/linux/clockchips.h +1
-0@@ -36,6 +36,7 @@ enum clock_event_nofitiers {CLOCK_EVT_NOTIFY_BROADCAST_EXIT ,CLOCK_EVT_NOTIFY_SUSPEND ,CLOCK_EVT_NOTIFY_RESUME , +CLOCK_EVT_NOTIFY_CPU_DYING , CLOCK_EVT_NOTIFY_CPU_DEAD, };kernel/hrtimer.c +4
-0@@ -1794,6 +1794,10 @@ static int __cpuinit hrtimer_cpu_notify(struct notifier_block *self,break ;#ifdef CONFIG_HOTPLUG_CPU +case CPU_DYING : +case CPU_DYING_FROZEN : +clockevents_notify (CLOCK_EVT_NOTIFY_CPU_DYING , &cpu); +break ;case CPU_DEAD :case CPU_DEAD_FROZEN :clockevents_notify (CLOCK_EVT_NOTIFY_CPU_DEAD , &cpu);kernel/time/tick-common.c +19
-7@@ -274,6 +274,21 @@ out_bc: }/* +* Transfer the do_timer job away from a dying cpu. +* +* Called with interrupts disabled. +*/ +static void tick_handover_do_timer (unsigned int *cpup) +{ +if (*cpup == tick_do_timer_cpu) { +int cpu =first_cpu (cpu_online_map); + + tick_do_timer_cpu = (cpu !=NR_CPUS ) ? cpu : +TICK_DO_TIMER_NONE ; + } +} + +/* * Shutdown an event device on a given cpu: * * This is called on a life CPU, when a CPU is dead. So we cannot @@ -297,13 +312,6 @@ static void tick_shutdown(unsigned int *cpup)clockevents_exchange_device (dev,NULL ); td->evtdev =NULL ; } -/* Transfer the do_timer job away from this cpu */ -if (*cpup == tick_do_timer_cpu) { -int cpu =first_cpu (cpu_online_map); - - tick_do_timer_cpu = (cpu !=NR_CPUS ) ? cpu : -TICK_DO_TIMER_NONE ; - }spin_unlock_irqrestore (&tick_device_lock, flags); }@@ -357,6 +365,10 @@ static int tick_notify(struct notifier_block *nb, unsigned long reason,tick_broadcast_oneshot_control (reason);break ; +case CLOCK_EVT_NOTIFY_CPU_DYING : +tick_handover_do_timer (dev); +break ; +case CLOCK_EVT_NOTIFY_CPU_DEAD :tick_shutdown_broadcast_oneshot (dev);tick_shutdown_broadcast (dev);Signature · 7 lines
1.6.0.1.308.gede4c -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Quoted 10 lines · Sebastien Dugue
Quoted 6 lines · On Mon, 1 Dec 2008 14:09:07 +0100 Sebastien Dugue <[email protected]>…
here is a patch that fixes a CPU hotplug hang I get on a Power6
box. It may not be the only possible fix but it appears to be the cleanest I can think of at the moment.Comments welcomed.
Ingo, Thomas, anybody care to comment on the hang and this possible fix?
FWIW, I was able to recreate this hang with 2.6.28 on a 8-way Power5 system, and Sebastien's patch does appear to fix the problem for my testcase, which does a few hundred offline/online operations.
Ingo Molnar
AppliedQuoted 8 lines · Nathan Lynch
Quoted 7 lines · Sebastien Dugue
Quoted 5 lines · On Mon, 1 Dec 2008 14:09:07 +0100 Sebastien Dugue <[email protected]>…
here is a patch that fixes a CPU hotplug hang I get on a Power6
box. It may not be the only possible fix but it appears to be the cleanest I can think of at the moment.
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Comments welcomed.
Ingo, Thomas, anybody care to comment on the hang and this possible fix?
FWIW, I was able to recreate this hang with 2.6.28 on a 8-way Power5 system, and Sebastien's patch does appear to fix the problem for my testcase, which does a few hundred offline/online operations.
i've applied Sebastien's patch to tip/timers/hrtimers. Good catch! I've also tagged it for a -stable backport.
Ingo