[RFC] sysfs cpu cleanup

[David Gibson]

Current 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 (physical_id and the pmc
stuff) 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 are only registered for online CPUs (and
deregistered again if a cpu goes offline).

Anyone see any problems with this approach?  Also, this has not yet
been tested in the presence of actual cpu hotplugging...

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-17 17:55:22.284033824 +1100
@@ -12,6 +12,7 @@
 #include <asm/hvcall.h>
 #include <asm/prom.h>
 
+static DEFINE_PER_CPU(struct cpu, cpu_devices);
 
 /* SMT stuff */
 
@@ -259,8 +260,29 @@
 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)
+/* Only valid if CPU is online. */
+static ssize_t show_physical_id(struct sys_device *dev, char *buf)
 {
+	struct cpu *cpu = container_of(dev, struct cpu, sysdev);
+
+	return sprintf(buf, "%u\n", get_hard_smp_processor_id(cpu->sysdev.id));
+}
+static SYSDEV_ATTR(physical_id, 0444, show_physical_id, NULL);
+
+void register_cpu_online(int cpu)
+{
+	struct cpu *c = &per_cpu(cpu_devices, cpu);
+	struct sys_device *s = &c->sysdev;
+
+	sysdev_create_file(s, &attr_physical_id);
+
+#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 +305,45 @@
 		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);
+
+	sysdev_remove_file(s, &attr_physical_id);
+
+#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 */
 
@@ -313,18 +374,6 @@
 #endif
 
 
-/* Only valid if CPU is online. */
-static ssize_t show_physical_id(struct sys_device *dev, char *buf)
-{
-	struct cpu *cpu = container_of(dev, struct cpu, sysdev);
-
-	return sprintf(buf, "%u\n", get_hard_smp_processor_id(cpu->sysdev.id));
-}
-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;
@@ -348,16 +397,12 @@
 		if (systemcfg->platform != PLATFORM_PSERIES_LPAR)
 			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);
+		if (cpu_online{cpu))
+			register_cpu_online(cpu);
 
-		sysdev_create_file(&c->sysdev, &attr_physical_id);
-
-#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.827035864 +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