[RFC] sysfs cpu cleanup

David Gibson david at gibson.dropbear.id.au
Fri Nov 19 13:30:59 EST 2004


On Thu, Nov 18, 2004 at 06:22:02PM -0600, Nathan Lynch wrote:
> On Thu, 2004-11-18 at 17:44, David Gibson wrote:
> > On Wed, Nov 17, 2004 at 08:59:12AM -0600, Nathan Lynch wrote:
> > > Of course, I haven't had much time to work on this lately.  Your patch
> > > looks fine to me except for the treatment of the physical_id attribute.
> > > We need this to be present even on offline cpus, because it is sometimes
> > > used to determine which cpu to start.
> > 
> > Well, I did wonder about the physical_id attribute.  However for the
> > time being, at least, the physical_id showed as 0 for all offline
> > CPUs, so at present it's not meaningful for offline CPUs - maybe it
> > should be, in which case there's another bug...
> 
> Hmm, are you sure that's the case?  Cpus which are possible but not
> present have the bogus physical_id, yes, and this has always bugged me,
> but a cpu which is offlined retains its physical_id value.  Example from
> a system with 2 present cpus and 8 possible cpus:
> 
> linux:~ # cat listcpus.sh
> #!/bin/bash
> 
> for cpu in $(find /sys/devices/system/cpu/ -type d -name 'cpu*') ; do
> echo -ne "$cpu"
> echo -ne "\tonline $(cat $cpu/online)"
> echo -ne "\tphysical_id $(cat $cpu/physical_id)"
> echo
> done
> 
> linux:~ # ./listcpus.sh 
> /sys/devices/system/cpu/cpu7    online 0        physical_id 0
> /sys/devices/system/cpu/cpu6    online 0        physical_id 0
> /sys/devices/system/cpu/cpu5    online 0        physical_id 0
> /sys/devices/system/cpu/cpu4    online 0        physical_id 0
> /sys/devices/system/cpu/cpu3    online 0        physical_id 0
> /sys/devices/system/cpu/cpu2    online 0        physical_id 0
> /sys/devices/system/cpu/cpu1    online 1        physical_id 1
> /sys/devices/system/cpu/cpu0    online 1        physical_id 0
> linux:~ # echo 0 > /sys/devices/system/cpu/cpu1/online 
> linux:~ # ./listcpus.sh 
> /sys/devices/system/cpu/cpu7    online 0        physical_id 0
> /sys/devices/system/cpu/cpu6    online 0        physical_id 0
> /sys/devices/system/cpu/cpu5    online 0        physical_id 0
> /sys/devices/system/cpu/cpu4    online 0        physical_id 0
> /sys/devices/system/cpu/cpu3    online 0        physical_id 0
> /sys/devices/system/cpu/cpu2    online 0        physical_id 0
> /sys/devices/system/cpu/cpu1    online 0        physical_id 1
> /sys/devices/system/cpu/cpu0    online 1        physical_id 0

Ah, I see, yes, same behaviour confirmed here.  I guess I was
confused, because the first time I was looking at the this the lpar
only had one "present" cpu, although 4 possible were showing up.  The
fact that there is no indication of which CPUs are present in there is
certainly a bad thing, which I guess your proposol to only register
present CPUs would address.  As would the change below.

> It looks like Olof's early processor spinup patch from a few days ago
> would change the physical_id attribute to show -1 for nonpresent cpus,
> fwiw.

That would be good.

Anyway, here is a new version of the patch which leaves physical_id
there for all possible CPUs.  If there are no problems that anyone can
see, I'll forward this on to Andrew Morton.

====

Currently the ppc64 sysfs code registers an entry for each possible
cpu in sysfs, rather than just online cpus.  That makes sense, since
the sysfs entries are needed to control onlining of the cpus.
However, this is done even if CONFIG_HOTPLUG_CPU is not set, or if it
is not a hotplug capable (DLPAR) machine, which is a bit misleading.
Secondly it also registers all the other sysfs entries (mostly
performance monitoring controls) on all possible cpus, although they
are quite meaningless on non-online cpus.

This patch alters the code to only register sysfs directories at boot
for cpus which are either online or could be onlined (cpu is possible,
and CONFIG_HOTPLUG_CPU and an lpar machine).  Furthermore, the entries
apart from 'online' itself and 'physical_id' are only registered for
online CPUs (and deregistered again if a cpu goes offline).

Index: working-2.6/arch/ppc64/kernel/sysfs.c
===================================================================
--- working-2.6.orig/arch/ppc64/kernel/sysfs.c	2004-10-19 13:37:21.000000000 +1000
+++ working-2.6/arch/ppc64/kernel/sysfs.c	2004-11-19 13:07:10.690978384 +1100
@@ -6,6 +6,7 @@
 #include <linux/init.h>
 #include <linux/sched.h>
 #include <linux/module.h>
+#include <linux/cpumask.h>
 #include <asm/current.h>
 #include <asm/processor.h>
 #include <asm/cputable.h>
@@ -13,6 +14,8 @@
 #include <asm/prom.h>
 
 
+static DEFINE_PER_CPU(struct cpu, cpu_devices);
+
 /* SMT stuff */
 
 #ifndef CONFIG_PPC_ISERIES
@@ -259,8 +262,18 @@
 static SYSDEV_ATTR(pmc8, 0600, show_pmc8, store_pmc8);
 static SYSDEV_ATTR(purr, 0600, show_purr, NULL);
 
-static void __init register_cpu_pmc(struct sys_device *s)
+void register_cpu_online(int cpu)
 {
+	struct cpu *c = &per_cpu(cpu_devices, cpu);
+	struct sys_device *s = &c->sysdev;
+
+#ifndef CONFIG_PPC_ISERIES
+	if (cur_cpu_spec->cpu_features & CPU_FTR_SMT)
+		sysdev_create_file(s, &attr_smt_snooze_delay);
+#endif
+
+	/* PMC stuff */
+
 	sysdev_create_file(s, &attr_mmcr0);
 	sysdev_create_file(s, &attr_mmcr1);
 
@@ -283,6 +296,43 @@
 		sysdev_create_file(s, &attr_purr);
 }
 
+#ifdef CONFIG_HOTPLUG_CPU
+void unregister_cpu_online(int cpu)
+{
+	struct cpu *c = &per_cpu(cpu_devices, cpu);
+	struct sys_device *s = &c->sysdev;
+
+	BUG_ON(c->no_control);
+
+#ifndef CONFIG_PPC_ISERIES
+	if (cur_cpu_spec->cpu_features & CPU_FTR_SMT)
+		sysdev_remove_file(s, &attr_smt_snooze_delay);
+#endif
+
+	/* PMC stuff */
+
+	sysdev_remove_file(s, &attr_mmcr0);
+	sysdev_remove_file(s, &attr_mmcr1);
+
+	if (cur_cpu_spec->cpu_features & CPU_FTR_MMCRA)
+		sysdev_remove_file(s, &attr_mmcra);
+
+	sysdev_remove_file(s, &attr_pmc1);
+	sysdev_remove_file(s, &attr_pmc2);
+	sysdev_remove_file(s, &attr_pmc3);
+	sysdev_remove_file(s, &attr_pmc4);
+	sysdev_remove_file(s, &attr_pmc5);
+	sysdev_remove_file(s, &attr_pmc6);
+
+	if (cur_cpu_spec->cpu_features & CPU_FTR_PMC8) {
+		sysdev_remove_file(s, &attr_pmc7);
+		sysdev_remove_file(s, &attr_pmc8);
+	}
+
+	if (cur_cpu_spec->cpu_features & CPU_FTR_SMT)
+		sysdev_remove_file(s, &attr_purr);
+}
+#endif /* CONFIG_HOTPLUG_CPU */
 
 /* NUMA stuff */
 
@@ -312,8 +362,7 @@
 }
 #endif
 
-
-/* Only valid if CPU is online. */
+/* Only valid if CPU is present. */
 static ssize_t show_physical_id(struct sys_device *dev, char *buf)
 {
 	struct cpu *cpu = container_of(dev, struct cpu, sysdev);
@@ -322,9 +371,6 @@
 }
 static SYSDEV_ATTR(physical_id, 0444, show_physical_id, NULL);
 
-
-static DEFINE_PER_CPU(struct cpu, cpu_devices);
-
 static int __init topology_init(void)
 {
 	int cpu;
@@ -345,19 +391,19 @@
 		 * CPU.  For instance, the boot cpu might never be valid
 		 * for hotplugging.
 		 */
+#ifdef CONFIG_HOTPLUG_CPU
 		if (systemcfg->platform != PLATFORM_PSERIES_LPAR)
+#endif
 			c->no_control = 1;
 
-		register_cpu(c, cpu, parent);
+		if (cpu_online(cpu) || (c->no_control == 0))
+			register_cpu(c, cpu, parent);
 
-		register_cpu_pmc(&c->sysdev);
+		sysdev_create_file(s, &attr_physical_id);
 
-		sysdev_create_file(&c->sysdev, &attr_physical_id);
+		if (cpu_online(cpu))
+			register_cpu_online(cpu);
 
-#ifndef CONFIG_PPC_ISERIES
-		if (cur_cpu_spec->cpu_features & CPU_FTR_SMT)
-			sysdev_create_file(&c->sysdev, &attr_smt_snooze_delay);
-#endif
 	}
 
 	return 0;
Index: working-2.6/arch/ppc64/kernel/smp.c
===================================================================
--- working-2.6.orig/arch/ppc64/kernel/smp.c	2004-10-19 13:37:56.000000000 +1000
+++ working-2.6/arch/ppc64/kernel/smp.c	2004-11-17 17:56:42.000000000 +1100
@@ -82,6 +82,8 @@
 void smp_call_function_interrupt(void);
 extern long register_vpa(unsigned long flags, unsigned long proc,
 			 unsigned long vpa);
+extern void register_cpu_online(int cpu);
+extern void unregister_cpu_online(int cpu);
 
 int smt_enabled_at_boot = 1;
 
@@ -291,6 +293,8 @@
 	int cpu_status;
 	unsigned int pcpu = get_hard_smp_processor_id(cpu);
 
+	unregister_cpu_online(cpu);
+
 	for (tries = 0; tries < 25; tries++) {
 		cpu_status = query_cpu_stopped(pcpu);
 		if (cpu_status == 0 || cpu_status == -1)
@@ -919,6 +923,11 @@
 	while (!cpu_online(cpu))
 		cpu_relax();
 
+#ifdef CONFIG_HOTPLUG_CPU
+	if (system_state >= SYSTEM_RUNNING) /* This is a hotplug */
+		register_cpu_online(cpu);
+#endif /* CONFIG_HOTPLUG_CPU */
+
 	return 0;
 }
 


-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist.  NOT _the_ _other_ _way_
				| _around_!
http://www.ozlabs.org/people/dgibson



More information about the Linuxppc64-dev mailing list