[PATCH v2] block: retry call probe after request_module in blk_request_module

[PATCH v2] block: retry call probe after request_module in blk_request_module

[Yang Erkun]

From: Yang Erkun <yangerkun@huawei.com>

Set kernel config:

 CONFIG_BLK_DEV_LOOP=m
 CONFIG_BLK_DEV_LOOP_MIN_COUNT=0

Do latter:

 mknod loop0 b 7 0
 exec 4<> loop0

Before commit e418de3abcda ("block: switch gendisk lookup to a simple
xarray"), lookup_gendisk will first use base_probe to load module loop,
and then the retry will call loop_probe to prepare the loop disk. Finally
open for this disk will success. However, after this commit, we lose the
retry logic, and open will fail with ENXIO. Block device autoloading is
deprecated and will be removed soon, but maybe we should keep open success
until we really remove it. So, give a retry to fix it.

Fixes: e418de3abcda ("block: switch gendisk lookup to a simple xarray")
Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Yang Erkun <yangerkun@huawei.com>
---
 block/genhd.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

v1->v2:
Rewrite this patch by adding helper blk_probe_dev to make code looks
more clear.

diff --git a/block/genhd.c b/block/genhd.c
index 79230c109fca..8a63be374220 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -798,7 +798,7 @@ static ssize_t disk_badblocks_store(struct device *dev,
 }
 
 #ifdef CONFIG_BLOCK_LEGACY_AUTOLOAD
-void blk_request_module(dev_t devt)
+static bool blk_probe_dev(dev_t devt)
 {
 	unsigned int major = MAJOR(devt);
 	struct blk_major_name **n;
@@ -808,14 +808,26 @@ void blk_request_module(dev_t devt)
 		if ((*n)->major == major && (*n)->probe) {
 			(*n)->probe(devt);
 			mutex_unlock(&major_names_lock);
-			return;
+			return true;
 		}
 	}
 	mutex_unlock(&major_names_lock);
+	return false;
+}
+
+void blk_request_module(dev_t devt)
+{
+	int error;
+
+	if (blk_probe_dev(devt))
+		return;
 
-	if (request_module("block-major-%d-%d", MAJOR(devt), MINOR(devt)) > 0)
-		/* Make old-style 2.4 aliases work */
-		request_module("block-major-%d", MAJOR(devt));
+	error = request_module("block-major-%d-%d", MAJOR(devt), MINOR(devt));
+	/* Make old-style 2.4 aliases work */
+	if (error > 0)
+		error = request_module("block-major-%d", MAJOR(devt));
+	if (!error)
+		blk_probe_dev(devt);
 }
 #endif /* CONFIG_BLOCK_LEGACY_AUTOLOAD */
 
-- 
2.39.2

Re: [PATCH v2] block: retry call probe after request_module in blk_request_module

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v2] block: retry call probe after request_module in blk_request_module

[yangerkun]

ping

在 2024/12/9 19:04, Yang Erkun 写道:
> From: Yang Erkun <yangerkun@huawei.com>
> 
> Set kernel config:
> 
>   CONFIG_BLK_DEV_LOOP=m
>   CONFIG_BLK_DEV_LOOP_MIN_COUNT=0
> 
> Do latter:
> 
>   mknod loop0 b 7 0
>   exec 4<> loop0
> 
> Before commit e418de3abcda ("block: switch gendisk lookup to a simple
> xarray"), lookup_gendisk will first use base_probe to load module loop,
> and then the retry will call loop_probe to prepare the loop disk. Finally
> open for this disk will success. However, after this commit, we lose the
> retry logic, and open will fail with ENXIO. Block device autoloading is
> deprecated and will be removed soon, but maybe we should keep open success
> until we really remove it. So, give a retry to fix it.
> 
> Fixes: e418de3abcda ("block: switch gendisk lookup to a simple xarray")
> Suggested-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Yang Erkun <yangerkun@huawei.com>
> ---
>   block/genhd.c | 22 +++++++++++++++++-----
>   1 file changed, 17 insertions(+), 5 deletions(-)
> 
> v1->v2:
> Rewrite this patch by adding helper blk_probe_dev to make code looks
> more clear.
> 
> diff --git a/block/genhd.c b/block/genhd.c
> index 79230c109fca..8a63be374220 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -798,7 +798,7 @@ static ssize_t disk_badblocks_store(struct device *dev,
>   }
>   
>   #ifdef CONFIG_BLOCK_LEGACY_AUTOLOAD
> -void blk_request_module(dev_t devt)
> +static bool blk_probe_dev(dev_t devt)
>   {
>   	unsigned int major = MAJOR(devt);
>   	struct blk_major_name **n;
> @@ -808,14 +808,26 @@ void blk_request_module(dev_t devt)
>   		if ((*n)->major == major && (*n)->probe) {
>   			(*n)->probe(devt);
>   			mutex_unlock(&major_names_lock);
> -			return;
> +			return true;
>   		}
>   	}
>   	mutex_unlock(&major_names_lock);
> +	return false;
> +}
> +
> +void blk_request_module(dev_t devt)
> +{
> +	int error;
> +
> +	if (blk_probe_dev(devt))
> +		return;
>   
> -	if (request_module("block-major-%d-%d", MAJOR(devt), MINOR(devt)) > 0)
> -		/* Make old-style 2.4 aliases work */
> -		request_module("block-major-%d", MAJOR(devt));
> +	error = request_module("block-major-%d-%d", MAJOR(devt), MINOR(devt));
> +	/* Make old-style 2.4 aliases work */
> +	if (error > 0)
> +		error = request_module("block-major-%d", MAJOR(devt));
> +	if (!error)
> +		blk_probe_dev(devt);
>   }
>   #endif /* CONFIG_BLOCK_LEGACY_AUTOLOAD */
>   

Re: [PATCH v2] block: retry call probe after request_module in blk_request_module

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v2] block: retry call probe after request_module in blk_request_module

[Jens Axboe]

On Mon, 09 Dec 2024 19:04:35 +0800, Yang Erkun wrote:
> Set kernel config:
> 
>  CONFIG_BLK_DEV_LOOP=m
>  CONFIG_BLK_DEV_LOOP_MIN_COUNT=0
> 
> Do latter:
> 
> [...]

Applied, thanks!

[1/1] block: retry call probe after request_module in blk_request_module
      commit: 457ef47c08d2979f3e59ce66267485c3faed70c8

Best regards,
-- 
Jens Axboe