[PATCH 0/3] loop: three small cleanups

[PATCH 0/3] loop: three small cleanups

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

A stale kerneldoc reference, a simple_strtol() left in the max_loop
setup code, and an unused function argument.

Tao Cui (3):
  loop: drop a stale reference to loop_validate_size()
  loop: replace simple_strtol() with kstrtoint()
  loop: drop the unused argument of lo_req_flush()

 drivers/block/loop.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

-- 
2.43.0

[PATCH v2 1/3] loop: drop a stale reference to loop_validate_size()

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

The kerneldoc of loop_set_size() points readers at loop_validate_size(),
which does not exist in the tree.  Drop the reference.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/block/loop.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 6f12976035b0..68a9cc7aeb13 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -212,9 +212,6 @@ static inline void loop_update_dio(struct loop_device *lo)
  * loop_set_size() - sets device size and notifies userspace
  * @lo: struct loop_device to set the size for
  * @size: new size of the loop device
- *
- * Callers must validate that the size passed into this function fits into
- * a sector_t, eg using loop_validate_size()
  */
 static void loop_set_size(struct loop_device *lo, loff_t size)
 {
-- 
2.43.0

Re: [PATCH v2 1/3] loop: drop a stale reference to loop_validate_size()

[Christoph Hellwig]

Looks good:

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

[PATCH v2 2/3] loop: replace simple_strtol() with kstrtoint()

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

simple_strtol() is deprecated and swallows errors.  Use kstrtoint()
in the max_loop setup code; an invalid option keeps the default and
now says so.

Only mark max_loop_specified when the option was parsed successfully.
With simple_strtol() a garbage string yields max_loop = 0, and
loop_probe()'s "max_loop_specified && max_loop" check short-circuits
on 0, so legacy autoloading is not capped.  Keeping max_loop at its
default while still setting max_loop_specified would instead turn the
default into a hard upper bound for dynamic device creation.  This
mirrors what max_loop_param_set_int() already does for the module
parameter.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>

---
Changes in v2:
- move max_loop_specified = true under the success path, pointed out
  by an AI-assisted review of v1.
---
 drivers/block/loop.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 68a9cc7aeb13..126f42580632 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -2341,7 +2341,10 @@ module_exit(loop_exit);
 #ifndef MODULE
 static int __init max_loop_setup(char *str)
 {
-	max_loop = simple_strtol(str, NULL, 0);
+	if (kstrtoint(str, 0, &max_loop)) {
+		pr_warn("loop: invalid max_loop, keeping default\n");
+		return 1;
+	}
 #ifdef CONFIG_BLOCK_LEGACY_AUTOLOAD
 	max_loop_specified = true;
 #endif
-- 
2.43.0

Re: [PATCH v2 2/3] loop: replace simple_strtol() with kstrtoint()

[Tao Cui]

Hi Bart,

在 2026/8/31 18:03, Tao Cui 写道:
> From: Tao Cui <cuitao@kylinos.cn>
> 
> simple_strtol() is deprecated and swallows errors.  Use kstrtoint()
> in the max_loop setup code; an invalid option keeps the default and
> now says so.
> 
> Only mark max_loop_specified when the option was parsed successfully.
> With simple_strtol() a garbage string yields max_loop = 0, and
> loop_probe()'s "max_loop_specified && max_loop" check short-circuits
> on 0, so legacy autoloading is not capped.  Keeping max_loop at its
> default while still setting max_loop_specified would instead turn the
> default into a hard upper bound for dynamic device creation.  This
> mirrors what max_loop_param_set_int() already does for the module
> parameter.
> 

Thanks for the review!

After sending v1 I noticed a regression in this patch, so I'll follow
up with a v2 of just this one (1/3 and 3/3 are unchanged and keep your
Reviewed-by).

With simple_strtol() a garbage max_loop= string yields max_loop = 0,
and loop_probe()'s "max_loop_specified && max_loop" check
short-circuits on 0, so legacy autoloading was not capped.  In v1 an
invalid string keeps max_loop at its default value while
max_loop_specified is still set to true, which turns the default into
a hard upper bound for dynamic device creation.  The v2 only marks
max_loop_specified when kstrtoint() succeeds, mirroring
max_loop_param_set_int():

 	if (kstrtoint(str, 0, &max_loop))
+	{
 		pr_warn("loop: invalid max_loop, keeping default\n");
+		return 1;
+	}
 #ifdef CONFIG_BLOCK_LEGACY_AUTOLOAD
 		max_loop_specified = true;
 #endif

This was pointed out by an AI-assisted review of v1, so I've left your
Reviewed-by off this one for now.  Could you take another look when
you have a chance?

Thanks,
Tao

> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
> 
> ---
> Changes in v2:
> - move max_loop_specified = true under the success path, pointed out
>   by an AI-assisted review of v1.
> ---
>  drivers/block/loop.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 68a9cc7aeb13..126f42580632 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -2341,7 +2341,10 @@ module_exit(loop_exit);
>  #ifndef MODULE
>  static int __init max_loop_setup(char *str)
>  {
> -	max_loop = simple_strtol(str, NULL, 0);
> +	if (kstrtoint(str, 0, &max_loop)) {
> +		pr_warn("loop: invalid max_loop, keeping default\n");
> +		return 1;
> +	}
>  #ifdef CONFIG_BLOCK_LEGACY_AUTOLOAD
>  	max_loop_specified = true;
>  #endif

[PATCH v2 3/3] loop: drop the unused argument of lo_req_flush()

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

lo_req_flush() never uses its request argument.  Drop it.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/block/loop.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 126f42580632..1aff40963b40 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -270,7 +270,7 @@ static int lo_fallocate(struct loop_device *lo, struct request *rq, loff_t pos,
 	return ret;
 }
 
-static int lo_req_flush(struct loop_device *lo, struct request *rq)
+static int lo_req_flush(struct loop_device *lo)
 {
 	int ret = vfs_fsync(lo->lo_backing_file, 0);
 	if (unlikely(ret && ret != -EINVAL))
@@ -411,7 +411,7 @@ static int do_req_filebacked(struct loop_device *lo, struct request *rq)
 
 	switch (req_op(rq)) {
 	case REQ_OP_FLUSH:
-		return lo_req_flush(lo, rq);
+		return lo_req_flush(lo);
 	case REQ_OP_WRITE_ZEROES:
 		/*
 		 * If the caller doesn't want deallocation, call zeroout to
-- 
2.43.0

Re: [PATCH v2 3/3] loop: drop the unused argument of lo_req_flush()

[Christoph Hellwig]

Looks good:

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