[PATCH] block: fix rootwait=

[PATCH] block: fix rootwait=

[Christoph Hellwig]

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 <festevam@gmail.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Tested-by: Fabio Estevam <festevam@gmail.com>
---

[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

Re: [PATCH] block: fix rootwait=

[Jens Axboe]

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 <festevam@gmail.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Tested-by: Fabio Estevam <festevam@gmail.com>
> ---
> 
> [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

Re: [PATCH] block: fix rootwait=

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 4018 bytes --]

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=

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] block: fix rootwait=

[Fabio Estevam]

Hi Mark,

On Thu, Jun 8, 2023 at 3:26 PM Mark Brown <broonie@kernel.org> 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 <festevam@gmail.com>

Thanks

Re: [PATCH] block: fix rootwait=

[Christoph Hellwig]

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.