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.
>From b3bf273f7a91a686db25112278fc554b47aa30c6 Mon 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().
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(-)
diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h
index ed3a5d4..c6de413 100644
--- a/include/linux/clockchips.h
+++ b/include/linux/clockchips.h
@@ -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,
};
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 47e6334..b870bd0 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -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);
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index df12434..152871c 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -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);
--
1.6.0.1.308.gede4c
On Mon, 1 Dec 2008 14:09:07 +0100 Sebastien Dugue <[email protected]> wrote:
>
> 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?
>
> Thanks,
>
> Sebastien.
>
>
> From b3bf273f7a91a686db25112278fc554b47aa30c6 Mon 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().
>
> 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(-)
>
> diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h
> index ed3a5d4..c6de413 100644
> --- a/include/linux/clockchips.h
> +++ b/include/linux/clockchips.h
> @@ -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,
> };
>
> diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
> index 47e6334..b870bd0 100644
> --- a/kernel/hrtimer.c
> +++ b/kernel/hrtimer.c
> @@ -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);
> diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
> index df12434..152871c 100644
> --- a/kernel/time/tick-common.c
> +++ b/kernel/time/tick-common.c
> @@ -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);
> --
> 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/
>
Sebastien Dugue wrote:
> On Mon, 1 Dec 2008 14:09:07 +0100 Sebastien Dugue <[email protected]> wrote:
>
> >
> > 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.
* Nathan Lynch <[email protected]> wrote:
> Sebastien Dugue wrote:
> > On Mon, 1 Dec 2008 14:09:07 +0100 Sebastien Dugue <[email protected]> wrote:
> >
> > >
> > > 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.
i've applied Sebastien's patch to tip/timers/hrtimers. Good catch! I've
also tagged it for a -stable backport.
Ingo