* [PATCH] smpboot: Add smpboot state variables instead of reusing CPU hotplug states
@ 2015-10-15 11:32 Daniel Wagner
2015-11-05 8:09 ` Daniel Wagner
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Daniel Wagner @ 2015-10-15 11:32 UTC (permalink / raw)
To: linux-kernel, xen-devel
Cc: Daniel Wagner, Thomas Gleixner, Paul E. McKenney, Peter Zijlstra
The cpu hotplug state machine in smpboot.c is reusing the states from
cpu.h. That is confusing when it comes to the CPU_DEAD_FROZEN usage.
Paul explained to me that he was in need of an additional state
for destinguishing between a CPU error states. For this he just
picked CPU_DEAD_FROZEN.
8038dad7e888581266c76df15d70ca457a3c5910 smpboot: Add common code for notification from dying CPU
2a442c9c6453d3d043dfd89f2e03a1deff8a6f06 x86: Use common outgoing-CPU-notification code
Instead of reusing the states, let's add new definition inside
the smpboot.c file with explenation what those states
mean. Thanks Paul for providing them.
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: xen-devel@lists.xenproject.org
Cc: linux-kernel@vger.kernel.org
---
arch/x86/xen/smp.c | 4 +--
include/linux/cpu.h | 3 +-
kernel/smpboot.c | 82 ++++++++++++++++++++++++++++++++++++++++-------------
3 files changed, 67 insertions(+), 22 deletions(-)
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 3f4ebf0..804bf5c 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -495,7 +495,7 @@ static int xen_cpu_up(unsigned int cpu, struct task_struct *idle)
rc = HYPERVISOR_vcpu_op(VCPUOP_up, cpu, NULL);
BUG_ON(rc);
- while (cpu_report_state(cpu) != CPU_ONLINE)
+ while (!cpu_check_online(cpu))
HYPERVISOR_sched_op(SCHEDOP_yield, NULL);
return 0;
@@ -767,7 +767,7 @@ static int xen_hvm_cpu_up(unsigned int cpu, struct task_struct *tidle)
* This can happen if CPU was offlined earlier and
* offlining timed out in common_cpu_die().
*/
- if (cpu_report_state(cpu) == CPU_DEAD_FROZEN) {
+ if (cpu_check_timeout(cpu)) {
xen_smp_intr_free(cpu);
xen_uninit_lock_cpu(cpu);
}
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 23c30bd..f78ab46 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -284,7 +284,8 @@ void arch_cpu_idle_dead(void);
DECLARE_PER_CPU(bool, cpu_dead_idle);
-int cpu_report_state(int cpu);
+int cpu_check_online(int cpu);
+int cpu_check_timeout(int cpu);
int cpu_check_up_prepare(int cpu);
void cpu_set_state_online(int cpu);
#ifdef CONFIG_HOTPLUG_CPU
diff --git a/kernel/smpboot.c b/kernel/smpboot.c
index a818cbc..75e5724 100644
--- a/kernel/smpboot.c
+++ b/kernel/smpboot.c
@@ -371,19 +371,63 @@ int smpboot_update_cpumask_percpu_thread(struct smp_hotplug_thread *plug_thread,
}
EXPORT_SYMBOL_GPL(smpboot_update_cpumask_percpu_thread);
+/* The CPU is offline, and its last offline operation was
+ * successful and proceeded normally. (Or, alternatively, the
+ * CPU never has come online, as this is the initial state.)
+ */
+#define CPUHP_POST_DEAD 0x01
+
+/* The CPU is in the process of coming online.
+ * Simple architectures can skip this state, and just invoke
+ * cpu_set_state_online() unconditionally instead.
+ */
+#define CPUHP_UP_PREPARE 0x02
+
+/* The CPU is now online. Simple architectures can skip this
+ * state, and just invoke cpu_wait_death() and cpu_report_death()
+ * unconditionally instead.
+ */
+#define CPUHP_ONLINE 0x03
+
+/* The CPU has gone offline, so that it may now be safely
+ * powered off (or whatever the architecture needs to do to it).
+ */
+#define CPUHP_DEAD 0x04
+
+/* The CPU did not go offline in a timely fashion, if at all,
+ * so it might need special processing at the next online (for
+ * example, simply refusing to bring it online).
+ */
+#define CPUHP_BROKEN 0x05
+
+/* The CPU eventually did go offline, but not in a timely
+ * fashion. If some sort of reset operation is required before it
+ * can be brought online, that reset operation needs to be carried
+ * out at online time. (Or, again, the architecture might simply
+ * refuse to bring it online.)
+ */
+#define CPUHP_TIMEOUT 0x06
+
static DEFINE_PER_CPU(atomic_t, cpu_hotplug_state) = ATOMIC_INIT(CPU_POST_DEAD);
/*
* Called to poll specified CPU's state, for example, when waiting for
* a CPU to come online.
*/
-int cpu_report_state(int cpu)
+int cpu_check_online(int cpu)
+{
+ return atomic_read(&per_cpu(cpu_hotplug_state, cpu)) ==
+ CPUHP_ONLINE;
+}
+
+int cpu_check_timeout(int cpu)
{
- return atomic_read(&per_cpu(cpu_hotplug_state, cpu));
+ return atomic_read(&per_cpu(cpu_hotplug_state, cpu)) ==
+ CPUHP_TIMEOUT;
}
/*
- * If CPU has died properly, set its state to CPU_UP_PREPARE and
+ * If CPU has died properly, set its state to CPUHP_UP_PREPARE and
* return success. Otherwise, return -EBUSY if the CPU died after
* cpu_wait_death() timed out. And yet otherwise again, return -EAGAIN
* if cpu_wait_death() timed out and the CPU still hasn't gotten around
@@ -397,19 +441,19 @@ int cpu_report_state(int cpu)
int cpu_check_up_prepare(int cpu)
{
if (!IS_ENABLED(CONFIG_HOTPLUG_CPU)) {
- atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPU_UP_PREPARE);
+ atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPUHP_UP_PREPARE);
return 0;
}
switch (atomic_read(&per_cpu(cpu_hotplug_state, cpu))) {
- case CPU_POST_DEAD:
+ case CPUHP_POST_DEAD:
/* The CPU died properly, so just start it up again. */
- atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPU_UP_PREPARE);
+ atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPUHP_UP_PREPARE);
return 0;
- case CPU_DEAD_FROZEN:
+ case CPUHP_TIMEOUT:
/*
* Timeout during CPU death, so let caller know.
@@ -424,7 +468,7 @@ int cpu_check_up_prepare(int cpu)
*/
return -EBUSY;
- case CPU_BROKEN:
+ case CPUHP_BROKEN:
/*
* The most likely reason we got here is that there was
@@ -452,7 +496,7 @@ int cpu_check_up_prepare(int cpu)
*/
void cpu_set_state_online(int cpu)
{
- (void)atomic_xchg(&per_cpu(cpu_hotplug_state, cpu), CPU_ONLINE);
+ (void)atomic_xchg(&per_cpu(cpu_hotplug_state, cpu), CPUHP_ONLINE);
}
#ifdef CONFIG_HOTPLUG_CPU
@@ -470,12 +514,12 @@ bool cpu_wait_death(unsigned int cpu, int seconds)
might_sleep();
/* The outgoing CPU will normally get done quite quickly. */
- if (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) == CPU_DEAD)
+ if (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) == CPUHP_DEAD)
goto update_state;
udelay(5);
/* But if the outgoing CPU dawdles, wait increasingly long times. */
- while (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) != CPU_DEAD) {
+ while (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) != CPUHP_DEAD) {
schedule_timeout_uninterruptible(sleep_jf);
jf_left -= sleep_jf;
if (jf_left <= 0)
@@ -484,14 +528,14 @@ bool cpu_wait_death(unsigned int cpu, int seconds)
}
update_state:
oldstate = atomic_read(&per_cpu(cpu_hotplug_state, cpu));
- if (oldstate == CPU_DEAD) {
+ if (oldstate == CPUHP_DEAD) {
/* Outgoing CPU died normally, update state. */
smp_mb(); /* atomic_read() before update. */
- atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPU_POST_DEAD);
+ atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPUHP_POST_DEAD);
} else {
/* Outgoing CPU still hasn't died, set state accordingly. */
if (atomic_cmpxchg(&per_cpu(cpu_hotplug_state, cpu),
- oldstate, CPU_BROKEN) != oldstate)
+ oldstate, CPUHP_BROKEN) != oldstate)
goto update_state;
ret = false;
}
@@ -502,7 +546,7 @@ update_state:
* Called by the outgoing CPU to report its successful death. Return
* false if this report follows the surviving CPU's timing out.
*
- * A separate "CPU_DEAD_FROZEN" is used when the surviving CPU
+ * A separate "CPUHP_TIMEOUT" is used when the surviving CPU
* timed out. This approach allows architectures to omit calls to
* cpu_check_up_prepare() and cpu_set_state_online() without defeating
* the next cpu_wait_death()'s polling loop.
@@ -515,13 +559,13 @@ bool cpu_report_death(void)
do {
oldstate = atomic_read(&per_cpu(cpu_hotplug_state, cpu));
- if (oldstate != CPU_BROKEN)
- newstate = CPU_DEAD;
+ if (oldstate != CPUHP_BROKEN)
+ newstate = CPUHP_DEAD;
else
- newstate = CPU_DEAD_FROZEN;
+ newstate = CPUHP_TIMEOUT;
} while (atomic_cmpxchg(&per_cpu(cpu_hotplug_state, cpu),
oldstate, newstate) != oldstate);
- return newstate == CPU_DEAD;
+ return newstate == CPUHP_DEAD;
}
#endif /* #ifdef CONFIG_HOTPLUG_CPU */
--
2.4.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] smpboot: Add smpboot state variables instead of reusing CPU hotplug states
2015-10-15 11:32 [PATCH] smpboot: Add smpboot state variables instead of reusing CPU hotplug states Daniel Wagner
@ 2015-11-05 8:09 ` Daniel Wagner
2015-11-05 8:09 ` Daniel Wagner
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Daniel Wagner @ 2015-11-05 8:09 UTC (permalink / raw)
To: linux-kernel, xen-devel, Paul E. McKenney; +Cc: Thomas Gleixner, Peter Zijlstra
Hi Paul,
I guess this patch got the summer conference period treatment. ACK,
NACK, completely STUPID idea?
cheers,
daniel
On 10/15/2015 01:32 PM, Daniel Wagner wrote:
> The cpu hotplug state machine in smpboot.c is reusing the states from
> cpu.h. That is confusing when it comes to the CPU_DEAD_FROZEN usage.
> Paul explained to me that he was in need of an additional state
> for destinguishing between a CPU error states. For this he just
> picked CPU_DEAD_FROZEN.
>
> 8038dad7e888581266c76df15d70ca457a3c5910 smpboot: Add common code for notification from dying CPU
> 2a442c9c6453d3d043dfd89f2e03a1deff8a6f06 x86: Use common outgoing-CPU-notification code
>
> Instead of reusing the states, let's add new definition inside
> the smpboot.c file with explenation what those states
> mean. Thanks Paul for providing them.
>
> Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: xen-devel@lists.xenproject.org
> Cc: linux-kernel@vger.kernel.org
> ---
> arch/x86/xen/smp.c | 4 +--
> include/linux/cpu.h | 3 +-
> kernel/smpboot.c | 82 ++++++++++++++++++++++++++++++++++++++++-------------
> 3 files changed, 67 insertions(+), 22 deletions(-)
>
> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
> index 3f4ebf0..804bf5c 100644
> --- a/arch/x86/xen/smp.c
> +++ b/arch/x86/xen/smp.c
> @@ -495,7 +495,7 @@ static int xen_cpu_up(unsigned int cpu, struct task_struct *idle)
> rc = HYPERVISOR_vcpu_op(VCPUOP_up, cpu, NULL);
> BUG_ON(rc);
>
> - while (cpu_report_state(cpu) != CPU_ONLINE)
> + while (!cpu_check_online(cpu))
> HYPERVISOR_sched_op(SCHEDOP_yield, NULL);
>
> return 0;
> @@ -767,7 +767,7 @@ static int xen_hvm_cpu_up(unsigned int cpu, struct task_struct *tidle)
> * This can happen if CPU was offlined earlier and
> * offlining timed out in common_cpu_die().
> */
> - if (cpu_report_state(cpu) == CPU_DEAD_FROZEN) {
> + if (cpu_check_timeout(cpu)) {
> xen_smp_intr_free(cpu);
> xen_uninit_lock_cpu(cpu);
> }
> diff --git a/include/linux/cpu.h b/include/linux/cpu.h
> index 23c30bd..f78ab46 100644
> --- a/include/linux/cpu.h
> +++ b/include/linux/cpu.h
> @@ -284,7 +284,8 @@ void arch_cpu_idle_dead(void);
>
> DECLARE_PER_CPU(bool, cpu_dead_idle);
>
> -int cpu_report_state(int cpu);
> +int cpu_check_online(int cpu);
> +int cpu_check_timeout(int cpu);
> int cpu_check_up_prepare(int cpu);
> void cpu_set_state_online(int cpu);
> #ifdef CONFIG_HOTPLUG_CPU
> diff --git a/kernel/smpboot.c b/kernel/smpboot.c
> index a818cbc..75e5724 100644
> --- a/kernel/smpboot.c
> +++ b/kernel/smpboot.c
> @@ -371,19 +371,63 @@ int smpboot_update_cpumask_percpu_thread(struct smp_hotplug_thread *plug_thread,
> }
> EXPORT_SYMBOL_GPL(smpboot_update_cpumask_percpu_thread);
>
> +/* The CPU is offline, and its last offline operation was
> + * successful and proceeded normally. (Or, alternatively, the
> + * CPU never has come online, as this is the initial state.)
> + */
> +#define CPUHP_POST_DEAD 0x01
> +
> +/* The CPU is in the process of coming online.
> + * Simple architectures can skip this state, and just invoke
> + * cpu_set_state_online() unconditionally instead.
> + */
> +#define CPUHP_UP_PREPARE 0x02
> +
> +/* The CPU is now online. Simple architectures can skip this
> + * state, and just invoke cpu_wait_death() and cpu_report_death()
> + * unconditionally instead.
> + */
> +#define CPUHP_ONLINE 0x03
> +
> +/* The CPU has gone offline, so that it may now be safely
> + * powered off (or whatever the architecture needs to do to it).
> + */
> +#define CPUHP_DEAD 0x04
> +
> +/* The CPU did not go offline in a timely fashion, if at all,
> + * so it might need special processing at the next online (for
> + * example, simply refusing to bring it online).
> + */
> +#define CPUHP_BROKEN 0x05
> +
> +/* The CPU eventually did go offline, but not in a timely
> + * fashion. If some sort of reset operation is required before it
> + * can be brought online, that reset operation needs to be carried
> + * out at online time. (Or, again, the architecture might simply
> + * refuse to bring it online.)
> + */
> +#define CPUHP_TIMEOUT 0x06
> +
> static DEFINE_PER_CPU(atomic_t, cpu_hotplug_state) = ATOMIC_INIT(CPU_POST_DEAD);
>
> /*
> * Called to poll specified CPU's state, for example, when waiting for
> * a CPU to come online.
> */
> -int cpu_report_state(int cpu)
> +int cpu_check_online(int cpu)
> +{
> + return atomic_read(&per_cpu(cpu_hotplug_state, cpu)) ==
> + CPUHP_ONLINE;
> +}
> +
> +int cpu_check_timeout(int cpu)
> {
> - return atomic_read(&per_cpu(cpu_hotplug_state, cpu));
> + return atomic_read(&per_cpu(cpu_hotplug_state, cpu)) ==
> + CPUHP_TIMEOUT;
> }
>
> /*
> - * If CPU has died properly, set its state to CPU_UP_PREPARE and
> + * If CPU has died properly, set its state to CPUHP_UP_PREPARE and
> * return success. Otherwise, return -EBUSY if the CPU died after
> * cpu_wait_death() timed out. And yet otherwise again, return -EAGAIN
> * if cpu_wait_death() timed out and the CPU still hasn't gotten around
> @@ -397,19 +441,19 @@ int cpu_report_state(int cpu)
> int cpu_check_up_prepare(int cpu)
> {
> if (!IS_ENABLED(CONFIG_HOTPLUG_CPU)) {
> - atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPU_UP_PREPARE);
> + atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPUHP_UP_PREPARE);
> return 0;
> }
>
> switch (atomic_read(&per_cpu(cpu_hotplug_state, cpu))) {
>
> - case CPU_POST_DEAD:
> + case CPUHP_POST_DEAD:
>
> /* The CPU died properly, so just start it up again. */
> - atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPU_UP_PREPARE);
> + atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPUHP_UP_PREPARE);
> return 0;
>
> - case CPU_DEAD_FROZEN:
> + case CPUHP_TIMEOUT:
>
> /*
> * Timeout during CPU death, so let caller know.
> @@ -424,7 +468,7 @@ int cpu_check_up_prepare(int cpu)
> */
> return -EBUSY;
>
> - case CPU_BROKEN:
> + case CPUHP_BROKEN:
>
> /*
> * The most likely reason we got here is that there was
> @@ -452,7 +496,7 @@ int cpu_check_up_prepare(int cpu)
> */
> void cpu_set_state_online(int cpu)
> {
> - (void)atomic_xchg(&per_cpu(cpu_hotplug_state, cpu), CPU_ONLINE);
> + (void)atomic_xchg(&per_cpu(cpu_hotplug_state, cpu), CPUHP_ONLINE);
> }
>
> #ifdef CONFIG_HOTPLUG_CPU
> @@ -470,12 +514,12 @@ bool cpu_wait_death(unsigned int cpu, int seconds)
> might_sleep();
>
> /* The outgoing CPU will normally get done quite quickly. */
> - if (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) == CPU_DEAD)
> + if (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) == CPUHP_DEAD)
> goto update_state;
> udelay(5);
>
> /* But if the outgoing CPU dawdles, wait increasingly long times. */
> - while (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) != CPU_DEAD) {
> + while (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) != CPUHP_DEAD) {
> schedule_timeout_uninterruptible(sleep_jf);
> jf_left -= sleep_jf;
> if (jf_left <= 0)
> @@ -484,14 +528,14 @@ bool cpu_wait_death(unsigned int cpu, int seconds)
> }
> update_state:
> oldstate = atomic_read(&per_cpu(cpu_hotplug_state, cpu));
> - if (oldstate == CPU_DEAD) {
> + if (oldstate == CPUHP_DEAD) {
> /* Outgoing CPU died normally, update state. */
> smp_mb(); /* atomic_read() before update. */
> - atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPU_POST_DEAD);
> + atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPUHP_POST_DEAD);
> } else {
> /* Outgoing CPU still hasn't died, set state accordingly. */
> if (atomic_cmpxchg(&per_cpu(cpu_hotplug_state, cpu),
> - oldstate, CPU_BROKEN) != oldstate)
> + oldstate, CPUHP_BROKEN) != oldstate)
> goto update_state;
> ret = false;
> }
> @@ -502,7 +546,7 @@ update_state:
> * Called by the outgoing CPU to report its successful death. Return
> * false if this report follows the surviving CPU's timing out.
> *
> - * A separate "CPU_DEAD_FROZEN" is used when the surviving CPU
> + * A separate "CPUHP_TIMEOUT" is used when the surviving CPU
> * timed out. This approach allows architectures to omit calls to
> * cpu_check_up_prepare() and cpu_set_state_online() without defeating
> * the next cpu_wait_death()'s polling loop.
> @@ -515,13 +559,13 @@ bool cpu_report_death(void)
>
> do {
> oldstate = atomic_read(&per_cpu(cpu_hotplug_state, cpu));
> - if (oldstate != CPU_BROKEN)
> - newstate = CPU_DEAD;
> + if (oldstate != CPUHP_BROKEN)
> + newstate = CPUHP_DEAD;
> else
> - newstate = CPU_DEAD_FROZEN;
> + newstate = CPUHP_TIMEOUT;
> } while (atomic_cmpxchg(&per_cpu(cpu_hotplug_state, cpu),
> oldstate, newstate) != oldstate);
> - return newstate == CPU_DEAD;
> + return newstate == CPUHP_DEAD;
> }
>
> #endif /* #ifdef CONFIG_HOTPLUG_CPU */
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] smpboot: Add smpboot state variables instead of reusing CPU hotplug states
2015-10-15 11:32 [PATCH] smpboot: Add smpboot state variables instead of reusing CPU hotplug states Daniel Wagner
2015-11-05 8:09 ` Daniel Wagner
@ 2015-11-05 8:09 ` Daniel Wagner
2015-11-07 7:17 ` Paul E. McKenney
2015-11-07 7:17 ` Paul E. McKenney
3 siblings, 0 replies; 8+ messages in thread
From: Daniel Wagner @ 2015-11-05 8:09 UTC (permalink / raw)
To: linux-kernel, xen-devel, Paul E. McKenney; +Cc: Peter Zijlstra, Thomas Gleixner
Hi Paul,
I guess this patch got the summer conference period treatment. ACK,
NACK, completely STUPID idea?
cheers,
daniel
On 10/15/2015 01:32 PM, Daniel Wagner wrote:
> The cpu hotplug state machine in smpboot.c is reusing the states from
> cpu.h. That is confusing when it comes to the CPU_DEAD_FROZEN usage.
> Paul explained to me that he was in need of an additional state
> for destinguishing between a CPU error states. For this he just
> picked CPU_DEAD_FROZEN.
>
> 8038dad7e888581266c76df15d70ca457a3c5910 smpboot: Add common code for notification from dying CPU
> 2a442c9c6453d3d043dfd89f2e03a1deff8a6f06 x86: Use common outgoing-CPU-notification code
>
> Instead of reusing the states, let's add new definition inside
> the smpboot.c file with explenation what those states
> mean. Thanks Paul for providing them.
>
> Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: xen-devel@lists.xenproject.org
> Cc: linux-kernel@vger.kernel.org
> ---
> arch/x86/xen/smp.c | 4 +--
> include/linux/cpu.h | 3 +-
> kernel/smpboot.c | 82 ++++++++++++++++++++++++++++++++++++++++-------------
> 3 files changed, 67 insertions(+), 22 deletions(-)
>
> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
> index 3f4ebf0..804bf5c 100644
> --- a/arch/x86/xen/smp.c
> +++ b/arch/x86/xen/smp.c
> @@ -495,7 +495,7 @@ static int xen_cpu_up(unsigned int cpu, struct task_struct *idle)
> rc = HYPERVISOR_vcpu_op(VCPUOP_up, cpu, NULL);
> BUG_ON(rc);
>
> - while (cpu_report_state(cpu) != CPU_ONLINE)
> + while (!cpu_check_online(cpu))
> HYPERVISOR_sched_op(SCHEDOP_yield, NULL);
>
> return 0;
> @@ -767,7 +767,7 @@ static int xen_hvm_cpu_up(unsigned int cpu, struct task_struct *tidle)
> * This can happen if CPU was offlined earlier and
> * offlining timed out in common_cpu_die().
> */
> - if (cpu_report_state(cpu) == CPU_DEAD_FROZEN) {
> + if (cpu_check_timeout(cpu)) {
> xen_smp_intr_free(cpu);
> xen_uninit_lock_cpu(cpu);
> }
> diff --git a/include/linux/cpu.h b/include/linux/cpu.h
> index 23c30bd..f78ab46 100644
> --- a/include/linux/cpu.h
> +++ b/include/linux/cpu.h
> @@ -284,7 +284,8 @@ void arch_cpu_idle_dead(void);
>
> DECLARE_PER_CPU(bool, cpu_dead_idle);
>
> -int cpu_report_state(int cpu);
> +int cpu_check_online(int cpu);
> +int cpu_check_timeout(int cpu);
> int cpu_check_up_prepare(int cpu);
> void cpu_set_state_online(int cpu);
> #ifdef CONFIG_HOTPLUG_CPU
> diff --git a/kernel/smpboot.c b/kernel/smpboot.c
> index a818cbc..75e5724 100644
> --- a/kernel/smpboot.c
> +++ b/kernel/smpboot.c
> @@ -371,19 +371,63 @@ int smpboot_update_cpumask_percpu_thread(struct smp_hotplug_thread *plug_thread,
> }
> EXPORT_SYMBOL_GPL(smpboot_update_cpumask_percpu_thread);
>
> +/* The CPU is offline, and its last offline operation was
> + * successful and proceeded normally. (Or, alternatively, the
> + * CPU never has come online, as this is the initial state.)
> + */
> +#define CPUHP_POST_DEAD 0x01
> +
> +/* The CPU is in the process of coming online.
> + * Simple architectures can skip this state, and just invoke
> + * cpu_set_state_online() unconditionally instead.
> + */
> +#define CPUHP_UP_PREPARE 0x02
> +
> +/* The CPU is now online. Simple architectures can skip this
> + * state, and just invoke cpu_wait_death() and cpu_report_death()
> + * unconditionally instead.
> + */
> +#define CPUHP_ONLINE 0x03
> +
> +/* The CPU has gone offline, so that it may now be safely
> + * powered off (or whatever the architecture needs to do to it).
> + */
> +#define CPUHP_DEAD 0x04
> +
> +/* The CPU did not go offline in a timely fashion, if at all,
> + * so it might need special processing at the next online (for
> + * example, simply refusing to bring it online).
> + */
> +#define CPUHP_BROKEN 0x05
> +
> +/* The CPU eventually did go offline, but not in a timely
> + * fashion. If some sort of reset operation is required before it
> + * can be brought online, that reset operation needs to be carried
> + * out at online time. (Or, again, the architecture might simply
> + * refuse to bring it online.)
> + */
> +#define CPUHP_TIMEOUT 0x06
> +
> static DEFINE_PER_CPU(atomic_t, cpu_hotplug_state) = ATOMIC_INIT(CPU_POST_DEAD);
>
> /*
> * Called to poll specified CPU's state, for example, when waiting for
> * a CPU to come online.
> */
> -int cpu_report_state(int cpu)
> +int cpu_check_online(int cpu)
> +{
> + return atomic_read(&per_cpu(cpu_hotplug_state, cpu)) ==
> + CPUHP_ONLINE;
> +}
> +
> +int cpu_check_timeout(int cpu)
> {
> - return atomic_read(&per_cpu(cpu_hotplug_state, cpu));
> + return atomic_read(&per_cpu(cpu_hotplug_state, cpu)) ==
> + CPUHP_TIMEOUT;
> }
>
> /*
> - * If CPU has died properly, set its state to CPU_UP_PREPARE and
> + * If CPU has died properly, set its state to CPUHP_UP_PREPARE and
> * return success. Otherwise, return -EBUSY if the CPU died after
> * cpu_wait_death() timed out. And yet otherwise again, return -EAGAIN
> * if cpu_wait_death() timed out and the CPU still hasn't gotten around
> @@ -397,19 +441,19 @@ int cpu_report_state(int cpu)
> int cpu_check_up_prepare(int cpu)
> {
> if (!IS_ENABLED(CONFIG_HOTPLUG_CPU)) {
> - atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPU_UP_PREPARE);
> + atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPUHP_UP_PREPARE);
> return 0;
> }
>
> switch (atomic_read(&per_cpu(cpu_hotplug_state, cpu))) {
>
> - case CPU_POST_DEAD:
> + case CPUHP_POST_DEAD:
>
> /* The CPU died properly, so just start it up again. */
> - atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPU_UP_PREPARE);
> + atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPUHP_UP_PREPARE);
> return 0;
>
> - case CPU_DEAD_FROZEN:
> + case CPUHP_TIMEOUT:
>
> /*
> * Timeout during CPU death, so let caller know.
> @@ -424,7 +468,7 @@ int cpu_check_up_prepare(int cpu)
> */
> return -EBUSY;
>
> - case CPU_BROKEN:
> + case CPUHP_BROKEN:
>
> /*
> * The most likely reason we got here is that there was
> @@ -452,7 +496,7 @@ int cpu_check_up_prepare(int cpu)
> */
> void cpu_set_state_online(int cpu)
> {
> - (void)atomic_xchg(&per_cpu(cpu_hotplug_state, cpu), CPU_ONLINE);
> + (void)atomic_xchg(&per_cpu(cpu_hotplug_state, cpu), CPUHP_ONLINE);
> }
>
> #ifdef CONFIG_HOTPLUG_CPU
> @@ -470,12 +514,12 @@ bool cpu_wait_death(unsigned int cpu, int seconds)
> might_sleep();
>
> /* The outgoing CPU will normally get done quite quickly. */
> - if (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) == CPU_DEAD)
> + if (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) == CPUHP_DEAD)
> goto update_state;
> udelay(5);
>
> /* But if the outgoing CPU dawdles, wait increasingly long times. */
> - while (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) != CPU_DEAD) {
> + while (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) != CPUHP_DEAD) {
> schedule_timeout_uninterruptible(sleep_jf);
> jf_left -= sleep_jf;
> if (jf_left <= 0)
> @@ -484,14 +528,14 @@ bool cpu_wait_death(unsigned int cpu, int seconds)
> }
> update_state:
> oldstate = atomic_read(&per_cpu(cpu_hotplug_state, cpu));
> - if (oldstate == CPU_DEAD) {
> + if (oldstate == CPUHP_DEAD) {
> /* Outgoing CPU died normally, update state. */
> smp_mb(); /* atomic_read() before update. */
> - atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPU_POST_DEAD);
> + atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPUHP_POST_DEAD);
> } else {
> /* Outgoing CPU still hasn't died, set state accordingly. */
> if (atomic_cmpxchg(&per_cpu(cpu_hotplug_state, cpu),
> - oldstate, CPU_BROKEN) != oldstate)
> + oldstate, CPUHP_BROKEN) != oldstate)
> goto update_state;
> ret = false;
> }
> @@ -502,7 +546,7 @@ update_state:
> * Called by the outgoing CPU to report its successful death. Return
> * false if this report follows the surviving CPU's timing out.
> *
> - * A separate "CPU_DEAD_FROZEN" is used when the surviving CPU
> + * A separate "CPUHP_TIMEOUT" is used when the surviving CPU
> * timed out. This approach allows architectures to omit calls to
> * cpu_check_up_prepare() and cpu_set_state_online() without defeating
> * the next cpu_wait_death()'s polling loop.
> @@ -515,13 +559,13 @@ bool cpu_report_death(void)
>
> do {
> oldstate = atomic_read(&per_cpu(cpu_hotplug_state, cpu));
> - if (oldstate != CPU_BROKEN)
> - newstate = CPU_DEAD;
> + if (oldstate != CPUHP_BROKEN)
> + newstate = CPUHP_DEAD;
> else
> - newstate = CPU_DEAD_FROZEN;
> + newstate = CPUHP_TIMEOUT;
> } while (atomic_cmpxchg(&per_cpu(cpu_hotplug_state, cpu),
> oldstate, newstate) != oldstate);
> - return newstate == CPU_DEAD;
> + return newstate == CPUHP_DEAD;
> }
>
> #endif /* #ifdef CONFIG_HOTPLUG_CPU */
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] smpboot: Add smpboot state variables instead of reusing CPU hotplug states
2015-10-15 11:32 [PATCH] smpboot: Add smpboot state variables instead of reusing CPU hotplug states Daniel Wagner
2015-11-05 8:09 ` Daniel Wagner
2015-11-05 8:09 ` Daniel Wagner
@ 2015-11-07 7:17 ` Paul E. McKenney
2015-11-12 14:34 ` Daniel Wagner
2015-11-12 14:34 ` Daniel Wagner
2015-11-07 7:17 ` Paul E. McKenney
3 siblings, 2 replies; 8+ messages in thread
From: Paul E. McKenney @ 2015-11-07 7:17 UTC (permalink / raw)
To: Daniel Wagner; +Cc: linux-kernel, xen-devel, Thomas Gleixner, Peter Zijlstra
On Thu, Oct 15, 2015 at 01:32:44PM +0200, Daniel Wagner wrote:
> The cpu hotplug state machine in smpboot.c is reusing the states from
> cpu.h. That is confusing when it comes to the CPU_DEAD_FROZEN usage.
> Paul explained to me that he was in need of an additional state
> for destinguishing between a CPU error states. For this he just
> picked CPU_DEAD_FROZEN.
>
> 8038dad7e888581266c76df15d70ca457a3c5910 smpboot: Add common code for notification from dying CPU
> 2a442c9c6453d3d043dfd89f2e03a1deff8a6f06 x86: Use common outgoing-CPU-notification code
>
> Instead of reusing the states, let's add new definition inside
> the smpboot.c file with explenation what those states
> mean. Thanks Paul for providing them.
>
> Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Apologies for the delay, I didn't realize that you were waiting on me.
Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: xen-devel@lists.xenproject.org
> Cc: linux-kernel@vger.kernel.org
> ---
> arch/x86/xen/smp.c | 4 +--
> include/linux/cpu.h | 3 +-
> kernel/smpboot.c | 82 ++++++++++++++++++++++++++++++++++++++++-------------
> 3 files changed, 67 insertions(+), 22 deletions(-)
>
> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
> index 3f4ebf0..804bf5c 100644
> --- a/arch/x86/xen/smp.c
> +++ b/arch/x86/xen/smp.c
> @@ -495,7 +495,7 @@ static int xen_cpu_up(unsigned int cpu, struct task_struct *idle)
> rc = HYPERVISOR_vcpu_op(VCPUOP_up, cpu, NULL);
> BUG_ON(rc);
>
> - while (cpu_report_state(cpu) != CPU_ONLINE)
> + while (!cpu_check_online(cpu))
> HYPERVISOR_sched_op(SCHEDOP_yield, NULL);
>
> return 0;
> @@ -767,7 +767,7 @@ static int xen_hvm_cpu_up(unsigned int cpu, struct task_struct *tidle)
> * This can happen if CPU was offlined earlier and
> * offlining timed out in common_cpu_die().
> */
> - if (cpu_report_state(cpu) == CPU_DEAD_FROZEN) {
> + if (cpu_check_timeout(cpu)) {
> xen_smp_intr_free(cpu);
> xen_uninit_lock_cpu(cpu);
> }
> diff --git a/include/linux/cpu.h b/include/linux/cpu.h
> index 23c30bd..f78ab46 100644
> --- a/include/linux/cpu.h
> +++ b/include/linux/cpu.h
> @@ -284,7 +284,8 @@ void arch_cpu_idle_dead(void);
>
> DECLARE_PER_CPU(bool, cpu_dead_idle);
>
> -int cpu_report_state(int cpu);
> +int cpu_check_online(int cpu);
> +int cpu_check_timeout(int cpu);
> int cpu_check_up_prepare(int cpu);
> void cpu_set_state_online(int cpu);
> #ifdef CONFIG_HOTPLUG_CPU
> diff --git a/kernel/smpboot.c b/kernel/smpboot.c
> index a818cbc..75e5724 100644
> --- a/kernel/smpboot.c
> +++ b/kernel/smpboot.c
> @@ -371,19 +371,63 @@ int smpboot_update_cpumask_percpu_thread(struct smp_hotplug_thread *plug_thread,
> }
> EXPORT_SYMBOL_GPL(smpboot_update_cpumask_percpu_thread);
>
> +/* The CPU is offline, and its last offline operation was
> + * successful and proceeded normally. (Or, alternatively, the
> + * CPU never has come online, as this is the initial state.)
> + */
> +#define CPUHP_POST_DEAD 0x01
> +
> +/* The CPU is in the process of coming online.
> + * Simple architectures can skip this state, and just invoke
> + * cpu_set_state_online() unconditionally instead.
> + */
> +#define CPUHP_UP_PREPARE 0x02
> +
> +/* The CPU is now online. Simple architectures can skip this
> + * state, and just invoke cpu_wait_death() and cpu_report_death()
> + * unconditionally instead.
> + */
> +#define CPUHP_ONLINE 0x03
> +
> +/* The CPU has gone offline, so that it may now be safely
> + * powered off (or whatever the architecture needs to do to it).
> + */
> +#define CPUHP_DEAD 0x04
> +
> +/* The CPU did not go offline in a timely fashion, if at all,
> + * so it might need special processing at the next online (for
> + * example, simply refusing to bring it online).
> + */
> +#define CPUHP_BROKEN 0x05
> +
> +/* The CPU eventually did go offline, but not in a timely
> + * fashion. If some sort of reset operation is required before it
> + * can be brought online, that reset operation needs to be carried
> + * out at online time. (Or, again, the architecture might simply
> + * refuse to bring it online.)
> + */
> +#define CPUHP_TIMEOUT 0x06
> +
> static DEFINE_PER_CPU(atomic_t, cpu_hotplug_state) = ATOMIC_INIT(CPU_POST_DEAD);
>
> /*
> * Called to poll specified CPU's state, for example, when waiting for
> * a CPU to come online.
> */
> -int cpu_report_state(int cpu)
> +int cpu_check_online(int cpu)
> +{
> + return atomic_read(&per_cpu(cpu_hotplug_state, cpu)) ==
> + CPUHP_ONLINE;
> +}
> +
> +int cpu_check_timeout(int cpu)
> {
> - return atomic_read(&per_cpu(cpu_hotplug_state, cpu));
> + return atomic_read(&per_cpu(cpu_hotplug_state, cpu)) ==
> + CPUHP_TIMEOUT;
> }
>
> /*
> - * If CPU has died properly, set its state to CPU_UP_PREPARE and
> + * If CPU has died properly, set its state to CPUHP_UP_PREPARE and
> * return success. Otherwise, return -EBUSY if the CPU died after
> * cpu_wait_death() timed out. And yet otherwise again, return -EAGAIN
> * if cpu_wait_death() timed out and the CPU still hasn't gotten around
> @@ -397,19 +441,19 @@ int cpu_report_state(int cpu)
> int cpu_check_up_prepare(int cpu)
> {
> if (!IS_ENABLED(CONFIG_HOTPLUG_CPU)) {
> - atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPU_UP_PREPARE);
> + atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPUHP_UP_PREPARE);
> return 0;
> }
>
> switch (atomic_read(&per_cpu(cpu_hotplug_state, cpu))) {
>
> - case CPU_POST_DEAD:
> + case CPUHP_POST_DEAD:
>
> /* The CPU died properly, so just start it up again. */
> - atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPU_UP_PREPARE);
> + atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPUHP_UP_PREPARE);
> return 0;
>
> - case CPU_DEAD_FROZEN:
> + case CPUHP_TIMEOUT:
>
> /*
> * Timeout during CPU death, so let caller know.
> @@ -424,7 +468,7 @@ int cpu_check_up_prepare(int cpu)
> */
> return -EBUSY;
>
> - case CPU_BROKEN:
> + case CPUHP_BROKEN:
>
> /*
> * The most likely reason we got here is that there was
> @@ -452,7 +496,7 @@ int cpu_check_up_prepare(int cpu)
> */
> void cpu_set_state_online(int cpu)
> {
> - (void)atomic_xchg(&per_cpu(cpu_hotplug_state, cpu), CPU_ONLINE);
> + (void)atomic_xchg(&per_cpu(cpu_hotplug_state, cpu), CPUHP_ONLINE);
> }
>
> #ifdef CONFIG_HOTPLUG_CPU
> @@ -470,12 +514,12 @@ bool cpu_wait_death(unsigned int cpu, int seconds)
> might_sleep();
>
> /* The outgoing CPU will normally get done quite quickly. */
> - if (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) == CPU_DEAD)
> + if (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) == CPUHP_DEAD)
> goto update_state;
> udelay(5);
>
> /* But if the outgoing CPU dawdles, wait increasingly long times. */
> - while (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) != CPU_DEAD) {
> + while (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) != CPUHP_DEAD) {
> schedule_timeout_uninterruptible(sleep_jf);
> jf_left -= sleep_jf;
> if (jf_left <= 0)
> @@ -484,14 +528,14 @@ bool cpu_wait_death(unsigned int cpu, int seconds)
> }
> update_state:
> oldstate = atomic_read(&per_cpu(cpu_hotplug_state, cpu));
> - if (oldstate == CPU_DEAD) {
> + if (oldstate == CPUHP_DEAD) {
> /* Outgoing CPU died normally, update state. */
> smp_mb(); /* atomic_read() before update. */
> - atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPU_POST_DEAD);
> + atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPUHP_POST_DEAD);
> } else {
> /* Outgoing CPU still hasn't died, set state accordingly. */
> if (atomic_cmpxchg(&per_cpu(cpu_hotplug_state, cpu),
> - oldstate, CPU_BROKEN) != oldstate)
> + oldstate, CPUHP_BROKEN) != oldstate)
> goto update_state;
> ret = false;
> }
> @@ -502,7 +546,7 @@ update_state:
> * Called by the outgoing CPU to report its successful death. Return
> * false if this report follows the surviving CPU's timing out.
> *
> - * A separate "CPU_DEAD_FROZEN" is used when the surviving CPU
> + * A separate "CPUHP_TIMEOUT" is used when the surviving CPU
> * timed out. This approach allows architectures to omit calls to
> * cpu_check_up_prepare() and cpu_set_state_online() without defeating
> * the next cpu_wait_death()'s polling loop.
> @@ -515,13 +559,13 @@ bool cpu_report_death(void)
>
> do {
> oldstate = atomic_read(&per_cpu(cpu_hotplug_state, cpu));
> - if (oldstate != CPU_BROKEN)
> - newstate = CPU_DEAD;
> + if (oldstate != CPUHP_BROKEN)
> + newstate = CPUHP_DEAD;
> else
> - newstate = CPU_DEAD_FROZEN;
> + newstate = CPUHP_TIMEOUT;
> } while (atomic_cmpxchg(&per_cpu(cpu_hotplug_state, cpu),
> oldstate, newstate) != oldstate);
> - return newstate == CPU_DEAD;
> + return newstate == CPUHP_DEAD;
> }
>
> #endif /* #ifdef CONFIG_HOTPLUG_CPU */
> --
> 2.4.3
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] smpboot: Add smpboot state variables instead of reusing CPU hotplug states
2015-10-15 11:32 [PATCH] smpboot: Add smpboot state variables instead of reusing CPU hotplug states Daniel Wagner
` (2 preceding siblings ...)
2015-11-07 7:17 ` Paul E. McKenney
@ 2015-11-07 7:17 ` Paul E. McKenney
3 siblings, 0 replies; 8+ messages in thread
From: Paul E. McKenney @ 2015-11-07 7:17 UTC (permalink / raw)
To: Daniel Wagner; +Cc: Peter Zijlstra, xen-devel, Thomas Gleixner, linux-kernel
On Thu, Oct 15, 2015 at 01:32:44PM +0200, Daniel Wagner wrote:
> The cpu hotplug state machine in smpboot.c is reusing the states from
> cpu.h. That is confusing when it comes to the CPU_DEAD_FROZEN usage.
> Paul explained to me that he was in need of an additional state
> for destinguishing between a CPU error states. For this he just
> picked CPU_DEAD_FROZEN.
>
> 8038dad7e888581266c76df15d70ca457a3c5910 smpboot: Add common code for notification from dying CPU
> 2a442c9c6453d3d043dfd89f2e03a1deff8a6f06 x86: Use common outgoing-CPU-notification code
>
> Instead of reusing the states, let's add new definition inside
> the smpboot.c file with explenation what those states
> mean. Thanks Paul for providing them.
>
> Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Apologies for the delay, I didn't realize that you were waiting on me.
Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: xen-devel@lists.xenproject.org
> Cc: linux-kernel@vger.kernel.org
> ---
> arch/x86/xen/smp.c | 4 +--
> include/linux/cpu.h | 3 +-
> kernel/smpboot.c | 82 ++++++++++++++++++++++++++++++++++++++++-------------
> 3 files changed, 67 insertions(+), 22 deletions(-)
>
> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
> index 3f4ebf0..804bf5c 100644
> --- a/arch/x86/xen/smp.c
> +++ b/arch/x86/xen/smp.c
> @@ -495,7 +495,7 @@ static int xen_cpu_up(unsigned int cpu, struct task_struct *idle)
> rc = HYPERVISOR_vcpu_op(VCPUOP_up, cpu, NULL);
> BUG_ON(rc);
>
> - while (cpu_report_state(cpu) != CPU_ONLINE)
> + while (!cpu_check_online(cpu))
> HYPERVISOR_sched_op(SCHEDOP_yield, NULL);
>
> return 0;
> @@ -767,7 +767,7 @@ static int xen_hvm_cpu_up(unsigned int cpu, struct task_struct *tidle)
> * This can happen if CPU was offlined earlier and
> * offlining timed out in common_cpu_die().
> */
> - if (cpu_report_state(cpu) == CPU_DEAD_FROZEN) {
> + if (cpu_check_timeout(cpu)) {
> xen_smp_intr_free(cpu);
> xen_uninit_lock_cpu(cpu);
> }
> diff --git a/include/linux/cpu.h b/include/linux/cpu.h
> index 23c30bd..f78ab46 100644
> --- a/include/linux/cpu.h
> +++ b/include/linux/cpu.h
> @@ -284,7 +284,8 @@ void arch_cpu_idle_dead(void);
>
> DECLARE_PER_CPU(bool, cpu_dead_idle);
>
> -int cpu_report_state(int cpu);
> +int cpu_check_online(int cpu);
> +int cpu_check_timeout(int cpu);
> int cpu_check_up_prepare(int cpu);
> void cpu_set_state_online(int cpu);
> #ifdef CONFIG_HOTPLUG_CPU
> diff --git a/kernel/smpboot.c b/kernel/smpboot.c
> index a818cbc..75e5724 100644
> --- a/kernel/smpboot.c
> +++ b/kernel/smpboot.c
> @@ -371,19 +371,63 @@ int smpboot_update_cpumask_percpu_thread(struct smp_hotplug_thread *plug_thread,
> }
> EXPORT_SYMBOL_GPL(smpboot_update_cpumask_percpu_thread);
>
> +/* The CPU is offline, and its last offline operation was
> + * successful and proceeded normally. (Or, alternatively, the
> + * CPU never has come online, as this is the initial state.)
> + */
> +#define CPUHP_POST_DEAD 0x01
> +
> +/* The CPU is in the process of coming online.
> + * Simple architectures can skip this state, and just invoke
> + * cpu_set_state_online() unconditionally instead.
> + */
> +#define CPUHP_UP_PREPARE 0x02
> +
> +/* The CPU is now online. Simple architectures can skip this
> + * state, and just invoke cpu_wait_death() and cpu_report_death()
> + * unconditionally instead.
> + */
> +#define CPUHP_ONLINE 0x03
> +
> +/* The CPU has gone offline, so that it may now be safely
> + * powered off (or whatever the architecture needs to do to it).
> + */
> +#define CPUHP_DEAD 0x04
> +
> +/* The CPU did not go offline in a timely fashion, if at all,
> + * so it might need special processing at the next online (for
> + * example, simply refusing to bring it online).
> + */
> +#define CPUHP_BROKEN 0x05
> +
> +/* The CPU eventually did go offline, but not in a timely
> + * fashion. If some sort of reset operation is required before it
> + * can be brought online, that reset operation needs to be carried
> + * out at online time. (Or, again, the architecture might simply
> + * refuse to bring it online.)
> + */
> +#define CPUHP_TIMEOUT 0x06
> +
> static DEFINE_PER_CPU(atomic_t, cpu_hotplug_state) = ATOMIC_INIT(CPU_POST_DEAD);
>
> /*
> * Called to poll specified CPU's state, for example, when waiting for
> * a CPU to come online.
> */
> -int cpu_report_state(int cpu)
> +int cpu_check_online(int cpu)
> +{
> + return atomic_read(&per_cpu(cpu_hotplug_state, cpu)) ==
> + CPUHP_ONLINE;
> +}
> +
> +int cpu_check_timeout(int cpu)
> {
> - return atomic_read(&per_cpu(cpu_hotplug_state, cpu));
> + return atomic_read(&per_cpu(cpu_hotplug_state, cpu)) ==
> + CPUHP_TIMEOUT;
> }
>
> /*
> - * If CPU has died properly, set its state to CPU_UP_PREPARE and
> + * If CPU has died properly, set its state to CPUHP_UP_PREPARE and
> * return success. Otherwise, return -EBUSY if the CPU died after
> * cpu_wait_death() timed out. And yet otherwise again, return -EAGAIN
> * if cpu_wait_death() timed out and the CPU still hasn't gotten around
> @@ -397,19 +441,19 @@ int cpu_report_state(int cpu)
> int cpu_check_up_prepare(int cpu)
> {
> if (!IS_ENABLED(CONFIG_HOTPLUG_CPU)) {
> - atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPU_UP_PREPARE);
> + atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPUHP_UP_PREPARE);
> return 0;
> }
>
> switch (atomic_read(&per_cpu(cpu_hotplug_state, cpu))) {
>
> - case CPU_POST_DEAD:
> + case CPUHP_POST_DEAD:
>
> /* The CPU died properly, so just start it up again. */
> - atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPU_UP_PREPARE);
> + atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPUHP_UP_PREPARE);
> return 0;
>
> - case CPU_DEAD_FROZEN:
> + case CPUHP_TIMEOUT:
>
> /*
> * Timeout during CPU death, so let caller know.
> @@ -424,7 +468,7 @@ int cpu_check_up_prepare(int cpu)
> */
> return -EBUSY;
>
> - case CPU_BROKEN:
> + case CPUHP_BROKEN:
>
> /*
> * The most likely reason we got here is that there was
> @@ -452,7 +496,7 @@ int cpu_check_up_prepare(int cpu)
> */
> void cpu_set_state_online(int cpu)
> {
> - (void)atomic_xchg(&per_cpu(cpu_hotplug_state, cpu), CPU_ONLINE);
> + (void)atomic_xchg(&per_cpu(cpu_hotplug_state, cpu), CPUHP_ONLINE);
> }
>
> #ifdef CONFIG_HOTPLUG_CPU
> @@ -470,12 +514,12 @@ bool cpu_wait_death(unsigned int cpu, int seconds)
> might_sleep();
>
> /* The outgoing CPU will normally get done quite quickly. */
> - if (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) == CPU_DEAD)
> + if (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) == CPUHP_DEAD)
> goto update_state;
> udelay(5);
>
> /* But if the outgoing CPU dawdles, wait increasingly long times. */
> - while (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) != CPU_DEAD) {
> + while (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) != CPUHP_DEAD) {
> schedule_timeout_uninterruptible(sleep_jf);
> jf_left -= sleep_jf;
> if (jf_left <= 0)
> @@ -484,14 +528,14 @@ bool cpu_wait_death(unsigned int cpu, int seconds)
> }
> update_state:
> oldstate = atomic_read(&per_cpu(cpu_hotplug_state, cpu));
> - if (oldstate == CPU_DEAD) {
> + if (oldstate == CPUHP_DEAD) {
> /* Outgoing CPU died normally, update state. */
> smp_mb(); /* atomic_read() before update. */
> - atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPU_POST_DEAD);
> + atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPUHP_POST_DEAD);
> } else {
> /* Outgoing CPU still hasn't died, set state accordingly. */
> if (atomic_cmpxchg(&per_cpu(cpu_hotplug_state, cpu),
> - oldstate, CPU_BROKEN) != oldstate)
> + oldstate, CPUHP_BROKEN) != oldstate)
> goto update_state;
> ret = false;
> }
> @@ -502,7 +546,7 @@ update_state:
> * Called by the outgoing CPU to report its successful death. Return
> * false if this report follows the surviving CPU's timing out.
> *
> - * A separate "CPU_DEAD_FROZEN" is used when the surviving CPU
> + * A separate "CPUHP_TIMEOUT" is used when the surviving CPU
> * timed out. This approach allows architectures to omit calls to
> * cpu_check_up_prepare() and cpu_set_state_online() without defeating
> * the next cpu_wait_death()'s polling loop.
> @@ -515,13 +559,13 @@ bool cpu_report_death(void)
>
> do {
> oldstate = atomic_read(&per_cpu(cpu_hotplug_state, cpu));
> - if (oldstate != CPU_BROKEN)
> - newstate = CPU_DEAD;
> + if (oldstate != CPUHP_BROKEN)
> + newstate = CPUHP_DEAD;
> else
> - newstate = CPU_DEAD_FROZEN;
> + newstate = CPUHP_TIMEOUT;
> } while (atomic_cmpxchg(&per_cpu(cpu_hotplug_state, cpu),
> oldstate, newstate) != oldstate);
> - return newstate == CPU_DEAD;
> + return newstate == CPUHP_DEAD;
> }
>
> #endif /* #ifdef CONFIG_HOTPLUG_CPU */
> --
> 2.4.3
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] smpboot: Add smpboot state variables instead of reusing CPU hotplug states
2015-11-07 7:17 ` Paul E. McKenney
@ 2015-11-12 14:34 ` Daniel Wagner
2015-11-12 14:34 ` Daniel Wagner
1 sibling, 0 replies; 8+ messages in thread
From: Daniel Wagner @ 2015-11-12 14:34 UTC (permalink / raw)
To: paulmck; +Cc: linux-kernel, xen-devel, Thomas Gleixner, Peter Zijlstra
Hi Paul,
On 11/07/2015 08:17 AM, Paul E. McKenney wrote:
> On Thu, Oct 15, 2015 at 01:32:44PM +0200, Daniel Wagner wrote:
>> The cpu hotplug state machine in smpboot.c is reusing the states from
>> cpu.h. That is confusing when it comes to the CPU_DEAD_FROZEN usage.
>> Paul explained to me that he was in need of an additional state
>> for destinguishing between a CPU error states. For this he just
>> picked CPU_DEAD_FROZEN.
>>
>> 8038dad7e888581266c76df15d70ca457a3c5910 smpboot: Add common code for notification from dying CPU
>> 2a442c9c6453d3d043dfd89f2e03a1deff8a6f06 x86: Use common outgoing-CPU-notification code
>>
>> Instead of reusing the states, let's add new definition inside
>> the smpboot.c file with explenation what those states
>> mean. Thanks Paul for providing them.
>>
>> Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
>
> Apologies for the delay, I didn't realize that you were waiting on me.
No apology needed. I didn't express my wish explicitly. I'll post it
again after the merge window is closed.
> Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Thanks,
Daniel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] smpboot: Add smpboot state variables instead of reusing CPU hotplug states
2015-11-07 7:17 ` Paul E. McKenney
2015-11-12 14:34 ` Daniel Wagner
@ 2015-11-12 14:34 ` Daniel Wagner
1 sibling, 0 replies; 8+ messages in thread
From: Daniel Wagner @ 2015-11-12 14:34 UTC (permalink / raw)
To: paulmck; +Cc: Peter Zijlstra, xen-devel, Thomas Gleixner, linux-kernel
Hi Paul,
On 11/07/2015 08:17 AM, Paul E. McKenney wrote:
> On Thu, Oct 15, 2015 at 01:32:44PM +0200, Daniel Wagner wrote:
>> The cpu hotplug state machine in smpboot.c is reusing the states from
>> cpu.h. That is confusing when it comes to the CPU_DEAD_FROZEN usage.
>> Paul explained to me that he was in need of an additional state
>> for destinguishing between a CPU error states. For this he just
>> picked CPU_DEAD_FROZEN.
>>
>> 8038dad7e888581266c76df15d70ca457a3c5910 smpboot: Add common code for notification from dying CPU
>> 2a442c9c6453d3d043dfd89f2e03a1deff8a6f06 x86: Use common outgoing-CPU-notification code
>>
>> Instead of reusing the states, let's add new definition inside
>> the smpboot.c file with explenation what those states
>> mean. Thanks Paul for providing them.
>>
>> Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
>
> Apologies for the delay, I didn't realize that you were waiting on me.
No apology needed. I didn't express my wish explicitly. I'll post it
again after the merge window is closed.
> Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Thanks,
Daniel
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] smpboot: Add smpboot state variables instead of reusing CPU hotplug states
@ 2015-10-15 11:32 Daniel Wagner
0 siblings, 0 replies; 8+ messages in thread
From: Daniel Wagner @ 2015-10-15 11:32 UTC (permalink / raw)
To: linux-kernel, xen-devel
Cc: Peter Zijlstra, Thomas Gleixner, Paul E. McKenney, Daniel Wagner
The cpu hotplug state machine in smpboot.c is reusing the states from
cpu.h. That is confusing when it comes to the CPU_DEAD_FROZEN usage.
Paul explained to me that he was in need of an additional state
for destinguishing between a CPU error states. For this he just
picked CPU_DEAD_FROZEN.
8038dad7e888581266c76df15d70ca457a3c5910 smpboot: Add common code for notification from dying CPU
2a442c9c6453d3d043dfd89f2e03a1deff8a6f06 x86: Use common outgoing-CPU-notification code
Instead of reusing the states, let's add new definition inside
the smpboot.c file with explenation what those states
mean. Thanks Paul for providing them.
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: xen-devel@lists.xenproject.org
Cc: linux-kernel@vger.kernel.org
---
arch/x86/xen/smp.c | 4 +--
include/linux/cpu.h | 3 +-
kernel/smpboot.c | 82 ++++++++++++++++++++++++++++++++++++++++-------------
3 files changed, 67 insertions(+), 22 deletions(-)
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 3f4ebf0..804bf5c 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -495,7 +495,7 @@ static int xen_cpu_up(unsigned int cpu, struct task_struct *idle)
rc = HYPERVISOR_vcpu_op(VCPUOP_up, cpu, NULL);
BUG_ON(rc);
- while (cpu_report_state(cpu) != CPU_ONLINE)
+ while (!cpu_check_online(cpu))
HYPERVISOR_sched_op(SCHEDOP_yield, NULL);
return 0;
@@ -767,7 +767,7 @@ static int xen_hvm_cpu_up(unsigned int cpu, struct task_struct *tidle)
* This can happen if CPU was offlined earlier and
* offlining timed out in common_cpu_die().
*/
- if (cpu_report_state(cpu) == CPU_DEAD_FROZEN) {
+ if (cpu_check_timeout(cpu)) {
xen_smp_intr_free(cpu);
xen_uninit_lock_cpu(cpu);
}
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 23c30bd..f78ab46 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -284,7 +284,8 @@ void arch_cpu_idle_dead(void);
DECLARE_PER_CPU(bool, cpu_dead_idle);
-int cpu_report_state(int cpu);
+int cpu_check_online(int cpu);
+int cpu_check_timeout(int cpu);
int cpu_check_up_prepare(int cpu);
void cpu_set_state_online(int cpu);
#ifdef CONFIG_HOTPLUG_CPU
diff --git a/kernel/smpboot.c b/kernel/smpboot.c
index a818cbc..75e5724 100644
--- a/kernel/smpboot.c
+++ b/kernel/smpboot.c
@@ -371,19 +371,63 @@ int smpboot_update_cpumask_percpu_thread(struct smp_hotplug_thread *plug_thread,
}
EXPORT_SYMBOL_GPL(smpboot_update_cpumask_percpu_thread);
+/* The CPU is offline, and its last offline operation was
+ * successful and proceeded normally. (Or, alternatively, the
+ * CPU never has come online, as this is the initial state.)
+ */
+#define CPUHP_POST_DEAD 0x01
+
+/* The CPU is in the process of coming online.
+ * Simple architectures can skip this state, and just invoke
+ * cpu_set_state_online() unconditionally instead.
+ */
+#define CPUHP_UP_PREPARE 0x02
+
+/* The CPU is now online. Simple architectures can skip this
+ * state, and just invoke cpu_wait_death() and cpu_report_death()
+ * unconditionally instead.
+ */
+#define CPUHP_ONLINE 0x03
+
+/* The CPU has gone offline, so that it may now be safely
+ * powered off (or whatever the architecture needs to do to it).
+ */
+#define CPUHP_DEAD 0x04
+
+/* The CPU did not go offline in a timely fashion, if at all,
+ * so it might need special processing at the next online (for
+ * example, simply refusing to bring it online).
+ */
+#define CPUHP_BROKEN 0x05
+
+/* The CPU eventually did go offline, but not in a timely
+ * fashion. If some sort of reset operation is required before it
+ * can be brought online, that reset operation needs to be carried
+ * out at online time. (Or, again, the architecture might simply
+ * refuse to bring it online.)
+ */
+#define CPUHP_TIMEOUT 0x06
+
static DEFINE_PER_CPU(atomic_t, cpu_hotplug_state) = ATOMIC_INIT(CPU_POST_DEAD);
/*
* Called to poll specified CPU's state, for example, when waiting for
* a CPU to come online.
*/
-int cpu_report_state(int cpu)
+int cpu_check_online(int cpu)
+{
+ return atomic_read(&per_cpu(cpu_hotplug_state, cpu)) ==
+ CPUHP_ONLINE;
+}
+
+int cpu_check_timeout(int cpu)
{
- return atomic_read(&per_cpu(cpu_hotplug_state, cpu));
+ return atomic_read(&per_cpu(cpu_hotplug_state, cpu)) ==
+ CPUHP_TIMEOUT;
}
/*
- * If CPU has died properly, set its state to CPU_UP_PREPARE and
+ * If CPU has died properly, set its state to CPUHP_UP_PREPARE and
* return success. Otherwise, return -EBUSY if the CPU died after
* cpu_wait_death() timed out. And yet otherwise again, return -EAGAIN
* if cpu_wait_death() timed out and the CPU still hasn't gotten around
@@ -397,19 +441,19 @@ int cpu_report_state(int cpu)
int cpu_check_up_prepare(int cpu)
{
if (!IS_ENABLED(CONFIG_HOTPLUG_CPU)) {
- atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPU_UP_PREPARE);
+ atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPUHP_UP_PREPARE);
return 0;
}
switch (atomic_read(&per_cpu(cpu_hotplug_state, cpu))) {
- case CPU_POST_DEAD:
+ case CPUHP_POST_DEAD:
/* The CPU died properly, so just start it up again. */
- atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPU_UP_PREPARE);
+ atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPUHP_UP_PREPARE);
return 0;
- case CPU_DEAD_FROZEN:
+ case CPUHP_TIMEOUT:
/*
* Timeout during CPU death, so let caller know.
@@ -424,7 +468,7 @@ int cpu_check_up_prepare(int cpu)
*/
return -EBUSY;
- case CPU_BROKEN:
+ case CPUHP_BROKEN:
/*
* The most likely reason we got here is that there was
@@ -452,7 +496,7 @@ int cpu_check_up_prepare(int cpu)
*/
void cpu_set_state_online(int cpu)
{
- (void)atomic_xchg(&per_cpu(cpu_hotplug_state, cpu), CPU_ONLINE);
+ (void)atomic_xchg(&per_cpu(cpu_hotplug_state, cpu), CPUHP_ONLINE);
}
#ifdef CONFIG_HOTPLUG_CPU
@@ -470,12 +514,12 @@ bool cpu_wait_death(unsigned int cpu, int seconds)
might_sleep();
/* The outgoing CPU will normally get done quite quickly. */
- if (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) == CPU_DEAD)
+ if (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) == CPUHP_DEAD)
goto update_state;
udelay(5);
/* But if the outgoing CPU dawdles, wait increasingly long times. */
- while (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) != CPU_DEAD) {
+ while (atomic_read(&per_cpu(cpu_hotplug_state, cpu)) != CPUHP_DEAD) {
schedule_timeout_uninterruptible(sleep_jf);
jf_left -= sleep_jf;
if (jf_left <= 0)
@@ -484,14 +528,14 @@ bool cpu_wait_death(unsigned int cpu, int seconds)
}
update_state:
oldstate = atomic_read(&per_cpu(cpu_hotplug_state, cpu));
- if (oldstate == CPU_DEAD) {
+ if (oldstate == CPUHP_DEAD) {
/* Outgoing CPU died normally, update state. */
smp_mb(); /* atomic_read() before update. */
- atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPU_POST_DEAD);
+ atomic_set(&per_cpu(cpu_hotplug_state, cpu), CPUHP_POST_DEAD);
} else {
/* Outgoing CPU still hasn't died, set state accordingly. */
if (atomic_cmpxchg(&per_cpu(cpu_hotplug_state, cpu),
- oldstate, CPU_BROKEN) != oldstate)
+ oldstate, CPUHP_BROKEN) != oldstate)
goto update_state;
ret = false;
}
@@ -502,7 +546,7 @@ update_state:
* Called by the outgoing CPU to report its successful death. Return
* false if this report follows the surviving CPU's timing out.
*
- * A separate "CPU_DEAD_FROZEN" is used when the surviving CPU
+ * A separate "CPUHP_TIMEOUT" is used when the surviving CPU
* timed out. This approach allows architectures to omit calls to
* cpu_check_up_prepare() and cpu_set_state_online() without defeating
* the next cpu_wait_death()'s polling loop.
@@ -515,13 +559,13 @@ bool cpu_report_death(void)
do {
oldstate = atomic_read(&per_cpu(cpu_hotplug_state, cpu));
- if (oldstate != CPU_BROKEN)
- newstate = CPU_DEAD;
+ if (oldstate != CPUHP_BROKEN)
+ newstate = CPUHP_DEAD;
else
- newstate = CPU_DEAD_FROZEN;
+ newstate = CPUHP_TIMEOUT;
} while (atomic_cmpxchg(&per_cpu(cpu_hotplug_state, cpu),
oldstate, newstate) != oldstate);
- return newstate == CPU_DEAD;
+ return newstate == CPUHP_DEAD;
}
#endif /* #ifdef CONFIG_HOTPLUG_CPU */
--
2.4.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-11-12 14:34 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-15 11:32 [PATCH] smpboot: Add smpboot state variables instead of reusing CPU hotplug states Daniel Wagner
2015-11-05 8:09 ` Daniel Wagner
2015-11-05 8:09 ` Daniel Wagner
2015-11-07 7:17 ` Paul E. McKenney
2015-11-12 14:34 ` Daniel Wagner
2015-11-12 14:34 ` Daniel Wagner
2015-11-07 7:17 ` Paul E. McKenney
2015-10-15 11:32 Daniel Wagner
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.