Re: [PATCH] do_mounts: try all available filesystems before panicking

[Linus Torvalds]

On Mon, May 5, 2014 at 11:34 AM, Plamen Petrov <plamen.sisi@xxxxxxxxx> wrote:
>
> The story short: on systems with btrfs root I have a kernel .config with ext4,
> xfs and btrfs built-in which works fine with 3.13.x, but 3.14.x panics. After
> inserting some debug printks, I got this info from mount_block_root:
>
> ---> EACCESS=13, EINVAL=22, Available filesystems: ext3 ext2 ext4 fuseblk xfs btrfs
> -----> Tried ext3, error code is -22.
> -----> Tried ext2, error code is -22.
> -----> Tried ext4, error code is -22.
> -----> Tried fuseblk, error code is -22.
> -----> Tried xfs, error code is -38.
> VFS: Cannot open root device "sda2" or unknown-block(8,2): error -38
> Please append a correct "root=" boot option; here are the available partitions:
>
> Last one tried is xfs, the needed btrfs in this case never gets a chance.
> Looking at the code in init/do_mounts.c we can see that it "continue"s only if
> the return code it got is EINVAL, yet xfs clearly does not fit - so the kernel
> panics. Maybe there are other filesystems like xfs - I did not check. This
> patch fixes mount_block_root to try all available filesystems first, and then
> panic. The patched 3.14.x works for me.

Hmm. I don't really dislike your patch, but it makes all the code
_after_ the switch-statement dead, since there is now no way to ever
fall through the switch statement.

So now that

                /*
                 * Allow the user to distinguish between failed sys_open
                 * and bad superblock on root device.
                 * and give them a list of the available devices
                 */

comment ends up being entirely stale, and the code after it is
pointless and it all looks very misleading. And I'm assuming somebody
cared about that difference at some point.

The fact is, I think xfs is just buggy. Returning 38 (ENOSYS) is
totally insane. "No such system call"? Somebody is on some bad bad
drugs. Not that the mount_block_root() loop and error handling might
not be a good thing to perhaps tweak _too_, but at the very least your
patch means that now it no longer prints out the error number at all.

Maybe just making it do something like the attached patch instead? It
doesn't panic on unrecognized errors, just prints them out (just once,
if it repeats). It also doesn't do the "goto repeat" if we already
have the RDONLY bit set, because if somebody is returning insane error
numbers, that could otherwise result in an endless loop.

Anyway, I'm also not seeing why that xfs error would be new to 3.14,
though.. Adding the XFS people to the cc.

Comments (patch obviously TOTALLY UNTESTED)

              Linus
 init/do_mounts.c | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/init/do_mounts.c b/init/do_mounts.c
index 82f22885c87e..a6a725f46f18 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -385,6 +385,7 @@ void __init mount_block_root(char *name, int flags)
 #else
 	const char *b = name;
 #endif
+	int last_err = 0;
 
 	get_fs_names(fs_names);
 retry:
@@ -394,29 +395,16 @@ retry:
 			case 0:
 				goto out;
 			case -EACCES:
+				if (flags & MS_RDONLY)
+					break;
 				flags |= MS_RDONLY;
 				goto retry;
 			case -EINVAL:
 				continue;
 		}
-	        /*
-		 * Allow the user to distinguish between failed sys_open
-		 * and bad superblock on root device.
-		 * and give them a list of the available devices
-		 */
-#ifdef CONFIG_BLOCK
-		__bdevname(ROOT_DEV, b);
-#endif
-		printk("VFS: Cannot open root device \"%s\" or %s: error %d\n",
-				root_device_name, b, err);
-		printk("Please append a correct \"root=\" boot option; here are the available partitions:\n");
-
-		printk_all_partitions();
-#ifdef CONFIG_DEBUG_BLOCK_EXT_DEVT
-		printk("DEBUG_BLOCK_EXT_DEVT is enabled, you need to specify "
-		       "explicit textual name for \"root=\" boot option.\n");
-#endif
-		panic("VFS: Unable to mount root fs on %s", b);
+		if (err != last_err)
+			printk("VFS: Cannot open root device \"%s\" or %s: error %d\n",
+					root_device_name, b, err);
 	}
 
 	printk("List of all partitions:\n");