[PATCh v2] loop: Increase bsize variable from unsigned short to unsigned int

[PATCh v2] loop: Increase bsize variable from unsigned short to unsigned int

[Li Wang]

This change allows the loopback driver to handle block size larger than
PAGE_SIZE and increases the consistency of data types used within the driver.
Especially to match the struct queue_limits.logical_block_size type.

Also, this is to get rid of the LTP/ioctl_loop06 test failure:

  12 ioctl_loop06.c:76: TINFO: Using LOOP_SET_BLOCK_SIZE with arg > PAGE_SIZE
  13 ioctl_loop06.c:59: TFAIL: Set block size succeed unexpectedly
  ...
  18 ioctl_loop06.c:76: TINFO: Using LOOP_CONFIGURE with block_size > PAGE_SIZE
  19 ioctl_loop06.c:59: TFAIL: Set block size succeed unexpectedly

Thoese fail due to the loop_reconfigure_limits() cast bsize to 'unsined short'
that never gets an expected error when testing invalid logical block size,
which was just exposed since 6.11-rc1 introduced patches:

  commit 9423c653fe61 ("loop: Don't bother validating blocksize")
  commit fe3d508ba95b ("block: Validate logical block size in blk_validate_limits()")

Link: https://lists.linux.it/pipermail/ltp/2024-August/039912.html
Signed-off-by: Li Wang <liwang@redhat.com>
Cc: John Garry <john.g.garry@oracle.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Jan Stancek <jstancek@redhat.com>
---
 drivers/block/loop.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 78a7bb28defe..86cc3b19faae 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -173,7 +173,7 @@ static loff_t get_loop_size(struct loop_device *lo, struct file *file)
 static bool lo_bdev_can_use_dio(struct loop_device *lo,
 		struct block_device *backing_bdev)
 {
-	unsigned short sb_bsize = bdev_logical_block_size(backing_bdev);
+	unsigned int sb_bsize = bdev_logical_block_size(backing_bdev);
 
 	if (queue_logical_block_size(lo->lo_queue) < sb_bsize)
 		return false;
@@ -977,7 +977,7 @@ loop_set_status_from_info(struct loop_device *lo,
 	return 0;
 }
 
-static unsigned short loop_default_blocksize(struct loop_device *lo,
+static unsigned int loop_default_blocksize(struct loop_device *lo,
 		struct block_device *backing_bdev)
 {
 	/* In case of direct I/O, match underlying block size */
@@ -986,7 +986,7 @@ static unsigned short loop_default_blocksize(struct loop_device *lo,
 	return SECTOR_SIZE;
 }
 
-static int loop_reconfigure_limits(struct loop_device *lo, unsigned short bsize)
+static int loop_reconfigure_limits(struct loop_device *lo, unsigned int bsize)
 {
 	struct file *file = lo->lo_backing_file;
 	struct inode *inode = file->f_mapping->host;
-- 
2.46.0

Re: [PATCh v2] loop: Increase bsize variable from unsigned short to unsigned int

[John Garry]

On 27/08/2024 09:17, Li Wang wrote:
> This change allows the loopback driver to handle block size larger than
> PAGE_SIZE and increases the consistency of data types used within the driver.
> Especially to match the struct queue_limits.logical_block_size type.
> 
> Also, this is to get rid of the LTP/ioctl_loop06 test failure:
> 
>    12 ioctl_loop06.c:76: TINFO: Using LOOP_SET_BLOCK_SIZE with arg > PAGE_SIZE
>    13 ioctl_loop06.c:59: TFAIL: Set block size succeed unexpectedly
>    ...
>    18 ioctl_loop06.c:76: TINFO: Using LOOP_CONFIGURE with block_size > PAGE_SIZE
>    19 ioctl_loop06.c:59: TFAIL: Set block size succeed unexpectedly
> 
> Thoese fail due to the loop_reconfigure_limits() cast bsize to 'unsined short'

these

> that never gets an expected error when testing invalid logical block size,
> which was just exposed since 6.11-rc1 introduced patches:
> 
>    commit 9423c653fe61 ("loop: Don't bother validating blocksize")
>    commit fe3d508ba95b ("block: Validate logical block size in blk_validate_limits()")

Maybe it's better to add a fixes tag for original commit which 
introduced unsigned short usage.

> 
> Link:https://urldefense.com/v3/__https://lists.linux.it/pipermail/ltp/2024- 
> August/039912.html__;!!ACWV5N9M2RV99hQ! 
> MxW7Cvs_YKyihvENQFEGgHt-1KbCFD9gkUJxwI8gguQlawan3UNneBoB_THVG4zNP8Sao2a0apcOu03lcg$ 
> Signed-off-by: Li Wang<liwang@redhat.com>
> Cc: John Garry<john.g.garry@oracle.com>
> Cc: Jens Axboe<axboe@kernel.dk>
> Cc: Stefan Hajnoczi<stefanha@redhat.com>
> Reviewed-by: Damien Le Moal<dlemoal@kernel.org>
> Reviewed-by: Jan Stancek<jstancek@redhat.com>


Reviewed-by: John Garry <john.g.garry@oracle.com>

Re: [PATCh v2] loop: Increase bsize variable from unsigned short to unsigned int

[Li Wang]

On Tue, Aug 27, 2024 at 4:41 PM John Garry <john.g.garry@oracle.com> wrote:
>
> On 27/08/2024 09:17, Li Wang wrote:
> > This change allows the loopback driver to handle block size larger than
> > PAGE_SIZE and increases the consistency of data types used within the driver.
> > Especially to match the struct queue_limits.logical_block_size type.
> >
> > Also, this is to get rid of the LTP/ioctl_loop06 test failure:
> >
> >    12 ioctl_loop06.c:76: TINFO: Using LOOP_SET_BLOCK_SIZE with arg > PAGE_SIZE
> >    13 ioctl_loop06.c:59: TFAIL: Set block size succeed unexpectedly
> >    ...
> >    18 ioctl_loop06.c:76: TINFO: Using LOOP_CONFIGURE with block_size > PAGE_SIZE
> >    19 ioctl_loop06.c:59: TFAIL: Set block size succeed unexpectedly
> >
> > Thoese fail due to the loop_reconfigure_limits() cast bsize to 'unsined short'
>
> these
>
> > that never gets an expected error when testing invalid logical block size,
> > which was just exposed since 6.11-rc1 introduced patches:
> >
> >    commit 9423c653fe61 ("loop: Don't bother validating blocksize")
> >    commit fe3d508ba95b ("block: Validate logical block size in blk_validate_limits()")
>
> Maybe it's better to add a fixes tag for original commit which
> introduced unsigned short usage.

I'm not sure that makes sense because at that moment loop_set_block_size
has a dedicated function blk_validate_block_size to validate bsize, after your
commit 9423c653fe61 optimize that then the problem appears.

  473516b36193 ("loop: use the atomic queue limits update API")


> Reviewed-by: John Garry <john.g.garry@oracle.com>

Thanks for reviewing.


-- 
Regards,
Li Wang