[RFC] sysfs cpu cleanup

David Gibson david at gibson.dropbear.id.au
Tue Nov 23 15:04:46 EST 2004


On Mon, Nov 22, 2004 at 12:58:54PM -0600, Nathan Lynch wrote:
> On Thu, 2004-11-18 at 20:30, David Gibson wrote:
> > Index: working-2.6/arch/ppc64/kernel/smp.c
> > ===================================================================
> > +++ 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;
> >  }
> 
> I apologize for not noticing this before :/
> 
> Instead of explicitly calling into the sysfs.c code from smp.c, I think
> it would be better to use a cpu hotplug notifier block.  Even without
> CONFIG_HOTPLUG_CPU turned on, you get the "online" notification at boot
> for each cpu as it is brought up.  See the ppc64 numa code for an
> example usage.

Ah, good point.  How about this, then.  Note that we still need to
register the stuff for each online cpu in topology_init(), since
smp_init() is called before the notifier block is registered.

====

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-23 14:38:33.422111976 +1100
@@ -6,6 +6,8 @@
 #include <linux/init.h>
 #include <linux/sched.h>
 #include <linux/module.h>
+#include <linux/cpumask.h>
+#include <linux/notifier.h>
 #include <asm/current.h>
 #include <asm/processor.h>
 #include <asm/cputable.h>
@@ -13,6 +15,8 @@
 #include <asm/prom.h>
 
 
+static DEFINE_PER_CPU(struct cpu, cpu_devices);
+
 /* SMT stuff */
 
 #ifndef CONFIG_PPC_ISERIES
@@ -259,8 +263,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)
+static void register_cpu_online(unsigned 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 +297,65 @@
 		sysdev_create_file(s, &attr_purr);
 }
 
+#ifdef CONFIG_HOTPLUG_CPU
+static void unregister_cpu_online(unsigned 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 */
+
+static int __devinit sysfs_cpu_notify(struct notifier_block *self, 
+				      unsigned long action, void *hcpu)
+{
+	unsigned int cpu = (unsigned int)(long)hcpu;
+
+	switch (action) {
+	case CPU_ONLINE:
+		register_cpu_online(cpu);
+		break;
+#ifdef CONFIG_HOTPLUG_CPU
+	case CPU_DEAD:
+		unregister_cpu_online(cpu);
+		break;
+#endif
+	}
+	return NOTIFY_OK;
+}
+
+static struct notifier_block __devinitdata sysfs_cpu_nb = {
+	.notifier_call	= sysfs_cpu_notify,
+};
 
 /* NUMA stuff */
 
@@ -312,8 +385,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 +394,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;
@@ -332,6 +401,8 @@
 
 	register_nodes();
 
+	register_cpu_notifier(&sysfs_cpu_nb);
+
 	for_each_cpu(cpu) {
 		struct cpu *c = &per_cpu(cpu_devices, cpu);
 
@@ -345,19 +416,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);
-
-		register_cpu_pmc(&c->sysdev);
+		if (cpu_online(cpu) || (c->no_control == 0)) {
+			register_cpu(c, cpu, parent);
 
-		sysdev_create_file(&c->sysdev, &attr_physical_id);
+			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
+		if (cpu_online(cpu))
+			register_cpu_online(cpu);
 	}
 
 	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-23 14:05:14.039119728 +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;
 


-- 
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