Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230518035321.1672-1-angus.chen@jaguarmicro.com> <20230518130221.81e8e1f474cfbfd6c93712c5@linux-foundation.org>
In-Reply-To: <20230518130221.81e8e1f474cfbfd6c93712c5@linux-foundation.org>
From:   Nick Desaulniers <ndesaulniers@google.com>
Date:   Thu, 18 May 2023 13:35:54 -0700
Message-ID: <CAKwvOdmtnpETT4P88NZKAtLNvj4kJ7MtotAwNJSdh7WVxa_xJA@mail.gmail.com>
Subject: Re: [PATCH] init: Add bdev fs printk if mount_block_root failed
To:     Andrew Morton <akpm@linux-foundation.org>,
        Angus Chen <angus.chen@jaguarmicro.com>
Cc:     masahiroy@kernel.org, vbabka@suse.cz, peterz@infradead.org,
        paulmck@kernel.org, rppt@kernel.org, linux-kernel@vger.kernel.org,
        Al Viro <viro@zeniv.linux.org.uk>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, May 18, 2023 at 1:02=E2=80=AFPM Andrew Morton <akpm@linux-foundatio=
n.org> wrote:
>
> On Thu, 18 May 2023 11:53:21 +0800 Angus Chen <angus.chen@jaguarmicro.com=
> wrote:
>
> > Attempt to printk all bdev fstype as root gives the following kernel pa=
nic:
> >
> > [    1.729006] VFS: Cannot open root device "vda" or unknown-block(253,=
0): error -19
> > [    1.730603] Please append a correct "root=3D" boot option; here are =
the available partitions:
> > [    1.732323] fd00          256000 vda
> > [    1.732329]  driver: virtio_blk
> > [    1.734194] Kernel panic - not syncing: VFS: Unable to mount root fs=
 on unknown-block(253,0)
> > [    1.734771] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.4.0-rc2+ #53
> > [    1.735194] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS=
 1.10.2-1ubuntu1 04/01/2014
> > [    1.735772] Call Trace:
> > [    1.735950]  <TASK>
> > [    1.736113]  dump_stack_lvl+0x32/0x50
> > [    1.736367]  panic+0x108/0x310
> > [    1.736570]  mount_block_root+0x161/0x310
> > [    1.736849]  ? rdinit_setup+0x40/0x40
> > [    1.737088]  prepare_namespace+0x10c/0x180
> > [    1.737393]  kernel_init_freeable+0x354/0x450
> > [    1.737707]  ? rest_init+0xd0/0xd0
> > [    1.737945]  kernel_init+0x16/0x130
> > [    1.738196]  ret_from_fork+0x1f/0x30
> >
> > QEMU command line:
> > "qemu-system-x86_64 -append root=3D/dev/vda rootfstype=3Dext4 ..."
> >
> > This error is because ext4 is not buildin and request ext4 module fail.

Cool! I'm glad this got picked up; I personally find it confusing when
trying to start from something like an allnoconfig build then start
enabling configs then hitting this panic; it's unclear to users that
they are missing the config for the FS they are trying to load and on
first glance looks like something much worse is going wrong.  In that
sense, I view this as a win. Thanks for the patch!

Acked-by: Nick Desaulniers <ndesaulniers@google.com>

I wish the commit message showed an example of the panic after the
patch, to contrast the before vs. after.

> >
> > As a hint, printk all the bdev fstype available for prompts.
> >
>
> Seems reasonable.  I reworded the changelog a bit:
>
> : Booting with the QEMU command line:
> : "qemu-system-x86_64 -append root=3D/dev/vda rootfstype=3Dext4 ..."
> : will panic if ext4 is not builtin and a request to load the ext4 module
> : fails.
> :
> : [    1.729006] VFS: Cannot open root device "vda" or unknown-block(253,=
0): error -19
> : [    1.730603] Please append a correct "root=3D" boot option; here are =
the available partitions:
> : [    1.732323] fd00          256000 vda
> : [    1.732329]  driver: virtio_blk
> : [    1.734194] Kernel panic - not syncing: VFS: Unable to mount root fs=
 on unknown-block(253,0)
> : [    1.734771] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.4.0-rc2+ #53
> : [    1.735194] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS=
 1.10.2-1ubuntu1 04/01/2014
> : [    1.735772] Call Trace:
> : [    1.735950]  <TASK>
> : [    1.736113]  dump_stack_lvl+0x32/0x50
> : [    1.736367]  panic+0x108/0x310
> : [    1.736570]  mount_block_root+0x161/0x310
> : [    1.736849]  ? rdinit_setup+0x40/0x40
> : [    1.737088]  prepare_namespace+0x10c/0x180
> : [    1.737393]  kernel_init_freeable+0x354/0x450
> : [    1.737707]  ? rest_init+0xd0/0xd0
> : [    1.737945]  kernel_init+0x16/0x130
> : [    1.738196]  ret_from_fork+0x1f/0x30
> :
> : As a hint, print all the bdev fstypes which are available.
>
> > --- a/init/do_mounts.c
> > +++ b/init/do_mounts.c
> > @@ -427,8 +427,19 @@ void __init mount_block_root(char *name, int flags=
)
> >               printk("VFS: Cannot open root device \"%s\" or %s: error =
%d\n",
> >                               root_device_name, b, err);
> >               printk("Please append a correct \"root=3D\" boot option; =
here are the available partitions:\n");
> > -
> >               printk_all_partitions();
> > +
> > +             if (root_fs_names)
> > +                     num_fs =3D list_bdev_fs_names(fs_names, PAGE_SIZE=
);
> > +             if (!num_fs)
> > +                     pr_err("Can't find any bdev filesystem to be used=
 for mount!\n");
> > +             else {
> > +                     pr_err("List of all bdev filesystem:\n");
> > +                     for (i =3D 0, p =3D fs_names; i < num_fs; i++, p =
+=3D strlen(p)+1)
> > +                             pr_err(" %s", p);
> > +                     pr_err("\n");
> > +             }
> > +
> >               panic("VFS: Unable to mount root fs on %s", b);
> >       }
> >       if (!(flags & SB_RDONLY)) {
>
> And I added a little fix.
>
> --- a/init/do_mounts.c~init-add-bdev-fs-printk-if-mount_block_root-failed=
-fix
> +++ a/init/do_mounts.c
> @@ -434,7 +434,7 @@ retry:
>                 if (!num_fs)
>                         pr_err("Can't find any bdev filesystem to be used=
 for mount!\n");
>                 else {
> -                       pr_err("List of all bdev filesystem:\n");
> +                       pr_err("List of all bdev filesystems:\n");
>                         for (i =3D 0, p =3D fs_names; i < num_fs; i++, p =
+=3D strlen(p)+1)
>                                 pr_err(" %s", p);
>                         pr_err("\n");
> _
>
>
> This function now uses a jumble of printk() and pr_err().  Perhaps
> someone will go through and rationalize all of this sometime.
>


--=20
Thanks,
~Nick Desaulniers