[PATCH] KERN_INFO for devfs

[PATCH] KERN_INFO for devfs

[Denis Vlasenko]

Primary purpose of this patch is to make KERN_WARNING and
KERN_INFO log levels closer to their original meaning.
Today they are quite far from what was intended.
Just look what kernel writes at the WARNING level
each time you boot your box!

Diff for devfs.
--
vda

diff --recursive -u linux-2.4.13-orig/fs/devfs/base.c linux-2.4.13-new/fs/devfs/base.c
--- linux-2.4.13-orig/fs/devfs/base.c	Thu Oct 11 04:23:24 2001
+++ linux-2.4.13-new/fs/devfs/base.c	Thu Nov  8 23:42:11 2001
@@ -3289,13 +3289,13 @@
 {
     int err;

-    printk ("%s: v%s Richard Gooch (rgooch@atnf.csiro.au)\n",
-	    DEVFS_NAME, DEVFS_VERSION);
+    printk (KERN_INFO DEVFS_NAME ": v" DEVFS_VERSION
+	" Richard Gooch (rgooch@atnf.csiro.au)\n");
 #ifdef CONFIG_DEVFS_DEBUG
     devfs_debug = devfs_debug_init;
-    printk ("%s: devfs_debug: 0x%0x\n", DEVFS_NAME, devfs_debug);
+    printk (KERN_INFO DEVFS_NAME ": devfs_debug: 0x%0x\n", devfs_debug);
 #endif
-    printk ("%s: boot_options: 0x%0x\n", DEVFS_NAME, boot_options);
+    printk (KERN_INFO DEVFS_NAME ": boot_options: 0x%0x\n", boot_options);
     err = register_filesystem (&devfs_fs_type);
     if (!err)
     {

Re: [PATCH] KERN_INFO for devfs

[Richard Gooch]

Denis Vlasenko writes:
> Primary purpose of this patch is to make KERN_WARNING and
> KERN_INFO log levels closer to their original meaning.
> Today they are quite far from what was intended.
> Just look what kernel writes at the WARNING level
> each time you boot your box!
> 
> Diff for devfs.
> --
> vda
> 
> diff --recursive -u linux-2.4.13-orig/fs/devfs/base.c linux-2.4.13-new/fs/devfs/base.c

This patch won't even remotely apply to 2.4.18-pre7. Please don't
submit patches which were generated against old kernels unless you've
verified that they apply to the latest kernel.

Furthermore, if you look at 2.4.18-pre7, you'll notice that devfs has
changed the way most of it's messages are generated, and uses the
KERN_ values quite a bit.

				Regards,

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

Re: [PATCH] KERN_INFO for devfs

[Denis Vlasenko]

> > diff --recursive -u linux-2.4.13-orig/fs/devfs/base.c
> > linux-2.4.13-new/fs/devfs/base.c
>
> This patch won't even remotely apply to 2.4.18-pre7. Please don't
> submit patches which were generated against old kernels unless you've
> verified that they apply to the latest kernel.

Didn't try but I'm sure you're right :-)

Diff against 2.4.18-pre6 will be attached as soon as diff will finish
(it's over NFS).

I changed "none" to "devfs" in do_mount("none", "/dev", "devfs", 0, ""):
"none is busy" is misleading at umount time :-)

Aha, it's ready!
--
vda


diff -u --recursive linux-2.4.18-pre6mhv_ll/fs/devfs/base.c 
linux-2.4.18-pre6mhv_ll.devfs/fs/devfs/base.c
--- linux-2.4.18-pre6mhv_ll/fs/devfs/base.c     Fri Jan 25 15:49:53 2002
+++ linux-2.4.18-pre6mhv_ll.devfs/fs/devfs/base.c       Mon Jan 28 23:05:44 
2002
@@ -3464,17 +3464,16 @@
 {
     int err;

-    printk ("%s: v%s Richard Gooch (rgooch@atnf.csiro.au)\n",
-           DEVFS_NAME, DEVFS_VERSION);
+    printk (KERN_INFO DEVFS_NAME ": v" DEVFS_VERSION " Richard Gooch 
(rgooch@atnf.csiro.au)\n");
     devfsd_buf_cache = kmem_cache_create ("devfsd_event",
                                          sizeof (struct devfsd_buf_entry),
                                          0, 0, NULL, NULL);
     if (!devfsd_buf_cache) OOPS ("(): unable to allocate event slab\n");
 #ifdef CONFIG_DEVFS_DEBUG
     devfs_debug = devfs_debug_init;
-    printk ("%s: devfs_debug: 0x%0x\n", DEVFS_NAME, devfs_debug);
+    printk (KERN_INFO DEVFS_NAME ": devfs_debug: 0x%0x\n", devfs_debug);
 #endif
-    printk ("%s: boot_options: 0x%0x\n", DEVFS_NAME, boot_options);
+    printk (KERN_INFO DEVFS_NAME ": boot_options: 0x%0x\n", boot_options);
     err = register_filesystem (&devfs_fs_type);
     if (!err)
     {
@@ -3490,8 +3489,8 @@
     int err;

     if ( !(boot_options & OPTION_MOUNT) ) return;
-    err = do_mount ("none", "/dev", "devfs", 0, "");
-    if (err == 0) printk ("Mounted devfs on /dev\n");
+    err = do_mount ("devfs", "/dev", "devfs", 0, "");
+    if (err == 0) printk (KERN_INFO "Mounted devfs on /dev\n");
     else printk ("Warning: unable to mount devfs, err: %d\n", err);
 }   /*  End Function mount_devfs_fs  */

Re: [PATCH] KERN_INFO for devfs

[Horst von Brand]

Denis Vlasenko <vda@port.imtp.ilyichevsk.odessa.ua> said:

[...]

> diff -u --recursive linux-2.4.18-pre6mhv_ll/fs/devfs/base.c 
> linux-2.4.18-pre6mhv_ll.devfs/fs/devfs/base.c
> --- linux-2.4.18-pre6mhv_ll/fs/devfs/base.c     Fri Jan 25 15:49:53 2002
> +++ linux-2.4.18-pre6mhv_ll.devfs/fs/devfs/base.c       Mon Jan 28 23:05:44 
> 2002
> @@ -3490,8 +3489,8 @@
>      int err;
> 
>      if ( !(boot_options & OPTION_MOUNT) ) return;
> -    err = do_mount ("none", "/dev", "devfs", 0, "");
> -    if (err == 0) printk ("Mounted devfs on /dev\n");
> +    err = do_mount ("devfs", "/dev", "devfs", 0, "");
> +    if (err == 0) printk (KERN_INFO "Mounted devfs on /dev\n");
>      else printk ("Warning: unable to mount devfs, err: %d\n", err);
                    ^^^^^
                     Missed this one
>  }   /*  End Function mount_devfs_fs  */
-- 
Horst von Brand			     http://counter.li.org # 22616

Re: [PATCH] KERN_INFO for devfs

[David Weinehall]

On Wed, Jan 30, 2002 at 11:03:05AM -0200, Denis Vlasenko wrote:
> On 29 January 2002 14:49, Horst von Brand wrote:
> > > diff -u --recursive linux-2.4.18-pre6mhv_ll/fs/devfs/base.c
> > > linux-2.4.18-pre6mhv_ll.devfs/fs/devfs/base.c
> > > --- linux-2.4.18-pre6mhv_ll/fs/devfs/base.c     Fri Jan 25 15:49:53 2002
> > > +++ linux-2.4.18-pre6mhv_ll.devfs/fs/devfs/base.c       Mon Jan 28
> > > 23:05:44 2002
> > > @@ -3490,8 +3489,8 @@
> > >      int err;
> > >
> > >      if ( !(boot_options & OPTION_MOUNT) ) return;
> > > -    err = do_mount ("none", "/dev", "devfs", 0, "");
> > > -    if (err == 0) printk ("Mounted devfs on /dev\n");
> > > +    err = do_mount ("devfs", "/dev", "devfs", 0, "");
> > > +    if (err == 0) printk (KERN_INFO "Mounted devfs on /dev\n");
> > >      else printk ("Warning: unable to mount devfs, err: %d\n", err);
> >
> >                     ^^^^^
> >                      Missed this one
> 
> Hmm. KERN_WARNING can be added there, but it is the default level anyway.

Yes, but that may change (in theory, at least.) Consistency is a virtue.


/David Weinehall
  _                                                                 _
 // David Weinehall <tao@acc.umu.se> /> Northern lights wander      \\
//  Maintainer of the v2.0 kernel   //  Dance across the winter sky //
\>  http://www.acc.umu.se/~tao/    </   Full colour fire           </

Re: [PATCH] KERN_INFO for devfs

[Zwane Mwaikambo]

On Wed, 30 Jan 2002, Denis Vlasenko wrote:

> I'll do this cleanup if my KERN_INFO patches will be accepted, at least some 
> of them. So far only Richard Gooch replied...

Heh, thats a lot to ask ;)

Re: [PATCH] KERN_INFO for devfs

[Denis Vlasenko]

On 29 January 2002 14:49, Horst von Brand wrote:
> > diff -u --recursive linux-2.4.18-pre6mhv_ll/fs/devfs/base.c
> > linux-2.4.18-pre6mhv_ll.devfs/fs/devfs/base.c
> > --- linux-2.4.18-pre6mhv_ll/fs/devfs/base.c     Fri Jan 25 15:49:53 2002
> > +++ linux-2.4.18-pre6mhv_ll.devfs/fs/devfs/base.c       Mon Jan 28
> > 23:05:44 2002
> > @@ -3490,8 +3489,8 @@
> >      int err;
> >
> >      if ( !(boot_options & OPTION_MOUNT) ) return;
> > -    err = do_mount ("none", "/dev", "devfs", 0, "");
> > -    if (err == 0) printk ("Mounted devfs on /dev\n");
> > +    err = do_mount ("devfs", "/dev", "devfs", 0, "");
> > +    if (err == 0) printk (KERN_INFO "Mounted devfs on /dev\n");
> >      else printk ("Warning: unable to mount devfs, err: %d\n", err);
>
>                     ^^^^^
>                      Missed this one

Hmm. KERN_WARNING can be added there, but it is the default level anyway.
--
vda

Re: [PATCH] KERN_INFO for devfs

[Alan Cox]

> > Yes, but that may change (in theory, at least.) Consistency is a virtue.
> 
> I'll do this cleanup if my KERN_INFO patches will be accepted, at least some 
> of them. So far only Richard Gooch replied...

I ran some of them into 7ac1 but got rejects so I've dumped them out for
now. They mostly look completely sensible

Re: [PATCH] KERN_INFO for devfs

[Denis Vlasenko]

On 30 January 2002 09:09, David Weinehall wrote:
> > > >      else printk ("Warning: unable to mount devfs, err: %d\n", err);
> > >
> > >                     ^^^^^
> > >                      Missed this one
> >
> > Hmm. KERN_WARNING can be added there, but it is the default level anyway.
>
> Yes, but that may change (in theory, at least.) Consistency is a virtue.

I'll do this cleanup if my KERN_INFO patches will be accepted, at least some 
of them. So far only Richard Gooch replied...
--
vda

Re: [PATCH] KERN_INFO for devfs

[Richard Gooch]

Alan Cox writes:
> > > Yes, but that may change (in theory, at least.) Consistency is a virtue.
> > 
> > I'll do this cleanup if my KERN_INFO patches will be accepted, at least some 
> > of them. So far only Richard Gooch replied...
> 
> I ran some of them into 7ac1 but got rejects so I've dumped them out
> for now. They mostly look completely sensible

I'd prefer if tree maintainers (that means you, Alan:-) don't apply
devfs patches that didn't come from me. I've already posted a patch
which cleans up *all* the remaining printk()'s. In fact, it's a pair
of patches, one for 2.4.x and one for 2.5.x. That was yesterday. Today
I'm still seeing this thread being beaten to death.

Besides, this is hardly an urgent fix, so there's no great rush to
apply a random patch from someone else, even if I did sit on it for a
week or two. Applying random patches will just end up generating more
merge work for me down the track.

				Regards,

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

Re: [PATCH] KERN_INFO for devfs

[Richard Gooch]

Alan Cox writes:
> > I'd prefer if tree maintainers (that means you, Alan:-) don't apply
> > devfs patches that didn't come from me. I've already posted a patch
> > which cleans up *all* the remaining printk()'s. In fact, it's a pair
> > of patches, one for 2.4.x and one for 2.5.x. That was yesterday. Today
> > I'm still seeing this thread being beaten to death.
> 
> I'll apply stuff to my tree that looks sane and see what happens, if I know
> there is an active maintainer I'll also replace it with newer stuff from
> the mainstream and I won't submit it on to Marcelo.
> 
> So no worries, I'm not going to screw your patchsets to Marcelo up

OK.

				Regards,

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

Re: [PATCH] KERN_INFO for devfs

[Alan Cox]

> I'd prefer if tree maintainers (that means you, Alan:-) don't apply
> devfs patches that didn't come from me. I've already posted a patch
> which cleans up *all* the remaining printk()'s. In fact, it's a pair
> of patches, one for 2.4.x and one for 2.5.x. That was yesterday. Today
> I'm still seeing this thread being beaten to death.

I'll apply stuff to my tree that looks sane and see what happens, if I know
there is an active maintainer I'll also replace it with newer stuff from
the mainstream and I won't submit it on to Marcelo.

So no worries, I'm not going to screw your patchsets to Marcelo up

Re: [PATCH] KERN_INFO for devfs

[Jeff Garzik]

On Wed, Jan 30, 2002 at 11:04:49AM -0700, Richard Gooch wrote:
> I'd prefer if tree maintainers (that means you, Alan:-) don't apply
> devfs patches that didn't come from me. I've already posted a patch
> which cleans up *all* the remaining printk()'s. In fact, it's a pair
> of patches, one for 2.4.x and one for 2.5.x. That was yesterday. Today
> I'm still seeing this thread being beaten to death.

First, before I get to my point, I want to say that I fully understand
your argument, and I also bitch when people patch the files I maintain.

But.

That is courtesy, not a right or need.  The -beauty- of open source is
that anyone can hack on my files, or devfs, and if the patch makes
sense, it will get applied, regardless of whether I am a crotchety old
a-hole or not.  (I am :))

	Jeff

Re: [PATCH] KERN_INFO for devfs

[Horst von Brand]

Richard Gooch <rgooch@ras.ucalgary.ca> said:

[...]

> I'd prefer if tree maintainers (that means you, Alan:-) don't apply
> devfs patches that didn't come from me. I've already posted a patch
> which cleans up *all* the remaining printk()'s. In fact, it's a pair
> of patches, one for 2.4.x and one for 2.5.x. That was yesterday. Today
> I'm still seeing this thread being beaten to death.

There goes the "just small-stuff maintainer"...
-- 
Horst von Brand			     http://counter.li.org # 22616

Re: [PATCH] KERN_INFO for devfs

[Richard Gooch]

Denis Vlasenko writes:
> > > Why do you think they _have to_ have "none"? Is it POSIXized or
> > > otherwise standardized? Where can I RTFM?
> >
> > I do not think they have to. They just are :-)
> >
> > fs/namespace.c:show_vfsmnt()
> >
> > ...
> > mangle(m, mnt->mnt_devname ? mnt->mnt_devname : "none");
> >
> >
> > I find this convention quite useful. It allows any program to easily
> > skip virtual filesystems. Using something like /dev or devfs in this
> > case does not add any bit of useful information but possibly adds to
> > confusion.
> 
> Maybe you're right. It's up to maintainer to decide.
> Richard, do you need updated patch without "none" -> "devfs"?

Don't bother. I've gone through the code and done it myself, making
some other minor changes as I go along.

				Regards,

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

Re: [PATCH] KERN_INFO for devfs

[Denis Vlasenko]

> > Why do you think they _have to_ have "none"? Is it POSIXized or
> > otherwise standardized? Where can I RTFM?
>
> I do not think they have to. They just are :-)
>
> fs/namespace.c:show_vfsmnt()
>
> ...
> mangle(m, mnt->mnt_devname ? mnt->mnt_devname : "none");
>
>
> I find this convention quite useful. It allows any program to easily
> skip virtual filesystems. Using something like /dev or devfs in this
> case does not add any bit of useful information but possibly adds to
> confusion.

Maybe you're right. It's up to maintainer to decide.
Richard, do you need updated patch without "none" -> "devfs"?
--
vda

Re: [PATCH] KERN_INFO for devfs

[Denis Vlasenko]

On 29 January 2002 05:27, Borsenkow Andrej wrote:
> > I changed "none" to "devfs" in do_mount("none", "/dev", "devfs", 0,
>
> ""):
> > "none is busy" is misleading at umount time :-)
>
> File systems that do not have real devices behind them have "none" as
> device. Please do not change it - it was correct. Having it later in
> /proc/mounts may confuse some user-level tools. If you want to fix it -
> fix umount to report something more sensible if device == none.

Why do you think they _have to_ have "none"? Is it POSIXized or otherwise 
standardized? Where can I RTFM?
--
vda

RE: [PATCH] KERN_INFO for devfs

[Borsenkow Andrej]

> On 29 January 2002 05:27, Borsenkow Andrej wrote:
> > > I changed "none" to "devfs" in do_mount("none", "/dev", "devfs",
0,
> >
> > ""):
> > > "none is busy" is misleading at umount time :-)
> >
> > File systems that do not have real devices behind them have "none"
as
> > device. Please do not change it - it was correct. Having it later in
> > /proc/mounts may confuse some user-level tools. If you want to fix
it -
> > fix umount to report something more sensible if device == none.
> 
> Why do you think they _have to_ have "none"? Is it POSIXized or
otherwise
> standardized? Where can I RTFM?

I do not think they have to. They just are :-)

fs/namespace.c:show_vfsmnt()

...
mangle(m, mnt->mnt_devname ? mnt->mnt_devname : "none");


I find this convention quite useful. It allows any program to easily
skip virtual filesystems. Using something like /dev or devfs in this
case does not add any bit of useful information but possibly adds to
confusion.

-andrej

Re: [PATCH] KERN_INFO for devfs

[Borsenkow Andrej]

> I changed "none" to "devfs" in do_mount("none", "/dev", "devfs", 0,
""): 
> "none is busy" is misleading at umount time :-) 

File systems that do not have real devices behind them have "none" as
device. Please do not change it - it was correct. Having it later in
/proc/mounts may confuse some user-level tools. If you want to fix it -
fix umount to report something more sensible if device == none.

-andrej