Re: /sys/devices/system/cpu/cpuX/online are missing

Giuliano Pochini [off-list ref] writes:

I had a look at arch/powerpc/kernel/smp.c but I'm not familiar at all with 
those parts of the kernel.

See arch/powerpc/kernel/sysfs.c:topology_init.  I don't think there is
anything to do here.  You probably don't have CONFIG_HOTPLUG_CPU enabled.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

On Tue, Mar 13, 2007 at 01:39:25AM +0100, Andreas Schwab wrote:

Giuliano Pochini [off-list ref] writes:

quoted

I had a look at arch/powerpc/kernel/smp.c but I'm not familiar at all with 
those parts of the kernel.

See arch/powerpc/kernel/sysfs.c:topology_init.  I don't think there is
anything to do here.  You probably don't have CONFIG_HOTPLUG_CPU enabled.

I was referring to arch/ppc not arch/powerpc. But it seems that arch/ppc
doesn't support cpu hotplug anyway. So I guess it's indeed just a missing
config option.

Grepping a bit further shows that arm suffered by the change that inverted
the logic if the 'online' attribute for cpus should appear. Since arm
supports cpu hotplug but the patch left arm out, it doesn't work there
anymore (cc'ing arm people: changeset 72486f1f8f0a2bc828b9d30cf4690cf2dd6807fc
is most probably disabling cpu hotplug support on arm like it did on s390).

On Tue, Mar 13, 2007 at 10:03:50AM +0100, Heiko Carstens wrote:

On Tue, Mar 13, 2007 at 01:39:25AM +0100, Andreas Schwab wrote:

quoted

Giuliano Pochini [off-list ref] writes:

quoted

I had a look at arch/powerpc/kernel/smp.c but I'm not familiar at all with 
those parts of the kernel.

See arch/powerpc/kernel/sysfs.c:topology_init.  I don't think there is
anything to do here.  You probably don't have CONFIG_HOTPLUG_CPU enabled.

I was referring to arch/ppc not arch/powerpc. But it seems that arch/ppc
doesn't support cpu hotplug anyway. So I guess it's indeed just a missing
config option.

Grepping a bit further shows that arm suffered by the change that inverted
the logic if the 'online' attribute for cpus should appear. Since arm
supports cpu hotplug but the patch left arm out, it doesn't work there
anymore (cc'ing arm people: changeset 72486f1f8f0a2bc828b9d30cf4690cf2dd6807fc
is most probably disabling cpu hotplug support on arm like it did on s390).

Should have cc'ed Suresh Siddha who caused the breakage ;)

On Tue, Mar 13, 2007 at 10:11:59AM +0100, Heiko Carstens wrote:

On Tue, Mar 13, 2007 at 10:03:50AM +0100, Heiko Carstens wrote:

quoted

On Tue, Mar 13, 2007 at 01:39:25AM +0100, Andreas Schwab wrote:

quoted

Giuliano Pochini [off-list ref] writes:

quoted

I had a look at arch/powerpc/kernel/smp.c but I'm not familiar at all with 
those parts of the kernel.

See arch/powerpc/kernel/sysfs.c:topology_init.  I don't think there is
anything to do here.  You probably don't have CONFIG_HOTPLUG_CPU enabled.

I was referring to arch/ppc not arch/powerpc. But it seems that arch/ppc
doesn't support cpu hotplug anyway. So I guess it's indeed just a missing
config option.

Grepping a bit further shows that arm suffered by the change that inverted
the logic if the 'online' attribute for cpus should appear. Since arm
supports cpu hotplug but the patch left arm out, it doesn't work there
anymore (cc'ing arm people: changeset 72486f1f8f0a2bc828b9d30cf4690cf2dd6807fc
is most probably disabling cpu hotplug support on arm like it did on s390).

Should have cc'ed Suresh Siddha who caused the breakage ;)

Welcome to why cleanups are bad news. ;(  Yes, ARM also needs to be fixed
and I'd ask that in future people doing cleanups in core code take a little
more time to review the code before submitting patches *AND* give heads-up
to *EVERYONE* who might be affected by the change.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

On Tue, 13 Mar 2007, Andreas Schwab wrote:

Giuliano Pochini [off-list ref] writes:

quoted

I had a look at arch/powerpc/kernel/smp.c but I'm not familiar at all with
those parts of the kernel.

See arch/powerpc/kernel/sysfs.c:topology_init.  I don't think there is
anything to do here.  You probably don't have CONFIG_HOTPLUG_CPU enabled.

I can't check it out right now, but I'm sure it is enabled as I wrote in 
my first message. I said arch/powerpc/kernel/smp.c because the respective 
file in arch/s390 was fixed recently for the same issue.


--
Giuliano.

On Tue, Mar 13, 2007 at 09:40:39AM +0000, Russell King wrote:

On Tue, Mar 13, 2007 at 10:11:59AM +0100, Heiko Carstens wrote:

quoted

On Tue, Mar 13, 2007 at 10:03:50AM +0100, Heiko Carstens wrote:

quoted

I was referring to arch/ppc not arch/powerpc. But it seems that arch/ppc
doesn't support cpu hotplug anyway. So I guess it's indeed just a missing
config option.

Grepping a bit further shows that arm suffered by the change that inverted
the logic if the 'online' attribute for cpus should appear. Since arm
supports cpu hotplug but the patch left arm out, it doesn't work there
anymore (cc'ing arm people: changeset 72486f1f8f0a2bc828b9d30cf4690cf2dd6807fc
is most probably disabling cpu hotplug support on arm like it did on s390).

Should have cc'ed Suresh Siddha who caused the breakage ;)

Welcome to why cleanups are bad news. ;(  Yes, ARM also needs to be fixed
and I'd ask that in future people doing cleanups in core code take a little
more time to review the code before submitting patches *AND* give heads-up
to *EVERYONE* who might be affected by the change.

Right, here's the ARM fix which is now in the ARM tree:

# Base git commit: 8b9909ded6922c33c221b105b26917780cfa497d
#	(Merge branch 'merge' of master.kernel.org:/pub/scm/linux/kernel/git/paulus/powerpc)
#
# Author:    Russell King (Tue Mar 13 09:54:21 GMT 2007)
# Committer: Russell King (Tue Mar 13 09:54:21 GMT 2007)
#   
#   [ARM] Fix breakage caused by 72486f1f8f0a2bc828b9d30cf4690cf2dd6807fc
#   
#   72486f1f8f0a2bc828b9d30cf4690cf2dd6807fc inverted the sense for
#   enabling hotplug CPU controls without reference to any other
#   architecture other than i386, ia64 and PowerPC.  This left
#   everyone else without hotplug CPU control.
#   
#   Fix ARM for this brain damage.
#   
#   Signed-off-by: Russell King
#
#    arch/arm/kernel/setup.c |    7 +++++--
#    1 files changed, 5 insertions(+), 2 deletions(-)
#

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 03e37af..0453dcc 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c

@@ -839,8 +839,11 @@ static int __init topology_init(void)
 {
 	int cpu;
 
-	for_each_possible_cpu(cpu)
-		register_cpu(&per_cpu(cpu_data, cpu).cpu, cpu);
+	for_each_possible_cpu(cpu) {
+		struct cpuinfo_arm *cpuinfo = &per_cpu(cpu_data, cpu);
+		cpuinfo->cpu.hotpluggable = 1;
+		register_cpu(&cpuinfo->cpu, cpu);
+	}
 
 	return 0;
 }

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

On Tue, 13 Mar 2007 09:56:52 +0000
Russell King [off-list ref] wrote:

Right, here's the ARM fix which is now in the ARM tree:
[...]

The following patch seems to fix the issue (+ minor style fix). I'm not sure
it's ok due to my poor knowledge of this code.


Signed-off-by: Giuliano Pochini <redacted>

--- linux-2.6.21rc3/arch/powerpc/kernel/setup_32.c__orig	2007-03-15 00:05:02.000000000 +0100
+++ linux-2.6.21rc3/arch/powerpc/kernel/setup_32.c	2007-03-15 00:07:02.000000000 +0100

@@ -195,18 +195,22 @@ EXPORT_SYMBOL(nvram_sync);
 
 #endif /* CONFIG_NVRAM */
 
-static struct cpu cpu_devices[NR_CPUS];
+static DEFINE_PER_CPU(struct cpu, cpu_devices);
 
 int __init ppc_init(void)
 {
-	int i;
+	int cpu;
 
 	/* clear the progress line */
-	if ( ppc_md.progress ) ppc_md.progress("             ", 0xffff);
+	if (ppc_md.progress)
+		ppc_md.progress("             ", 0xffff);
 
 	/* register CPU devices */
-	for_each_possible_cpu(i)
-		register_cpu(&cpu_devices[i], i);
+	for_each_possible_cpu(cpu) {
+		struct cpu *c = &per_cpu(cpu_devices, cpu);
+		c->hotpluggable = 1;
+		register_cpu(c, cpu);
+	}
 
 	/* call platform init */
 	if (ppc_md.init != NULL) {


--

Giuliano.