Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[Matt Dainty]

On Sun, Jan 06, 2002 at 06:40:58PM -0700, Richard Gooch wrote:
> 
> So I'd like to propose a new file (say kernel/smp.c) which has generic
> startup code for each CPU. To start with, it can have a
> generic_cpu_init() function, which is called by each arch. Note that
> this function would be called for the boot CPU too.

Would this also be hacked into whatever Hotswap CPU support exists? Such
that plugging in a new CPU spawns a new cpu/%d directory, (and removing one
deletes the directory), or do we just scan for the total number of possible
slots on boot and rely on any nodes to return -ENODEV or whatever when
there's no CPU physically present? (Probably easier)

(Maybe not smp.c, as this is for Uniprocessor boxen as well, cpu.c? :-)

Matt
-- 
"Phased plasma rifle in a forty-watt range?"
"Hey, just what you see, pal"

[PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[Matt Dainty]

[-- Attachment #1: Type: text/plain, Size: 499 bytes --]

Hi,

Please find attached a patch to add support for devfs to the i386 cpuid and
msr drivers. Not only that, but it fixes a problem with loading these
drivers as modules in that the exit hooks on the module never run, (simply
changing the function prototypes to include 'static' seems to fix this).

Patch is against 2.4.17. SMP environment isn't tested, but I can't see any
reason why it wouldn't work...

Cheers

Matt
-- 
"Phased plasma rifle in a forty-watt range?"
"Hey, just what you see, pal"

[-- Attachment #2: patch-2.4.17-cpuid-msr-devfs.patch --]
[-- Type: text/plain, Size: 4306 bytes --]

diff -ur linux-2.4.17.orig/arch/i386/kernel/cpuid.c linux-2.4.17/arch/i386/kernel/cpuid.c
--- linux-2.4.17.orig/arch/i386/kernel/cpuid.c	Sat Jan  5 19:20:08 2002
+++ linux-2.4.17/arch/i386/kernel/cpuid.c	Sun Jan  6 18:09:08 2002
@@ -23,6 +23,11 @@
  *
  * This driver uses /dev/cpu/%d/cpuid where %d is the minor number, and on
  * an SMP box will direct the access to CPU %d.
+ *
+ * ChangeLog
+ *
+ * 2002-01-06	Matt Dainty <matt@bodgit-n-scarper.com>
+ *		Added support for devfs
  */
 
 #include <linux/module.h>
@@ -35,12 +40,16 @@
 #include <linux/poll.h>
 #include <linux/smp.h>
 #include <linux/major.h>
+#include <linux/threads.h>
+#include <linux/devfs_fs_kernel.h>
 
 #include <asm/processor.h>
 #include <asm/msr.h>
 #include <asm/uaccess.h>
 #include <asm/system.h>
 
+static devfs_handle_t devfs_handle[NR_CPUS];
+
 #ifdef CONFIG_SMP
 
 struct cpuid_command {
@@ -140,24 +149,45 @@
   open:		cpuid_open,
 };
 
-int __init cpuid_init(void)
+static int __init cpuid_init(void)
 {
-  if (register_chrdev(CPUID_MAJOR, "cpu/cpuid", &cpuid_fops)) {
+  int i;
+  char name[16];
+
+  if (devfs_register_chrdev(CPUID_MAJOR, "cpu/%d/cpuid", &cpuid_fops)) {
     printk(KERN_ERR "cpuid: unable to get major %d for cpuid\n",
 	   CPUID_MAJOR);
     return -EBUSY;
   }
+  for(i = 0; i < NR_CPUS; i++) {
+    sprintf(name, "cpu/%d/cpuid", i);
+    if ((devfs_handle[i] = devfs_register(NULL, name, DEVFS_FL_DEFAULT,
+					  CPUID_MAJOR, i,
+					  S_IFCHR | S_IRUSR | S_IRGRP,
+					  &cpuid_fops, NULL)) == NULL) {
+      printk(KERN_ERR "cpuid: failed to devfs_register()\n");
+    }
+  }
 
   return 0;
 }
 
-void __exit cpuid_exit(void)
+static void __exit cpuid_exit(void)
 {
-  unregister_chrdev(CPUID_MAJOR, "cpu/cpuid");
+  int i;
+  devfs_handle_t parent;
+
+  for(i = 0; i < NR_CPUS; i++) {
+    parent = devfs_get_parent(devfs_handle[i]);
+    devfs_unregister(devfs_handle[i]);
+    if(devfs_get_first_child(parent) == NULL)
+      devfs_unregister(parent);
+  }
+  devfs_unregister_chrdev(CPUID_MAJOR, "cpu/%d/cpuid");
 }
 
 module_init(cpuid_init);
-module_exit(cpuid_exit)
+module_exit(cpuid_exit);
 
 EXPORT_NO_SYMBOLS;
 
diff -ur linux-2.4.17.orig/arch/i386/kernel/msr.c linux-2.4.17/arch/i386/kernel/msr.c
--- linux-2.4.17.orig/arch/i386/kernel/msr.c	Sat Jan  5 19:20:08 2002
+++ linux-2.4.17/arch/i386/kernel/msr.c	Sun Jan  6 18:09:18 2002
@@ -22,6 +22,11 @@
  *
  * This driver uses /dev/cpu/%d/msr where %d is the minor number, and on
  * an SMP box will direct the access to CPU %d.
+ *
+ * ChangeLog
+ *
+ * 2002-01-06   Matt Dainty <matt@bodgit-n-scarper.com>
+ *              Added support for devfs
  */
 
 #include <linux/module.h>
@@ -34,12 +39,16 @@
 #include <linux/poll.h>
 #include <linux/smp.h>
 #include <linux/major.h>
+#include <linux/threads.h>
+#include <linux/devfs_fs_kernel.h>
 
 #include <asm/processor.h>
 #include <asm/msr.h>
 #include <asm/uaccess.h>
 #include <asm/system.h>
 
+static devfs_handle_t devfs_handle[NR_CPUS];
+
 /* Note: "err" is handled in a funny way below.  Otherwise one version
    of gcc or another breaks. */
 
@@ -248,24 +257,45 @@
   open:		msr_open,
 };
 
-int __init msr_init(void)
+static int __init msr_init(void)
 {
-  if (register_chrdev(MSR_MAJOR, "cpu/msr", &msr_fops)) {
+  int i;
+  char name[16];
+
+  if (devfs_register_chrdev(MSR_MAJOR, "cpu/%d/msr", &msr_fops)) {
     printk(KERN_ERR "msr: unable to get major %d for msr\n",
 	   MSR_MAJOR);
     return -EBUSY;
   }
+  for(i = 0; i < NR_CPUS; i++) {
+    sprintf(name, "cpu/%d/msr", i);
+    if ((devfs_handle[i] = devfs_register(NULL, name, DEVFS_FL_DEFAULT,
+					  MSR_MAJOR, i,
+					  S_IFCHR | S_IRUSR | S_IRGRP | S_IWUSR,
+					  &msr_fops, NULL)) == NULL) {
+      printk(KERN_ERR "msr: failed to devfs_register()\n");
+    }
+  }
   
   return 0;
 }
 
-void __exit msr_exit(void)
+static void __exit msr_exit(void)
 {
-  unregister_chrdev(MSR_MAJOR, "cpu/msr");
+  int i;
+  devfs_handle_t parent;
+
+  for(i = 0; i < NR_CPUS; i++) {
+    parent = devfs_get_parent(devfs_handle[i]);
+    devfs_unregister(devfs_handle[i]);
+    if(devfs_get_first_child(parent) == NULL)
+      devfs_unregister(parent);
+  }
+  devfs_unregister_chrdev(MSR_MAJOR, "cpu/%d/msr");
 }
 
 module_init(msr_init);
-module_exit(msr_exit)
+module_exit(msr_exit);
 
 EXPORT_NO_SYMBOLS;
 

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[Richard Gooch]

Matt Dainty writes:
> Please find attached a patch to add support for devfs to the i386 cpuid and
> msr drivers. Not only that, but it fixes a problem with loading these
> drivers as modules in that the exit hooks on the module never run, (simply
> changing the function prototypes to include 'static' seems to fix this).
> 
> Patch is against 2.4.17. SMP environment isn't tested, but I can't see any
> reason why it wouldn't work...

Looks mostly reasonable, except for:

> -void __exit cpuid_exit(void)
> +static void __exit cpuid_exit(void)
>  {
> -  unregister_chrdev(CPUID_MAJOR, "cpu/cpuid");
> +  int i;
> +  devfs_handle_t parent;
> +
> +  for(i = 0; i < NR_CPUS; i++) {
> +    parent = devfs_get_parent(devfs_handle[i]);
> +    devfs_unregister(devfs_handle[i]);
> +    if(devfs_get_first_child(parent) == NULL)
> +      devfs_unregister(parent);
> +  }
> +  devfs_unregister_chrdev(CPUID_MAJOR, "cpu/%d/cpuid");
>  }

There is no need to remove the parent /dev/cpu/%d directory, and in
fact it's better not to. All you need is this simpler loop:
	for(i = 0; i < NR_CPUS; i++)
		devfs_unregister(devfs_handle[i]);

You do something similar in the MSR driver.

				Regards,

					Richard....
Permanent: rgooch@atnf.csiro.au
Current:   rgooch@ras.ucalgary.ca

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[Matt Dainty]

[-- Attachment #1: Type: text/plain, Size: 328 bytes --]

Okey dokey,

Please find attached a second, better patch to add devfs support to the i386
cpuid and msr drivers. Now it doesn't nuke the cpu/X directories on
unloading and only enumerates CPUs based on smp_num_cpus instead of NR_CPUS.

Cheers

Matt
-- 
"Phased plasma rifle in a forty-watt range?"
"Hey, just what you see, pal"

[-- Attachment #2: patch-2.4.17-cpuid-msr-devfs.patch --]
[-- Type: text/plain, Size: 4002 bytes --]

diff -ur linux-2.4.17.orig/arch/i386/kernel/cpuid.c linux-2.4.17/arch/i386/kernel/cpuid.c
--- linux-2.4.17.orig/arch/i386/kernel/cpuid.c	Sat Jan  5 19:20:08 2002
+++ linux-2.4.17/arch/i386/kernel/cpuid.c	Sun Jan  6 20:29:07 2002
@@ -23,6 +23,11 @@
  *
  * This driver uses /dev/cpu/%d/cpuid where %d is the minor number, and on
  * an SMP box will direct the access to CPU %d.
+ *
+ * ChangeLog
+ *
+ * 2002-01-06	Matt Dainty <matt@bodgit-n-scarper.com>
+ *		Added support for devfs
  */
 
 #include <linux/module.h>
@@ -35,12 +40,16 @@
 #include <linux/poll.h>
 #include <linux/smp.h>
 #include <linux/major.h>
+#include <linux/threads.h>
+#include <linux/devfs_fs_kernel.h>
 
 #include <asm/processor.h>
 #include <asm/msr.h>
 #include <asm/uaccess.h>
 #include <asm/system.h>
 
+static devfs_handle_t devfs_handle[NR_CPUS];
+
 #ifdef CONFIG_SMP
 
 struct cpuid_command {
@@ -140,24 +149,40 @@
   open:		cpuid_open,
 };
 
-int __init cpuid_init(void)
+static int __init cpuid_init(void)
 {
-  if (register_chrdev(CPUID_MAJOR, "cpu/cpuid", &cpuid_fops)) {
+  int i;
+  char name[16];
+
+  if (devfs_register_chrdev(CPUID_MAJOR, "cpu/%d/cpuid", &cpuid_fops)) {
     printk(KERN_ERR "cpuid: unable to get major %d for cpuid\n",
 	   CPUID_MAJOR);
     return -EBUSY;
   }
+  for(i = 0; i < smp_num_cpus; i++) {
+    sprintf(name, "cpu/%d/cpuid", i);
+    if ((devfs_handle[i] = devfs_register(NULL, name, DEVFS_FL_DEFAULT,
+					  CPUID_MAJOR, i,
+					  S_IFCHR | S_IRUSR | S_IRGRP,
+					  &cpuid_fops, NULL)) == NULL) {
+      printk(KERN_ERR "cpuid: failed to devfs_register()\n");
+    }
+  }
 
   return 0;
 }
 
-void __exit cpuid_exit(void)
+static void __exit cpuid_exit(void)
 {
-  unregister_chrdev(CPUID_MAJOR, "cpu/cpuid");
+  int i;
+
+  for(i = 0; i < smp_num_cpus; i++)
+    devfs_unregister(devfs_handle[i]);
+  devfs_unregister_chrdev(CPUID_MAJOR, "cpu/%d/cpuid");
 }
 
 module_init(cpuid_init);
-module_exit(cpuid_exit)
+module_exit(cpuid_exit);
 
 EXPORT_NO_SYMBOLS;
 
diff -ur linux-2.4.17.orig/arch/i386/kernel/msr.c linux-2.4.17/arch/i386/kernel/msr.c
--- linux-2.4.17.orig/arch/i386/kernel/msr.c	Sat Jan  5 19:20:08 2002
+++ linux-2.4.17/arch/i386/kernel/msr.c	Sun Jan  6 20:29:19 2002
@@ -22,6 +22,11 @@
  *
  * This driver uses /dev/cpu/%d/msr where %d is the minor number, and on
  * an SMP box will direct the access to CPU %d.
+ *
+ * ChangeLog
+ *
+ * 2002-01-06   Matt Dainty <matt@bodgit-n-scarper.com>
+ *              Added support for devfs
  */
 
 #include <linux/module.h>
@@ -34,12 +39,16 @@
 #include <linux/poll.h>
 #include <linux/smp.h>
 #include <linux/major.h>
+#include <linux/threads.h>
+#include <linux/devfs_fs_kernel.h>
 
 #include <asm/processor.h>
 #include <asm/msr.h>
 #include <asm/uaccess.h>
 #include <asm/system.h>
 
+static devfs_handle_t devfs_handle[NR_CPUS];
+
 /* Note: "err" is handled in a funny way below.  Otherwise one version
    of gcc or another breaks. */
 
@@ -248,24 +257,40 @@
   open:		msr_open,
 };
 
-int __init msr_init(void)
+static int __init msr_init(void)
 {
-  if (register_chrdev(MSR_MAJOR, "cpu/msr", &msr_fops)) {
+  int i;
+  char name[16];
+
+  if (devfs_register_chrdev(MSR_MAJOR, "cpu/%d/msr", &msr_fops)) {
     printk(KERN_ERR "msr: unable to get major %d for msr\n",
 	   MSR_MAJOR);
     return -EBUSY;
   }
+  for(i = 0; i < smp_num_cpus; i++) {
+    sprintf(name, "cpu/%d/msr", i);
+    if ((devfs_handle[i] = devfs_register(NULL, name, DEVFS_FL_DEFAULT,
+					  MSR_MAJOR, i,
+					  S_IFCHR | S_IRUSR | S_IRGRP | S_IWUSR,
+					  &msr_fops, NULL)) == NULL) {
+      printk(KERN_ERR "msr: failed to devfs_register()\n");
+    }
+  }
   
   return 0;
 }
 
-void __exit msr_exit(void)
+static void __exit msr_exit(void)
 {
-  unregister_chrdev(MSR_MAJOR, "cpu/msr");
+  int i;
+
+  for(i = 0; i < smp_num_cpus; i++)
+    devfs_unregister(devfs_handle[i]);
+  devfs_unregister_chrdev(MSR_MAJOR, "cpu/%d/msr");
 }
 
 module_init(msr_init);
-module_exit(msr_exit)
+module_exit(msr_exit);
 
 EXPORT_NO_SYMBOLS;
 

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[H. Peter Anvin]

Matt Dainty wrote:

> Okey dokey,
> 
> Please find attached a second, better patch to add devfs support to the i386
> cpuid and msr drivers. Now it doesn't nuke the cpu/X directories on
> unloading and only enumerates CPUs based on smp_num_cpus instead of NR_CPUS.
> 


If you don't understand why this is idiotic, then let me enlighten you: 
there is no sensible reason why /dev/cpu/%d should only be populated 
after having run a CPU-dependent device driver.  /dev/cpu/%d should be 
always populated; heck, that's the only way you can sensibly handle 
hotswapping CPUs.

I WILL NOT accept a patch as long as devfs is as fucked in the head as 
it currently is.  Unfortunately, that seems like it will be a long long 
time.

	-hpa

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[Richard Gooch]

H. Peter Anvin writes:
> Matt Dainty wrote:
> 
> > Okey dokey,
> > 
> > Please find attached a second, better patch to add devfs support to the i386
> > cpuid and msr drivers. Now it doesn't nuke the cpu/X directories on
> > unloading and only enumerates CPUs based on smp_num_cpus instead of NR_CPUS.
> 
> If you don't understand why this is idiotic, then let me enlighten you: 
> there is no sensible reason why /dev/cpu/%d should only be populated 
> after having run a CPU-dependent device driver.  /dev/cpu/%d should be 
> always populated; heck, that's the only way you can sensibly handle 
> hotswapping CPUs.

I've already privately told Matt that it would be nice if creation of
/dev/cpu/%d was handled by generic boot code, and not a driver.
However, I don't see that as essential for the CPUID and MSR drivers.

> I WILL NOT accept a patch as long as devfs is as fucked in the head
> as it currently is.  Unfortunately, that seems like it will be a
> long long time.

No need to get rude.

				Regards,

					Richard....
Permanent: rgooch@atnf.csiro.au
Current:   rgooch@ras.ucalgary.ca

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[H. Peter Anvin]

Richard Gooch wrote:

>>>
>>If you don't understand why this is idiotic, then let me enlighten you: 
>>there is no sensible reason why /dev/cpu/%d should only be populated 
>>after having run a CPU-dependent device driver.  /dev/cpu/%d should be 
>>always populated; heck, that's the only way you can sensibly handle 
>>hotswapping CPUs.
> 
> I've already privately told Matt that it would be nice if creation of
> /dev/cpu/%d was handled by generic boot code, and not a driver.
> However, I don't see that as essential for the CPUID and MSR drivers.
> 


I don't see putting a lot of devfs shit into the CPUID and MSR drivers 
as essential in any shape, way or form.  Do it right or don't do it at all.

	-hpa

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[H. Peter Anvin]

Once again, this shit does not belong in N drivers; it is core code.

	-hpa


Richard Gooch wrote:

> Matt Dainty writes:
> 
>>Please find attached a patch to add support for devfs to the i386 cpuid and
>>msr drivers. Not only that, but it fixes a problem with loading these
>>drivers as modules in that the exit hooks on the module never run, (simply
>>changing the function prototypes to include 'static' seems to fix this).
>>
>>Patch is against 2.4.17. SMP environment isn't tested, but I can't see any
>>reason why it wouldn't work...
>>
> 
> Looks mostly reasonable, except for:
> 
> 
>>-void __exit cpuid_exit(void)
>>+static void __exit cpuid_exit(void)
>> {
>>-  unregister_chrdev(CPUID_MAJOR, "cpu/cpuid");
>>+  int i;
>>+  devfs_handle_t parent;
>>+
>>+  for(i = 0; i < NR_CPUS; i++) {
>>+    parent = devfs_get_parent(devfs_handle[i]);
>>+    devfs_unregister(devfs_handle[i]);
>>+    if(devfs_get_first_child(parent) == NULL)
>>+      devfs_unregister(parent);
>>+  }
>>+  devfs_unregister_chrdev(CPUID_MAJOR, "cpu/%d/cpuid");
>> }
>>
> 
> There is no need to remove the parent /dev/cpu/%d directory, and in
> fact it's better not to. All you need is this simpler loop:
> 	for(i = 0; i < NR_CPUS; i++)
> 		devfs_unregister(devfs_handle[i]);
> 
> You do something similar in the MSR driver.
> 
> 				Regards,
> 
> 					Richard....
> Permanent: rgooch@atnf.csiro.au
> Current:   rgooch@ras.ucalgary.ca
> 

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[Richard Gooch]

H. Peter Anvin writes:
> Once again, this shit does not belong in N drivers; it is core code.

Drivers have to register their own device nodes. What kind of API do
you propose?

				Regards,

					Richard....
Permanent: rgooch@atnf.csiro.au
Current:   rgooch@ras.ucalgary.ca

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[H. Peter Anvin]

Richard Gooch wrote:

> H. Peter Anvin writes:
> 
>>Once again, this shit does not belong in N drivers; it is core code.
>>
> 
> Drivers have to register their own device nodes. What kind of API do
> you propose?
> 


The existence of a CPU creates /dev/cpu/# and registering a node 
replicates across the /dev/cpu directories.

	-hpa

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[Russell King]

On Sun, Jan 06, 2002 at 01:10:10PM -0800, H. Peter Anvin wrote:
> The existence of a CPU creates /dev/cpu/# and registering a node 
> replicates across the /dev/cpu directories.

And, thus, we decend into more /proc crappyness.

After *lots* of discussion and months of waiting, it was decided between
Alan, David Jones, Jeff Garzik, and other affected parties that
/proc/sys/cpu/#/whatever would be a reasonable.  It has even appeared on
lkml a couple of times in the past.

Currently, there is an allocated sysctl number in include/linux/sysctl.h
for /proc/sys/cpu, and it is used by the cpufreq code to provide:

  /proc/sys/cpu/#/speed
  /proc/sys/cpu/#/speed-max
  /proc/sys/cpu/#/speed-min

However, it's true that some of that needs to be pulled out so anything
can use /proc/sys/cpu/#.  Just takes the necessary parties to get together
to do the hard work, and with the right hardware to test it out.

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[H. Peter Anvin]

Russell King wrote:

> On Sun, Jan 06, 2002 at 01:10:10PM -0800, H. Peter Anvin wrote:
> 
>>The existence of a CPU creates /dev/cpu/# and registering a node 
>>replicates across the /dev/cpu directories.
>>
> 
> And, thus, we decend into more /proc crappyness.
> 
> After *lots* of discussion and months of waiting, it was decided between
> Alan, David Jones, Jeff Garzik, and other affected parties that
> /proc/sys/cpu/#/whatever would be a reasonable.  It has even appeared on
> lkml a couple of times in the past.
> 


Ummm... this isn't about /proc.

	-hpa

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[Richard Gooch]

H. Peter Anvin writes:
> Richard Gooch wrote:
> 
> > H. Peter Anvin writes:
> > 
> >>Once again, this shit does not belong in N drivers; it is core code.
> >>
> > 
> > Drivers have to register their own device nodes. What kind of API do
> > you propose?
> 
> The existence of a CPU creates /dev/cpu/# and registering a node 
> replicates across the /dev/cpu directories.

So you mean something like this:

void devfs_per_cpu_register (const char *leafname, unsigned int flags,
			     unsigned int major, unsigned int minor_start,
			     umode_t mode, void *ops);

void devfs_per_cpu_unregister (const char *leafname);

with code in the per-cpu boot code to create the /dev/cpu/%d
directories.

				Regards,

					Richard....
Permanent: rgooch@atnf.csiro.au
Current:   rgooch@ras.ucalgary.ca

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[H. Peter Anvin]

Richard Gooch wrote:

> 
> So you mean something like this:
> 
> void devfs_per_cpu_register (const char *leafname, unsigned int flags,
> 			     unsigned int major, unsigned int minor_start,
> 			     umode_t mode, void *ops);
> 
> void devfs_per_cpu_unregister (const char *leafname);
> 
> with code in the per-cpu boot code to create the /dev/cpu/%d
> directories.
> 


Yes, that sounds like a good way to do it.

	-hpa

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[Richard Gooch]

H. Peter Anvin writes:
> Richard Gooch wrote:
> 
> > 
> > So you mean something like this:
> > 
> > void devfs_per_cpu_register (const char *leafname, unsigned int flags,
> > 			     unsigned int major, unsigned int minor_start,
> > 			     umode_t mode, void *ops);
> > 
> > void devfs_per_cpu_unregister (const char *leafname);
> > 
> > with code in the per-cpu boot code to create the /dev/cpu/%d
> > directories.
> 
> Yes, that sounds like a good way to do it.

Unfortunately, there doesn't seem to be a really nice place to put
generic SMP startup code. Each arch defines it's own per-cpu startup
code. As Matt asked in a private email, it would require hacks to each
arch to support this (Matt: this is my reply:-). While it would be
fairly simple to add a call to a devfs_cpu_register() function to each
arch, this does seem a bit hackish.

So I'd like to propose a new file (say kernel/smp.c) which has generic
startup code for each CPU. To start with, it can have a
generic_cpu_init() function, which is called by each arch. Note that
this function would be called for the boot CPU too.

Comments?

				Regards,

					Richard....
Permanent: rgooch@atnf.csiro.au
Current:   rgooch@ras.ucalgary.ca

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[H. Peter Anvin]

Richard Gooch wrote:

> 
> Unfortunately, there doesn't seem to be a really nice place to put
> generic SMP startup code. Each arch defines it's own per-cpu startup
> code. As Matt asked in a private email, it would require hacks to each
> arch to support this (Matt: this is my reply:-). While it would be
> fairly simple to add a call to a devfs_cpu_register() function to each
> arch, this does seem a bit hackish.
> 
> So I'd like to propose a new file (say kernel/smp.c) which has generic
> startup code for each CPU. To start with, it can have a
> generic_cpu_init() function, which is called by each arch. Note that
> this function would be called for the boot CPU too.
> 


Makes sense.

	-hpa

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[Rusty Russell]

On Tue, 8 Jan 2002 11:13:02 +0000
Matt Dainty <matt@bodgit-n-scarper.com> wrote:

> On Sun, Jan 06, 2002 at 06:40:58PM -0700, Richard Gooch wrote:
> > 
> > So I'd like to propose a new file (say kernel/smp.c) which has generic
> > startup code for each CPU. To start with, it can have a
> > generic_cpu_init() function, which is called by each arch. Note that
> > this function would be called for the boot CPU too.
> 
> Would this also be hacked into whatever Hotswap CPU support exists? Such

We use /proc/sys/cpu/#/.  I don't understand what /dev/cpu/xxx is supposed to
do.

It's unfortunate that /proc/sys and /proc suck so hard that good coders go
to great lengths to do anything else [see previous /proc/sys patches].

Rusty.
-- 
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[Matt Dainty]

On Tue, Jan 08, 2002 at 08:14:51PM +1100, Rusty Russell wrote:
> On Tue, 8 Jan 2002 11:13:02 +0000
> Matt Dainty <matt@bodgit-n-scarper.com> wrote:
> 
> > On Sun, Jan 06, 2002 at 06:40:58PM -0700, Richard Gooch wrote:
> > > 
> > > So I'd like to propose a new file (say kernel/smp.c) which has generic
> > > startup code for each CPU. To start with, it can have a
> > > generic_cpu_init() function, which is called by each arch. Note that
> > > this function would be called for the boot CPU too.
> > 
> > Would this also be hacked into whatever Hotswap CPU support exists? Such
> 
> We use /proc/sys/cpu/#/.  I don't understand what /dev/cpu/xxx is supposed to
> do.

/dev/cpu/[0...]/... contains (on i386 at least), the cpuid and msr character
devices, except on devfs-enabled boxen, these don't appear automatically.

Matt
-- 
"Phased plasma rifle in a forty-watt range?"
"Hey, just what you see, pal"

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[H. Peter Anvin]

Followup to:  <20020108201451.088f7f99.rusty@rustcorp.com.au>
By author:    Rusty Russell <rusty@rustcorp.com.au>
In newsgroup: linux.dev.kernel
> 
> It's unfortunate that /proc/sys and /proc suck so hard that good coders go
> to great lengths to do anything else [see previous /proc/sys patches].
> 

/proc/sys is pretty cool.  However, procfs has no permission control
system set up, unlike /dev.  This is inherent; adjusting sysctls is a
root-only function and cannot be made otherwise.

	-hpa
-- 
<hpa@transmeta.com> at work, <hpa@zytor.com> in private!
"Unix gives you enough rope to shoot yourself in the foot."
http://www.zytor.com/~hpa/puzzle.txt	<amsp@zytor.com>

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[Richard Gooch]

Matt Dainty writes:
> On Tue, Jan 08, 2002 at 08:14:51PM +1100, Rusty Russell wrote:
> > On Tue, 8 Jan 2002 11:13:02 +0000
> > Matt Dainty <matt@bodgit-n-scarper.com> wrote:
> > 
> > > On Sun, Jan 06, 2002 at 06:40:58PM -0700, Richard Gooch wrote:
> > > > 
> > > > So I'd like to propose a new file (say kernel/smp.c) which has generic
> > > > startup code for each CPU. To start with, it can have a
> > > > generic_cpu_init() function, which is called by each arch. Note that
> > > > this function would be called for the boot CPU too.
> > > 
> > > Would this also be hacked into whatever Hotswap CPU support exists? Such

Hm, that would be nice.

> > We use /proc/sys/cpu/#/.  I don't understand what /dev/cpu/xxx is supposed to
> > do.
> 
> /dev/cpu/[0...]/... contains (on i386 at least), the cpuid and msr
> character devices, except on devfs-enabled boxen, these don't appear
> automatically.

Indeed.

				Regards,

					Richard....
Permanent: rgooch@atnf.csiro.au
Current:   rgooch@ras.ucalgary.ca

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[Rusty Russell]

On 8 Jan 2002 09:17:29 -0800
"H. Peter Anvin" <hpa@zytor.com> wrote:
> /proc/sys is pretty cool.  However, procfs has no permission control
> system set up, unlike /dev.  This is inherent; adjusting sysctls is a
> root-only function and cannot be made otherwise.

Incorrect.  See my new /proc/sys implementation patch.  It's hidden in the
flames somewhere...

Cheers,
Rusty.
-- 
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[H. Peter Anvin]

Followup to:  <20020109120108.39bcf7ad.rusty@rustcorp.com.au>
By author:    Rusty Russell <rusty@rustcorp.com.au>
In newsgroup: linux.dev.kernel
> 
> Incorrect.  See my new /proc/sys implementation patch.  It's hidden in the
> flames somewhere...
> 

So you chown an entry, then a module is unloaded and reloaded, now
what happens?

It's the old "virtual filesystem which really wants persistence" issue
again...

	-hpa
-- 
<hpa@transmeta.com> at work, <hpa@zytor.com> in private!
"Unix gives you enough rope to shoot yourself in the foot."
http://www.zytor.com/~hpa/puzzle.txt	<amsp@zytor.com>

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[Rusty Russell]

On 8 Jan 2002 19:16:30 -0800
"H. Peter Anvin" <hpa@zytor.com> wrote:
> So you chown an entry, then a module is unloaded and reloaded, now
> what happens?

Sorry, was responding to the second sentence, not the first:

> However, procfs has no permission control
> system set up, unlike /dev.  This is inherent; adjusting sysctls is a
> root-only function and cannot be made otherwise.

In practice, my /proc/sys perms are four bits: ALL READ, ALL WRITE, ROOT READ, ROOT WRITE.

Hope that helps,
Rusty.
-- 
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[Richard Gooch]

H. Peter Anvin writes:
> By author:    Rusty Russell <rusty@rustcorp.com.au>
> > Incorrect.  See my new /proc/sys implementation patch.  It's hidden in the
> > flames somewhere...
> > 
> 
> So you chown an entry, then a module is unloaded and reloaded, now
> what happens?
> 
> It's the old "virtual filesystem which really wants persistence"
> issue again...

Works beautifully with devfs+devfsd :-)

Permissions get saved elsewhere in the namespace (perhaps even to the
underlying /dev) as you chown(2)/chmod(2)/mknod(2), and are restored
when entries are (re)created and/or at startup.

My /dev has persistence behaviour which looks like a FS with backing
store.

				Regards,

					Richard....
Permanent: rgooch@atnf.csiro.au
Current:   rgooch@ras.ucalgary.ca

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[H. Peter Anvin]

Richard Gooch wrote:

>>>
>>So you chown an entry, then a module is unloaded and reloaded, now
>>what happens?
>>
>>It's the old "virtual filesystem which really wants persistence"
>>issue again...
>>
> 
> Works beautifully with devfs+devfsd :-)
> 
> Permissions get saved elsewhere in the namespace (perhaps even to the
> underlying /dev) as you chown(2)/chmod(2)/mknod(2), and are restored
> when entries are (re)created and/or at startup.
> 
> My /dev has persistence behaviour which looks like a FS with backing
> store.
> 


Yes, after quite a few years it finally got in there.  This is a Good 
Thing[TM].  Now apply the same problem to /proc.

	-hpa

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[Richard Gooch]

H. Peter Anvin writes:
> Richard Gooch wrote:
> 
> >>>
> >>So you chown an entry, then a module is unloaded and reloaded, now
> >>what happens?
> >>
> >>It's the old "virtual filesystem which really wants persistence"
> >>issue again...
> >>
> > 
> > Works beautifully with devfs+devfsd :-)
> > 
> > Permissions get saved elsewhere in the namespace (perhaps even to the
> > underlying /dev) as you chown(2)/chmod(2)/mknod(2), and are restored
> > when entries are (re)created and/or at startup.
> > 
> > My /dev has persistence behaviour which looks like a FS with backing
> > store.
> 
> Yes, after quite a few years it finally got in there.  This is a Good 
> Thing[TM].  Now apply the same problem to /proc.

Actually, the COPY action has been available since APR-2000 :-)

But as for applying this to /proc, that's a good point. My current
plans are to write v2.0 of the devfs core, which will use the dcache
to maintain the devfs tree structure, rather than having my own code
to do it. That should result in a significant reduction in code side,
and maybe finally people won't hate the idea so much.

So then, one approach would be to have control files where you want to
have permissions persistence placed in devfs. Of course, for
consistency, you'd want all control files under devfs. A /dev/sys
hierarchy perhaps.

Another idea is to turn devfs into some kind of "kernfs", and have one
instance for /dev, another for /proc/sys (or /system if you like), and
run an instance of devfsd (renamed to "kernfsd" perhaps) for each of
these instances.

Of course, it depends on whether people *want* permissions persistence
for /proc/sys. Is there much call for this?

				Regards,

					Richard....
Permanent: rgooch@atnf.csiro.au
Current:   rgooch@ras.ucalgary.ca

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[Ishan Jayawardena]

[-- Attachment #1: Type: text/plain, Size: 570 bytes --]

Perhaps this will suit your taste better...

I'd love to extend this work to support Paul Russell's hotplug cpu work, but 
I have absolutely no way at present to test it. (If some kind party would 
like to donate a quad pentium 4 Xeon with hotplug cpu support, I'll be more 
than happy to do so ;)

This is against 2.4.18-pre9-ac4 with devfs v199.9

Relevent comments, advise, improvisations, flames, welcome.

I. O. Jayawardena
.



_________________________________________________________________
Send and receive Hotmail on your mobile device: http://mobile.msn.com

[-- Attachment #2: devfs.patch --]
[-- Type: application/octet-stream, Size: 8004 bytes --]

--- linux/fs/devfs/base.c	Mon Feb 18 16:07:49 2002
+++ linux-ioj/fs/devfs/base.c	Mon Feb 18 12:45:42 2002
@@ -631,6 +631,7 @@
 #include <linux/init.h>
 #include <linux/devfs_fs.h>
 #include <linux/devfs_fs_kernel.h>
+#include <linux/threads.h>
 #include <linux/smp_lock.h>
 #include <linux/smp.h>
 #include <linux/version.h>
@@ -1624,6 +1625,72 @@ devfs_handle_t devfs_register (devfs_han
 
 
 /**
+ * 	devfs_register_per_cpu - register a device node across all 
+ *		cpu-specific directories.
+ *	@leafname - the name of the device node
+ *
+ *	All other parameters are for devfs_register(). Returns zero on success.
+ */
+
+int devfs_register_per_cpu (const char *leafname, unsigned int flags, 
+			unsigned int major, unsigned int minor_start,
+			umode_t mode, void *ops, void *info)
+{
+    int i;
+    char *name;
+    char tmp[5] = { "0" }; /* FIXME: this string may overflow, but we don't know
+			the sizeof smp_num_cpus until we get it into a string */
+
+    if (smp_num_cpus > 1) sprintf (tmp, "%d", smp_num_cpus - 1);
+    name = kmalloc (strlen (leafname) + strlen (tmp) + sizeof (char) * 6, 
+			GFP_KERNEL);
+    if (!name) return -ENOMEM;
+    for(i = 0; i < smp_num_cpus; i++) {
+	sprintf (name, "cpu/%d/%s", i, leafname);
+	if (!devfs_register (NULL, name, flags, major, minor_start + i, 
+					mode, ops, info)) {
+	    PRINTK ("(): devfs_register() failed.\n");
+	    kfree (name);
+	    return -EAGAIN;
+	}
+    }
+    kfree (name);
+    return 0;
+} /* End function devfs_register_per_cpu */
+
+
+/**
+ *	devfs_unregister_per_cpu - unregister a node across /dev/cpu/
+ *	@leafname - the name of the device node
+ *
+ *	Unregisters a number of nodes (probably registered with
+ *	devfs_register_per_cpu() )
+ */
+
+void devfs_unregister_per_cpu (const char *leafname)
+{
+    int i;
+    char *name;
+    char tmp[5] = { "0" };
+    devfs_handle_t dh;
+
+    if (smp_num_cpus > 1) sprintf (tmp, "%d", smp_num_cpus - 1);
+    name = kmalloc (strlen (leafname) + strlen (tmp) + sizeof (char) * 6,
+			GFP_KERNEL);
+    if (!name) return;
+    for (i = 0; i < smp_num_cpus; i++) {
+	sprintf (name, "cpu/%d/%s", i, leafname);
+	dh = _devfs_walk_path (NULL, name, strlen (name), 0);
+	if (!dh) {
+	    kfree (name);
+	    return;
+	}
+	devfs_unregister (dh);
+    }
+    kfree (name);
+} /* End function devfs_unregister_per_cpu */
+
+/**
  *	_devfs_unhook - Unhook a device entry from its parents list
  *	@de: The entry to unhook.
  *
@@ -2355,7 +2422,9 @@ __setup("devfs=", devfs_setup);
 
 EXPORT_SYMBOL(devfs_put);
 EXPORT_SYMBOL(devfs_register);
+EXPORT_SYMBOL(devfs_register_per_cpu);
 EXPORT_SYMBOL(devfs_unregister);
+EXPORT_SYMBOL(devfs_unregister_per_cpu);
 EXPORT_SYMBOL(devfs_mk_symlink);
 EXPORT_SYMBOL(devfs_mk_dir);
 EXPORT_SYMBOL(devfs_get_handle);
@@ -3172,6 +3241,17 @@ static int devfs_mknod (struct inode *di
 			  inode->i_uid, inode->i_gid, fs_info, 0);
     return 0;
 }   /*  End Function devfs_mknod  */
+
+void devfs_mk_dir_per_cpu (void)
+{
+    int i;
+    char name[7];
+
+    for (i = 0; i < smp_num_cpus; i++) {
+	sprintf (name, "cpu/%d", i);
+	devfs_mk_dir (NULL, name, NULL);
+    }
+}
 
 static int devfs_readlink (struct dentry *dentry, char *buffer, int buflen)
 {
--- linux/include/linux/devfs_fs_kernel.h	Mon Feb 18 16:05:21 2002
+++ linux-ioj/include/linux/devfs_fs_kernel.h	Mon Feb 18 14:08:38 2002
@@ -73,12 +73,17 @@ extern devfs_handle_t devfs_register (de
 				      unsigned int flags,
 				      unsigned int major, unsigned int minor,
 				      umode_t mode, void *ops, void *info);
+extern int devfs_register_per_cpu (const char *leafname, unsigned int flags,
+				   unsigned int major, unsigned int minor_start,
+				   umode_t mode, void *ops, void *info);
 extern void devfs_unregister (devfs_handle_t de);
+extern void devfs_unregister_per_cpu (const char *leafname);
 extern int devfs_mk_symlink (devfs_handle_t dir, const char *name,
 			     unsigned int flags, const char *link,
 			     devfs_handle_t *handle, void *info);
 extern devfs_handle_t devfs_mk_dir (devfs_handle_t dir, const char *name,
 				    void *info);
+extern void devfs_mk_dir_per_cpu (void);
 extern devfs_handle_t devfs_get_handle (devfs_handle_t dir, const char *name,
 					unsigned int major,unsigned int minor,
 					char type, int traverse_symlinks);
@@ -149,9 +154,20 @@ static inline devfs_handle_t devfs_regis
 {
     return NULL;
 }
+static inline int devfs_register_per_cpu (const char *name,
+					  unsigned int flags, 
+					  unsigned int major, 
+					  unsigned int minor_start,
+					  umode_t mode, void *ops, void *info);
+{
+    return NULL;
+}
 static inline void devfs_unregister (devfs_handle_t de)
 {
-    return;
+}
+static inline void devfs_unregister_per_cpu (const char *name)
+{
+    return NULL;
 }
 static inline int devfs_mk_symlink (devfs_handle_t dir, const char *name,
 				    unsigned int flags, const char *link,
@@ -163,6 +179,9 @@ static inline devfs_handle_t devfs_mk_di
 					   const char *name, void *info)
 {
     return NULL;
+}
+static inline void devfs_mk_dir_per_cpu (void)
+{
 }
 static inline devfs_handle_t devfs_get_handle (devfs_handle_t dir,
 					       const char *name,
--- linux/init/main.c	Mon Feb 18 16:07:37 2002
+++ linux-ioj/init/main.c	Sun Feb 17 23:26:43 2002
@@ -497,9 +497,13 @@ unsigned long wait_init_idle;
 static void __init smp_init(void)
 {
 	APIC_init_uniprocessor();
+
+	devfs_mk_dir_per_cpu();
 }
 #else
-#define smp_init()	do { } while (0)
+#define smp_init()	do { \
+		devfs_mk_dir_per_cpu(); \
+	} while (0)
 #endif
 
 #else
@@ -523,6 +527,8 @@ static void __init smp_init(void)
 		barrier();
 	}
 	printk("All processors have done init_idle\n");
+
+	devfs_mk_dir_per_cpu();
 }
 
 #endif
--- linux/arch/i386/kernel/cpuid.c	Thu Oct 11 12:04:57 2001
+++ linux-ioj/arch/i386/kernel/cpuid.c	Mon Feb 18 15:19:32 2002
@@ -32,6 +32,7 @@
 #include <linux/errno.h>
 #include <linux/fcntl.h>
 #include <linux/init.h>
+#include <linux/devfs_fs_kernel.h>
 #include <linux/poll.h>
 #include <linux/smp.h>
 #include <linux/major.h>
@@ -140,24 +141,26 @@ static struct file_operations cpuid_fops
   open:		cpuid_open,
 };
 
-int __init cpuid_init(void)
+static int __init cpuid_init(void)
 {
-  if (register_chrdev(CPUID_MAJOR, "cpu/cpuid", &cpuid_fops)) {
+  if (devfs_register_chrdev(CPUID_MAJOR, "cpu/%d/cpuid", &cpuid_fops)) {
     printk(KERN_ERR "cpuid: unable to get major %d for cpuid\n",
 	   CPUID_MAJOR);
     return -EBUSY;
   }
 
-  return 0;
+  return devfs_register_per_cpu("cpuid", DEVFS_FL_DEFAULT, CPUID_MAJOR, 0,
+		S_IFCHR | S_IRUSR | S_IRGRP, &cpuid_fops, NULL);
 }
 
 void __exit cpuid_exit(void)
 {
-  unregister_chrdev(CPUID_MAJOR, "cpu/cpuid");
+  devfs_unregister_chrdev(CPUID_MAJOR, "cpu/%d/cpuid");
+  devfs_unregister_per_cpu("cpuid");
 }
 
 module_init(cpuid_init);
-module_exit(cpuid_exit)
+module_exit(cpuid_exit);
 
 EXPORT_NO_SYMBOLS;
 
--- linux/arch/i386/kernel/msr.c	Thu Oct 11 12:04:57 2001
+++ linux-ioj/arch/i386/kernel/msr.c	Mon Feb 18 15:19:05 2002
@@ -31,6 +31,7 @@
 #include <linux/errno.h>
 #include <linux/fcntl.h>
 #include <linux/init.h>
+#include <linux/devfs_fs_kernel.h>
 #include <linux/poll.h>
 #include <linux/smp.h>
 #include <linux/major.h>
@@ -248,24 +249,26 @@ static struct file_operations msr_fops =
   open:		msr_open,
 };
 
-int __init msr_init(void)
+static int __init msr_init(void)
 {
-  if (register_chrdev(MSR_MAJOR, "cpu/msr", &msr_fops)) {
+  if (devfs_register_chrdev(MSR_MAJOR, "cpu/%d/msr", &msr_fops)) {
     printk(KERN_ERR "msr: unable to get major %d for msr\n",
 	   MSR_MAJOR);
     return -EBUSY;
   }
   
-  return 0;
+  return devfs_register_per_cpu("msr", DEVFS_FL_DEFAULT, MSR_MAJOR, 0,
+			S_IFCHR | S_IRUSR | S_IRGRP, &msr_fops, NULL);
 }
 
 void __exit msr_exit(void)
 {
-  unregister_chrdev(MSR_MAJOR, "cpu/msr");
+  devfs_unregister_chrdev(MSR_MAJOR, "cpu/%d/msr");
+  devfs_unregister_per_cpu("msr");
 }
 
 module_init(msr_init);
-module_exit(msr_exit)
+module_exit(msr_exit);
 
 EXPORT_NO_SYMBOLS;
 

Re: [PATCH] DevFS support for /dev/cpu/X/(cpuid|msr)

[Richard Gooch]

Ishan Jayawardena writes:
> This is a multi-part message in MIME format.

First comment: please send the patch inline, not MIME-encoded.

> I'd love to extend this work to support Paul Russell's hotplug cpu work, but 
> I have absolutely no way at present to test it. (If some kind party would 
> like to donate a quad pentium 4 Xeon with hotplug cpu support, I'll be more 
> than happy to do so ;)
> 
> This is against 2.4.18-pre9-ac4 with devfs v199.9
> 
> Relevent comments, advise, improvisations, flames, welcome.

Second comment: devfs_(un)register_per_cpu() doesn't belong in
fs/devfs/base.c. I don't even think it should be in fs/devfs/util.c.
I think it belongs kernel/smp.c (that's right, create a new file).
There's too much duplicated SMP code in each arch/ tree. Let's create
a place for new stuff to be shared.

So, please send along your next version of the patch (inlined, of
course:-).

				Regards,

					Richard....
Permanent: rgooch@atnf.csiro.au
Current:   rgooch@ras.ucalgary.ca