[PATCH 2.6.6-rc2 RFT] Add's class support to cpuid.c

[PATCH 2.6.6-rc2 RFT] Add's class support to cpuid.c

[Hanna Linder]

This patch adds class support to arch/i386/kernel/cpuid.c. This enables udev
support. I have tested on a 2-way SMP system and on a 2-way built as UP.
Here are the results for the SMP:

[hlinder@w-hlinder2 hlinder]$ tree /sys/class/cpuid
/sys/class/cpuid
|-- cpu0
|   `-- dev
`-- cpu1
    `-- dev

2 directories, 2 files
[hlinder@w-hlinder2 hlinder]$ more /sys/class/cpuid/cpu0/dev
203:0
[hlinder@w-hlinder2 hlinder]$ more /sys/class/cpuid/cpu1/dev
203:1
[hlinder@w-hlinder2 hlinder]$

And for the UP:

[root@w-hlinder2 root]# tree /sys/class/cpuid
/sys/class/cpuid
`-- cpu0
    `-- dev

1 directory, 1 file

Please review and consider for inclusion for further testing.

Thanks.

Hanna Linder
IBM Linux Technology Center
----------------
diff -Nrup -Xdontdiff linux-2.6.7-rc2/arch/i386/kernel/cpuid.c linux-2.6.7-rc2p/arch/i386/kernel/cpuid.c
--- linux-2.6.7-rc2/arch/i386/kernel/cpuid.c	2004-06-01 16:40:10.000000000 -0700
+++ linux-2.6.7-rc2p/arch/i386/kernel/cpuid.c	2004-06-02 14:22:27.000000000 -0700
@@ -36,12 +36,15 @@
 #include <linux/fs.h>
 #include <linux/smp_lock.h>
 #include <linux/fs.h>
+#include <linux/device.h>
 
 #include <asm/processor.h>
 #include <asm/msr.h>
 #include <asm/uaccess.h>
 #include <asm/system.h>
 
+static struct class_simple *cpuid_class;
+
 #ifdef CONFIG_SMP
 
 struct cpuid_command {
@@ -155,17 +158,48 @@ static struct file_operations cpuid_fops
 
 int __init cpuid_init(void)
 {
+	int i, err = 0;
+	struct class_device *class_err;
+	i = 0;
+
 	if (register_chrdev(CPUID_MAJOR, "cpu/cpuid", &cpuid_fops)) {
 		printk(KERN_ERR "cpuid: unable to get major %d for cpuid\n",
 		       CPUID_MAJOR);
-		return -EBUSY;
+		err = -EBUSY;
+		goto out;
+	}
+	cpuid_class = class_simple_create(THIS_MODULE, "cpuid");
+	if (IS_ERR(cpuid_class)){
+		err = PTR_ERR(cpuid_class);
+		goto out_chrdev;
 	}
+	for_each_online_cpu(i){
+		class_err = class_simple_device_add(cpuid_class, MKDEV(CPUID_MAJOR, i), NULL, "cpu%d",i);
+		if (IS_ERR(class_err)){
+			err = PTR_ERR(class_err);
+			goto out_class;
+		}
+	}
+	err = 0;
+	goto out;
 
-	return 0;
+out_class:
+	i = 0;
+	for_each_online_cpu(i)
+		class_simple_device_remove(MKDEV(CPUID_MAJOR, i));
+	class_simple_destroy(cpuid_class);
+out_chrdev:
+	unregister_chrdev(CPUID_MAJOR, "cpu/cpuid");	
+out:
+	return err;
 }
 
 void __exit cpuid_exit(void)
 {
+	int i = 0;
+	for_each_online_cpu(i)
+		class_simple_device_remove(MKDEV(CPUID_MAJOR, i));
+	class_simple_destroy(cpuid_class);
 	unregister_chrdev(CPUID_MAJOR, "cpu/cpuid");
 }
 

Re: [PATCH 2.6.6-rc2 RFT] Add's class support to cpuid.c

[H. Peter Anvin]

Hanna Linder wrote:
> This patch adds class support to arch/i386/kernel/cpuid.c. This enables udev
> support. I have tested on a 2-way SMP system and on a 2-way built as UP.
> Here are the results for the SMP:

I think it would be better if udev could handle /dev/cpu as an iterator 
instead of needing O(N) kernel memory for all per-CPU devices; I made the same 
comments about devfs.

As it is, it also mishandles the hotswap CPU scenario.

> [hlinder@w-hlinder2 hlinder]$ tree /sys/class/cpuid
> /sys/class/cpuid
> |-- cpu0
> |   `-- dev
> `-- cpu1
>     `-- dev
> 
> 2 directories, 2 files
> [hlinder@w-hlinder2 hlinder]$ more /sys/class/cpuid/cpu0/dev
> 203:0
> [hlinder@w-hlinder2 hlinder]$ more /sys/class/cpuid/cpu1/dev
> 203:1
> [hlinder@w-hlinder2 hlinder]$

	-hpa

Re: [PATCH 2.6.6-rc2 RFT] Add's class support to cpuid.c

[Greg KH]

On Wed, Jun 02, 2004 at 05:11:21PM -0700, H. Peter Anvin wrote:
> Hanna Linder wrote:
> >This patch adds class support to arch/i386/kernel/cpuid.c. This enables 
> >udev
> >support. I have tested on a 2-way SMP system and on a 2-way built as UP.
> >Here are the results for the SMP:
> 
> I think it would be better if udev could handle /dev/cpu as an iterator 
> instead of needing O(N) kernel memory for all per-CPU devices; I made the 
> same comments about devfs.

Sorry, but that's not going to happen at this time.

> As it is, it also mishandles the hotswap CPU scenario.

I agree, but that can be easily added with a second patch on top of this
one, right Hanna?  :)

I'll go add this to the driver-2.6 bk tree to show up in the next -mm.

thanks,

greg k-h

Re: [PATCH 2.6.6-rc2 RFT] Add's class support to cpuid.c

[Hanna Linder]

--On Thursday, June 03, 2004 12:32:56 PM -0700 Greg KH <greg@kroah.com> wrote:

>> As it is, it also mishandles the hotswap CPU scenario.
> 
> I agree, but that can be easily added with a second patch on top of this
> one, right Hanna?  :)
> 
> I'll go add this to the driver-2.6 bk tree to show up in the next -mm.

Thanks. Im working on it now.

Hanna

Re: [PATCH 2.6.6-rc2 RFT] Add's class support to cpuid.c

[Hanna Linder]

--On Thursday, June 03, 2004 12:32:56 PM -0700 Greg KH <greg@kroah.com> wrote:
> On Wed, Jun 02, 2004 at 05:11:21PM -0700, H. Peter Anvin wrote:
>> As it is, it also mishandles the hotswap CPU scenario.
> 
> I agree, but that can be easily added with a second patch on top of this
> one, right Hanna?  :)

Here is the patch that uses a cpu hotplug callback, to allow dynamic support 
of cpu id for classes in sysfs.

This patch applies on top of the one I sent out earlier that Greg included.
I do not have access to hardware that supports cpu hotswapping (virtually or not) 
so have not been able to test that aspect of the patch. However, the original
functionality of listing static cpu's still works.

Please consider for testing or inclusion. 

Signed-off-by Hanna Linder <hannal@us.ibm.com>

Thanks.

Hanna Linder
IBM Linux Technology Center
-------

diff -Nrup -Xdontdiff linux-2.6.7-rc2/arch/i386/kernel/cpuid.c linux-2.6.7-rc2p/arch/i386/kernel/cpuid.c
--- linux-2.6.7-rc2/arch/i386/kernel/cpuid.c	2004-06-03 15:51:37.000000000 -0700
+++ linux-2.6.7-rc2p/arch/i386/kernel/cpuid.c	2004-06-07 15:52:50.000000000 -0700
@@ -37,6 +37,8 @@
 #include <linux/smp_lock.h>
 #include <linux/fs.h>
 #include <linux/device.h>
+#include <linux/cpu.h>
+#include <linux/notifier.h>
 
 #include <asm/processor.h>
 #include <asm/msr.h>
@@ -156,10 +158,48 @@ static struct file_operations cpuid_fops
 	.open = cpuid_open,
 };
 
+static void cpuid_class_simple_device_remove(void)
+{
+	int i = 0;
+	for_each_online_cpu(i)
+		class_simple_device_remove(MKDEV(CPUID_MAJOR, i));
+	return;
+}
+
+static int cpuid_class_simple_device_add(int i) 
+{
+	int err = 0;
+	struct class_device *class_err;
+
+	class_err = class_simple_device_add(cpuid_class, MKDEV(CPUID_MAJOR, i), NULL, "cpu%d",i);
+	if (IS_ERR(class_err)) {
+		err = PTR_ERR(class_err);
+	}
+	return err;
+}
+static int __devinit cpuid_class_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
+{
+	unsigned int cpu = (unsigned long)hcpu;
+
+	switch(action) {
+	case CPU_ONLINE:
+		cpuid_class_simple_device_add(cpu);
+		break;
+	case CPU_DEAD:
+		cpuid_class_simple_device_remove();
+		break;
+	}
+	return NOTIFY_OK;
+}
+
+static struct notifier_block cpuid_class_cpu_notifier =
+{
+	.notifier_call = cpuid_class_cpu_callback,
+};
+
 int __init cpuid_init(void)
 {
 	int i, err = 0;
-	struct class_device *class_err;
 	i = 0;
 
 	if (register_chrdev(CPUID_MAJOR, "cpu/cpuid", &cpuid_fops)) {
@@ -174,33 +214,31 @@ int __init cpuid_init(void)
 		goto out_chrdev;
 	}
 	for_each_online_cpu(i) {
-		class_err = class_simple_device_add(cpuid_class, MKDEV(CPUID_MAJOR, i), NULL, "cpu%d",i);
-		if (IS_ERR(class_err)) {
-			err = PTR_ERR(class_err);
+		err = cpuid_class_simple_device_add(i);
+		if (err != 0) 
 			goto out_class;
-		}
 	}
+	register_cpu_notifier(&cpuid_class_cpu_notifier);
+
 	err = 0;
 	goto out;
 
 out_class:
-	i = 0;
-	for_each_online_cpu(i)
-		class_simple_device_remove(MKDEV(CPUID_MAJOR, i));
+	cpuid_class_simple_device_remove();
 	class_simple_destroy(cpuid_class);
 out_chrdev:
 	unregister_chrdev(CPUID_MAJOR, "cpu/cpuid");	
+	unregister_cpu_notifier(&cpuid_class_cpu_notifier);
 out:
 	return err;
 }
 
 void __exit cpuid_exit(void)
 {
-	int i = 0;
-	for_each_online_cpu(i)
-		class_simple_device_remove(MKDEV(CPUID_MAJOR, i));
+	cpuid_class_simple_device_remove();
 	class_simple_destroy(cpuid_class);
 	unregister_chrdev(CPUID_MAJOR, "cpu/cpuid");
+	unregister_cpu_notifier(&cpuid_class_cpu_notifier);
 }
 
 module_init(cpuid_init);

Re: [PATCH 2.6.6-rc2 RFT] Add's class support to cpuid.c

[Hanna Linder]

> 
> Here is the patch that uses a cpu hotplug callback, to allow dynamic support 
> of cpu id for classes in sysfs.
> 
> This patch applies on top of the one I sent out earlier that Greg included.
> I do not have access to hardware that supports cpu hotswapping (virtually or not) 
> so have not been able to test that aspect of the patch. However, the original
> functionality of listing static cpu's still works.
> 
> Please consider for testing or inclusion. 
> 
> Signed-off-by Hanna Linder <hannal@us.ibm.com>
> 
> Thanks.
> 
> Hanna Linder
> IBM Linux Technology Center

Gregkh found a small bug in the error path. Here is the fixed version:

diff -Nrup linux-2.6.7-rc2/arch/i386/kernel/cpuid.c linux-2.6.7-rc2p/arch/i386/kernel/cpuid.c
--- linux-2.6.7-rc2/arch/i386/kernel/cpuid.c	2004-06-03 15:51:37.000000000 -0700
+++ linux-2.6.7-rc2p/arch/i386/kernel/cpuid.c	2004-06-08 14:31:22.000000000 -0700
@@ -37,6 +37,8 @@
 #include <linux/smp_lock.h>
 #include <linux/fs.h>
 #include <linux/device.h>
+#include <linux/cpu.h>
+#include <linux/notifier.h>
 
 #include <asm/processor.h>
 #include <asm/msr.h>
@@ -156,10 +158,48 @@ static struct file_operations cpuid_fops
 	.open = cpuid_open,
 };
 
+static void cpuid_class_simple_device_remove(void)
+{
+	int i = 0;
+	for_each_online_cpu(i)
+		class_simple_device_remove(MKDEV(CPUID_MAJOR, i));
+	return;
+}
+
+static int cpuid_class_simple_device_add(int i) 
+{
+	int err = 0;
+	struct class_device *class_err;
+
+	class_err = class_simple_device_add(cpuid_class, MKDEV(CPUID_MAJOR, i), NULL, "cpu%d",i);
+	if (IS_ERR(class_err)) {
+		err = PTR_ERR(class_err);
+	}
+	return err;
+}
+static int __devinit cpuid_class_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
+{
+	unsigned int cpu = (unsigned long)hcpu;
+
+	switch(action) {
+	case CPU_ONLINE:
+		cpuid_class_simple_device_add(cpu);
+		break;
+	case CPU_DEAD:
+		cpuid_class_simple_device_remove();
+		break;
+	}
+	return NOTIFY_OK;
+}
+
+static struct notifier_block cpuid_class_cpu_notifier =
+{
+	.notifier_call = cpuid_class_cpu_callback,
+};
+
 int __init cpuid_init(void)
 {
 	int i, err = 0;
-	struct class_device *class_err;
 	i = 0;
 
 	if (register_chrdev(CPUID_MAJOR, "cpu/cpuid", &cpuid_fops)) {
@@ -174,19 +214,17 @@ int __init cpuid_init(void)
 		goto out_chrdev;
 	}
 	for_each_online_cpu(i) {
-		class_err = class_simple_device_add(cpuid_class, MKDEV(CPUID_MAJOR, i), NULL, "cpu%d",i);
-		if (IS_ERR(class_err)) {
-			err = PTR_ERR(class_err);
+		err = cpuid_class_simple_device_add(i);
+		if (err != 0) 
 			goto out_class;
-		}
 	}
+	register_cpu_notifier(&cpuid_class_cpu_notifier);
+
 	err = 0;
 	goto out;
 
 out_class:
-	i = 0;
-	for_each_online_cpu(i)
-		class_simple_device_remove(MKDEV(CPUID_MAJOR, i));
+	cpuid_class_simple_device_remove();
 	class_simple_destroy(cpuid_class);
 out_chrdev:
 	unregister_chrdev(CPUID_MAJOR, "cpu/cpuid");	
@@ -196,11 +234,10 @@ out:
 
 void __exit cpuid_exit(void)
 {
-	int i = 0;
-	for_each_online_cpu(i)
-		class_simple_device_remove(MKDEV(CPUID_MAJOR, i));
+	cpuid_class_simple_device_remove();
 	class_simple_destroy(cpuid_class);
 	unregister_chrdev(CPUID_MAJOR, "cpu/cpuid");
+	unregister_cpu_notifier(&cpuid_class_cpu_notifier);
 }
 
 module_init(cpuid_init);

Re: [PATCH 2.6.6-rc2 RFT] Add's class support to cpuid.c

[Greg KH]

On Tue, Jun 08, 2004 at 03:15:47PM -0700, Hanna Linder wrote:
> > 
> > Here is the patch that uses a cpu hotplug callback, to allow dynamic support 
> > of cpu id for classes in sysfs.
> > 
> > This patch applies on top of the one I sent out earlier that Greg included.
> > I do not have access to hardware that supports cpu hotswapping (virtually or not) 
> > so have not been able to test that aspect of the patch. However, the original
> > functionality of listing static cpu's still works.
> > 
> > Please consider for testing or inclusion. 
> > 
> > Signed-off-by Hanna Linder <hannal@us.ibm.com>
> > 
> > Thanks.
> > 
> > Hanna Linder
> > IBM Linux Technology Center
> 
> Gregkh found a small bug in the error path. Here is the fixed version:

Looks good, I've added it to my trees.

thanks,

greg k-h

Re: [PATCH 2.6.6-rc2 RFT] Add's class support to cpuid.c

[Zwane Mwaikambo]

On Tue, 8 Jun 2004, Hanna Linder wrote:

> +static void cpuid_class_simple_device_remove(void)
> +{
> +	int i = 0;
> +	for_each_online_cpu(i)
> +		class_simple_device_remove(MKDEV(CPUID_MAJOR, i));
> +	return;
> +}

My understanding is that the above removes the class for each online cpu.

> +static int __devinit cpuid_class_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
> +{
> +	unsigned int cpu = (unsigned long)hcpu;
> +
> +	switch(action) {
> +	case CPU_ONLINE:
> +		cpuid_class_simple_device_add(cpu);
> +		break;
> +	case CPU_DEAD:
> +		cpuid_class_simple_device_remove();
> +		break;

So the above will remove the class for all online processors when one
processor goes down? By the way, you can use i386 SMP to test cpu hotplug
code paths.

	Zwane

Re: [PATCH 2.6.6-rc2 RFT] Add's class support to cpuid.c

[Greg KH]

On Tue, Jun 08, 2004 at 10:52:09PM -0400, Zwane Mwaikambo wrote:
> My understanding is that the above removes the class for each online cpu.

Ick, good catch.  This change should fix this.  Thanks for letting me
know.

greg k-h

# cpuid: fix hotplug cpu remove bug for class device.
#
# Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
#
diff -Nru a/arch/i386/kernel/cpuid.c b/arch/i386/kernel/cpuid.c
--- a/arch/i386/kernel/cpuid.c	2004-06-08 22:44:26 -07:00
+++ b/arch/i386/kernel/cpuid.c	2004-06-08 22:44:26 -07:00
@@ -158,35 +158,27 @@
 	.open = cpuid_open,
 };
 
-static void cpuid_class_simple_device_remove(void)
-{
-	int i = 0;
-	for_each_online_cpu(i)
-		class_simple_device_remove(MKDEV(CPUID_MAJOR, i));
-	return;
-}
-
 static int cpuid_class_simple_device_add(int i) 
 {
 	int err = 0;
 	struct class_device *class_err;
 
 	class_err = class_simple_device_add(cpuid_class, MKDEV(CPUID_MAJOR, i), NULL, "cpu%d",i);
-	if (IS_ERR(class_err)) {
+	if (IS_ERR(class_err))
 		err = PTR_ERR(class_err);
-	}
 	return err;
 }
+
 static int __devinit cpuid_class_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
 {
 	unsigned int cpu = (unsigned long)hcpu;
 
-	switch(action) {
+	switch (action) {
 	case CPU_ONLINE:
 		cpuid_class_simple_device_add(cpu);
 		break;
 	case CPU_DEAD:
-		cpuid_class_simple_device_remove();
+		class_simple_device_remove(MKDEV(CPUID_MAJOR, cpu));
 		break;
 	}
 	return NOTIFY_OK;
@@ -224,7 +216,10 @@
 	goto out;
 
 out_class:
-	cpuid_class_simple_device_remove();
+	i = 0;
+	for_each_online_cpu(i) {
+		class_simple_device_remove(MKDEV(CPUID_MAJOR, i));
+	}
 	class_simple_destroy(cpuid_class);
 out_chrdev:
 	unregister_chrdev(CPUID_MAJOR, "cpu/cpuid");	
@@ -234,7 +229,10 @@
 
 void __exit cpuid_exit(void)
 {
-	cpuid_class_simple_device_remove();
+	int cpu = 0;
+
+	for_each_online_cpu(cpu)
+		class_simple_device_remove(MKDEV(CPUID_MAJOR, cpu));
 	class_simple_destroy(cpuid_class);
 	unregister_chrdev(CPUID_MAJOR, "cpu/cpuid");
 	unregister_cpu_notifier(&cpuid_class_cpu_notifier);