Re: [PATCH v2 3/3] xfs/008: use block size instead of the pagesize

["Pankaj Raghav (Samsung)" <kernel@pankajraghav.com>]

On Thu, May 23, 2024 at 09:55:25AM +0530, Ritesh Harjani wrote:
> "Pankaj Raghav (Samsung)" <kernel@pankajraghav.com> writes:
> 
> > From: Pankaj Raghav <p.raghav@samsung.com>
> >
> > The testcase estimates to have ratio of 1:3/4 for holes:filesize. This
> > holds true where the blocksize is always less than or equal to pagesize
> > and the total size of the file is calculated based on the pagesize.
> > There is an implicit assumption that blocksize will always be less than
> > the pagesize.
> >
> > LBS support will enable bs > ps where a minimum IO size is one block,
> > which can be greater than a page. Adjust the size calculation to be
> > based on the blocksize and not the pagesize.
> >
> > Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> > Reviewed-by: Zorro Lang <zlang@redhat.com>
> > ---
> >  tests/xfs/008     | 19 ++++++++++---------
> >  tests/xfs/008.out |  8 ++++----
> >  2 files changed, 14 insertions(+), 13 deletions(-)
> >
> > diff --git a/tests/xfs/008 b/tests/xfs/008
> > index e7d6153b..e37e435a 100755
> > --- a/tests/xfs/008
> > +++ b/tests/xfs/008
> > @@ -11,7 +11,8 @@ _begin_fstest rw ioctl auto quick
> >  
> >  status=0	# success is the default!
> >  pgsize=`$here/src/feature -s`
> > -
> > +fileblksize=$(_get_file_block_size "$TEST_DIR")
> > +blksize=$((fileblksize > pgsize ? fileblksize : pgsize))
> 
> I assume when the test might be written it might have assumed blocksize =
> pagesize. Hence the dependency on pagesize in this test. 
> If that assumption is correct, then we might not need these two paths
> and can just make it blocksize.
> 
> Do you see any problem with blocksize for any of the paths
> (bs = ps, bs < ps and bs > ps)?
Thanks for the comments Ritesh. You are right about not needing to
fallback to pgsize.

I made your suggested changes and I will send a new version soon.
> 
> 
> >  # Override the default cleanup function.
> >  _cleanup()
> >  {
> > @@ -21,7 +22,7 @@ _cleanup()
> >  
> >  _filter()
> >  {
> > -    sed -e "s/-b $pgsize/-b PGSIZE/g" \
> > +    sed -e "s/-b $blksize/-b BLKSIZE/g" \
> >  	-e "s/-l .* -c/-l FSIZE -c/g"
> >  }
> >  
> > @@ -73,17 +74,17 @@ _require_test
> >  # We are trying to create roughly 50 or 100 holes in a file
> >  # using random writes. Assuming a good distribution of 50 writes
> >  # in a file, the file only needs to be 3-4x the size of the write
> > -# size muliplied by the number of writes. Hence we use 200 * pgsize
> > -# for files we want 50 holes in and 400 * pgsize for files we want
> > +# size muliplied by the number of writes. Hence we use 200 * blksize
> > +# for files we want 50 holes in and 400 * blksize for files we want
> >  # 100 holes in. This keeps the runtime down as low as possible.
> >  #
> > -_do_test 1 50 "-l `expr 200 \* $pgsize` -c 50 -b $pgsize"
> > -_do_test 2 100 "-l `expr 400 \* $pgsize` -c 100 -b $pgsize"
> > -_do_test 3 100 "-l `expr 400 \* $pgsize` -c 100 -b 512"   # test partial pages
> > +_do_test 1 50 "-l `expr 200 \* $blksize` -c 50 -b $blksize"
> > +_do_test 2 100 "-l `expr 400 \* $blksize` -c 100 -b $blksize"
> > +_do_test 3 100 "-l `expr 400 \* $blksize` -c 100 -b 512"   # test partial blocks
> >  
> >  # rinse, lather, repeat for direct IO
> > -_do_test 4 50 "-d -l `expr 200 \* $pgsize` -c 50 -b $pgsize"
> > -_do_test 5 100 "-d -l `expr 400 \* $pgsize` -c 100 -b $pgsize"
> > +_do_test 4 50 "-d -l `expr 200 \* $blksize` -c 50 -b $blksize"
> > +_do_test 5 100 "-d -l `expr 400 \* $blksize` -c 100 -b $blksize"
> >  # note: direct IO requires page aligned IO
> 
> ^^^ This last comment about direct-io alignment is not valid anymore since kernel
> 2.6. Maybe it's time to rip that comment off.
> 
> From man 2 open
> O_DIRECT
> <...>
>        If  none  of  the  above is available, then direct I/O support and alignment restrictions can only be assumed from known characteristics of the filesystem, the individual file, the underlying storage device(s), and the kernel
>        version.  In Linux 2.4, most filesystems based on block devices require that the file offset and the length and memory address of all I/O segments be multiples of the filesystem block size (typically 4096  bytes).   In  Linux
>        2.6.0, this was relaxed to the logical block size of the block device (typically 512 bytes).  A block device's logical block size can be determined using the ioctl(2) BLKSSZGET operation or from the shell using the command:
> 
> -ritesh
> 
> >  
> >  # todo: realtime.
> > diff --git a/tests/xfs/008.out b/tests/xfs/008.out
> > index 5e3ae8e3..0941e218 100644
> > --- a/tests/xfs/008.out
> > +++ b/tests/xfs/008.out
> > @@ -1,10 +1,10 @@
> >  QA output created by 008
> >  
> > -randholes.1 : -l FSIZE -c 50 -b PGSIZE
> > +randholes.1 : -l FSIZE -c 50 -b BLKSIZE
> >  ------------------------------------------
> >  holes is in range
> >  
> > -randholes.2 : -l FSIZE -c 100 -b PGSIZE
> > +randholes.2 : -l FSIZE -c 100 -b BLKSIZE
> >  ------------------------------------------
> >  holes is in range
> >  
> > @@ -12,10 +12,10 @@ randholes.3 : -l FSIZE -c 100 -b 512
> >  ------------------------------------------
> >  holes is in range
> >  
> > -randholes.4 : -d -l FSIZE -c 50 -b PGSIZE
> > +randholes.4 : -d -l FSIZE -c 50 -b BLKSIZE
> >  ------------------------------------------
> >  holes is in range
> >  
> > -randholes.5 : -d -l FSIZE -c 100 -b PGSIZE
> > +randholes.5 : -d -l FSIZE -c 100 -b BLKSIZE
> >  ------------------------------------------
> >  holes is in range
> > -- 
> > 2.34.1

-- 
Pankaj Raghav