[Top] [All Lists]

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

To: Plamen Petrov <plamen.sisi@xxxxxxxxx>, Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH] do_mounts: try all available filesystems before panicking
From: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Date: Sun, 25 May 2014 13:04:09 -0700
Cc: LKML <linux-kernel@xxxxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=3e1kBOwFLPUWo+Kjfar38sOGzPRVi0D0JYe9NopixFI=; b=gNwnmye7XymTyh58DbeskUf5X+vuS6avF2NH6CXEN1X3bzRevkWxNUeaUapeMfVhLJ oh7kEzkd3xX8rQGJPH5SRVVZikLxak+nRyfmw18EK6ZU0XeV2FH5JHix2r1UZTNY2mgu QXkVONVcNGDhBQ+g8JU7SbSV1QRFixauoAtUbTZRs4rHj07TmBMs1CvFvjdXSq42mzyn U8/+2lYu32qnPSdWgAm7vBLGsJrrTx8j9RTGdTa7Mxj3cj0jL3fo7eTUkH1SwCbQgqTW CsAtkw3968wU6fGMGPLJmF4UEUngbh+VII9GKVBNyql6ElS6g+7Kn8D54toiBfnrZROH OCOA==
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=3e1kBOwFLPUWo+Kjfar38sOGzPRVi0D0JYe9NopixFI=; b=araTl0/A76+GKOPIJiJ+YixSEGjpwWqXeXhh6lrmdiENj3Y4k58iVzXGvQtU0RRfHl T2OrMTP/1fzEzh1Bo433UWCHEIdhYMtnzSlxLT80U7VAyrRRB5Nct+XftthdT3ZGZjgR bMKSWeX7GWw7WcpW4ektBxANDtomchQNBDJ2w=
In-reply-to: <1399314889-9829-1-git-send-email-plamen.sisi@xxxxxxxxx>
References: <1399314889-9829-1-git-send-email-plamen.sisi@xxxxxxxxx>
Sender: linus971@xxxxxxxxx
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)


Attachment: patch.diff
Description: Text document

<Prev in Thread] Current Thread [Next in Thread>