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"
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: [email protected]
Current: [email protected]
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"
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: [email protected]
> Current: [email protected]
>
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: [email protected]
Current: [email protected]
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
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
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: [email protected]
Current: [email protected]
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
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 ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html
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
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: [email protected]
Current: [email protected]
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
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: [email protected]
Current: [email protected]
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
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"
On Tue, 8 Jan 2002 11:13:02 +0000
Matt Dainty <[email protected]> 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.
On Tue, Jan 08, 2002 at 08:14:51PM +1100, Rusty Russell wrote:
> On Tue, 8 Jan 2002 11:13:02 +0000
> Matt Dainty <[email protected]> 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"
Followup to: <[email protected]>
By author: Rusty Russell <[email protected]>
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
--
<[email protected]> at work, <[email protected]> in private!
"Unix gives you enough rope to shoot yourself in the foot."
http://www.zytor.com/~hpa/puzzle.txt <[email protected]>
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 <[email protected]> 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: [email protected]
Current: [email protected]
On 8 Jan 2002 09:17:29 -0800
"H. Peter Anvin" <[email protected]> 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.
Followup to: <[email protected]>
By author: Rusty Russell <[email protected]>
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
--
<[email protected]> at work, <[email protected]> in private!
"Unix gives you enough rope to shoot yourself in the foot."
http://www.zytor.com/~hpa/puzzle.txt <[email protected]>
H. Peter Anvin writes:
> By author: Rusty Russell <[email protected]>
> > 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: [email protected]
Current: [email protected]
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
On 8 Jan 2002 19:16:30 -0800
"H. Peter Anvin" <[email protected]> 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.
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: [email protected]
Current: [email protected]
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: [email protected]
Current: [email protected]
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