linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Wagner <daniel.wagner@bmw-carit.de>
To: linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org
Cc: Daniel Wagner <daniel.wagner@bmw-carit.de>,
	Thomas Gleixner <tglx@linutronix.de>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Peter Zijlstra <peterz@infradead.org>
Subject: [PATCH] smpboot: Add smpboot state variables instead of reusing CPU hotplug states
Date: Thu, 15 Oct 2015 13:32:44 +0200	[thread overview]
Message-ID: <1444908764-9057-1-git-send-email-daniel.wagner@bmw-carit.de> (raw)

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


             reply	other threads:[~2015-10-15 11:32 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-15 11:32 Daniel Wagner [this message]
2015-11-05  8:09 ` [PATCH] smpboot: Add smpboot state variables instead of reusing CPU hotplug states Daniel Wagner
2015-11-07  7:17 ` Paul E. McKenney
2015-11-12 14:34   ` Daniel Wagner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1444908764-9057-1-git-send-email-daniel.wagner@bmw-carit.de \
    --to=daniel.wagner@bmw-carit.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).