CONFIG_EXT4_USE_FOR_EXT23: rootfs shows as ext2 instead of ext4
Subsystems (2)
Ted et al.
I've only just noticed this, so I have no idea how long it has been this way.
When I build a kernel with CONFIG_EXT4_USE_FOR_EXT23=y and boot from it, the ext4 root filesystem shows up as "ext2" mode, rather than "ext4".
This looks very wrong to me, and quite dangerous.
Eg. I test it by building my own kernel (2.6.38.2), with ext4 built-in, no initramfs required, and boot:
root=/dev/sda1 init=/bin/bash
...
$ mount /proc
$ cat /proc/mounts
/dev/root / ext2 ro,relatime,barrier=1,data=ordered 0 0
So.. it shows "ext2" instead of "ext4". That really looks like a bug. Especially since it appears to be using journaling regardless.
Building the kernel without CONFIG_EXT4_USE_FOR_EXT23 results in a proper "ext4" mount entry in /proc/mounts.
Ted Ts'o
maintainerQuoted 8 lines · Mark Lord
Ted et al.
I've only just noticed this, so I have no idea how long it has been this way.
When I build a kernel with
CONFIG_EXT4_USE_FOR_EXT23=yand boot from it, the ext4 root filesystem shows up as "ext2" mode, rather than "ext4".This looks very wrong to me, and quite dangerous.
It's a cosemtic bug, I agree, but I'm not sure why you consider it dangerous.
CONFIG_EXT4_USE_FOR_EXT23 means that ext4 registers itself as ext2 and/or ext3, if ext2 and/or ext3 are not configured into the kernel. Since the kernel tries to mount the file system as ext2, ext3, and then ext4, and uses whichever one works first.
- Ted
Quoted 2 lines · Ted Ts'o
It's a cosemtic bug, I agree
Great. Got a patch to fix it for me to try?
Since it currently misidentifies the filesystem type, some of the disk management tools I have get confused. I don't like taking any chances with user data like that.
Thanks.
Quoted 5 lines · Ted Ts'o
Quoted 4 lines · Mark Lord
Ted et al.
I've only just noticed this, so I have no idea how long it has been this way.
Quoted 7 lines
Quoted 4 lines
When I build a kernel with
CONFIG_EXT4_USE_FOR_EXT23=yand boot from it, the ext4 root filesystem shows up as "ext2" mode, rather than "ext4".This looks very wrong to me, and quite dangerous.
It's a cosemtic bug, I agree, but I'm not sure why you consider it dangerous.
It is dangerous if it really has mounted an ext4 as ext2; the on-disk formats are not 100% compatible.
I don't know what it is doing, so it looks quite dangerous. Perhaps it really did mount as ext4 though, in which case not.
Thanks.
Theodore Tso
maintainerQuoted 6 lines · Mark Lord
It is dangerous if it really has mounted an ext4 as ext2; the on-disk formats are not 100% compatible.
I don't know what it is doing, so it looks quite dangerous. Perhaps it really did mount as ext4 though, in which case not.
As the config option name: CONFIG_EXT4_USE_FOR_EXT23 might mave suggested to you, what this means is that we are
using the ext4 file system for ext2/3 file systems. So yes, we are
using the ext4 file system driver when the ext2 file system is
requested. Since the kernel tries mounting the "ext2" file system
first, since we are using the ext4 file system driver, it succeeds, and since it was mounted when the requested name was "ext2", that is what is recorded in the mount table.
It's not dangerous at all. Again, as you can tell if you were to actually
look at your .config carefully, the ext2 file system was not compiled
into your kernel at all. Ext4 will only try masquerading as ext2 ifCONFIG_EXT2_FS is disabled. So it couldn't have possibly been using ext2 code, if you thought about it for a second.
I can write up a patch which explicitly tests for feature flags that go beyond ext2 as of a particular version, and if so, refuse the mount
when ext4 is masquerading as ext2, and do the same for ext3. I
probably will do this to avoid user questions, when I have some spare time.
However, please note that this will have no actual effect on how anything int he kernel will behave --- none ---- except for a one
character change in /proc/mounts: s/2/4/. (This is because the
kernel will now try ext2, and fail, try ext3 and fail, and then succeed when the exact same file system driver is used. Oh, it might also extend the boot time by a few milliseconds.)
Have I made it clear enough now to assuage your fears?
-- Ted
Quoted 5 lines · Theodore Tso
I can write up a patch which explicitly tests for feature flags that go beyond ext2 as of a particular version, and if so, refuse the mount
when ext4 is masquerading as ext2, and do the same for ext3. I
probably will do this to avoid user questions, when I have some spare time.
This is the correct behavior. Few people understand the
filesystem type test ordering, and fewer (these days) modify their own .config. They expect that the name in /proc/mounts reflects the format on the platter. If we say 'ext2', they think it's a non-journaled FFS.
Errors in the other direction are less confusing. If you wanted
a quick hack, you could just have ext4 always fail ext2/3 mounts and report itself as ext4 no matter what the physical disk looks like. People would understand that 'ext4' in /proc/mounts means that the ext4 driver has mounted an extN filesystem much faster than they would understand that 'ext2' means the ext4 driver has mounted an ext4 filesystem but with the scanning name of ext2.
Joel
Signature · 5 lines
"If at first you don't succeed, cover all traces that you tried."
-Unknown
http://www.jlbec.org/
[email protected]PATCH 1/1 ext4: register ext2 and ext3 alias after ext4
The intent of CONFIG_EXT4_USE_FOR_EXT23 is to provide an alias so that the ext4 module is sufficient even if user space calls out ext2. However, it currently registers the alias names before registering ext4. This means they show up first in /proc/filesystems and ext3 and ext4 formatted file systems show up in the mount list as ext4.
Register the filesystem aliases after ext4 so they show as ext4 until the filesystems is filtered. This will avoid users or tools configuring a kernel with just ext2 and wondering why there system broke. Also register in the same order as listed in fs/Makefile.
---
Untested but simple code movement. Mark will you please test?
Is it safe to initialize ext4_li_info and ext4_li_mtx after we register the filesystem? What keeps the init after the first call to mount a filesystem?
I checked unregister_filesystem and it is safe when the file system type is and is not registered.
fs/ext4/super.c +2 -2
@@ -4898,11 +4898,11 @@ static int __init ext4_init_fs(void) err =init_inodecache ();if (err) goto out1; -register_as_ext2 (); -register_as_ext3 (); err =register_filesystem (&ext4_fs_type);if (err) goto out; +register_as_ext3 (); +register_as_ext2 (); ext4_li_info =NULL ;mutex_init (&ext4_li_mtx);
PATCH 1/1 fs: synchronize_rcu when unregister_filesystem success not failure
While checking unregister_filesystem for saftey vs extra calls for "ext4: register ext2 and ext3 alias after ext4" I realized that the synchronize_rcu() was called on the error path but not on the success path.
Should we call it in both?
fs/filesystems.c +1 -2
@@ -110,14 +110,13 @@ int unregister_filesystem(struct file_sy *tmp = fs->next; fs->next =NULL ;write_unlock (&file_systems_lock); +synchronize_rcu ();return 0 ; } tmp = &(*tmp)->next; }write_unlock (&file_systems_lock); -synchronize_rcu (); -return -EINVAL ; }
Re: [PATCH] fs: synchronize_rcu when unregister_filesystem success not failure
Quoted 5 lines · Milton Miller
While checking
unregister_filesystemfor saftey vs extra calls for "ext4: register ext2 and ext3 alias after ext4" I realized that thesynchronize_rcu()was called on the error path but not on the success path.
Good catch.
I think this is the bug that then caused us to do commit d863b50ab013 ("vfs: call rcu_barrier after ->kill_sb()")
That said, that commit says that "synchronize_rcu()" isn't enough, and uses rcu_barrier().
Which _should_ mean that there are no actual users that care about RCU events by the time you actually hit "unregister_filesystem()".
So I think your patch is correct, but won't actually matter. But maybe I'm missing something.
Quoted 1 line
Should we call it in both?
No, I think the success path is the one that would matter.
Comments?
Linus
Re: [PATCH] fs: synchronize_rcu when unregister_filesystem success not failure
Il 14/04/2011 17:52, Linus Torvalds ha scritto:
Quoted 18 lines · Milton Miller
Quoted 5 lines
While checking
unregister_filesystemfor saftey vs extra calls for "ext4: register ext2 and ext3 alias after ext4" I realized that thesynchronize_rcu()was called on the error path but not on the success path.Good catch.
I think this is the bug that then caused us to do commit
d863b50ab013("vfs: callrcu_barrierafter ->kill_sb()")That said, that commit says that "
synchronize_rcu()" isn't enough, and usesrcu_barrier().Which _should_ mean that there are no actual users that care about RCU events by the time you actually hit "
unregister_filesystem()".
Quoted 9 lines
So I think your patch is correct, but won't actually matter. But maybe I'm missing something.
Quoted 1 line
Should we call it in both?
No, I think the success path is the one that would matter.
Comments?
If I well remember the rcu_barrier was needed for the fs module unloading problem. In that case synchronize_rcu() wasn't enough. That said, I agree with you, it won't have any impact.
Marco
Re: [PATCH] fs: synchronize_rcu when unregister_filesystem success not failure
In reply to Linus TorvaldsQuoted 9 lines · Linus Torvalds
Quoted 5 lines · Milton Miller
While checking
unregister_filesystemfor saftey vs extra calls for "ext4: register ext2 and ext3 alias after ext4" I realized that thesynchronize_rcu()was called on the error path but not on the success path.Good catch.
Quoted 8 lines
I think this is the bug that then caused us to do commit
d863b50ab013("vfs: callrcu_barrierafter ->kill_sb()")That said, that commit says that "
synchronize_rcu()" isn't enough, and usesrcu_barrier().Which _should_ mean that there are no actual users that care about RCU events by the time you actually hit "
unregister_filesystem()".
Is that true of older kernels? (eg. 2.6.38 has the same bug)
IOW, is this a -stable candidate?
Re: [PATCH] ext4: register ext2 and ext3 alias after ext4
In reply to Milton MillerQuoted 5 lines · Milton Miller
The intent of
CONFIG_EXT4_USE_FOR_EXT23is to provide an alias so that the ext4 module is sufficient even if user space calls out ext2. However, it currently registers the alias names before registering ext4. This means they show up first in/proc/filesystemsand ext3 and ext4 formatted file systems show up in the mount list as ext4.
Shouldn't that say:
This means they show up first in /proc/filesystems, causing ext3 and ext4 formatted file systems to show up in the mount list as ext2.
Thanks. Other than that, it all looks good here with ext2 and ext4 file systems. I didn't try an ext3 filesystem, but with it being between the other two, I expect it to also be fine.
Quoted 35 lines
Register the filesystem aliases after ext4 so they show as ext4 until the filesystems is filtered. This will avoid users or tools configuring a kernel with just ext2 and wondering why there system broke. Also register in the same order as listed in fs/Makefile.
Signed-off-by Milton Miller <[email protected]>---
Untested but simple code movement. Mark will you please test?
Is it safe to initialize
ext4_li_infoandext4_li_mtxafter we register the filesystem? What keeps the init after the first call to mount a filesystem?I checked
unregister_filesystemand it is safe when the file system type is and is not registered.Index:
work.git/fs/ext4/super.c
===================================================================fs/ext4/super.c +2
-2@@ -4898,11 +4898,11 @@ static int __init ext4_init_fs(void) err =init_inodecache ();if (err) goto out1; -register_as_ext2 (); -register_as_ext3 (); err =register_filesystem (&ext4_fs_type);if (err) goto out; +register_as_ext3 (); +register_as_ext2 (); ext4_li_info =NULL ;mutex_init (&ext4_li_mtx);
Ted Ts'o
maintainerRe: [PATCH] ext4: register ext2 and ext3 alias after ext4
In reply to Milton MillerQuoted 5 lines · Milton Miller
The intent of
CONFIG_EXT4_USE_FOR_EXT23is to provide an alias so that the ext4 module is sufficient even if user space calls out ext2. However, it currently registers the alias names before registering ext4. This means they show up first in/proc/filesystemsand ext3 and ext4 formatted file systems show up in the mount list as ext4.
Note this is only true for the root file system.
Quoted 7 lines
Register the filesystem aliases after ext4 so they show as ext4 until the filesystems is filtered. This will avoid users or tools configuring a kernel with just ext2 and wondering why there system broke. Also register in the same order as listed in fs/Makefile.
Signed-off-by Milton Miller <[email protected]>
This is actually the better way to fix Mark's complaint, as it provides the correct behaviour if someone tries explicitly mounting an ext4 file system via "mount -t ext2 /dev/sda". It also provides a better explanation of what happens when CONFIG_EXT4_USE_EXT23 is enabled and ext2 and ext3 are deconfigured, and the user has an ext2 or ext3 root file system. Now they will something like this:
EXT4-fs (sda): mounting ext2 file system using the ext4 subsystem EXT4-fs (sda): mounted filesystem without journal. Opts: (null)
which makes clearer what is going on.
- Ted
Quoted 1 line
From
5a5baedb6306edb9effc86add73e9522e2a70532Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <[email protected]>
Date: Sat, 16 Apr 2011 21:31:57 -0400
Subject: [PATCH] ext4: check for ext[23] file system features when mounting as ext[23]
Provide better emulation for ext[23] mode by enforcing that the file system does not have any unsupported file system features as defined by ext[23] when emulating the ext[23] file system driver when CONFIG_EXT4_USE_FOR_EXT23 is defined.
This causes the file system type information in /proc/mounts to be correct for the automatically mounted root file system. This also means that "mount -t ext2 /dev/sda /mnt" will fail if /dev/sda contains an ext3 or ext4 file system, just as one would expect if the original ext2 file system driver were in use.
--- fs/ext4/ext4.h | 15 +++++++++++ fs/ext4/super.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 79 insertions(+), 8 deletions(-)
fs/ext4/ext4.h +15 -0
@@ -1351,6 +1351,21 @@ static inline void ext4_clear_state_flags(struct ext4_inode_info *ei)#define EXT4_FEATURE_INCOMPAT_EA_INODE 0x0400/* EA in inode */ #define EXT4_FEATURE_INCOMPAT_DIRDATA 0x1000/* data in dirent */ +#define EXT2_FEATURE_COMPAT_SUPP EXT4_FEATURE_COMPAT_EXT_ATTR +#define EXT2_FEATURE_INCOMPAT_SUPP (EXT4_FEATURE_INCOMPAT_FILETYPE| \ + EXT4_FEATURE_INCOMPAT_META_BG) +#define EXT2_FEATURE_RO_COMPAT_SUPP (EXT4_FEATURE_RO_COMPAT_SPARSE_SUPER| \ + EXT4_FEATURE_RO_COMPAT_LARGE_FILE| \ + EXT4_FEATURE_RO_COMPAT_BTREE_DIR) + +#define EXT3_FEATURE_COMPAT_SUPP EXT4_FEATURE_COMPAT_EXT_ATTR +#define EXT3_FEATURE_INCOMPAT_SUPP (EXT4_FEATURE_INCOMPAT_FILETYPE| \ + EXT4_FEATURE_INCOMPAT_RECOVER| \ + EXT4_FEATURE_INCOMPAT_META_BG) +#define EXT3_FEATURE_RO_COMPAT_SUPP (EXT4_FEATURE_RO_COMPAT_SPARSE_SUPER| \ + EXT4_FEATURE_RO_COMPAT_LARGE_FILE| \ + EXT4_FEATURE_RO_COMPAT_BTREE_DIR) +#define EXT4_FEATURE_COMPAT_SUPP EXT2_FEATURE_COMPAT_EXT_ATTR#define EXT4_FEATURE_INCOMPAT_SUPP (EXT4_FEATURE_INCOMPAT_FILETYPE| \ EXT4_FEATURE_INCOMPAT_RECOVER| \
fs/ext4/super.c +64 -8
115 lines not on this page · raw message
Signature · 1 line
1.7.3.1
Quoted 13 lines · Theodore Tso
It's not dangerous at all. Again, as you can tell if you were to actually
look at your .config carefully, the ext2 file system was not compiled
into your kernel at all. Ext4 will only try masquerading as ext2 ifCONFIG_EXT2_FSis disabled. So it couldn't have possibly been using ext2 code, if you thought about it for a second.I can write up a patch which explicitly tests for feature flags that go beyond ext2 as of a particular version, and if so, refuse the mount
when ext4 is masquerading as ext2, and do the same for ext3. I
probably will do this to avoid user questions, when I have some spare time.
Quoted 8 lines
However, please note that this will have no actual effect on how anything int he kernel will behave --- none ---- except for a one
character change in/proc/mounts: s/2/4/. (This is because the
kernel will now try ext2, and fail, try ext3 and fail, and then succeed when the exact same file system driver is used. Oh, it might also extend the boot time by a few milliseconds.)Have I made it clear enough now to assuage your fears?
No, very much the contrary, despite the condescending attempt at "explanation".
config EXT4_USE_FOR_EXT23
bool "Use ext4 for ext2/ext3 file systems"
depends on EXT4_FS
depends on EXT3_FS=n || EXT2_FS=n
default y
help
Allow the ext4 file system driver code to be used for ext2 or ext3 file system mounts. This allows users to reduce their compiled kernel size by using one file system driver for ext2, ext3, and ext4 file systems.
So the option is supposed to allow ext4 code to be used to access ext2 and ext3 filesystems, saving some space in the binary kernel. That much is obvious.
But far far less obvious is why the kernel reports having mounted my ext4 filesystem as "ext2", and what exactly it means by that.
And I've been a kernel developer as long as you have, Ted, so just imagine the confusion this might be causing lots of other less experienced people.
Thanks.
Theodore Tso
maintainerQuoted 3 lines · Mark Lord
But far far less obvious is why the kernel reports having mounted my ext4 filesystem as "ext2", and what exactly it means by that.
Because the kernel reports the type that it successfully matched against when the mount succeeded, and in order for ext4 to take over ext2, it calls register_filesystem() as ext2. Since the kernel (a) tries mounting the root file system as ext2 (it tries blindly until it succeeds), (b) when ext4 is taking care of ext2 mounts, an filesystem with ext4 features can be mounted by the ext4 file system driver, (c) the base VFS layer controls what types is listed, and since it succeeded when it tried mounting as ext2, it reports it as ext2.
In other words, what the kernel reports is not what file system driver is currently servicing your block device, but the name of the fs type first succeeded when the root file system mount did a brute-force search. (BTW, some distributions may use an initrd that uses a more sophisticated userspace fs probing logic that will correctly determine that the file system is really an ext4 file system, and explicitly request an ext4 mount, and then the kernel will report that it was mounted with an ext4 fs type.) The brute-force search nature of the kernel can potentially get you in trouble, since if a block device was originally formatted as a FAT file system, and then later reformatted as some other file system, such as btrfs, ext4, xfs, etc., in theory, depending on the order the kernel tri
es its brute force search, it could conceivably get things wrong. This is a long-known problem, and we mostly work around it by making the mkfs programs zero out enough data blocks that it's unlikely that a reused block device might get mistaken as something else.
I repeat; this is a cosmetic issue _only_. I suspect most users won't be looking that closely at what the kernel outputs, but I agree it's something that we should fix. OK?
-- Ted
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Ted" == Ted Ts'o <[email protected]> writes:
Quoted 8 lines · Mark Lord
Quoted 8 lines
Ted et al.
I've only just noticed this, so I have no idea how long it has been this way.
When I build a kernel with
CONFIG_EXT4_USE_FOR_EXT23=yand boot from it, the ext4 root filesystem shows up as "ext2" mode, rather than "ext4".This looks very wrong to me, and quite dangerous.
Ted> It's a cosemtic bug, I agree, but I'm not sure why you consider it Ted> dangerous.
Ted> CONFIG_EXT4_USE_FOR_EXT23 means that ext4 registers itself as Ted> ext2 and/or ext3, if ext2 and/or ext3 are not configured into the Ted> kernel. Since the kernel tries to mount the file system as ext2, Ted> ext3, and then ext4, and uses whichever one works first.
Should it instead be: CONFIG_EXT4_MASQUERADE_AS_EXT23 instead, so it's blindingly obvious what's going on here.
John
Quoted 18 lines · John Stoffel
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Ted" == Ted Ts'o<[email protected]> writes:
Quoted 8 lines · Mark Lord
Quoted 8 lines
Ted et al.
I've only just noticed this, so I have no idea how long it has been this way.
When I build a kernel with
CONFIG_EXT4_USE_FOR_EXT23=yand boot from it, the ext4 root filesystem shows up as "ext2" mode, rather than "ext4".This looks very wrong to me, and quite dangerous.
Ted> It's a cosemtic bug, I agree, but I'm not sure why you consider it Ted> dangerous.
Ted> CONFIG_EXT4_USE_FOR_EXT23 means that ext4 registers itself as Ted> ext2 and/or ext3, if ext2 and/or ext3 are not configured into the Ted> kernel. Since the kernel tries to mount the file system as ext2, Ted> ext3, and then ext4, and uses whichever one works first.
Quoted 4 lines
Should it instead be:
CONFIG_EXT4_MASQUERADE_AS_EXT23instead, so it's blindingly obvious what's going on here.John
No - the idea is to have use eliminate duplicate code and still be able to run ext2 and ext3 in the same way.
How close we come to that goal will probably vary between ext2 and ext3 now but it would be great to get that done eventually.
We did discuss this at LSF last week and we had some enthusiasm for keeping ext2 code around forever more or less as "the simple" file system template :)
Ric