[RFC] [PATCH] cpu hotplug on power based systems.

[RFC] [PATCH] cpu hotplug on power based systems.

[Srinivasa Ds]

[-- Attachment #1: Type: text/plain, Size: 901 bytes --]


Linux kernel uses some of the rtas token to perform cpu hotplug on power 
systems. Some of the systems may not provide all the rtas services,which 
are required to perform cpu hotplug. Like for example
   1) JS20 doesn't provide "stop-self" token and cpu hotplug operations 
on these systems causes system to crash.
   2) some of the p630 systems doesn't provide "query-cpu-stopped-state" 
token and we are not sure of whether cpu is under stopped state or not  
or stop-self is still in progress .
So we can't take decision on whether cpu really has gone offline or not.

So we need to make sure that all required rtas tokens for cpu hotplug  
are available during rtas initialization phase and to disable cpu 
hotplug if they are not available.
I have developed the patch which does the above thing. Please let me 
know your comments on this.

Signed-off-by: Srinivasa DS <srinivasa@in.ibm.com>

 

[-- Attachment #2: cpu_final.fix --]
[-- Type: text/plain, Size: 4208 bytes --]

 arch/powerpc/kernel/rtas.c |    6 ++++++
 include/linux/cpu.h        |    4 ++++
 kernel/cpu.c               |   24 ++++++++++++++++++++++--
 3 files changed, 32 insertions(+), 2 deletions(-)

Index: linux-2.6.19-rc5/arch/powerpc/kernel/rtas.c
===================================================================
--- linux-2.6.19-rc5.orig/arch/powerpc/kernel/rtas.c
+++ linux-2.6.19-rc5/arch/powerpc/kernel/rtas.c
@@ -19,6 +19,7 @@
 #include <linux/init.h>
 #include <linux/capability.h>
 #include <linux/delay.h>
+#include <linux/cpu.h>
 
 #include <asm/prom.h>
 #include <asm/rtas.h>
@@ -843,6 +844,7 @@ void rtas_stop_self(void)
 void __init rtas_initialize(void)
 {
 	unsigned long rtas_region = RTAS_INSTANTIATE_MAX;
+	int qcss_tok;
 
 	/* Get RTAS dev node and fill up our "rtas" structure with infos
 	 * about it.
@@ -881,6 +883,10 @@ void __init rtas_initialize(void)
 
 #ifdef CONFIG_HOTPLUG_CPU
 	rtas_stop_self_args.token = rtas_token("stop-self");
+	qcss_tok = rtas_token("query-cpu-stopped-state");
+	if(rtas_stop_self_args.token == RTAS_UNKNOWN_SERVICE ||
+				 qcss_tok == RTAS_UNKNOWN_SERVICE)
+		disable_cpu_hotplug_perm();
 #endif /* CONFIG_HOTPLUG_CPU */
 #ifdef CONFIG_RTAS_ERROR_LOGGING
 	rtas_last_error_token = rtas_token("rtas-last-error");
Index: linux-2.6.19-rc5/kernel/cpu.c
===================================================================
--- linux-2.6.19-rc5.orig/kernel/cpu.c
+++ linux-2.6.19-rc5/kernel/cpu.c
@@ -63,8 +63,20 @@ void unlock_cpu_hotplug(void)
 }
 EXPORT_SYMBOL_GPL(unlock_cpu_hotplug);
 
+void disable_cpu_hotplug_perm(void)
+{
+	mutex_lock(&cpu_add_remove_lock);
+	cpu_hotplug_disabled = PERM_DISABLED_CPU_HOTPLUG;
+	mutex_unlock(&cpu_add_remove_lock);
+}
+
 #endif	/* CONFIG_HOTPLUG_CPU */
 
+static int is_cpu_hotplug_perm_disabled()
+{
+	return cpu_hotplug_disabled == PERM_DISABLED_CPU_HOTPLUG;
+}
+
 /* Need to know about CPUs going up/down? */
 int __cpuinit register_cpu_notifier(struct notifier_block *nb)
 {
@@ -193,7 +205,7 @@ int cpu_down(unsigned int cpu)
 	int err = 0;
 
 	mutex_lock(&cpu_add_remove_lock);
-	if (cpu_hotplug_disabled)
+	if (cpu_hotplug_disabled || is_cpu_hotplug_perm_disabled())
 		err = -EBUSY;
 	else
 		err = _cpu_down(cpu);
@@ -244,7 +256,7 @@ int __devinit cpu_up(unsigned int cpu)
 	int err = 0;
 
 	mutex_lock(&cpu_add_remove_lock);
-	if (cpu_hotplug_disabled)
+	if (cpu_hotplug_disabled || is_cpu_hotplug_perm_disabled())
 		err = -EBUSY;
 	else
 		err = _cpu_up(cpu);
@@ -261,6 +273,10 @@ int disable_nonboot_cpus(void)
 	int cpu, first_cpu, error;
 
 	mutex_lock(&cpu_add_remove_lock);
+	if(is_cpu_hotplug_perm_disabled()) {
+		error = -EBUSY;
+		goto out;
+	}
 	first_cpu = first_cpu(cpu_present_map);
 	if (!cpu_online(first_cpu)) {
 		error = _cpu_up(first_cpu);
@@ -311,6 +327,10 @@ void enable_nonboot_cpus(void)
 
 	/* Allow everyone to use the CPU hotplug again */
 	mutex_lock(&cpu_add_remove_lock);
+	if(is_cpu_hotplug_perm_disabled()) {
+		mutex_unlock(&cpu_add_remove_lock);
+		return;
+	}
 	cpu_hotplug_disabled = 0;
 	mutex_unlock(&cpu_add_remove_lock);
 
Index: linux-2.6.19-rc5/include/linux/cpu.h
===================================================================
--- linux-2.6.19-rc5.orig/include/linux/cpu.h
+++ linux-2.6.19-rc5/include/linux/cpu.h
@@ -31,6 +31,8 @@ struct cpu {
 	struct sys_device sysdev;
 };
 
+#define  PERM_DISABLED_CPU_HOTPLUG -1
+
 extern int register_cpu(struct cpu *cpu, int num);
 extern struct sys_device *get_cpu_sysdev(unsigned cpu);
 #ifdef CONFIG_HOTPLUG_CPU
@@ -68,6 +70,7 @@ extern struct sysdev_class cpu_sysdev_cl
 /* Stop CPUs going up and down. */
 extern void lock_cpu_hotplug(void);
 extern void unlock_cpu_hotplug(void);
+extern void disable_cpu_hotplug_perm(void);
 #define hotcpu_notifier(fn, pri) {				\
 	static struct notifier_block fn##_nb =			\
 		{ .notifier_call = fn, .priority = pri };	\
@@ -80,6 +83,7 @@ int cpu_down(unsigned int cpu);
 #else
 #define lock_cpu_hotplug()	do { } while (0)
 #define unlock_cpu_hotplug()	do { } while (0)
+#define disable_cpu_hotplug_perm()     do { } while (0)
 #define lock_cpu_hotplug_interruptible() 0
 #define hotcpu_notifier(fn, pri)	do { } while (0)
 #define register_hotcpu_notifier(nb)	do { } while (0)

Re: [RFC] [PATCH] cpu hotplug on power based systems.

[Nathan Lynch]

Srinivasa Ds wrote:
> 
> Linux kernel uses some of the rtas token to perform cpu hotplug on power 
> systems. Some of the systems may not provide all the rtas services,which 
> are required to perform cpu hotplug. Like for example
>   1) JS20 doesn't provide "stop-self" token and cpu hotplug operations 
> on these systems causes system to crash.
>   2) some of the p630 systems doesn't provide "query-cpu-stopped-state" 
> token and we are not sure of whether cpu is under stopped state or not  
> or stop-self is still in progress .

Neither of these systems (well, the p630 in "SMP" mode) have
hypervisors so their firmwares don't provide the rtas primitives for
CPU offline.

> So we can't take decision on whether cpu really has gone offline or not.
> 
> So we need to make sure that all required rtas tokens for cpu hotplug  
> are available during rtas initialization phase and to disable cpu 
> hotplug if they are not available.
> I have developed the patch which does the above thing. Please let me 
> know your comments on this.

Would be better to either a) inhibit creation of the sysfs 'online'
cpu attributes (I thought we used to handle this correctly on these
systems) or b) use the generic cpu hotplug operations which have no
dependency on RTAS.

Re: [RFC] [PATCH] cpu hotplug on power based systems.

[Linas Vepstas]

On Thu, Nov 16, 2006 at 09:40:51AM -0600, Nathan Lynch wrote:
> Srinivasa Ds wrote:
> > 
> > Linux kernel uses some of the rtas token to perform cpu hotplug on power 
> > systems. Some of the systems may not provide all the rtas services,which 
> > are required to perform cpu hotplug. Like for example
> >   1) JS20 doesn't provide "stop-self" token and cpu hotplug operations 
> > on these systems causes system to crash.
> >   2) some of the p630 systems doesn't provide "query-cpu-stopped-state" 
> > token and we are not sure of whether cpu is under stopped state or not  
> > or stop-self is still in progress .
> 
> Neither of these systems (well, the p630 in "SMP" mode) have
> hypervisors so their firmwares don't provide the rtas primitives for
> CPU offline.
> 
> > So we can't take decision on whether cpu really has gone offline or not.
> > 
> > So we need to make sure that all required rtas tokens for cpu hotplug  
> > are available during rtas initialization phase and to disable cpu 
> > hotplug if they are not available.
> > I have developed the patch which does the above thing. Please let me 
> > know your comments on this.
> 
> Would be better to either a) inhibit creation of the sysfs 'online'
> cpu attributes (I thought we used to handle this correctly on these
> systems) or b) use the generic cpu hotplug operations which have no
> dependency on RTAS.

Yeah, good point. It seemes cleaner to do this, than to add a new
state ("per disabled") that adds complexity to the generic kernel code. 

Perhaps we could set this up so that, if rtas "stop-self" is not
defined, then generic_cpu_die() is installed instead of pseries_cpu_die()?

--linas

[PATCH] Reorganise and then fixup the pseries cpu hotplug code

[Michael Ellerman]

The pseries cpu hotplug code is currently spread between ./kernel/rtas.c,
./platforms/pseries/smp.c and ./platforms/pseries/setup.c. Some of it is
not #ifdef CONFIG_HOTPLUG_CPU, but it should be.

So move all the cpu hotplug code into platforms/pseries/hotplug-cpu.c
While we're moving, rename studly caps functions to normal caps, they're
all static so no harm done. Fixup some long lines also, and make things
static that can be, now we're all in the same file.

Currently we unconditionally hookup pSeries_mach_cpu_die to ppc_md.cpu_die,
even if we don't have CONFIG_HOTPLUG_CPU enabled. This is wrong, as it
signals the sysfs code to create the online attribute for cpu nodes,
allowing the user to attempt an offline when it's not actually supported.

There is also a problem on systems that don't have the correct RTAS tokens
available to do RTAS-based cpu hotplug, we still indicate via sysfs that
we support cpu hotplug - and then attempt to do so with missing RTAS tokens.

Both problems are solved by conditionally registering the cpu hotplug
callbacks, only when CONFIG_HOTPLUG_CPU is enabled, and only after we've
found the requisite RTAS tokens.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---

OK, what do people think of this? I think it solves the problems we've
seen lately. This supersedes Linas patch to wrap the pSeries bits in
CONFIG_HOTPLUG_CPU.

I haven't tested this - if someone can that'd be great - otherwise I'll
have a go on Monday.

---
 arch/powerpc/kernel/rtas.c                   |   28 --
 arch/powerpc/platforms/pseries/Makefile      |    2 
 arch/powerpc/platforms/pseries/hotplug-cpu.c |  272 +++++++++++++++++++++++++++
 arch/powerpc/platforms/pseries/setup.c       |   12 -
 arch/powerpc/platforms/pseries/smp.c         |  199 -------------------
 include/asm-powerpc/rtas.h                   |    4 
 6 files changed, 274 insertions(+), 243 deletions(-)

Index: powerpc/arch/powerpc/kernel/rtas.c
===================================================================
--- powerpc.orig/arch/powerpc/kernel/rtas.c
+++ powerpc/arch/powerpc/kernel/rtas.c
@@ -810,31 +810,6 @@ asmlinkage int ppc_rtas(struct rtas_args
 	return 0;
 }
 
-/* This version can't take the spinlock, because it never returns */
-
-struct rtas_args rtas_stop_self_args = {
-	/* The token is initialized for real in setup_system() */
-	.token = RTAS_UNKNOWN_SERVICE,
-	.nargs = 0,
-	.nret = 1,
-	.rets = &rtas_stop_self_args.args[0],
-};
-
-void rtas_stop_self(void)
-{
-	struct rtas_args *rtas_args = &rtas_stop_self_args;
-
-	local_irq_disable();
-
-	BUG_ON(rtas_args->token == RTAS_UNKNOWN_SERVICE);
-
-	printk("cpu %u (hwid %u) Ready to die...\n",
-	       smp_processor_id(), hard_smp_processor_id());
-	enter_rtas(__pa(rtas_args));
-
-	panic("Alas, I survived.\n");
-}
-
 /*
  * Call early during boot, before mem init or bootmem, to retrieve the RTAS
  * informations from the device-tree and allocate the RMO buffer for userland
@@ -879,9 +854,6 @@ void __init rtas_initialize(void)
 #endif
 	rtas_rmo_buf = lmb_alloc_base(RTAS_RMOBUF_MAX, PAGE_SIZE, rtas_region);
 
-#ifdef CONFIG_HOTPLUG_CPU
-	rtas_stop_self_args.token = rtas_token("stop-self");
-#endif /* CONFIG_HOTPLUG_CPU */
 #ifdef CONFIG_RTAS_ERROR_LOGGING
 	rtas_last_error_token = rtas_token("rtas-last-error");
 #endif
Index: powerpc/arch/powerpc/platforms/pseries/Makefile
===================================================================
--- powerpc.orig/arch/powerpc/platforms/pseries/Makefile
+++ powerpc/arch/powerpc/platforms/pseries/Makefile
@@ -10,6 +10,8 @@ obj-$(CONFIG_XICS)	+= xics.o
 obj-$(CONFIG_SCANLOG)	+= scanlog.o
 obj-$(CONFIG_EEH)	+= eeh.o eeh_cache.o eeh_driver.o eeh_event.o
 
+obj-$(CONFIG_HOTPLUG_CPU)	+= hotplug-cpu.o
+
 obj-$(CONFIG_HVC_CONSOLE)	+= hvconsole.o
 obj-$(CONFIG_HVCS)		+= hvcserver.o
 obj-$(CONFIG_HCALL_STATS)	+= hvCall_inst.o
Index: powerpc/arch/powerpc/platforms/pseries/hotplug-cpu.c
===================================================================
--- /dev/null
+++ powerpc/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -0,0 +1,272 @@
+/*
+ * pseries CPU Hotplug infrastructure.
+ *
+ * Split out from arch/powerpc/platforms/pseries/setup.c
+ *  arch/powerpc/kernel/rtas.c, and arch/powerpc/platforms/pseries/smp.c
+ *
+ * Peter Bergner, IBM	March 2001.
+ * Copyright (C) 2001 IBM.
+ * Dave Engebretsen, Peter Bergner, and
+ * Mike Corrigan {engebret|bergner|mikec}@us.ibm.com
+ * Plus various changes from other IBM teams...
+ *
+ * Copyright (C) 2006 Michael Ellerman, IBM Corporation
+ *
+ *      This program is free software; you can redistribute it and/or
+ *      modify it under the terms of the GNU General Public License
+ *      as published by the Free Software Foundation; either version
+ *      2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include <linux/delay.h>
+#include <linux/cpu.h>
+#include <asm/system.h>
+#include <asm/prom.h>
+#include <asm/rtas.h>
+#include <asm/firmware.h>
+#include <asm/machdep.h>
+#include <asm/vdso_datapage.h>
+#include <asm/pSeries_reconfig.h>
+#include "xics.h"
+
+/* This version can't take the spinlock, because it never returns */
+static struct rtas_args rtas_stop_self_args = {
+	.token = RTAS_UNKNOWN_SERVICE,
+	.nargs = 0,
+	.nret = 1,
+	.rets = &rtas_stop_self_args.args[0],
+};
+
+static void rtas_stop_self(void)
+{
+	struct rtas_args *args = &rtas_stop_self_args;
+
+	local_irq_disable();
+
+	BUG_ON(args->token == RTAS_UNKNOWN_SERVICE);
+
+	printk("cpu %u (hwid %u) Ready to die...\n",
+	       smp_processor_id(), hard_smp_processor_id());
+	enter_rtas(__pa(args));
+
+	panic("Alas, I survived.\n");
+}
+
+static void pseries_mach_cpu_die(void)
+{
+	local_irq_disable();
+	idle_task_exit();
+	xics_teardown_cpu(0);
+	rtas_stop_self();
+	/* Should never get here... */
+	BUG();
+	for(;;);
+}
+
+static int qcss_tok;	/* query-cpu-stopped-state token */
+
+/* Get state of physical CPU.
+ * Return codes:
+ *	0	- The processor is in the RTAS stopped state
+ *	1	- stop-self is in progress
+ *	2	- The processor is not in the RTAS stopped state
+ *	-1	- Hardware Error
+ *	-2	- Hardware Busy, Try again later.
+ */
+static int query_cpu_stopped(unsigned int pcpu)
+{
+	int cpu_status, status;
+
+	status = rtas_call(qcss_tok, 1, 2, &cpu_status, pcpu);
+	if (status != 0) {
+		printk(KERN_ERR
+		       "RTAS query-cpu-stopped-state failed: %i\n", status);
+		return status;
+	}
+
+	return cpu_status;
+}
+
+static int pseries_cpu_disable(void)
+{
+	int cpu = smp_processor_id();
+
+	cpu_clear(cpu, cpu_online_map);
+	vdso_data->processorCount--;
+
+	/*fix boot_cpuid here*/
+	if (cpu == boot_cpuid)
+		boot_cpuid = any_online_cpu(cpu_online_map);
+
+	/* FIXME: abstract this to not be platform specific later on */
+	xics_migrate_irqs_away();
+	return 0;
+}
+
+static void pseries_cpu_die(unsigned int cpu)
+{
+	int tries;
+	int cpu_status;
+	unsigned int pcpu = get_hard_smp_processor_id(cpu);
+
+	for (tries = 0; tries < 25; tries++) {
+		cpu_status = query_cpu_stopped(pcpu);
+		if (cpu_status == 0 || cpu_status == -1)
+			break;
+		msleep(200);
+	}
+	if (cpu_status != 0) {
+		printk("Querying DEAD? cpu %i (%i) shows %i\n",
+		       cpu, pcpu, cpu_status);
+	}
+
+	/* Isolation and deallocation are definatly done by
+	 * drslot_chrp_cpu.  If they were not they would be
+	 * done here.  Change isolate state to Isolate and
+	 * change allocation-state to Unusable.
+	 */
+	paca[cpu].cpu_start = 0;
+}
+
+/*
+ * Update cpu_present_map and paca(s) for a new cpu node.  The wrinkle
+ * here is that a cpu device node may represent up to two logical cpus
+ * in the SMT case.  We must honor the assumption in other code that
+ * the logical ids for sibling SMT threads x and y are adjacent, such
+ * that x^1 == y and y^1 == x.
+ */
+static int pseries_add_processor(struct device_node *np)
+{
+	unsigned int cpu;
+	cpumask_t candidate_map, tmp = CPU_MASK_NONE;
+	int err = -ENOSPC, len, nthreads, i;
+	const u32 *intserv;
+
+	intserv = get_property(np, "ibm,ppc-interrupt-server#s", &len);
+	if (!intserv)
+		return 0;
+
+	nthreads = len / sizeof(u32);
+	for (i = 0; i < nthreads; i++)
+		cpu_set(i, tmp);
+
+	lock_cpu_hotplug();
+
+	BUG_ON(!cpus_subset(cpu_present_map, cpu_possible_map));
+
+	/* Get a bitmap of unoccupied slots. */
+	cpus_xor(candidate_map, cpu_possible_map, cpu_present_map);
+	if (cpus_empty(candidate_map)) {
+		/* If we get here, it most likely means that NR_CPUS is
+		 * less than the partition's max processors setting.
+		 */
+		printk(KERN_ERR "Cannot add cpu %s; this system configuration"
+		       " supports %d logical cpus.\n", np->full_name,
+		       cpus_weight(cpu_possible_map));
+		goto out_unlock;
+	}
+
+	while (!cpus_empty(tmp))
+		if (cpus_subset(tmp, candidate_map))
+			/* Found a range where we can insert the new cpu(s) */
+			break;
+		else
+			cpus_shift_left(tmp, tmp, nthreads);
+
+	if (cpus_empty(tmp)) {
+		printk(KERN_ERR "Unable to find space in cpu_present_map for"
+		       " processor %s with %d thread(s)\n", np->name,
+		       nthreads);
+		goto out_unlock;
+	}
+
+	for_each_cpu_mask(cpu, tmp) {
+		BUG_ON(cpu_isset(cpu, cpu_present_map));
+		cpu_set(cpu, cpu_present_map);
+		set_hard_smp_processor_id(cpu, *intserv++);
+	}
+	err = 0;
+out_unlock:
+	unlock_cpu_hotplug();
+	return err;
+}
+
+/*
+ * Update the present map for a cpu node which is going away, and set
+ * the hard id in the paca(s) to -1 to be consistent with boot time
+ * convention for non-present cpus.
+ */
+static void pseries_remove_processor(struct device_node *np)
+{
+	unsigned int cpu;
+	int len, nthreads, i;
+	const u32 *intserv;
+
+	intserv = get_property(np, "ibm,ppc-interrupt-server#s", &len);
+	if (!intserv)
+		return;
+
+	nthreads = len / sizeof(u32);
+
+	lock_cpu_hotplug();
+	for (i = 0; i < nthreads; i++) {
+		for_each_present_cpu(cpu) {
+			if (get_hard_smp_processor_id(cpu) != intserv[i])
+				continue;
+			BUG_ON(cpu_online(cpu));
+			cpu_clear(cpu, cpu_present_map);
+			set_hard_smp_processor_id(cpu, -1);
+			break;
+		}
+		if (cpu == NR_CPUS)
+			printk(KERN_WARNING "Could not find cpu to remove "
+			       "with physical id 0x%x\n", intserv[i]);
+	}
+	unlock_cpu_hotplug();
+}
+
+static int pseries_smp_notifier(struct notifier_block *nb,
+		unsigned long action, void *node)
+{
+	int err = NOTIFY_OK;
+
+	switch (action) {
+	case PSERIES_RECONFIG_ADD:
+		if (pseries_add_processor(node))
+			err = NOTIFY_BAD;
+		break;
+	case PSERIES_RECONFIG_REMOVE:
+		pseries_remove_processor(node);
+		break;
+	default:
+		err = NOTIFY_DONE;
+		break;
+	}
+	return err;
+}
+
+static struct notifier_block pseries_smp_nb = {
+	.notifier_call = pseries_smp_notifier,
+};
+
+static int __init pseries_cpu_hotplug_init(void)
+{
+	rtas_stop_self_args.token = rtas_token("stop-self");
+	qcss_tok = rtas_token("query-cpu-stopped-state");
+
+	if (rtas_stop_self_args.token == RTAS_UNKNOWN_SERVICE ||
+			qcss_tok == RTAS_UNKNOWN_SERVICE)
+		return 1;
+
+	ppc_md.cpu_die = pseries_mach_cpu_die;
+	smp_ops->cpu_disable = pseries_cpu_disable;
+	smp_ops->cpu_die = pseries_cpu_die;
+
+	/* Processors can be added/removed only on LPAR */
+	if (firmware_has_feature(FW_FEATURE_LPAR))
+		pSeries_reconfig_notifier_register(&pseries_smp_nb);
+
+	return 0;
+}
+arch_initcall(pseries_cpu_hotplug_init);
Index: powerpc/arch/powerpc/platforms/pseries/setup.c
===================================================================
--- powerpc.orig/arch/powerpc/platforms/pseries/setup.c
+++ powerpc/arch/powerpc/platforms/pseries/setup.c
@@ -347,17 +347,6 @@ static int __init pSeries_init_panel(voi
 }
 arch_initcall(pSeries_init_panel);
 
-static void pSeries_mach_cpu_die(void)
-{
-	local_irq_disable();
-	idle_task_exit();
-	xics_teardown_cpu(0);
-	rtas_stop_self();
-	/* Should never get here... */
-	BUG();
-	for(;;);
-}
-
 static int pseries_set_dabr(unsigned long dabr)
 {
 	return plpar_hcall_norets(H_SET_DABR, dabr);
@@ -558,7 +547,6 @@ define_machine(pseries) {
 	.power_off		= rtas_power_off,
 	.halt			= rtas_halt,
 	.panic			= rtas_os_term,
-	.cpu_die		= pSeries_mach_cpu_die,
 	.get_boot_time		= rtas_get_boot_time,
 	.get_rtc_time		= rtas_get_rtc_time,
 	.set_rtc_time		= rtas_set_rtc_time,
Index: powerpc/arch/powerpc/platforms/pseries/smp.c
===================================================================
--- powerpc.orig/arch/powerpc/platforms/pseries/smp.c
+++ powerpc/arch/powerpc/platforms/pseries/smp.c
@@ -64,196 +64,6 @@ static cpumask_t of_spin_map;
 
 extern void generic_secondary_smp_init(unsigned long);
 
-#ifdef CONFIG_HOTPLUG_CPU
-
-/* Get state of physical CPU.
- * Return codes:
- *	0	- The processor is in the RTAS stopped state
- *	1	- stop-self is in progress
- *	2	- The processor is not in the RTAS stopped state
- *	-1	- Hardware Error
- *	-2	- Hardware Busy, Try again later.
- */
-static int query_cpu_stopped(unsigned int pcpu)
-{
-	int cpu_status;
-	int status, qcss_tok;
-
-	qcss_tok = rtas_token("query-cpu-stopped-state");
-	if (qcss_tok == RTAS_UNKNOWN_SERVICE)
-		return -1;
-	status = rtas_call(qcss_tok, 1, 2, &cpu_status, pcpu);
-	if (status != 0) {
-		printk(KERN_ERR
-		       "RTAS query-cpu-stopped-state failed: %i\n", status);
-		return status;
-	}
-
-	return cpu_status;
-}
-
-static int pSeries_cpu_disable(void)
-{
-	int cpu = smp_processor_id();
-
-	cpu_clear(cpu, cpu_online_map);
-	vdso_data->processorCount--;
-
-	/*fix boot_cpuid here*/
-	if (cpu == boot_cpuid)
-		boot_cpuid = any_online_cpu(cpu_online_map);
-
-	/* FIXME: abstract this to not be platform specific later on */
-	xics_migrate_irqs_away();
-	return 0;
-}
-
-static void pSeries_cpu_die(unsigned int cpu)
-{
-	int tries;
-	int cpu_status;
-	unsigned int pcpu = get_hard_smp_processor_id(cpu);
-
-	for (tries = 0; tries < 25; tries++) {
-		cpu_status = query_cpu_stopped(pcpu);
-		if (cpu_status == 0 || cpu_status == -1)
-			break;
-		msleep(200);
-	}
-	if (cpu_status != 0) {
-		printk("Querying DEAD? cpu %i (%i) shows %i\n",
-		       cpu, pcpu, cpu_status);
-	}
-
-	/* Isolation and deallocation are definatly done by
-	 * drslot_chrp_cpu.  If they were not they would be
-	 * done here.  Change isolate state to Isolate and
-	 * change allocation-state to Unusable.
-	 */
-	paca[cpu].cpu_start = 0;
-}
-
-/*
- * Update cpu_present_map and paca(s) for a new cpu node.  The wrinkle
- * here is that a cpu device node may represent up to two logical cpus
- * in the SMT case.  We must honor the assumption in other code that
- * the logical ids for sibling SMT threads x and y are adjacent, such
- * that x^1 == y and y^1 == x.
- */
-static int pSeries_add_processor(struct device_node *np)
-{
-	unsigned int cpu;
-	cpumask_t candidate_map, tmp = CPU_MASK_NONE;
-	int err = -ENOSPC, len, nthreads, i;
-	const u32 *intserv;
-
-	intserv = get_property(np, "ibm,ppc-interrupt-server#s", &len);
-	if (!intserv)
-		return 0;
-
-	nthreads = len / sizeof(u32);
-	for (i = 0; i < nthreads; i++)
-		cpu_set(i, tmp);
-
-	lock_cpu_hotplug();
-
-	BUG_ON(!cpus_subset(cpu_present_map, cpu_possible_map));
-
-	/* Get a bitmap of unoccupied slots. */
-	cpus_xor(candidate_map, cpu_possible_map, cpu_present_map);
-	if (cpus_empty(candidate_map)) {
-		/* If we get here, it most likely means that NR_CPUS is
-		 * less than the partition's max processors setting.
-		 */
-		printk(KERN_ERR "Cannot add cpu %s; this system configuration"
-		       " supports %d logical cpus.\n", np->full_name,
-		       cpus_weight(cpu_possible_map));
-		goto out_unlock;
-	}
-
-	while (!cpus_empty(tmp))
-		if (cpus_subset(tmp, candidate_map))
-			/* Found a range where we can insert the new cpu(s) */
-			break;
-		else
-			cpus_shift_left(tmp, tmp, nthreads);
-
-	if (cpus_empty(tmp)) {
-		printk(KERN_ERR "Unable to find space in cpu_present_map for"
-		       " processor %s with %d thread(s)\n", np->name,
-		       nthreads);
-		goto out_unlock;
-	}
-
-	for_each_cpu_mask(cpu, tmp) {
-		BUG_ON(cpu_isset(cpu, cpu_present_map));
-		cpu_set(cpu, cpu_present_map);
-		set_hard_smp_processor_id(cpu, *intserv++);
-	}
-	err = 0;
-out_unlock:
-	unlock_cpu_hotplug();
-	return err;
-}
-
-/*
- * Update the present map for a cpu node which is going away, and set
- * the hard id in the paca(s) to -1 to be consistent with boot time
- * convention for non-present cpus.
- */
-static void pSeries_remove_processor(struct device_node *np)
-{
-	unsigned int cpu;
-	int len, nthreads, i;
-	const u32 *intserv;
-
-	intserv = get_property(np, "ibm,ppc-interrupt-server#s", &len);
-	if (!intserv)
-		return;
-
-	nthreads = len / sizeof(u32);
-
-	lock_cpu_hotplug();
-	for (i = 0; i < nthreads; i++) {
-		for_each_present_cpu(cpu) {
-			if (get_hard_smp_processor_id(cpu) != intserv[i])
-				continue;
-			BUG_ON(cpu_online(cpu));
-			cpu_clear(cpu, cpu_present_map);
-			set_hard_smp_processor_id(cpu, -1);
-			break;
-		}
-		if (cpu == NR_CPUS)
-			printk(KERN_WARNING "Could not find cpu to remove "
-			       "with physical id 0x%x\n", intserv[i]);
-	}
-	unlock_cpu_hotplug();
-}
-
-static int pSeries_smp_notifier(struct notifier_block *nb, unsigned long action, void *node)
-{
-	int err = NOTIFY_OK;
-
-	switch (action) {
-	case PSERIES_RECONFIG_ADD:
-		if (pSeries_add_processor(node))
-			err = NOTIFY_BAD;
-		break;
-	case PSERIES_RECONFIG_REMOVE:
-		pSeries_remove_processor(node);
-		break;
-	default:
-		err = NOTIFY_DONE;
-		break;
-	}
-	return err;
-}
-
-static struct notifier_block pSeries_smp_nb = {
-	.notifier_call = pSeries_smp_notifier,
-};
-
-#endif /* CONFIG_HOTPLUG_CPU */
 
 /**
  * smp_startup_cpu() - start the given cpu
@@ -422,15 +232,6 @@ static void __init smp_init_pseries(void
 
 	DBG(" -> smp_init_pSeries()\n");
 
-#ifdef CONFIG_HOTPLUG_CPU
-	smp_ops->cpu_disable = pSeries_cpu_disable;
-	smp_ops->cpu_die = pSeries_cpu_die;
-
-	/* Processors can be added/removed only on LPAR */
-	if (firmware_has_feature(FW_FEATURE_LPAR))
-		pSeries_reconfig_notifier_register(&pSeries_smp_nb);
-#endif
-
 	/* Mark threads which are still spinning in hold loops. */
 	if (cpu_has_feature(CPU_FTR_SMT)) {
 		for_each_present_cpu(i) { 
Index: powerpc/include/asm-powerpc/rtas.h
===================================================================
--- powerpc.orig/include/asm-powerpc/rtas.h
+++ powerpc/include/asm-powerpc/rtas.h
@@ -54,8 +54,6 @@ struct rtas_args {
 	rtas_arg_t *rets;     /* Pointer to return values in args[]. */
 };  
 
-extern struct rtas_args rtas_stop_self_args;
-
 struct rtas_t {
 	unsigned long entry;		/* physical address pointer */
 	unsigned long base;		/* physical address pointer */
@@ -223,8 +221,6 @@ extern int rtas_get_error_log_max(void);
 extern spinlock_t rtas_data_buf_lock;
 extern char rtas_data_buf[RTAS_DATA_BUF_SIZE];
 
-extern void rtas_stop_self(void);
-
 /* RMO buffer reserved for user-space RTAS use */
 extern unsigned long rtas_rmo_buf;
 

Re: [PATCH] Reorganise and then fixup the pseries cpu hotplug code

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 2130 bytes --]

On Fri, 2006-11-17 at 14:36 +1100, Michael Ellerman wrote:
> The pseries cpu hotplug code is currently spread between ./kernel/rtas.c,
> ./platforms/pseries/smp.c and ./platforms/pseries/setup.c. Some of it is
> not #ifdef CONFIG_HOTPLUG_CPU, but it should be.
> 
> So move all the cpu hotplug code into platforms/pseries/hotplug-cpu.c
> While we're moving, rename studly caps functions to normal caps, they're
> all static so no harm done. Fixup some long lines also, and make things
> static that can be, now we're all in the same file.
> 
> Currently we unconditionally hookup pSeries_mach_cpu_die to ppc_md.cpu_die,
> even if we don't have CONFIG_HOTPLUG_CPU enabled. This is wrong, as it
> signals the sysfs code to create the online attribute for cpu nodes,
> allowing the user to attempt an offline when it's not actually supported.
> 
> There is also a problem on systems that don't have the correct RTAS tokens
> available to do RTAS-based cpu hotplug, we still indicate via sysfs that
> we support cpu hotplug - and then attempt to do so with missing RTAS tokens.
> 
> Both problems are solved by conditionally registering the cpu hotplug
> callbacks, only when CONFIG_HOTPLUG_CPU is enabled, and only after we've
> found the requisite RTAS tokens.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> ---
> 
> OK, what do people think of this? I think it solves the problems we've
> seen lately. This supersedes Linas patch to wrap the pSeries bits in
> CONFIG_HOTPLUG_CPU.
> 
> I haven't tested this - if someone can that'd be great - otherwise I'll
> have a go on Monday.

I should add .. The callbacks now get setup in an arch_initcall() - this
is not as early, by a long shot, as it used to be - however AFAICT it
should make no difference as there's no way to trigger a cpu hotplug
until later on anyway.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Reorganise and then fixup the pseries cpu hotplug code

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 641 bytes --]

One small nit ...

On Fri, 17 Nov 2006 14:36:35 +1100 Michael Ellerman <michael@ellerman.id.au> wrote:
>
> +static int __init pseries_cpu_hotplug_init(void)
> +{
> +	rtas_stop_self_args.token = rtas_token("stop-self");
> +	qcss_tok = rtas_token("query-cpu-stopped-state");
> +
> +	if (rtas_stop_self_args.token == RTAS_UNKNOWN_SERVICE ||
> +			qcss_tok == RTAS_UNKNOWN_SERVICE)
> +		return 1;

initcall fucntions should return 0 or -<error>;  -ENODEV is ignored,
other nonzero values cause a log message if initcall debugging is enabled.

--
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Reorganise and then fixup the pseries cpu hotplug code

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1093 bytes --]

On Fri, 2006-11-17 at 15:31 +1100, Stephen Rothwell wrote:
> One small nit ...
> 
> On Fri, 17 Nov 2006 14:36:35 +1100 Michael Ellerman <michael@ellerman.id.au> wrote:
> >
> > +static int __init pseries_cpu_hotplug_init(void)
> > +{
> > +	rtas_stop_self_args.token = rtas_token("stop-self");
> > +	qcss_tok = rtas_token("query-cpu-stopped-state");
> > +
> > +	if (rtas_stop_self_args.token == RTAS_UNKNOWN_SERVICE ||
> > +			qcss_tok == RTAS_UNKNOWN_SERVICE)
> > +		return 1;
> 
> initcall fucntions should return 0 or -<error>;  -ENODEV is ignored,
> other nonzero values cause a log message if initcall debugging is enabled.

OK. I wanted to return an error, so there's something in the log to show
that cpu hotplug was disabled - but I didn't check what to return. I'll
fix it up on Monday to return -ENOENT.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Reorganise and then fixup the pseries cpu hotplug code

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 741 bytes --]

On Fri, 17 Nov 2006 15:44:00 +1100 Michael Ellerman <michael@ellerman.id.au> wrote:
>
> > initcall fucntions should return 0 or -<error>;  -ENODEV is ignored,
> > other nonzero values cause a log message if initcall debugging is enabled.
>
> OK. I wanted to return an error, so there's something in the log to show
> that cpu hotplug was disabled - but I didn't check what to return. I'll
> fix it up on Monday to return -ENOENT.

Which also won't do anything unless initcall debugging is enabled and,
even then, will just give an obscure message in the boot logs.  If you
want to log something, then you should explicitly call printk().

--
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Reorganise and then fixup the pseries cpu hotplug code

[Linas Vepstas]

On Fri, Nov 17, 2006 at 02:36:35PM +1100, Michael Ellerman wrote:
> The pseries cpu hotplug code is currently spread between ./kernel/rtas.c,
> ./platforms/pseries/smp.c and ./platforms/pseries/setup.c. Some of it is
> not #ifdef CONFIG_HOTPLUG_CPU, but it should be.
> 
> So move all the cpu hotplug code into platforms/pseries/hotplug-cpu.c

Yes, much cleaner this way.

> +	/* Isolation and deallocation are definatly done by
                                        ^^^^^^^^^
typo

> +	 * drslot_chrp_cpu.  If they were not they would be
> +	 * done here.  Change isolate state to Isolate and
> +	 * change allocation-state to Unusable.
> +	 */
> +	paca[cpu].cpu_start = 0;

I can't figure out what the comment means with respect 
to this code ... 

--linas

Re: [PATCH] Reorganise and then fixup the pseries cpu hotplug code

[Linas Vepstas]

On Fri, Nov 17, 2006 at 03:44:00PM +1100, Michael Ellerman wrote:
> On Fri, 2006-11-17 at 15:31 +1100, Stephen Rothwell wrote:
> > One small nit ...
> > 
> > On Fri, 17 Nov 2006 14:36:35 +1100 Michael Ellerman <michael@ellerman.id.au> wrote:
> > >
> > > +static int __init pseries_cpu_hotplug_init(void)
> > > +{
> > > +	rtas_stop_self_args.token = rtas_token("stop-self");
> > > +	qcss_tok = rtas_token("query-cpu-stopped-state");
> > > +
> > > +	if (rtas_stop_self_args.token == RTAS_UNKNOWN_SERVICE ||
> > > +			qcss_tok == RTAS_UNKNOWN_SERVICE)
> > > +		return 1;
> > 
> > initcall fucntions should return 0 or -<error>;  -ENODEV is ignored,
> > other nonzero values cause a log message if initcall debugging is enabled.
> 
> OK. I wanted to return an error, so there's something in the log to show
> that cpu hotplug was disabled - but I didn't check what to return. I'll
> fix it up on Monday to return -ENOENT.

I presume the default config from RedHat/SuSE is to have
CONFIG_HOTPLUG_CPU turned on. In this case, all sorts of 
pseries boxes will spew a warning. I suppose this is a 
form of subliminal advertising: "Hey Schmo, upgrade your 
box to something that supports CPU hotplug!". 

--linas

Re: [PATCH] Reorganise and then fixup the pseries cpu hotplug code

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1966 bytes --]

On Fri, 2006-11-17 at 12:11 -0600, Linas Vepstas wrote:
> On Fri, Nov 17, 2006 at 03:44:00PM +1100, Michael Ellerman wrote:
> > On Fri, 2006-11-17 at 15:31 +1100, Stephen Rothwell wrote:
> > > One small nit ...
> > > 
> > > On Fri, 17 Nov 2006 14:36:35 +1100 Michael Ellerman <michael@ellerman.id.au> wrote:
> > > >
> > > > +static int __init pseries_cpu_hotplug_init(void)
> > > > +{
> > > > +	rtas_stop_self_args.token = rtas_token("stop-self");
> > > > +	qcss_tok = rtas_token("query-cpu-stopped-state");
> > > > +
> > > > +	if (rtas_stop_self_args.token == RTAS_UNKNOWN_SERVICE ||
> > > > +			qcss_tok == RTAS_UNKNOWN_SERVICE)
> > > > +		return 1;
> > > 
> > > initcall fucntions should return 0 or -<error>;  -ENODEV is ignored,
> > > other nonzero values cause a log message if initcall debugging is enabled.
> > 
> > OK. I wanted to return an error, so there's something in the log to show
> > that cpu hotplug was disabled - but I didn't check what to return. I'll
> > fix it up on Monday to return -ENOENT.
> 
> I presume the default config from RedHat/SuSE is to have
> CONFIG_HOTPLUG_CPU turned on. In this case, all sorts of 
> pseries boxes will spew a warning. I suppose this is a 
> form of subliminal advertising: "Hey Schmo, upgrade your 
> box to something that supports CPU hotplug!". 

Yeah the distros would have it on for everything. I'm not sure what
machines support it, but I think it'd best to have some indication in
the log that we disabled it.

I'll change it to printk, and always return success. As an aside, I
don't really see the point of having return codes from init routines if
nothing is done with them by default ... but whatever.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Reorganise and then fixup the pseries cpu hotplug code

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1779 bytes --]

On Fri, 2006-11-17 at 12:04 -0600, Linas Vepstas wrote:
> On Fri, Nov 17, 2006 at 02:36:35PM +1100, Michael Ellerman wrote:
> > The pseries cpu hotplug code is currently spread between ./kernel/rtas.c,
> > ./platforms/pseries/smp.c and ./platforms/pseries/setup.c. Some of it is
> > not #ifdef CONFIG_HOTPLUG_CPU, but it should be.
> > 
> > So move all the cpu hotplug code into platforms/pseries/hotplug-cpu.c
> 
> Yes, much cleaner this way.

I agree :)

> > +	/* Isolation and deallocation are definatly done by
>                                         ^^^^^^^^^
> typo
> 
> > +	 * drslot_chrp_cpu.  If they were not they would be
> > +	 * done here.  Change isolate state to Isolate and
> > +	 * change allocation-state to Unusable.
> > +	 */
> > +	paca[cpu].cpu_start = 0;
> 
> I can't figure out what the comment means with respect 
> to this code ... 

Me either. It's unchanged from the original merge of cpu hotplug:
http://kernel.org/git/?p=linux/kernel/git/torvalds/old-2.6-bkcvs.git;a=commit;h=24c13f21f0a6abe07020a959990da2b134e6734f
http://kernel.org/git/?p=linux/kernel/git/torvalds/old-2.6-bkcvs.git;a=blobdiff;h=72144b6122f9336e7fa242e50b65b099df0cd72b;hp=f671515c0676e61064ae414273fa4546d0c04938;hb=24c13f21f0a6abe07020a959990da2b134e6734f;f=arch/ppc64/kernel/smp.c

Which carries the name of one Joel Schopp :)

Grepping the tree at that point finds no other mention of
drslot_chrp_cpu, so I think it's a stale comment - we should probably
just rip it out - Joel?

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Reorganise and then fixup the pseries cpu hotplug code

[jschopp]

>>> +	/* Isolation and deallocation are definatly done by
>>                                         ^^^^^^^^^
>> typo

That word definitely gives me trouble.  I'd imagine the typo originated with me.

>>
>>> +	 * drslot_chrp_cpu.  If they were not they would be
>>> +	 * done here.  Change isolate state to Isolate and
>>> +	 * change allocation-state to Unusable.
>>> +	 */
>>> +	paca[cpu].cpu_start = 0;
>> I can't figure out what the comment means with respect 
>> to this code ... 
> 
> Me either. It's unchanged from the original merge of cpu hotplug:
> http://kernel.org/git/?p=linux/kernel/git/torvalds/old-2.6-bkcvs.git;a=commit;h=24c13f21f0a6abe07020a959990da2b134e6734f
> http://kernel.org/git/?p=linux/kernel/git/torvalds/old-2.6-bkcvs.git;a=blobdiff;h=72144b6122f9336e7fa242e50b65b099df0cd72b;hp=f671515c0676e61064ae414273fa4546d0c04938;hb=24c13f21f0a6abe07020a959990da2b134e6734f;f=arch/ppc64/kernel/smp.c
> 
> Which carries the name of one Joel Schopp :)
> 
> Grepping the tree at that point finds no other mention of
> drslot_chrp_cpu, so I think it's a stale comment - we should probably
> just rip it out - Joel?

It might be clearer by saying "the userspace program drslot_chrp_cpu" or "isolation and 
deallocation occur in userspace, so just stop the cpu.  If isolation and deallocation ever 
move from userpace to kernel space they would go here".  On pseries dynamic partitioning 
or cpu guard (the things that initiate a cpu hotplug) all call a userspace program drmgr, 
which calls drslot_chrp_cpu, which then talks to the firmware to isolate and unallocate 
(on remove, the opposite on add) after it has hotplug removed the cpu via the /sys 
interface.  This allows the cpu to be reassigned to another partition.

At least that's what I remember from back then when we did it.  Though I don't think it's 
changed any.  Feel free to make the comment more intelligible.

Re: [PATCH] Reorganise and then fixup the pseries cpu hotplug code

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 2631 bytes --]

On Sun, 2006-11-19 at 22:22 -0600, jschopp wrote:
> >>> +	/* Isolation and deallocation are definatly done by
> >>                                         ^^^^^^^^^
> >> typo
> 
> That word definitely gives me trouble.  I'd imagine the typo originated with me.

Yeah it's definitely a tricky one ;)

> >>
> >>> +	 * drslot_chrp_cpu.  If they were not they would be
> >>> +	 * done here.  Change isolate state to Isolate and
> >>> +	 * change allocation-state to Unusable.
> >>> +	 */
> >>> +	paca[cpu].cpu_start = 0;
> >> I can't figure out what the comment means with respect 
> >> to this code ... 
> > 
> > Me either. It's unchanged from the original merge of cpu hotplug:
> > http://kernel.org/git/?p=linux/kernel/git/torvalds/old-2.6-bkcvs.git;a=commit;h=24c13f21f0a6abe07020a959990da2b134e6734f
> > http://kernel.org/git/?p=linux/kernel/git/torvalds/old-2.6-bkcvs.git;a=blobdiff;h=72144b6122f9336e7fa242e50b65b099df0cd72b;hp=f671515c0676e61064ae414273fa4546d0c04938;hb=24c13f21f0a6abe07020a959990da2b134e6734f;f=arch/ppc64/kernel/smp.c
> > 
> > Which carries the name of one Joel Schopp :)
> > 
> > Grepping the tree at that point finds no other mention of
> > drslot_chrp_cpu, so I think it's a stale comment - we should probably
> > just rip it out - Joel?
> 
> It might be clearer by saying "the userspace program drslot_chrp_cpu" or "isolation and 
> deallocation occur in userspace, so just stop the cpu.  If isolation and deallocation ever 
> move from userpace to kernel space they would go here".  On pseries dynamic partitioning 
> or cpu guard (the things that initiate a cpu hotplug) all call a userspace program drmgr, 
> which calls drslot_chrp_cpu, which then talks to the firmware to isolate and unallocate 
> (on remove, the opposite on add) after it has hotplug removed the cpu via the /sys 
> interface.  This allows the cpu to be reassigned to another partition.
> 
> At least that's what I remember from back then when we did it.  Though I don't think it's 
> changed any.  Feel free to make the comment more intelligible.

OK, it didn't occur to me that it was referring to userspace. I'll
change it to:

/*
 * Isolation and deallocation occur in userspace, so just stop the cpu.
 * If isolation and deallocation ever move from userpace to kernel space
 * we would do them here.
 */

Sound OK?

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Reorganise and then fixup the pseries cpu hotplug code

[Nathan Lynch]

Michael Ellerman wrote:
> 
> OK, it didn't occur to me that it was referring to userspace. I'll
> change it to:
> 
> /*
>  * Isolation and deallocation occur in userspace, so just stop the cpu.
>  * If isolation and deallocation ever move from userpace to kernel space
>  * we would do them here.
>  */
> 
> Sound OK?

I favor just dropping the comment.  Offline and isolate are distinct
-- offline is a per-thread operation while isolate is per-processor.
Offlining a thread does not imply that its processor will be returned
to the hypervisor (e.g. going to single thread mode from SMT).