Installed some machine with a reiserfs rootfs.
The boot messages contain a lot of garbage spouted by Intermezzo
and ufs_read_super caused by the fact that the kernel tried lots
of other filesystems before hitting on reiserfs.
On the one hand this is solved by "rootfstype=reiserfs".
But on the other hand, the kernel should not complain about the
guessing it does itself. There is a parameter "silent" for this
purpose, but in this case I think it cleaner just to remove the
complaint and tighten the requirement.
Andries
--- /linux/2.6/linux-2.6.1/linux/fs/ufs/super.c 2003-12-18 03:57:57.000000000 +0100
+++ super.c 2004-01-25 01:05:47.000000000 +0100
@@ -516,14 +516,10 @@
printk("wrong mount options\n");
goto failed;
}
- if (!(sbi->s_mount_opt & UFS_MOUNT_UFSTYPE)) {
- printk("You didn't specify the type of your ufs filesystem\n\n"
- "mount -t ufs -o ufstype="
- "sun|sunx86|44bsd|old|hp|nextstep|netxstep-cd|openstep ...\n\n"
- ">>>WARNING<<< Wrong ufstype may corrupt your filesystem, "
- "default is ufstype=old\n");
- ufs_set_opt (sbi->s_mount_opt, UFSTYPE_OLD);
- }
+
+ /* "old" used to be default - now: always specify type */
+ if (!(sbi->s_mount_opt & UFS_MOUNT_UFSTYPE))
+ goto failed;
sbi->s_uspi = uspi =
kmalloc (sizeof(struct ufs_sb_private_info), GFP_KERNEL);
At Sun, 25 Jan 2004 01:17:47 +0100 (MET),
[email protected] wrote:
> Installed some machine with a reiserfs rootfs.
> The boot messages contain a lot of garbage spouted by Intermezzo
> and ufs_read_super caused by the fact that the kernel tried lots
> of other filesystems before hitting on reiserfs.
>
> On the one hand this is solved by "rootfstype=reiserfs".
>
> But on the other hand, the kernel should not complain about the
> guessing it does itself. There is a parameter "silent" for this
> purpose, but in this case I think it cleaner just to remove the
> complaint and tighten the requirement.
I wonder this modification is really ok because user can't find why he
can't mount his ufs if he does not specify ufstype=. Putting only
one line is not acceptable for you?
Regards,
-- gotom
--- fs/ufs/super.c 2003-10-20 12:50:24.000000000 +0900
+++ fs/ufs/super.c.new 2004-01-27 10:18:22.000000000 +0900
@@ -517,11 +517,8 @@
goto failed;
}
if (!(sbi->s_mount_opt & UFS_MOUNT_UFSTYPE)) {
- printk("You didn't specify the type of your ufs filesystem\n\n"
- "mount -t ufs -o ufstype="
- "sun|sunx86|44bsd|old|hp|nextstep|netxstep-cd|openstep ...\n\n"
- ">>>WARNING<<< Wrong ufstype may corrupt your filesystem, "
- "default is ufstype=old\n");
+ printk("to mount ufs, specify -o ufstype="
+ "sun|sunx86|44bsd|old(default)|hp|nextstep|netxstep-cd|openstep\n");
ufs_set_opt (sbi->s_mount_opt, UFSTYPE_OLD);
}
From [email protected] Tue Jan 27 02:44:00 2004
I wonder this modification is really ok because user can't find why he
can't mount his ufs if he does not specify ufstype=. Putting only
one line is not acceptable for you?
But you see, it wasn't the user at all, and it wasn't a ufs filesystem.
It is kernel probing that causes error messages. That is unwanted.
So, your version is wrong.
If it is really very desirable to warn the user the condition if(!silent)
should be added.
But in my opinion such a warning is not desirable at all.
mount(8) and Documentation/filesystems/ufs.txt explain the details.
Andries
On Tue, 27 Jan 2004 [email protected] wrote:
>
> But you see, it wasn't the user at all, and it wasn't a ufs filesystem.
> It is kernel probing that causes error messages. That is unwanted.
> So, your version is wrong.
Yes.
However, I think the _real_ bug is that we have reiserfs near the tail of
filesystems to try.
The filesystems in fs/Makefile are listed in order of being probed, so I
really thing the real fix is to ignore the whole verbose thing entirely,
and just move reiserfs upwards.
Because if you actually get far enough that you try to mount UFS as your
root filesystem, you have other problems, and verbosity at boot is not
your real issue.
Can you test that alternate patch instead?
Linus
From: Linus Torvalds <[email protected]>
> But you see, it wasn't the user at all, and it wasn't a ufs filesystem.
> It is kernel probing that causes error messages. That is unwanted.
> So, your version is wrong.
Yes.
However, I think the _real_ bug is that we have reiserfs near the tail of
filesystems to try.
Can you test that alternate patch instead?
Funny how we alternate - when I choose the pure, theoretical point of view
you prefer practice, when I prefer practice you become pure.
This time you prefer practice: the list of filesystems is full of garbage
and good filesystems should be near the top.
I prefer theory: the kernel should not probe at all, so everybody who
forgets rootfstype= gets what he deserves.
Be that as it may - below a patch as I suppose you had in mind.
I don't like it very much. Ordering constraints in makefiles are bad.
Have not compiled or tested.
You can apply it I suppose, but after doing so my earlier patch is still
meaningful. Maybe you should also apply that (and the Doc update).
Andries
diff -u --recursive --new-file -X /linux/dontdiff a/fs/Makefile b/fs/Makefile
--- a/fs/Makefile 2003-12-18 03:58:57.000000000 +0100
+++ b/fs/Makefile 2004-01-27 04:51:33.000000000 +0100
@@ -48,6 +48,7 @@
obj-$(CONFIG_EXT3_FS) += ext3/ # Before ext2 so root fs can be ext3
obj-$(CONFIG_JBD) += jbd/
obj-$(CONFIG_EXT2_FS) += ext2/
+obj-$(CONFIG_REISERFS_FS) += reiserfs/
obj-$(CONFIG_CRAMFS) += cramfs/
obj-$(CONFIG_RAMFS) += ramfs/
obj-$(CONFIG_HUGETLBFS) += hugetlbfs/
@@ -84,7 +85,6 @@
obj-$(CONFIG_AUTOFS_FS) += autofs/
obj-$(CONFIG_AUTOFS4_FS) += autofs4/
obj-$(CONFIG_ADFS_FS) += adfs/
-obj-$(CONFIG_REISERFS_FS) += reiserfs/
obj-$(CONFIG_UDF_FS) += udf/
obj-$(CONFIG_SUN_OPENPROMFS) += openpromfs/
obj-$(CONFIG_JFS_FS) += jfs/
At Tue, 27 Jan 2004 05:07:50 +0100 (MET),
[email protected] wrote:
> From: Linus Torvalds <[email protected]>
>
> > But you see, it wasn't the user at all, and it wasn't a ufs filesystem.
> > It is kernel probing that causes error messages. That is unwanted.
> > So, your version is wrong.
>
> Yes.
>
> However, I think the _real_ bug is that we have reiserfs near the tail of
> filesystems to try.
>
> Can you test that alternate patch instead?
>
> Funny how we alternate - when I choose the pure, theoretical point of view
> you prefer practice, when I prefer practice you become pure.
>
> This time you prefer practice: the list of filesystems is full of garbage
> and good filesystems should be near the top.
> I prefer theory: the kernel should not probe at all, so everybody who
> forgets rootfstype= gets what he deserves.
>
> Be that as it may - below a patch as I suppose you had in mind.
> I don't like it very much. Ordering constraints in makefiles are bad.
>From the user point of view, I think it's weilcome that "popular"
filesystems like reiserfs used for rootfs is moved upwards, because
boot up speed is accelerated. It's difficult to define what "popular"
is, but apparently hugetlbfs should be lower than reiserfs. So your
patch seems fine, I think.
> Have not compiled or tested.
> You can apply it I suppose, but after doing so my earlier patch is still
> meaningful. Maybe you should also apply that (and the Doc update).
I misunderstand what the real problem is.
The problem as you previously wrote was that UFS does not use
ufs_fill_super third argument "silent" value, so some warning messages
are scattered during boot up time. I agree that this is exactly just
a noise. Andries removed those warnings instead of using "silent",
but in fact such messages are important for users who want to mount
ufs filesystem to tell for kernel the proper UFS type. To be honest,
I sometimes forget to add "ufstype=" option to mount UFS, so this
warning is useful for me. I attached patch which uses "silent" value
for this problem. I checked this patch which removes all scattered
UFS messages from boot up screen. I think this one is acceptable for
even Andries instead of your UFS patch, is it OK?
Regards,
-- gotom
--- fs/ufs/super.c.org 2003-10-20 12:50:24.000000000 +0900
+++ fs/ufs/super.c 2004-01-27 13:26:05.000000000 +0900
@@ -516,7 +516,7 @@
printk("wrong mount options\n");
goto failed;
}
- if (!(sbi->s_mount_opt & UFS_MOUNT_UFSTYPE)) {
+ if (!(sbi->s_mount_opt & UFS_MOUNT_UFSTYPE) && !silent) {
printk("You didn't specify the type of your ufs filesystem\n\n"
"mount -t ufs -o ufstype="
"sun|sunx86|44bsd|old|hp|nextstep|netxstep-cd|openstep ...\n\n"
@@ -575,7 +575,7 @@
uspi->s_sbsize = super_block_size = 2048;
uspi->s_sbbase = 0;
flags |= UFS_DE_OLD | UFS_UID_OLD | UFS_ST_OLD | UFS_CG_OLD;
- if (!(sb->s_flags & MS_RDONLY)) {
+ if (!(sb->s_flags & MS_RDONLY) && !silent) {
printk(KERN_INFO "ufstype=old is supported read-only\n");
sb->s_flags |= MS_RDONLY;
}
@@ -589,7 +589,7 @@
uspi->s_sbsize = super_block_size = 2048;
uspi->s_sbbase = 0;
flags |= UFS_DE_OLD | UFS_UID_OLD | UFS_ST_OLD | UFS_CG_OLD;
- if (!(sb->s_flags & MS_RDONLY)) {
+ if (!(sb->s_flags & MS_RDONLY) && !silent) {
printk(KERN_INFO "ufstype=nextstep is supported read-only\n");
sb->s_flags |= MS_RDONLY;
}
@@ -603,7 +603,7 @@
uspi->s_sbsize = super_block_size = 2048;
uspi->s_sbbase = 0;
flags |= UFS_DE_OLD | UFS_UID_OLD | UFS_ST_OLD | UFS_CG_OLD;
- if (!(sb->s_flags & MS_RDONLY)) {
+ if (!(sb->s_flags & MS_RDONLY) && !silent) {
printk(KERN_INFO "ufstype=nextstep-cd is supported read-only\n");
sb->s_flags |= MS_RDONLY;
}
@@ -617,7 +617,7 @@
uspi->s_sbsize = super_block_size = 2048;
uspi->s_sbbase = 0;
flags |= UFS_DE_44BSD | UFS_UID_44BSD | UFS_ST_44BSD | UFS_CG_44BSD;
- if (!(sb->s_flags & MS_RDONLY)) {
+ if (!(sb->s_flags & MS_RDONLY) && !silent) {
printk(KERN_INFO "ufstype=openstep is supported read-only\n");
sb->s_flags |= MS_RDONLY;
}
@@ -631,13 +631,14 @@
uspi->s_sbsize = super_block_size = 2048;
uspi->s_sbbase = 0;
flags |= UFS_DE_OLD | UFS_UID_OLD | UFS_ST_OLD | UFS_CG_OLD;
- if (!(sb->s_flags & MS_RDONLY)) {
+ if (!(sb->s_flags & MS_RDONLY) && !silent) {
printk(KERN_INFO "ufstype=hp is supported read-only\n");
sb->s_flags |= MS_RDONLY;
}
break;
default:
- printk("unknown ufstype\n");
+ if (!silent)
+ printk("unknown ufstype\n");
goto failed;
}
@@ -687,7 +688,8 @@
uspi->s_sbbase += 8;
goto again;
}
- printk("ufs_read_super: bad magic number\n");
+ if (!silent)
+ printk("ufs_read_super: bad magic number\n");
goto failed;
magic_found:
On Tue, 27 Jan 2004 [email protected] wrote:
>
> Funny how we alternate - when I choose the pure, theoretical point of view
> you prefer practice, when I prefer practice you become pure.
Heh.
I'm actually usually very easy to predict:
- when it comes to "core technology" bugs, I'd much rather fix them
_right_. To the point where I prefer to not fix them at all if the fix
is only hiding the real bug. Then I'd rather leave it as a known bug
and hope the _real_ fix comes in.
- but when it comes to things that are more about "usability", I tend to
try to take the very practical approach. So we'll disagree on things
like "should the kernel autodetect", because I think that's a usability
issue, and consider that it should be as easy for users as possible.
The reiserfs/ufs issue to me is about "usability", not "core technology".
As such, to me it falls under the "practical" heading, and the solution
should be the pragmatic trivial "just test reiserfs first" kind of silly
thing.
Linus
Hi,
There's a semantic change introduced by this patch.
I don't know enough about UFS to call it a bug, but it
certainly looks suspicious.
>--- fs/ufs/super.c.org 2003-10-20 12:50:24.000000000 +0900
>+++ fs/ufs/super.c 2004-01-27 13:26:05.000000000 +0900
>@@ -516,7 +516,7 @@
> printk("wrong mount options\n");
> goto failed;
> }
>- if (!(sbi->s_mount_opt & UFS_MOUNT_UFSTYPE)) {
>+ if (!(sbi->s_mount_opt & UFS_MOUNT_UFSTYPE) && !silent) {
> printk("You didn't specify the type of your ufs filesystem\n\n"
> "mount -t ufs -o ufstype="
> "sun|sunx86|44bsd|old|hp|nextstep|netxstep-cd|openstep ...\n\n"
>@@ -575,7 +575,7 @@
> uspi->s_sbsize = super_block_size = 2048;
> uspi->s_sbbase = 0;
> flags |= UFS_DE_OLD | UFS_UID_OLD | UFS_ST_OLD | UFS_CG_OLD;
>- if (!(sb->s_flags & MS_RDONLY)) {
>+ if (!(sb->s_flags & MS_RDONLY) && !silent) {
> printk(KERN_INFO "ufstype=old is supported read-only\n");
> sb->s_flags |= MS_RDONLY;
If "silent" is set, this variable assignment is skipped. I
would have thought that the assignment should happen, and only
the printk() should be skipped.
> }
>@@ -589,7 +589,7 @@
> uspi->s_sbsize = super_block_size = 2048;
> uspi->s_sbbase = 0;
> flags |= UFS_DE_OLD | UFS_UID_OLD | UFS_ST_OLD | UFS_CG_OLD;
>- if (!(sb->s_flags & MS_RDONLY)) {
>+ if (!(sb->s_flags & MS_RDONLY) && !silent) {
> printk(KERN_INFO "ufstype=nextstep is supported read-only\n");
> sb->s_flags |= MS_RDONLY;
Same here.
> }
>@@ -603,7 +603,7 @@
> uspi->s_sbsize = super_block_size = 2048;
> uspi->s_sbbase = 0;
> flags |= UFS_DE_OLD | UFS_UID_OLD | UFS_ST_OLD | UFS_CG_OLD;
>- if (!(sb->s_flags & MS_RDONLY)) {
>+ if (!(sb->s_flags & MS_RDONLY) && !silent) {
> printk(KERN_INFO "ufstype=nextstep-cd is supported read-only\n");
> sb->s_flags |= MS_RDONLY;
And again.
> }
>@@ -617,7 +617,7 @@
> uspi->s_sbsize = super_block_size = 2048;
> uspi->s_sbbase = 0;
> flags |= UFS_DE_44BSD | UFS_UID_44BSD | UFS_ST_44BSD | UFS_CG_44BSD;
>- if (!(sb->s_flags & MS_RDONLY)) {
>+ if (!(sb->s_flags & MS_RDONLY) && !silent) {
> printk(KERN_INFO "ufstype=openstep is supported read-only\n");
> sb->s_flags |= MS_RDONLY;
Another one.
> }
>@@ -631,13 +631,14 @@
> uspi->s_sbsize = super_block_size = 2048;
> uspi->s_sbbase = 0;
> flags |= UFS_DE_OLD | UFS_UID_OLD | UFS_ST_OLD | UFS_CG_OLD;
>- if (!(sb->s_flags & MS_RDONLY)) {
>+ if (!(sb->s_flags & MS_RDONLY) && !silent) {
> printk(KERN_INFO "ufstype=hp is supported read-only\n");
> sb->s_flags |= MS_RDONLY;
And again.
> }
> break;
> default:
>- printk("unknown ufstype\n");
>+ if (!silent)
>+ printk("unknown ufstype\n");
> goto failed;
> }
>
>@@ -687,7 +688,8 @@
> uspi->s_sbbase += 8;
> goto again;
> }
>- printk("ufs_read_super: bad magic number\n");
>+ if (!silent)
>+ printk("ufs_read_super: bad magic number\n");
> goto failed;
>
> magic_found:
Kind regards,
Jon
At Wed, 28 Jan 2004 22:24:30 +0000,
Example wrote:
> There's a semantic change introduced by this patch.
> I don't know enough about UFS to call it a bug, but it
> certainly looks suspicious.
Exactly. Thanks for your check, and I'm sorry the previous patch
which includes such wrong change. I attached the update one.
Regards,
-- gotom
--- fs/ufs/super.c.org 2003-10-20 12:50:24.000000000 +0900
+++ fs/ufs/super.c 2004-01-29 08:34:15.000000000 +0900
@@ -517,11 +517,12 @@
goto failed;
}
if (!(sbi->s_mount_opt & UFS_MOUNT_UFSTYPE)) {
- printk("You didn't specify the type of your ufs filesystem\n\n"
- "mount -t ufs -o ufstype="
- "sun|sunx86|44bsd|old|hp|nextstep|netxstep-cd|openstep ...\n\n"
- ">>>WARNING<<< Wrong ufstype may corrupt your filesystem, "
- "default is ufstype=old\n");
+ if (!silent)
+ printk("You didn't specify the type of your ufs filesystem\n\n"
+ "mount -t ufs -o ufstype="
+ "sun|sunx86|44bsd|old|hp|nextstep|netxstep-cd|openstep ...\n\n"
+ ">>>WARNING<<< Wrong ufstype may corrupt your filesystem, "
+ "default is ufstype=old\n");
ufs_set_opt (sbi->s_mount_opt, UFSTYPE_OLD);
}
@@ -576,7 +577,8 @@
uspi->s_sbbase = 0;
flags |= UFS_DE_OLD | UFS_UID_OLD | UFS_ST_OLD | UFS_CG_OLD;
if (!(sb->s_flags & MS_RDONLY)) {
- printk(KERN_INFO "ufstype=old is supported read-only\n");
+ if (!silent)
+ printk(KERN_INFO "ufstype=old is supported read-only\n");
sb->s_flags |= MS_RDONLY;
}
break;
@@ -590,7 +592,8 @@
uspi->s_sbbase = 0;
flags |= UFS_DE_OLD | UFS_UID_OLD | UFS_ST_OLD | UFS_CG_OLD;
if (!(sb->s_flags & MS_RDONLY)) {
- printk(KERN_INFO "ufstype=nextstep is supported read-only\n");
+ if (!silent)
+ printk(KERN_INFO "ufstype=nextstep is supported read-only\n");
sb->s_flags |= MS_RDONLY;
}
break;
@@ -604,7 +607,8 @@
uspi->s_sbbase = 0;
flags |= UFS_DE_OLD | UFS_UID_OLD | UFS_ST_OLD | UFS_CG_OLD;
if (!(sb->s_flags & MS_RDONLY)) {
- printk(KERN_INFO "ufstype=nextstep-cd is supported read-only\n");
+ if (!silent)
+ printk(KERN_INFO "ufstype=nextstep-cd is supported read-only\n");
sb->s_flags |= MS_RDONLY;
}
break;
@@ -618,7 +622,8 @@
uspi->s_sbbase = 0;
flags |= UFS_DE_44BSD | UFS_UID_44BSD | UFS_ST_44BSD | UFS_CG_44BSD;
if (!(sb->s_flags & MS_RDONLY)) {
- printk(KERN_INFO "ufstype=openstep is supported read-only\n");
+ if (!silent)
+ printk(KERN_INFO "ufstype=openstep is supported read-only\n");
sb->s_flags |= MS_RDONLY;
}
break;
@@ -632,12 +637,14 @@
uspi->s_sbbase = 0;
flags |= UFS_DE_OLD | UFS_UID_OLD | UFS_ST_OLD | UFS_CG_OLD;
if (!(sb->s_flags & MS_RDONLY)) {
- printk(KERN_INFO "ufstype=hp is supported read-only\n");
+ if (!silent)
+ printk(KERN_INFO "ufstype=hp is supported read-only\n");
sb->s_flags |= MS_RDONLY;
}
break;
default:
- printk("unknown ufstype\n");
+ if (!silent)
+ printk("unknown ufstype\n");
goto failed;
}
@@ -687,7 +694,8 @@
uspi->s_sbbase += 8;
goto again;
}
- printk("ufs_read_super: bad magic number\n");
+ if (!silent)
+ printk("ufs_read_super: bad magic number\n");
goto failed;
magic_found: