| Message ID | 20240827081757.37646-1-liwang@redhat.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [v2] loop: Increase bsize variable from unsigned short to unsigned int | expand |
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>
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.
Hi John, Jens, Can any of you help merge this patch? Or should I send a new patch version? (Our CI jobs continuously complain the test failures) On Tue, Aug 27, 2024 at 7:59 PM Li Wang <liwang@redhat.com> wrote: > 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 >
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;
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(-)