fstests.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] more lbs test fixes
@ 2024-05-06 15:01 Pankaj Raghav (Samsung)
  2024-05-06 15:01 ` [PATCH 1/3] xfs/161: adapt the test case for 64k FS blocksize Pankaj Raghav (Samsung)
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Pankaj Raghav (Samsung) @ 2024-05-06 15:01 UTC (permalink / raw)
  To: fstests
  Cc: djwong, gost.dev, mcgrof, kernel, ritesh.list, zlang, Pankaj Raghav

From: Pankaj Raghav <p.raghav@samsung.com>

These are some of the tests that failed when we were working on LBS
patches [1].

generic/436 and xfs/008 are adapted to work with bs > ps systems.

xfs/161 is an upstream failure, and it happens even on bs <= ps systems.

[1] https://lore.kernel.org/linux-xfs/20240503095353.3798063-1-mcgrof@kernel.org/

Pankaj Raghav (3):
  xfs/161: adapt the test case for 64k FS blocksize
  generic/436: round up bufsz to nearest filesystem blksz
  xfs/008: use block size instead of the pagesize

 src/seek_sanity_test.c |  8 ++++----
 tests/xfs/008          | 19 ++++++++++---------
 tests/xfs/008.out      |  8 ++++----
 tests/xfs/161          | 10 ++++++++--
 4 files changed, 26 insertions(+), 19 deletions(-)


base-commit: b26d68da08e47e6508a96bee72b25823040ab67e
-- 
2.34.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/3] xfs/161: adapt the test case for 64k FS blocksize
  2024-05-06 15:01 [PATCH 0/3] more lbs test fixes Pankaj Raghav (Samsung)
@ 2024-05-06 15:01 ` Pankaj Raghav (Samsung)
  2024-05-07 22:23   ` Darrick J. Wong
  2024-05-06 15:01 ` [PATCH 2/3] generic/436: round up bufsz to nearest filesystem blksz Pankaj Raghav (Samsung)
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Pankaj Raghav (Samsung) @ 2024-05-06 15:01 UTC (permalink / raw)
  To: fstests
  Cc: djwong, gost.dev, mcgrof, kernel, ritesh.list, zlang, Pankaj Raghav

From: Pankaj Raghav <p.raghav@samsung.com>

This test fails when xfs is formatted with 64k filesystem block size*.
It fails because the soft quota is not exceeded with the hardcoded 64k
pwrite, thereby, the grace time is not set. Even though soft quota is
set to 12k for uid1, it is rounded up to the nearest blocksize.

*** Report for user quotas on device /dev/sdb3
Block grace time: 7days; Inode grace time: 7days
                        Block limits                File limits
User            used    soft    hard  grace    used  soft  hard  grace
----------------------------------------------------------------------
0        --       0       0       0      0       3     0     0      0
1        --      64      64    1024      0       1     0     0      0
2        --      64       0       0      0       1     0     0      0

Adapt the pwrite to do more than 64k write when the FS blocksize is 64k.

Cap the blksz to be at least 64k to retain the same behaviour as before
for smaller filesystem blocksizes.

* This happens even on a 64k pagesize system and it is not related to
  LBS effort.

Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 tests/xfs/161 | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tests/xfs/161 b/tests/xfs/161
index 486fa6ca..94290f18 100755
--- a/tests/xfs/161
+++ b/tests/xfs/161
@@ -38,9 +38,15 @@ _qmount_option "usrquota"
 _scratch_xfs_db -c 'version' -c 'sb 0' -c 'p' >> $seqres.full
 _scratch_mount >> $seqres.full
 
+min_blksz=65536
+file_blksz=$(_get_file_block_size "$SCRATCH_MNT")
+# Write more than one block to exceed the soft block quota limit.
+blksz=$(( 2 * $file_blksz))
+
+blksz=$(( blksz > min_blksz ? blksz : min_blksz ))
 # Force the block counters for uid 1 and 2 above zero
-_pwrite_byte 0x61 0 64k $SCRATCH_MNT/a >> $seqres.full
-_pwrite_byte 0x61 0 64k $SCRATCH_MNT/b >> $seqres.full
+_pwrite_byte 0x61 0 $blksz $SCRATCH_MNT/a >> $seqres.full
+_pwrite_byte 0x61 0 $blksz $SCRATCH_MNT/b >> $seqres.full
 sync
 chown 1 $SCRATCH_MNT/a
 chown 2 $SCRATCH_MNT/b
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/3] generic/436: round up bufsz to nearest filesystem blksz
  2024-05-06 15:01 [PATCH 0/3] more lbs test fixes Pankaj Raghav (Samsung)
  2024-05-06 15:01 ` [PATCH 1/3] xfs/161: adapt the test case for 64k FS blocksize Pankaj Raghav (Samsung)
@ 2024-05-06 15:01 ` Pankaj Raghav (Samsung)
  2024-05-07 18:10   ` Zorro Lang
  2024-05-07 22:24   ` Darrick J. Wong
  2024-05-06 15:01 ` [PATCH 3/3] xfs/008: use block size instead of the pagesize Pankaj Raghav (Samsung)
  2024-05-11  5:04 ` [PATCH 0/3] more lbs test fixes Zorro Lang
  3 siblings, 2 replies; 16+ messages in thread
From: Pankaj Raghav (Samsung) @ 2024-05-06 15:01 UTC (permalink / raw)
  To: fstests
  Cc: djwong, gost.dev, mcgrof, kernel, ritesh.list, zlang, Pankaj Raghav

From: Pankaj Raghav <p.raghav@samsung.com>

SEEK_HOLE and SEEK_DATA work in filesystem block size granularity. So
while filling up the buffer for test 13 - 16, round up the bufsz to the
closest filesystem blksz.

As we only allowed blocksizes lower than the pagesize, this was never an
issue and it always aligned. Once we have blocksize > pagesize, this
assumption will break.

Fixes the test for LBS configuration.

Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 src/seek_sanity_test.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
index 48b3ccc0..bc30f77c 100644
--- a/src/seek_sanity_test.c
+++ b/src/seek_sanity_test.c
@@ -541,7 +541,7 @@ static int test16(int fd, int testnum)
 {
 	int ret = 0;
 	char *buf = NULL;
-	int bufsz = sysconf(_SC_PAGE_SIZE);
+	int bufsz = roundup(sysconf(_SC_PAGE_SIZE) * 14, alloc_size);
 	int filsz = 4 << 20;
 
 	if (!unwritten_extents) {
@@ -591,7 +591,7 @@ static int test15(int fd, int testnum)
 {
 	int ret = 0;
 	char *buf = NULL;
-	int bufsz = sysconf(_SC_PAGE_SIZE);
+	int bufsz = roundup(sysconf(_SC_PAGE_SIZE) * 14, alloc_size);
 	int filsz = 4 << 20;
 
 	if (!unwritten_extents) {
@@ -643,7 +643,7 @@ static int test14(int fd, int testnum)
 {
 	int ret = 0;
 	char *buf = NULL;
-	int bufsz = sysconf(_SC_PAGE_SIZE) * 14;
+	int bufsz = roundup(sysconf(_SC_PAGE_SIZE) * 14, alloc_size);
 	int filsz = 4 << 20;
 
 	if (!unwritten_extents) {
@@ -692,7 +692,7 @@ static int test13(int fd, int testnum)
 {
 	int ret = 0;
 	char *buf = NULL;
-	int bufsz = sysconf(_SC_PAGE_SIZE) * 14;
+	int bufsz = roundup(sysconf(_SC_PAGE_SIZE) * 14, alloc_size);
 	int filsz = 4 << 20;
 
 	if (!unwritten_extents) {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/3] xfs/008: use block size instead of the pagesize
  2024-05-06 15:01 [PATCH 0/3] more lbs test fixes Pankaj Raghav (Samsung)
  2024-05-06 15:01 ` [PATCH 1/3] xfs/161: adapt the test case for 64k FS blocksize Pankaj Raghav (Samsung)
  2024-05-06 15:01 ` [PATCH 2/3] generic/436: round up bufsz to nearest filesystem blksz Pankaj Raghav (Samsung)
@ 2024-05-06 15:01 ` Pankaj Raghav (Samsung)
  2024-05-07 18:10   ` Zorro Lang
  2024-05-11  5:04 ` [PATCH 0/3] more lbs test fixes Zorro Lang
  3 siblings, 1 reply; 16+ messages in thread
From: Pankaj Raghav (Samsung) @ 2024-05-06 15:01 UTC (permalink / raw)
  To: fstests
  Cc: djwong, gost.dev, mcgrof, kernel, ritesh.list, zlang, Pankaj Raghav

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>
---
 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))
 # 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
 
 # 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] xfs/008: use block size instead of the pagesize
  2024-05-06 15:01 ` [PATCH 3/3] xfs/008: use block size instead of the pagesize Pankaj Raghav (Samsung)
@ 2024-05-07 18:10   ` Zorro Lang
  0 siblings, 0 replies; 16+ messages in thread
From: Zorro Lang @ 2024-05-07 18:10 UTC (permalink / raw)
  To: Pankaj Raghav (Samsung)
  Cc: fstests, djwong, gost.dev, mcgrof, ritesh.list, Pankaj Raghav

On Mon, May 06, 2024 at 05:01:19PM +0200, Pankaj Raghav (Samsung) wrote:
> 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>
> ---

Looks good to me, thanks!

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))
>  # 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
>  
>  # 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
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] generic/436: round up bufsz to nearest filesystem blksz
  2024-05-06 15:01 ` [PATCH 2/3] generic/436: round up bufsz to nearest filesystem blksz Pankaj Raghav (Samsung)
@ 2024-05-07 18:10   ` Zorro Lang
  2024-05-07 22:24   ` Darrick J. Wong
  1 sibling, 0 replies; 16+ messages in thread
From: Zorro Lang @ 2024-05-07 18:10 UTC (permalink / raw)
  To: Pankaj Raghav (Samsung)
  Cc: fstests, djwong, gost.dev, mcgrof, ritesh.list, Pankaj Raghav

On Mon, May 06, 2024 at 05:01:18PM +0200, Pankaj Raghav (Samsung) wrote:
> From: Pankaj Raghav <p.raghav@samsung.com>
> 
> SEEK_HOLE and SEEK_DATA work in filesystem block size granularity. So
> while filling up the buffer for test 13 - 16, round up the bufsz to the
> closest filesystem blksz.
> 
> As we only allowed blocksizes lower than the pagesize, this was never an
> issue and it always aligned. Once we have blocksize > pagesize, this
> assumption will break.
> 
> Fixes the test for LBS configuration.
> 
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---

Makes sense to me,

Reviewed-by: Zorro Lang <zlang@redhat.com>

>  src/seek_sanity_test.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
> index 48b3ccc0..bc30f77c 100644
> --- a/src/seek_sanity_test.c
> +++ b/src/seek_sanity_test.c
> @@ -541,7 +541,7 @@ static int test16(int fd, int testnum)
>  {
>  	int ret = 0;
>  	char *buf = NULL;
> -	int bufsz = sysconf(_SC_PAGE_SIZE);
> +	int bufsz = roundup(sysconf(_SC_PAGE_SIZE) * 14, alloc_size);
>  	int filsz = 4 << 20;
>  
>  	if (!unwritten_extents) {
> @@ -591,7 +591,7 @@ static int test15(int fd, int testnum)
>  {
>  	int ret = 0;
>  	char *buf = NULL;
> -	int bufsz = sysconf(_SC_PAGE_SIZE);
> +	int bufsz = roundup(sysconf(_SC_PAGE_SIZE) * 14, alloc_size);
>  	int filsz = 4 << 20;
>  
>  	if (!unwritten_extents) {
> @@ -643,7 +643,7 @@ static int test14(int fd, int testnum)
>  {
>  	int ret = 0;
>  	char *buf = NULL;
> -	int bufsz = sysconf(_SC_PAGE_SIZE) * 14;
> +	int bufsz = roundup(sysconf(_SC_PAGE_SIZE) * 14, alloc_size);
>  	int filsz = 4 << 20;
>  
>  	if (!unwritten_extents) {
> @@ -692,7 +692,7 @@ static int test13(int fd, int testnum)
>  {
>  	int ret = 0;
>  	char *buf = NULL;
> -	int bufsz = sysconf(_SC_PAGE_SIZE) * 14;
> +	int bufsz = roundup(sysconf(_SC_PAGE_SIZE) * 14, alloc_size);
>  	int filsz = 4 << 20;
>  
>  	if (!unwritten_extents) {
> -- 
> 2.34.1
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] xfs/161: adapt the test case for 64k FS blocksize
  2024-05-06 15:01 ` [PATCH 1/3] xfs/161: adapt the test case for 64k FS blocksize Pankaj Raghav (Samsung)
@ 2024-05-07 22:23   ` Darrick J. Wong
  2024-05-08  2:50     ` Ritesh Harjani
  2024-05-08 10:58     ` Pankaj Raghav (Samsung)
  0 siblings, 2 replies; 16+ messages in thread
From: Darrick J. Wong @ 2024-05-07 22:23 UTC (permalink / raw)
  To: Pankaj Raghav (Samsung)
  Cc: fstests, gost.dev, mcgrof, ritesh.list, zlang, Pankaj Raghav

On Mon, May 06, 2024 at 05:01:17PM +0200, Pankaj Raghav (Samsung) wrote:
> From: Pankaj Raghav <p.raghav@samsung.com>
> 
> This test fails when xfs is formatted with 64k filesystem block size*.
> It fails because the soft quota is not exceeded with the hardcoded 64k
> pwrite, thereby, the grace time is not set. Even though soft quota is
> set to 12k for uid1, it is rounded up to the nearest blocksize.
> 
> *** Report for user quotas on device /dev/sdb3
> Block grace time: 7days; Inode grace time: 7days
>                         Block limits                File limits
> User            used    soft    hard  grace    used  soft  hard  grace
> ----------------------------------------------------------------------
> 0        --       0       0       0      0       3     0     0      0
> 1        --      64      64    1024      0       1     0     0      0
> 2        --      64       0       0      0       1     0     0      0
> 
> Adapt the pwrite to do more than 64k write when the FS blocksize is 64k.
> 
> Cap the blksz to be at least 64k to retain the same behaviour as before
> for smaller filesystem blocksizes.
> 
> * This happens even on a 64k pagesize system and it is not related to
>   LBS effort.
> 
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
>  tests/xfs/161 | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/xfs/161 b/tests/xfs/161
> index 486fa6ca..94290f18 100755
> --- a/tests/xfs/161
> +++ b/tests/xfs/161
> @@ -38,9 +38,15 @@ _qmount_option "usrquota"
>  _scratch_xfs_db -c 'version' -c 'sb 0' -c 'p' >> $seqres.full
>  _scratch_mount >> $seqres.full
>  
> +min_blksz=65536
> +file_blksz=$(_get_file_block_size "$SCRATCH_MNT")
> +# Write more than one block to exceed the soft block quota limit.
> +blksz=$(( 2 * $file_blksz))
> +
> +blksz=$(( blksz > min_blksz ? blksz : min_blksz ))

If we don't set $min_blksize and always write (2 * $file_blksz) does the
test still work?

--D

>  # Force the block counters for uid 1 and 2 above zero
> -_pwrite_byte 0x61 0 64k $SCRATCH_MNT/a >> $seqres.full
> -_pwrite_byte 0x61 0 64k $SCRATCH_MNT/b >> $seqres.full
> +_pwrite_byte 0x61 0 $blksz $SCRATCH_MNT/a >> $seqres.full
> +_pwrite_byte 0x61 0 $blksz $SCRATCH_MNT/b >> $seqres.full
>  sync
>  chown 1 $SCRATCH_MNT/a
>  chown 2 $SCRATCH_MNT/b
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] generic/436: round up bufsz to nearest filesystem blksz
  2024-05-06 15:01 ` [PATCH 2/3] generic/436: round up bufsz to nearest filesystem blksz Pankaj Raghav (Samsung)
  2024-05-07 18:10   ` Zorro Lang
@ 2024-05-07 22:24   ` Darrick J. Wong
  2024-05-08 10:05     ` Pankaj Raghav (Samsung)
  1 sibling, 1 reply; 16+ messages in thread
From: Darrick J. Wong @ 2024-05-07 22:24 UTC (permalink / raw)
  To: Pankaj Raghav (Samsung)
  Cc: fstests, gost.dev, mcgrof, ritesh.list, zlang, Pankaj Raghav

On Mon, May 06, 2024 at 05:01:18PM +0200, Pankaj Raghav (Samsung) wrote:
> From: Pankaj Raghav <p.raghav@samsung.com>
> 
> SEEK_HOLE and SEEK_DATA work in filesystem block size granularity. So
> while filling up the buffer for test 13 - 16, round up the bufsz to the
> closest filesystem blksz.
> 
> As we only allowed blocksizes lower than the pagesize, this was never an
> issue and it always aligned. Once we have blocksize > pagesize, this
> assumption will break.
> 
> Fixes the test for LBS configuration.
> 
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
>  src/seek_sanity_test.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
> index 48b3ccc0..bc30f77c 100644
> --- a/src/seek_sanity_test.c
> +++ b/src/seek_sanity_test.c
> @@ -541,7 +541,7 @@ static int test16(int fd, int testnum)
>  {
>  	int ret = 0;
>  	char *buf = NULL;
> -	int bufsz = sysconf(_SC_PAGE_SIZE);
> +	int bufsz = roundup(sysconf(_SC_PAGE_SIZE) * 14, alloc_size);

Why is the page size multiplied by 14?

--D

>  	int filsz = 4 << 20;
>  
>  	if (!unwritten_extents) {
> @@ -591,7 +591,7 @@ static int test15(int fd, int testnum)
>  {
>  	int ret = 0;
>  	char *buf = NULL;
> -	int bufsz = sysconf(_SC_PAGE_SIZE);
> +	int bufsz = roundup(sysconf(_SC_PAGE_SIZE) * 14, alloc_size);
>  	int filsz = 4 << 20;
>  
>  	if (!unwritten_extents) {
> @@ -643,7 +643,7 @@ static int test14(int fd, int testnum)
>  {
>  	int ret = 0;
>  	char *buf = NULL;
> -	int bufsz = sysconf(_SC_PAGE_SIZE) * 14;
> +	int bufsz = roundup(sysconf(_SC_PAGE_SIZE) * 14, alloc_size);
>  	int filsz = 4 << 20;
>  
>  	if (!unwritten_extents) {
> @@ -692,7 +692,7 @@ static int test13(int fd, int testnum)
>  {
>  	int ret = 0;
>  	char *buf = NULL;
> -	int bufsz = sysconf(_SC_PAGE_SIZE) * 14;
> +	int bufsz = roundup(sysconf(_SC_PAGE_SIZE) * 14, alloc_size);
>  	int filsz = 4 << 20;
>  
>  	if (!unwritten_extents) {
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] xfs/161: adapt the test case for 64k FS blocksize
  2024-05-07 22:23   ` Darrick J. Wong
@ 2024-05-08  2:50     ` Ritesh Harjani
  2024-05-08 16:06       ` Darrick J. Wong
  2024-05-08 10:58     ` Pankaj Raghav (Samsung)
  1 sibling, 1 reply; 16+ messages in thread
From: Ritesh Harjani @ 2024-05-08  2:50 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Pankaj Raghav (Samsung), fstests, gost.dev, mcgrof, zlang, Pankaj Raghav

On Wed, May 8, 2024 at 3:53 AM Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Mon, May 06, 2024 at 05:01:17PM +0200, Pankaj Raghav (Samsung) wrote:
> > From: Pankaj Raghav <p.raghav@samsung.com>
> >
> > This test fails when xfs is formatted with 64k filesystem block size*.
> > It fails because the soft quota is not exceeded with the hardcoded 64k
> > pwrite, thereby, the grace time is not set. Even though soft quota is
> > set to 12k for uid1, it is rounded up to the nearest blocksize.
> >
> > *** Report for user quotas on device /dev/sdb3
> > Block grace time: 7days; Inode grace time: 7days
> >                         Block limits                File limits
> > User            used    soft    hard  grace    used  soft  hard  grace
> > ----------------------------------------------------------------------
> > 0        --       0       0       0      0       3     0     0      0
> > 1        --      64      64    1024      0       1     0     0      0
> > 2        --      64       0       0      0       1     0     0      0
> >
> > Adapt the pwrite to do more than 64k write when the FS blocksize is 64k.
> >
> > Cap the blksz to be at least 64k to retain the same behaviour as before
> > for smaller filesystem blocksizes.
> >
> > * This happens even on a 64k pagesize system and it is not related to
> >   LBS effort.
> >
> > Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> > ---
> >  tests/xfs/161 | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/xfs/161 b/tests/xfs/161
> > index 486fa6ca..94290f18 100755
> > --- a/tests/xfs/161
> > +++ b/tests/xfs/161
> > @@ -38,9 +38,15 @@ _qmount_option "usrquota"
> >  _scratch_xfs_db -c 'version' -c 'sb 0' -c 'p' >> $seqres.full
> >  _scratch_mount >> $seqres.full
> >
> > +min_blksz=65536
> > +file_blksz=$(_get_file_block_size "$SCRATCH_MNT")
> > +# Write more than one block to exceed the soft block quota limit.

Maybe we should improve this comment to -

# Write more than the soft block quota limit of 12k which is set later
via xfs_quota
# On 64k bs this will get rounded to the nearest blocksize which is 64k

> > +blksz=$(( 2 * $file_blksz))
> > +
> > +blksz=$(( blksz > min_blksz ? blksz : min_blksz ))
>
> If we don't set $min_blksize and always write (2 * $file_blksz) does the
> test still work?

I guess it won't (even for bs=4k), because we set the bsoft=12k via xfs_quota.
So we have to write more than 12k to trigger the grace timer.

-ritesh

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] generic/436: round up bufsz to nearest filesystem blksz
  2024-05-07 22:24   ` Darrick J. Wong
@ 2024-05-08 10:05     ` Pankaj Raghav (Samsung)
  0 siblings, 0 replies; 16+ messages in thread
From: Pankaj Raghav (Samsung) @ 2024-05-08 10:05 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: fstests, gost.dev, mcgrof, ritesh.list, zlang, Pankaj Raghav

On Tue, May 07, 2024 at 03:24:07PM -0700, Darrick J. Wong wrote:
> On Mon, May 06, 2024 at 05:01:18PM +0200, Pankaj Raghav (Samsung) wrote:
> > From: Pankaj Raghav <p.raghav@samsung.com>
> > 
> > SEEK_HOLE and SEEK_DATA work in filesystem block size granularity. So
> > while filling up the buffer for test 13 - 16, round up the bufsz to the
> > closest filesystem blksz.
> > 
> > As we only allowed blocksizes lower than the pagesize, this was never an
> > issue and it always aligned. Once we have blocksize > pagesize, this
> > assumption will break.
> > 
> > Fixes the test for LBS configuration.
> > 
> > Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> > ---
> >  src/seek_sanity_test.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
> > index 48b3ccc0..bc30f77c 100644
> > --- a/src/seek_sanity_test.c
> > +++ b/src/seek_sanity_test.c
> > @@ -541,7 +541,7 @@ static int test16(int fd, int testnum)
> >  {
> >  	int ret = 0;
> >  	char *buf = NULL;
> > -	int bufsz = sysconf(_SC_PAGE_SIZE);
> > +	int bufsz = roundup(sysconf(_SC_PAGE_SIZE) * 14, alloc_size);
> 
> Why is the page size multiplied by 14?

Ah, that is a copy pasta error from test 14 and test 15. I will change
it in the next version. Thanks.

> 
> --D
> 
> >  	int filsz = 4 << 20;
> >  
> >  	if (!unwritten_extents) {
> > @@ -591,7 +591,7 @@ static int test15(int fd, int testnum)
> >  {
> >  	int ret = 0;
> >  	char *buf = NULL;
> > -	int bufsz = sysconf(_SC_PAGE_SIZE);
> > +	int bufsz = roundup(sysconf(_SC_PAGE_SIZE) * 14, alloc_size);
> >  	int filsz = 4 << 20;
> >  
> >  	if (!unwritten_extents) {
> > @@ -643,7 +643,7 @@ static int test14(int fd, int testnum)
> >  {
> >  	int ret = 0;
> >  	char *buf = NULL;
> > -	int bufsz = sysconf(_SC_PAGE_SIZE) * 14;
> > +	int bufsz = roundup(sysconf(_SC_PAGE_SIZE) * 14, alloc_size);
> >  	int filsz = 4 << 20;
> >  
> >  	if (!unwritten_extents) {
> > @@ -692,7 +692,7 @@ static int test13(int fd, int testnum)
> >  {
> >  	int ret = 0;
> >  	char *buf = NULL;
> > -	int bufsz = sysconf(_SC_PAGE_SIZE) * 14;
> > +	int bufsz = roundup(sysconf(_SC_PAGE_SIZE) * 14, alloc_size);
> >  	int filsz = 4 << 20;
> >  
> >  	if (!unwritten_extents) {
> > -- 
> > 2.34.1
> > 

-- 
Pankaj Raghav

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] xfs/161: adapt the test case for 64k FS blocksize
  2024-05-07 22:23   ` Darrick J. Wong
  2024-05-08  2:50     ` Ritesh Harjani
@ 2024-05-08 10:58     ` Pankaj Raghav (Samsung)
  2024-05-08 14:49       ` Pankaj Raghav (Samsung)
  2024-05-09 17:33       ` Ritesh Harjani
  1 sibling, 2 replies; 16+ messages in thread
From: Pankaj Raghav (Samsung) @ 2024-05-08 10:58 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: fstests, gost.dev, mcgrof, ritesh.list, zlang, Pankaj Raghav

On Tue, May 07, 2024 at 03:23:23PM -0700, Darrick J. Wong wrote:
> On Mon, May 06, 2024 at 05:01:17PM +0200, Pankaj Raghav (Samsung) wrote:
> > From: Pankaj Raghav <p.raghav@samsung.com>
> > 
> > This test fails when xfs is formatted with 64k filesystem block size*.
> > It fails because the soft quota is not exceeded with the hardcoded 64k
> > pwrite, thereby, the grace time is not set. Even though soft quota is
> > set to 12k for uid1, it is rounded up to the nearest blocksize.
> > 
> > *** Report for user quotas on device /dev/sdb3
> > Block grace time: 7days; Inode grace time: 7days
> >                         Block limits                File limits
> > User            used    soft    hard  grace    used  soft  hard  grace
> > ----------------------------------------------------------------------
> > 0        --       0       0       0      0       3     0     0      0
> > 1        --      64      64    1024      0       1     0     0      0
> > 2        --      64       0       0      0       1     0     0      0
> > 
> > Adapt the pwrite to do more than 64k write when the FS blocksize is 64k.
> > 
> > Cap the blksz to be at least 64k to retain the same behaviour as before
> > for smaller filesystem blocksizes.
> > 
> > * This happens even on a 64k pagesize system and it is not related to
> >   LBS effort.
> > 
> > Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> > ---
> >  tests/xfs/161 | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/xfs/161 b/tests/xfs/161
> > index 486fa6ca..94290f18 100755
> > --- a/tests/xfs/161
> > +++ b/tests/xfs/161
> > @@ -38,9 +38,15 @@ _qmount_option "usrquota"
> >  _scratch_xfs_db -c 'version' -c 'sb 0' -c 'p' >> $seqres.full
> >  _scratch_mount >> $seqres.full
> >  
> > +min_blksz=65536
> > +file_blksz=$(_get_file_block_size "$SCRATCH_MNT")
> > +# Write more than one block to exceed the soft block quota limit.
> > +blksz=$(( 2 * $file_blksz))
> > +
> > +blksz=$(( blksz > min_blksz ? blksz : min_blksz ))
> 
> If we don't set $min_blksize and always write (2 * $file_blksz) does the
> test still work?

I think something like this is more clean where we don't have anymore
hardcoded variables:

Author: Pankaj Raghav <p.raghav@samsung.com>
Date:   Thu Jan 18 18:40:39 2024 +0100

    xfs/161: adapt the test case for 64k FS blocksize
    
    This test fails when xfs is formatted with 64k filesystem block size*.
    It fails because the soft quota is not exceeded with the hardcoded 64k
    pwrite, thereby, the grace time is not set. Even though soft quota is
    set to 12k for uid1, it is rounded up to the nearest blocksize.
    
    *** Report for user quotas on device /dev/sdb3
    Block grace time: 7days; Inode grace time: 7days
                            Block limits                File limits
    User            used    soft    hard  grace    used  soft  hard  grace
    ----------------------------------------------------------------------
    0        --       0       0       0      0       3     0     0      0
    1        --      64      64    1024      0       1     0     0      0
    2        --      64       0       0      0       1     0     0      0
    
    Adapt the pwrite to do twice the FS block size and set the soft limit
    to be 1 FS block.
    
    * This happens even on a 64k pagesize system and it is not related to
      LBS effort.
    
    Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>

diff --git a/tests/xfs/161 b/tests/xfs/161
index 486fa6ca..074acddc 100755
--- a/tests/xfs/161
+++ b/tests/xfs/161
@@ -38,15 +38,21 @@ _qmount_option "usrquota"
 _scratch_xfs_db -c 'version' -c 'sb 0' -c 'p' >> $seqres.full
 _scratch_mount >> $seqres.full
 
+
+pgsize=`$here/src/feature -s`
+file_blksz=$(_get_file_block_size "$SCRATCH_MNT")
+# Write more than one block to exceed the soft block quota limit.
+blksz=$(( 2 * $file_blksz))
+
 # Force the block counters for uid 1 and 2 above zero
-_pwrite_byte 0x61 0 64k $SCRATCH_MNT/a >> $seqres.full
-_pwrite_byte 0x61 0 64k $SCRATCH_MNT/b >> $seqres.full
+_pwrite_byte 0x61 0 $blksz $SCRATCH_MNT/a >> $seqres.full
+_pwrite_byte 0x61 0 $blksz $SCRATCH_MNT/b >> $seqres.full
 sync
 chown 1 $SCRATCH_MNT/a
 chown 2 $SCRATCH_MNT/b
 
 # Set quota limits on uid 1 before upgrading
-$XFS_QUOTA_PROG -x -c 'limit -u bsoft=12k bhard=1m 1' $SCRATCH_MNT
+$XFS_QUOTA_PROG -x -c 'limit -u bsoft='"$file_blksz"' bhard=1m 1' $SCRATCH_MNT
 
 # Make sure the grace period is at /some/ point in the future.  We have to
 # use bc because not all bashes can handle integer comparisons with 64-bit
@@ -71,7 +77,7 @@ _scratch_mount
 
 # Set a very generous grace period and quota limits on uid 2 after upgrading
 $XFS_QUOTA_PROG -x -c 'timer -u -b -d 2147483647' $SCRATCH_MNT
-$XFS_QUOTA_PROG -x -c 'limit -u bsoft=10000 bhard=150000 2' $SCRATCH_MNT
+$XFS_QUOTA_PROG -x -c 'limit -u bsoft='"$file_blksz"' bhard=150000 2' $SCRATCH_MNT
 
 # Query the grace periods to see if they got set properly after the upgrade.
 repquota -upn $SCRATCH_MNT > $tmp.repquota

> 
> --D
> 
> >  # Force the block counters for uid 1 and 2 above zero
> > -_pwrite_byte 0x61 0 64k $SCRATCH_MNT/a >> $seqres.full
> > -_pwrite_byte 0x61 0 64k $SCRATCH_MNT/b >> $seqres.full
> > +_pwrite_byte 0x61 0 $blksz $SCRATCH_MNT/a >> $seqres.full
> > +_pwrite_byte 0x61 0 $blksz $SCRATCH_MNT/b >> $seqres.full
> >  sync
> >  chown 1 $SCRATCH_MNT/a
> >  chown 2 $SCRATCH_MNT/b
> > -- 
> > 2.34.1
> > 

-- 
Pankaj Raghav

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] xfs/161: adapt the test case for 64k FS blocksize
  2024-05-08 10:58     ` Pankaj Raghav (Samsung)
@ 2024-05-08 14:49       ` Pankaj Raghav (Samsung)
  2024-05-09 17:33       ` Ritesh Harjani
  1 sibling, 0 replies; 16+ messages in thread
From: Pankaj Raghav (Samsung) @ 2024-05-08 14:49 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: fstests, gost.dev, mcgrof, ritesh.list, zlang, Pankaj Raghav

> 
> diff --git a/tests/xfs/161 b/tests/xfs/161
> index 486fa6ca..074acddc 100755
> --- a/tests/xfs/161
> +++ b/tests/xfs/161
> @@ -38,15 +38,21 @@ _qmount_option "usrquota"
>  _scratch_xfs_db -c 'version' -c 'sb 0' -c 'p' >> $seqres.full
>  _scratch_mount >> $seqres.full
>  
> +
> +pgsize=`$here/src/feature -s`
Stale pgsize variable.

> +file_blksz=$(_get_file_block_size "$SCRATCH_MNT")
> +# Write more than one block to exceed the soft block quota limit.
> +blksz=$(( 2 * $file_blksz))
> +
>  # Force the block counters for uid 1 and 2 above zero
> -_pwrite_byte 0x61 0 64k $SCRATCH_MNT/a >> $seqres.full
> -_pwrite_byte 0x61 0 64k $SCRATCH_MNT/b >> $seqres.full
> +_pwrite_byte 0x61 0 $blksz $SCRATCH_MNT/a >> $seqres.full
> +_pwrite_byte 0x61 0 $blksz $SCRATCH_MNT/b >> $seqres.full
>  sync
>  chown 1 $SCRATCH_MNT/a
>  chown 2 $SCRATCH_MNT/b
>  
>  # Set quota limits on uid 1 before upgrading
> -$XFS_QUOTA_PROG -x -c 'limit -u bsoft=12k bhard=1m 1' $SCRATCH_MNT
> +$XFS_QUOTA_PROG -x -c 'limit -u bsoft='"$file_blksz"' bhard=1m 1' $SCRATCH_MNT
>  
>  # Make sure the grace period is at /some/ point in the future.  We have to
>  # use bc because not all bashes can handle integer comparisons with 64-bit
> @@ -71,7 +77,7 @@ _scratch_mount
>  
>  # Set a very generous grace period and quota limits on uid 2 after upgrading
>  $XFS_QUOTA_PROG -x -c 'timer -u -b -d 2147483647' $SCRATCH_MNT
> -$XFS_QUOTA_PROG -x -c 'limit -u bsoft=10000 bhard=150000 2' $SCRATCH_MNT
> +$XFS_QUOTA_PROG -x -c 'limit -u bsoft='"$file_blksz"' bhard=150000 2' $SCRATCH_MNT
>  
>  # Query the grace periods to see if they got set properly after the upgrade.
>  repquota -upn $SCRATCH_MNT > $tmp.repquota
> 
> > 
> > --D
> > 
> > >  # Force the block counters for uid 1 and 2 above zero
> > > -_pwrite_byte 0x61 0 64k $SCRATCH_MNT/a >> $seqres.full
> > > -_pwrite_byte 0x61 0 64k $SCRATCH_MNT/b >> $seqres.full
> > > +_pwrite_byte 0x61 0 $blksz $SCRATCH_MNT/a >> $seqres.full
> > > +_pwrite_byte 0x61 0 $blksz $SCRATCH_MNT/b >> $seqres.full
> > >  sync
> > >  chown 1 $SCRATCH_MNT/a
> > >  chown 2 $SCRATCH_MNT/b
> > > -- 
> > > 2.34.1
> > > 
> 
> -- 
> Pankaj Raghav

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] xfs/161: adapt the test case for 64k FS blocksize
  2024-05-08  2:50     ` Ritesh Harjani
@ 2024-05-08 16:06       ` Darrick J. Wong
  2024-05-09 13:01         ` Pankaj Raghav (Samsung)
  0 siblings, 1 reply; 16+ messages in thread
From: Darrick J. Wong @ 2024-05-08 16:06 UTC (permalink / raw)
  To: Ritesh Harjani
  Cc: Pankaj Raghav (Samsung), fstests, gost.dev, mcgrof, zlang, Pankaj Raghav

On Wed, May 08, 2024 at 08:20:01AM +0530, Ritesh Harjani wrote:
> On Wed, May 8, 2024 at 3:53 AM Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > On Mon, May 06, 2024 at 05:01:17PM +0200, Pankaj Raghav (Samsung) wrote:
> > > From: Pankaj Raghav <p.raghav@samsung.com>
> > >
> > > This test fails when xfs is formatted with 64k filesystem block size*.
> > > It fails because the soft quota is not exceeded with the hardcoded 64k
> > > pwrite, thereby, the grace time is not set. Even though soft quota is
> > > set to 12k for uid1, it is rounded up to the nearest blocksize.
> > >
> > > *** Report for user quotas on device /dev/sdb3
> > > Block grace time: 7days; Inode grace time: 7days
> > >                         Block limits                File limits
> > > User            used    soft    hard  grace    used  soft  hard  grace
> > > ----------------------------------------------------------------------
> > > 0        --       0       0       0      0       3     0     0      0
> > > 1        --      64      64    1024      0       1     0     0      0
> > > 2        --      64       0       0      0       1     0     0      0
> > >
> > > Adapt the pwrite to do more than 64k write when the FS blocksize is 64k.
> > >
> > > Cap the blksz to be at least 64k to retain the same behaviour as before
> > > for smaller filesystem blocksizes.
> > >
> > > * This happens even on a 64k pagesize system and it is not related to
> > >   LBS effort.
> > >
> > > Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> > > ---
> > >  tests/xfs/161 | 10 ++++++++--
> > >  1 file changed, 8 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/tests/xfs/161 b/tests/xfs/161
> > > index 486fa6ca..94290f18 100755
> > > --- a/tests/xfs/161
> > > +++ b/tests/xfs/161
> > > @@ -38,9 +38,15 @@ _qmount_option "usrquota"
> > >  _scratch_xfs_db -c 'version' -c 'sb 0' -c 'p' >> $seqres.full
> > >  _scratch_mount >> $seqres.full
> > >
> > > +min_blksz=65536
> > > +file_blksz=$(_get_file_block_size "$SCRATCH_MNT")
> > > +# Write more than one block to exceed the soft block quota limit.
> 
> Maybe we should improve this comment to -
> 
> # Write more than the soft block quota limit of 12k which is set later
> via xfs_quota
> # On 64k bs this will get rounded to the nearest blocksize which is 64k
> 
> > > +blksz=$(( 2 * $file_blksz))
> > > +
> > > +blksz=$(( blksz > min_blksz ? blksz : min_blksz ))
> >
> > If we don't set $min_blksize and always write (2 * $file_blksz) does the
> > test still work?
> 
> I guess it won't (even for bs=4k), because we set the bsoft=12k via xfs_quota.
> So we have to write more than 12k to trigger the grace timer.

Ah, ok.  Yes, that makes sense with the improved comment. :)

--D

> -ritesh
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] xfs/161: adapt the test case for 64k FS blocksize
  2024-05-08 16:06       ` Darrick J. Wong
@ 2024-05-09 13:01         ` Pankaj Raghav (Samsung)
  0 siblings, 0 replies; 16+ messages in thread
From: Pankaj Raghav (Samsung) @ 2024-05-09 13:01 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Ritesh Harjani, fstests, gost.dev, mcgrof, zlang, Pankaj Raghav

On Wed, May 08, 2024 at 09:06:27AM -0700, Darrick J. Wong wrote:
> On Wed, May 08, 2024 at 08:20:01AM +0530, Ritesh Harjani wrote:
> > On Wed, May 8, 2024 at 3:53 AM Darrick J. Wong <djwong@kernel.org> wrote:
> > >
> > > On Mon, May 06, 2024 at 05:01:17PM +0200, Pankaj Raghav (Samsung) wrote:
> > > > From: Pankaj Raghav <p.raghav@samsung.com>
> > > >
> > > > This test fails when xfs is formatted with 64k filesystem block size*.
> > > > It fails because the soft quota is not exceeded with the hardcoded 64k
> > > > pwrite, thereby, the grace time is not set. Even though soft quota is
> > > > set to 12k for uid1, it is rounded up to the nearest blocksize.
> > > >
> > > > *** Report for user quotas on device /dev/sdb3
> > > > Block grace time: 7days; Inode grace time: 7days
> > > >                         Block limits                File limits
> > > > User            used    soft    hard  grace    used  soft  hard  grace
> > > > ----------------------------------------------------------------------
> > > > 0        --       0       0       0      0       3     0     0      0
> > > > 1        --      64      64    1024      0       1     0     0      0
> > > > 2        --      64       0       0      0       1     0     0      0
> > > >
> > > > Adapt the pwrite to do more than 64k write when the FS blocksize is 64k.
> > > >
> > > > Cap the blksz to be at least 64k to retain the same behaviour as before
> > > > for smaller filesystem blocksizes.
> > > >
> > > > * This happens even on a 64k pagesize system and it is not related to
> > > >   LBS effort.
> > > >
> > > > Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> > > > ---
> > > >  tests/xfs/161 | 10 ++++++++--
> > > >  1 file changed, 8 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/tests/xfs/161 b/tests/xfs/161
> > > > index 486fa6ca..94290f18 100755
> > > > --- a/tests/xfs/161
> > > > +++ b/tests/xfs/161
> > > > @@ -38,9 +38,15 @@ _qmount_option "usrquota"
> > > >  _scratch_xfs_db -c 'version' -c 'sb 0' -c 'p' >> $seqres.full
> > > >  _scratch_mount >> $seqres.full
> > > >
> > > > +min_blksz=65536
> > > > +file_blksz=$(_get_file_block_size "$SCRATCH_MNT")
> > > > +# Write more than one block to exceed the soft block quota limit.
> > 
> > Maybe we should improve this comment to -
> > 
> > # Write more than the soft block quota limit of 12k which is set later
> > via xfs_quota
> > # On 64k bs this will get rounded to the nearest blocksize which is 64k
> > 
> > > > +blksz=$(( 2 * $file_blksz))
> > > > +
> > > > +blksz=$(( blksz > min_blksz ? blksz : min_blksz ))
> > >
> > > If we don't set $min_blksize and always write (2 * $file_blksz) does the
> > > test still work?
> > 
> > I guess it won't (even for bs=4k), because we set the bsoft=12k via xfs_quota.
> > So we have to write more than 12k to trigger the grace timer.
> 
> Ah, ok.  Yes, that makes sense with the improved comment. :)

@ritesh and @darrick: I think we can also change the bsoft right? In
the follow up email [1], I made pwrite to 2 fs block and bsoft to be 1
fsblock.


That looks like a cleaner solution than having hardcoded values. Let me
know what you all think.


[1] https://lore.kernel.org/fstests/20240508105852.nfjtlp53v24xb3tw@quentin/
> 
> --D
> 
> > -ritesh
> > 

-- 
Pankaj Raghav

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] xfs/161: adapt the test case for 64k FS blocksize
  2024-05-08 10:58     ` Pankaj Raghav (Samsung)
  2024-05-08 14:49       ` Pankaj Raghav (Samsung)
@ 2024-05-09 17:33       ` Ritesh Harjani
  1 sibling, 0 replies; 16+ messages in thread
From: Ritesh Harjani @ 2024-05-09 17:33 UTC (permalink / raw)
  To: Pankaj Raghav (Samsung), Darrick J. Wong
  Cc: fstests, gost.dev, mcgrof, zlang, Pankaj Raghav

"Pankaj Raghav (Samsung)" <kernel@pankajraghav.com> writes:

> On Tue, May 07, 2024 at 03:23:23PM -0700, Darrick J. Wong wrote:
>> On Mon, May 06, 2024 at 05:01:17PM +0200, Pankaj Raghav (Samsung) wrote:
>> > From: Pankaj Raghav <p.raghav@samsung.com>
>> > 
>> > This test fails when xfs is formatted with 64k filesystem block size*.
>> > It fails because the soft quota is not exceeded with the hardcoded 64k
>> > pwrite, thereby, the grace time is not set. Even though soft quota is
>> > set to 12k for uid1, it is rounded up to the nearest blocksize.
>> > 
>> > *** Report for user quotas on device /dev/sdb3
>> > Block grace time: 7days; Inode grace time: 7days
>> >                         Block limits                File limits
>> > User            used    soft    hard  grace    used  soft  hard  grace
>> > ----------------------------------------------------------------------
>> > 0        --       0       0       0      0       3     0     0      0
>> > 1        --      64      64    1024      0       1     0     0      0
>> > 2        --      64       0       0      0       1     0     0      0
>> > 
>> > Adapt the pwrite to do more than 64k write when the FS blocksize is 64k.
>> > 
>> > Cap the blksz to be at least 64k to retain the same behaviour as before
>> > for smaller filesystem blocksizes.
>> > 
>> > * This happens even on a 64k pagesize system and it is not related to
>> >   LBS effort.
>> > 
>> > Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
>> > ---
>> >  tests/xfs/161 | 10 ++++++++--
>> >  1 file changed, 8 insertions(+), 2 deletions(-)
>> > 
>> > diff --git a/tests/xfs/161 b/tests/xfs/161
>> > index 486fa6ca..94290f18 100755
>> > --- a/tests/xfs/161
>> > +++ b/tests/xfs/161
>> > @@ -38,9 +38,15 @@ _qmount_option "usrquota"
>> >  _scratch_xfs_db -c 'version' -c 'sb 0' -c 'p' >> $seqres.full
>> >  _scratch_mount >> $seqres.full
>> >  
>> > +min_blksz=65536
>> > +file_blksz=$(_get_file_block_size "$SCRATCH_MNT")
>> > +# Write more than one block to exceed the soft block quota limit.
>> > +blksz=$(( 2 * $file_blksz))
>> > +
>> > +blksz=$(( blksz > min_blksz ? blksz : min_blksz ))
>> 
>> If we don't set $min_blksize and always write (2 * $file_blksz) does the
>> test still work?
>
> I think something like this is more clean where we don't have anymore
> hardcoded variables:
>

Yes, why not.

> Author: Pankaj Raghav <p.raghav@samsung.com>
> Date:   Thu Jan 18 18:40:39 2024 +0100
>
>     xfs/161: adapt the test case for 64k FS blocksize
>     
>     This test fails when xfs is formatted with 64k filesystem block size*.
>     It fails because the soft quota is not exceeded with the hardcoded 64k
>     pwrite, thereby, the grace time is not set. Even though soft quota is
>     set to 12k for uid1, it is rounded up to the nearest blocksize.
>     
>     *** Report for user quotas on device /dev/sdb3
>     Block grace time: 7days; Inode grace time: 7days
>                             Block limits                File limits
>     User            used    soft    hard  grace    used  soft  hard  grace
>     ----------------------------------------------------------------------
>     0        --       0       0       0      0       3     0     0      0
>     1        --      64      64    1024      0       1     0     0      0
>     2        --      64       0       0      0       1     0     0      0
>     
>     Adapt the pwrite to do twice the FS block size and set the soft limit
>     to be 1 FS block.
>     
>     * This happens even on a 64k pagesize system and it is not related to
>       LBS effort.
>     
>     Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
>
> diff --git a/tests/xfs/161 b/tests/xfs/161
> index 486fa6ca..074acddc 100755
> --- a/tests/xfs/161
> +++ b/tests/xfs/161
> @@ -38,15 +38,21 @@ _qmount_option "usrquota"
>  _scratch_xfs_db -c 'version' -c 'sb 0' -c 'p' >> $seqres.full
>  _scratch_mount >> $seqres.full
>  
> +
> +pgsize=`$here/src/feature -s`
> +file_blksz=$(_get_file_block_size "$SCRATCH_MNT")

small bit: maybe just $blksz is fine.

> +# Write more than one block to exceed the soft block quota limit.
> +blksz=$(( 2 * $file_blksz))

small nit: Instead of blksz here maybe writesz or filesz then?

Then let's make:
lim_bsoft=$blksz, lim_bhard=$(( 10 * blksz )) and writesz=$(( 2 * blksz ))


> +
>  # Force the block counters for uid 1 and 2 above zero
> -_pwrite_byte 0x61 0 64k $SCRATCH_MNT/a >> $seqres.full
> -_pwrite_byte 0x61 0 64k $SCRATCH_MNT/b >> $seqres.full
> +_pwrite_byte 0x61 0 $blksz $SCRATCH_MNT/a >> $seqres.full
> +_pwrite_byte 0x61 0 $blksz $SCRATCH_MNT/b >> $seqres.full
>  sync
>  chown 1 $SCRATCH_MNT/a
>  chown 2 $SCRATCH_MNT/b
>  
>  # Set quota limits on uid 1 before upgrading
> -$XFS_QUOTA_PROG -x -c 'limit -u bsoft=12k bhard=1m 1' $SCRATCH_MNT
> +$XFS_QUOTA_PROG -x -c 'limit -u bsoft='"$file_blksz"' bhard=1m 1' $SCRATCH_MNT

Then we can use $lim_bsoft and $lim_bhard here.

>  
>  # Make sure the grace period is at /some/ point in the future.  We have to
>  # use bc because not all bashes can handle integer comparisons with 64-bit
> @@ -71,7 +77,7 @@ _scratch_mount
>  
>  # Set a very generous grace period and quota limits on uid 2 after upgrading
>  $XFS_QUOTA_PROG -x -c 'timer -u -b -d 2147483647' $SCRATCH_MNT
> -$XFS_QUOTA_PROG -x -c 'limit -u bsoft=10000 bhard=150000 2' $SCRATCH_MNT
> +$XFS_QUOTA_PROG -x -c 'limit -u bsoft='"$file_blksz"' bhard=150000 2' $SCRATCH_MNT
>  
>  # Query the grace periods to see if they got set properly after the upgrade.
>  repquota -upn $SCRATCH_MNT > $tmp.repquota
>


-ritesh

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/3] more lbs test fixes
  2024-05-06 15:01 [PATCH 0/3] more lbs test fixes Pankaj Raghav (Samsung)
                   ` (2 preceding siblings ...)
  2024-05-06 15:01 ` [PATCH 3/3] xfs/008: use block size instead of the pagesize Pankaj Raghav (Samsung)
@ 2024-05-11  5:04 ` Zorro Lang
  3 siblings, 0 replies; 16+ messages in thread
From: Zorro Lang @ 2024-05-11  5:04 UTC (permalink / raw)
  To: Pankaj Raghav (Samsung)
  Cc: fstests, djwong, gost.dev, mcgrof, ritesh.list, Pankaj Raghav

Hi Pankaj,

Although this patchset got RVB, but looks like you still want to
change it a bit, so I won't merge this patchset this week, will
wait for your next version :)

Thanks,
Zorro

On Mon, May 06, 2024 at 05:01:16PM +0200, Pankaj Raghav (Samsung) wrote:
> From: Pankaj Raghav <p.raghav@samsung.com>
> 
> These are some of the tests that failed when we were working on LBS
> patches [1].
> 
> generic/436 and xfs/008 are adapted to work with bs > ps systems.
> 
> xfs/161 is an upstream failure, and it happens even on bs <= ps systems.
> 
> [1] https://lore.kernel.org/linux-xfs/20240503095353.3798063-1-mcgrof@kernel.org/
> 
> Pankaj Raghav (3):
>   xfs/161: adapt the test case for 64k FS blocksize
>   generic/436: round up bufsz to nearest filesystem blksz
>   xfs/008: use block size instead of the pagesize
> 
>  src/seek_sanity_test.c |  8 ++++----
>  tests/xfs/008          | 19 ++++++++++---------
>  tests/xfs/008.out      |  8 ++++----
>  tests/xfs/161          | 10 ++++++++--
>  4 files changed, 26 insertions(+), 19 deletions(-)
> 
> 
> base-commit: b26d68da08e47e6508a96bee72b25823040ab67e
> -- 
> 2.34.1
> 
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2024-05-11  5:04 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-06 15:01 [PATCH 0/3] more lbs test fixes Pankaj Raghav (Samsung)
2024-05-06 15:01 ` [PATCH 1/3] xfs/161: adapt the test case for 64k FS blocksize Pankaj Raghav (Samsung)
2024-05-07 22:23   ` Darrick J. Wong
2024-05-08  2:50     ` Ritesh Harjani
2024-05-08 16:06       ` Darrick J. Wong
2024-05-09 13:01         ` Pankaj Raghav (Samsung)
2024-05-08 10:58     ` Pankaj Raghav (Samsung)
2024-05-08 14:49       ` Pankaj Raghav (Samsung)
2024-05-09 17:33       ` Ritesh Harjani
2024-05-06 15:01 ` [PATCH 2/3] generic/436: round up bufsz to nearest filesystem blksz Pankaj Raghav (Samsung)
2024-05-07 18:10   ` Zorro Lang
2024-05-07 22:24   ` Darrick J. Wong
2024-05-08 10:05     ` Pankaj Raghav (Samsung)
2024-05-06 15:01 ` [PATCH 3/3] xfs/008: use block size instead of the pagesize Pankaj Raghav (Samsung)
2024-05-07 18:10   ` Zorro Lang
2024-05-11  5:04 ` [PATCH 0/3] more lbs test fixes Zorro Lang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).