2023-06-07 14:06:25

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH] block: fix rootwait=

Failures to look up the gendisk must return -ENODEV so that rootwait
retries the lookup instead of -EINVAL which exits early.

Fixes: cf056a431215 ("init: improve the name_to_dev_t interface")
Reported-by: Fabio Estevam <[email protected]>
Signed-off-by: Christoph Hellwig <[email protected]>
Tested-by: Fabio Estevam <[email protected]>
---

[Jens, if you need to rebase for any reason it might make sense to fold
this into the offending patch]

block/early-lookup.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/early-lookup.c b/block/early-lookup.c
index 3ff0d2e4dcbfb8..48ea3e982419cc 100644
--- a/block/early-lookup.c
+++ b/block/early-lookup.c
@@ -181,7 +181,7 @@ static int __init devt_from_devname(const char *name, dev_t *devt)
*p = '\0';
*devt = blk_lookup_devt(s, part);
if (*devt)
- return 0;
+ return -ENODEV;

/* try disk name without p<part number> */
if (p < s + 2 || !isdigit(p[-2]) || p[-1] != 'p')
@@ -190,7 +190,7 @@ static int __init devt_from_devname(const char *name, dev_t *devt)
*devt = blk_lookup_devt(s, part);
if (*devt)
return 0;
- return -EINVAL;
+ return -ENODEV;
}

static int __init devt_from_devnum(const char *name, dev_t *devt)
--
2.39.2



2023-06-07 14:24:43

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] block: fix rootwait=

On 6/7/23 7:57 AM, Christoph Hellwig wrote:
> Failures to look up the gendisk must return -ENODEV so that rootwait
> retries the lookup instead of -EINVAL which exits early.
>
> Fixes: cf056a431215 ("init: improve the name_to_dev_t interface")
> Reported-by: Fabio Estevam <[email protected]>
> Signed-off-by: Christoph Hellwig <[email protected]>
> Tested-by: Fabio Estevam <[email protected]>
> ---
>
> [Jens, if you need to rebase for any reason it might make sense to fold
> this into the offending patch]

I have applied it for now, and probably not a high likelihood of rebasing
this late. But if I do, I'll fold it in.

--
Jens Axboe



2023-06-08 18:44:25

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] block: fix rootwait=

On Wed, Jun 07, 2023 at 03:57:46PM +0200, Christoph Hellwig wrote:

> Failures to look up the gendisk must return -ENODEV so that rootwait
> retries the lookup instead of -EINVAL which exits early.

This appears to be causing failures to find a root filesystem located on
a partition on a virtio device virtio-blk when booting on Arm FVP
virtual platforms on today's -next. With root=/dev/vda1 we find the
device:

[ 0.593310] virtio_blk virtio0: 1/0/0 default/read/poll queues
[ 0.594413] virtio_blk virtio0: [vda] 2929688 512-byte logical blocks (1.50 GB/1.40 GiB)
[ 0.598898] vda: vda1

but then when it comes time to mount the rootfs we get:

[ 1.035643] Waiting for root device /dev/vda1...

instead of mounting the device we already knew about. A bisect (log
below) lands on this commit and reverting the first hunk:

> @@ -181,7 +181,7 @@ static int __init devt_from_devname(const char *name, dev_t *devt)
> *p = '\0';
> *devt = blk_lookup_devt(s, part);
> if (*devt)
> - return 0;
> + return -ENODEV;

avoids the issue and allows us to boot as normal. I need to stare at
the code a bit more to fully follow the intent of that bit of the change
but I'm about done for today, the commit log just mentions replacing
-EINVAL rather than 0.

Bisect log:

git bisect start
# bad: [db10f126048021b86e43a5d6a335ec49dec10155] Add linux-next specific files for 20230608
git bisect bad db10f126048021b86e43a5d6a335ec49dec10155
# good: [5f63595ebd82f56a2dd36ca013dd7f5ff2e2416a] Merge tag 'input-for-v6.4-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input
git bisect good 5f63595ebd82f56a2dd36ca013dd7f5ff2e2416a
# good: [dc3648f0663a83e9399f44faa473ce8891f8a469] Merge branch 'drm-next' of git://git.freedesktop.org/git/drm/drm.git
git bisect good dc3648f0663a83e9399f44faa473ce8891f8a469
# bad: [80f5925ba46062fd05697d83717d67162ae18812] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
git bisect bad 80f5925ba46062fd05697d83717d67162ae18812
# good: [2c5161ad2db3e89b608b89e4994e23ab58db7162] Merge branch 'for-linux-next' of git://anongit.freedesktop.org/drm-intel
git bisect good 2c5161ad2db3e89b608b89e4994e23ab58db7162
# good: [0e425a6896902e0112a4da8cbf71c067ec6d6d8f] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
git bisect good 0e425a6896902e0112a4da8cbf71c067ec6d6d8f
# good: [2dc4eeb36feb4b5baae9ec9ef145c974007d94b8] Merge branch 'for-6.5/block' into for-next
git bisect good 2dc4eeb36feb4b5baae9ec9ef145c974007d94b8
# bad: [e0e9727312109e289bcce88bbd2bbe86b9a209ca] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/dlemoal/libata.git
git bisect bad e0e9727312109e289bcce88bbd2bbe86b9a209ca
# good: [182faecd1a321d626190cb7e87d814da04c2a083] Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git
git bisect good 182faecd1a321d626190cb7e87d814da04c2a083
# bad: [f3e6f4e23cc82971bda01ef5cacb716fb564c7aa] Merge branch 'for-6.5/io_uring' into for-next
git bisect bad f3e6f4e23cc82971bda01ef5cacb716fb564c7aa
# bad: [80d994d2a71f88e0809dcaccef2259c791d2e3ef] pktcdvd: Use DEFINE_SHOW_ATTRIBUTE() to simplify code
git bisect bad 80d994d2a71f88e0809dcaccef2259c791d2e3ef
# bad: [3a41db531e5124adaa3a9ab9ca0c724aee85b10c] pktcdvd: Get rid of custom printing macros
git bisect bad 3a41db531e5124adaa3a9ab9ca0c724aee85b10c
# good: [a7cfa0af0c88353b4eb59db5a2a0fbe35329b3f9] blk-ioc: fix recursive spin_lock/unlock_irq() in ioc_clear_queue()
git bisect good a7cfa0af0c88353b4eb59db5a2a0fbe35329b3f9
# bad: [1341c7d2ccf42ed91aea80b8579d35bc1ea381e2] block: fix rootwait=
git bisect bad 1341c7d2ccf42ed91aea80b8579d35bc1ea381e2
# good: [3d2af77e31ade05ff7ccc3658c3635ec1bea0979] blk-cgroup: Reinit blkg_iostat_set after clearing in blkcg_reset_stats()
git bisect good 3d2af77e31ade05ff7ccc3658c3635ec1bea0979
# first bad commit: [1341c7d2ccf42ed91aea80b8579d35bc1ea381e2] block: fix rootwait=


Attachments:
(No filename) (3.99 kB)
signature.asc (499.00 B)
Download all attachments

2023-06-09 01:09:49

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH] block: fix rootwait=

Hi Mark,

On Thu, Jun 8, 2023 at 3:26 PM Mark Brown <[email protected]> wrote:

> instead of mounting the device we already knew about. A bisect (log
> below) lands on this commit and reverting the first hunk:
>
> > @@ -181,7 +181,7 @@ static int __init devt_from_devname(const char *name, dev_t *devt)
> > *p = '\0';
> > *devt = blk_lookup_devt(s, part);
> > if (*devt)
> > - return 0;
> > + return -ENODEV;

I have tested reverting this first hunk and it also works for me, so
if someone submits this fix:

Tested-by: Fabio Estevam <[email protected]>

Thanks

2023-06-09 04:57:59

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] block: fix rootwait=

On Thu, Jun 08, 2023 at 10:02:00PM -0300, Fabio Estevam wrote:
> > below) lands on this commit and reverting the first hunk:
> >
> > > @@ -181,7 +181,7 @@ static int __init devt_from_devname(const char *name, dev_t *devt)
> > > *p = '\0';
> > > *devt = blk_lookup_devt(s, part);
> > > if (*devt)
> > > - return 0;
> > > + return -ENODEV;
>
> I have tested reverting this first hunk and it also works for me, so
> if someone submits this fix:

Yes, the first hunk is completely bogus, sorry.

I'll send the fix.