2002-01-28 07:56:00

by Denis Vlasenko

[permalink] [raw]
Subject: [PATCH] KERN_INFO for devfs

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 ([email protected])\n",
- DEVFS_NAME, DEVFS_VERSION);
+ printk (KERN_INFO DEVFS_NAME ": v" DEVFS_VERSION
+ " Richard Gooch ([email protected])\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)
{


2002-01-28 16:45:21

by Richard Gooch

[permalink] [raw]
Subject: Re: [PATCH] KERN_INFO for devfs

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: [email protected]
Current: [email protected]

2002-01-28 21:22:12

by Denis Vlasenko

[permalink] [raw]
Subject: Re: [PATCH] KERN_INFO for devfs

> > 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 ([email protected])\n",
- DEVFS_NAME, DEVFS_VERSION);
+ printk (KERN_INFO DEVFS_NAME ": v" DEVFS_VERSION " Richard Gooch
([email protected])\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 */

2002-01-29 07:28:18

by Borsenkow Andrej

[permalink] [raw]
Subject: Re: [PATCH] KERN_INFO for devfs

> 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


2002-01-29 10:22:25

by Denis Vlasenko

[permalink] [raw]
Subject: Re: [PATCH] KERN_INFO for devfs

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

2002-01-29 10:39:56

by Borsenkow Andrej

[permalink] [raw]
Subject: RE: [PATCH] KERN_INFO for devfs


> 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

2002-01-29 12:02:27

by Denis Vlasenko

[permalink] [raw]
Subject: Re: [PATCH] KERN_INFO for devfs

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

2002-01-29 17:49:08

by Richard Gooch

[permalink] [raw]
Subject: Re: [PATCH] KERN_INFO for devfs

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: [email protected]
Current: [email protected]

2002-01-30 07:54:03

by Horst von Brand

[permalink] [raw]
Subject: Re: [PATCH] KERN_INFO for devfs

Denis Vlasenko <[email protected]> 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

2002-01-30 09:05:05

by Denis Vlasenko

[permalink] [raw]
Subject: Re: [PATCH] KERN_INFO for devfs

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

2002-01-30 11:10:54

by David Weinehall

[permalink] [raw]
Subject: Re: [PATCH] KERN_INFO for devfs

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 <[email protected]> /> Northern lights wander \\
// Maintainer of the v2.0 kernel // Dance across the winter sky //
\> http://www.acc.umu.se/~tao/ </ Full colour fire </

2002-01-30 12:35:28

by Denis Vlasenko

[permalink] [raw]
Subject: Re: [PATCH] KERN_INFO for devfs

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

2002-01-30 12:40:19

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: [PATCH] KERN_INFO for devfs

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


2002-01-30 15:04:56

by Alan

[permalink] [raw]
Subject: Re: [PATCH] KERN_INFO for devfs

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

2002-01-30 18:07:06

by Richard Gooch

[permalink] [raw]
Subject: Re: [PATCH] KERN_INFO for devfs

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: [email protected]
Current: [email protected]

2002-01-30 18:23:28

by Richard Gooch

[permalink] [raw]
Subject: Re: [PATCH] KERN_INFO for devfs

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: [email protected]
Current: [email protected]

2002-01-30 18:47:59

by Alan

[permalink] [raw]
Subject: Re: [PATCH] KERN_INFO for devfs

> 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

2002-01-30 19:38:16

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] KERN_INFO for devfs

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



2002-02-01 07:41:01

by Horst von Brand

[permalink] [raw]
Subject: Re: [PATCH] KERN_INFO for devfs

Richard Gooch <[email protected]> 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