public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] btrfs: Misc test fixes for large block/node sizes
@ 2025-08-19 12:00 Nirjhar Roy (IBM)
  2025-08-19 12:00 ` [PATCH v2 1/4] btrfs/301: Make the test compatible with all the supported block sizes Nirjhar Roy (IBM)
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-08-19 12:00 UTC (permalink / raw)
  To: fstests
  Cc: linux-btrfs, ritesh.list, ojaswin, djwong, zlang, fdmanana,
	nirjhar.roy.lists, quwenruo.btrfs

Some of the btrfs and generic tests are written with 4k block/node size in mind.
This patch fixes some of those tests to be compatible with other block/node sizes too.
We caught these bugs while running the auto group tests on btrfs with large
block/node sizes.

[v1] -> v2:
1. Removed the patch for btrfs/200 of [v1] - need more analysis on this.
2. Removed the first 2 patches of [v1] which introduced 2 new helper functions
3. btrfs/{137,301} and generic/274 - Instead of scaling the test dynamically
   based on the underlying disk block size, I have hardcoded the pwrite blocksizes
   and offsets to 64k which is aligned to all underlying fs block sizes <= 64.
4. For generic/563 - Doubled the iosize instead of btrfs specific hack to cover
   for btrfs write ranges.
5. Updated the commit messages

[v1] - https://lore.kernel.org/all/cover.1753769382.git.nirjhar.roy.lists@gmail.com/

Nirjhar Roy (IBM) (4):
  btrfs/301: Make the test compatible with all the supported block sizes
  generic/274: Make the pwrite block sizes and offsets to 64k
  btrfs/137: Make this test compatible with all supported block sizes
  generic/563: Increase the iosize to to cover for btrfs

 tests/btrfs/137     | 11 ++++----
 tests/btrfs/137.out | 66 ++++++++++++++++++++++-----------------------
 tests/btrfs/301     |  2 +-
 tests/generic/274   | 16 +++++------
 tests/generic/563   |  2 +-
 5 files changed, 49 insertions(+), 48 deletions(-)

--
2.34.1


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

* [PATCH v2 1/4] btrfs/301: Make the test compatible with all the supported block sizes
  2025-08-19 12:00 [PATCH v2 0/4] btrfs: Misc test fixes for large block/node sizes Nirjhar Roy (IBM)
@ 2025-08-19 12:00 ` Nirjhar Roy (IBM)
  2025-08-19 21:59   ` Qu Wenruo
  2025-08-19 12:00 ` [PATCH v2 2/4] generic/274: Make the pwrite block sizes and offsets to 64k Nirjhar Roy (IBM)
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-08-19 12:00 UTC (permalink / raw)
  To: fstests
  Cc: linux-btrfs, ritesh.list, ojaswin, djwong, zlang, fdmanana,
	nirjhar.roy.lists, quwenruo.btrfs

With large block sizes like 64k the test failed with the
following logs:

     QA output created by 301
     basic accounting
    +subvol 256 mismatched usage 33947648 vs 4587520 (expected data 4194304 expected meta 393216 diff 29360128)
    +subvol 256 mismatched usage 168165376 vs 138805248 (expected data 138412032 expected meta 393216 diff 29360128)
    +subvol 256 mismatched usage 33947648 vs 4587520 (expected data 4194304 expected meta 393216 diff 29360128)
    +subvol 256 mismatched usage 33947648 vs 4587520 (expected data 4194304 expected meta 393216 diff 29360128)
     fallocate: Disk quota exceeded

The test creates nr_fill files each of size 8k. Now with 64k
block size, 8k sized files occupy more than the expected sizes (i.e, 8k)
due to internal fragmentation, since 1 file will occupy at least 1
fsblock. Fix this by making the file size 64k, which is aligned
with all the supported block sizes.

Reported-by: Disha Goel <disgoel@linux.ibm.com>
Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
---
 tests/btrfs/301 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/btrfs/301 b/tests/btrfs/301
index 6b59749d..be346f52 100755
--- a/tests/btrfs/301
+++ b/tests/btrfs/301
@@ -23,7 +23,7 @@ subv=$SCRATCH_MNT/subv
 nested=$SCRATCH_MNT/subv/nested
 snap=$SCRATCH_MNT/snap
 nr_fill=512
-fill_sz=$((8 * 1024))
+fill_sz=$((64 * 1024))
 total_fill=$(($nr_fill * $fill_sz))
 nodesize=$($BTRFS_UTIL_PROG inspect-internal dump-super $SCRATCH_DEV | \
 					grep nodesize | $AWK_PROG '{print $2}')
-- 
2.34.1


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

* [PATCH v2 2/4] generic/274: Make the pwrite block sizes and offsets to 64k
  2025-08-19 12:00 [PATCH v2 0/4] btrfs: Misc test fixes for large block/node sizes Nirjhar Roy (IBM)
  2025-08-19 12:00 ` [PATCH v2 1/4] btrfs/301: Make the test compatible with all the supported block sizes Nirjhar Roy (IBM)
@ 2025-08-19 12:00 ` Nirjhar Roy (IBM)
  2025-08-19 22:09   ` Qu Wenruo
  2025-08-19 12:00 ` [PATCH v2 3/4] btrfs/137: Make this test compatible with all supported block sizes Nirjhar Roy (IBM)
  2025-08-19 12:00 ` [PATCH v2 4/4] generic/563: Increase the iosize to to cover for btrfs Nirjhar Roy (IBM)
  3 siblings, 1 reply; 13+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-08-19 12:00 UTC (permalink / raw)
  To: fstests
  Cc: linux-btrfs, ritesh.list, ojaswin, djwong, zlang, fdmanana,
	nirjhar.roy.lists, quwenruo.btrfs

This test was written with 4k block size in mind and it fails with
64k block size when tested with btrfs.
The test first does pre-allocation, then fills up the
filesystem. After that it tries to fragment and fill holes at offsets
of 4k(i.e, 1 fsblock) - which works fine with 4k block size, but with
64k block size, the test tries to fragment and fill holes within
1 fsblock(of size 64k). This results in overwrite of 64k fsblocks
and the write fails. The reason for this failure is that during
overwrite, there is no more space available for COW.
Fix this by changing the pwrite block size and offsets to 64k
so that the test never tries to punch holes or overwrite within 1 fsblock
and the test becomes compatible with all block sizes.

For non-COW filesystems/files, this test should work even if the
underlying filesytem block size > 64k.

Reported-by: Disha Goel <disgoel@linux.ibm.com>
Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
---
 tests/generic/274 | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tests/generic/274 b/tests/generic/274
index 916c7173..47c5ef5b 100755
--- a/tests/generic/274
+++ b/tests/generic/274
@@ -40,20 +40,20 @@ _scratch_unmount 2>/dev/null
 _scratch_mkfs_sized $((2 * 1024 * 1024 * 1024)) >>$seqres.full 2>&1
 _scratch_mount
 
-# Create a 4k file and Allocate 4M past EOF on that file
-$XFS_IO_PROG -f -c "pwrite 0 4k" -c "falloc -k 4k 4m" $SCRATCH_MNT/test \
+# Create a 64k file and Allocate 64M past EOF on that file
+$XFS_IO_PROG -f -c "pwrite 0 64k" -c "falloc -k 64k 64m" $SCRATCH_MNT/test \
 	>>$seqres.full 2>&1 || _fail "failed to create test file"
 
 # Fill the rest of the fs completely
 # Note, this will show ENOSPC errors in $seqres.full, that's ok.
 echo "Fill fs with 1M IOs; ENOSPC expected" >> $seqres.full
 dd if=/dev/zero of=$SCRATCH_MNT/tmp1 bs=1M >>$seqres.full 2>&1
-echo "Fill fs with 4K IOs; ENOSPC expected" >> $seqres.full
-dd if=/dev/zero of=$SCRATCH_MNT/tmp2 bs=4K >>$seqres.full 2>&1
+echo "Fill fs with 64K IOs; ENOSPC expected" >> $seqres.full
+dd if=/dev/zero of=$SCRATCH_MNT/tmp2 bs=64K >>$seqres.full 2>&1
 _scratch_sync
 # Last effort, use O_SYNC
-echo "Fill fs with 4K DIOs; ENOSPC expected" >> $seqres.full
-dd if=/dev/zero of=$SCRATCH_MNT/tmp3 bs=4K oflag=sync >>$seqres.full 2>&1
+echo "Fill fs with 64K DIOs; ENOSPC expected" >> $seqres.full
+dd if=/dev/zero of=$SCRATCH_MNT/tmp3 bs=64K oflag=sync >>$seqres.full 2>&1
 # Save space usage info
 echo "Post-fill space:" >> $seqres.full
 df $SCRATCH_MNT >>$seqres.full 2>&1
@@ -63,7 +63,7 @@ df $SCRATCH_MNT >>$seqres.full 2>&1
 echo "Fill in prealloc space; fragment at offsets:" >> $seqres.full
 for i in `seq 1 2 1023`; do
 	echo -n "$i " >> $seqres.full
-	dd if=/dev/zero of=$SCRATCH_MNT/test seek=$i bs=4K count=1 conv=notrunc \
+	dd if=/dev/zero of=$SCRATCH_MNT/test seek=$i bs=64K count=1 conv=notrunc \
 		>>$seqres.full 2>/dev/null || _fail "failed to write to test file"
 done
 _scratch_sync
@@ -71,7 +71,7 @@ echo >> $seqres.full
 echo "Fill in prealloc space; fill holes at offsets:" >> $seqres.full
 for i in `seq 2 2 1023`; do
 	echo -n "$i " >> $seqres.full
-	dd if=/dev/zero of=$SCRATCH_MNT/test seek=$i bs=4K count=1 conv=notrunc \
+	dd if=/dev/zero of=$SCRATCH_MNT/test seek=$i bs=64K count=1 conv=notrunc \
 		>>$seqres.full 2>/dev/null || _fail "failed to fill test file"
 done
 _scratch_sync
-- 
2.34.1


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

* [PATCH v2 3/4] btrfs/137: Make this test compatible with all supported block sizes
  2025-08-19 12:00 [PATCH v2 0/4] btrfs: Misc test fixes for large block/node sizes Nirjhar Roy (IBM)
  2025-08-19 12:00 ` [PATCH v2 1/4] btrfs/301: Make the test compatible with all the supported block sizes Nirjhar Roy (IBM)
  2025-08-19 12:00 ` [PATCH v2 2/4] generic/274: Make the pwrite block sizes and offsets to 64k Nirjhar Roy (IBM)
@ 2025-08-19 12:00 ` Nirjhar Roy (IBM)
  2025-08-19 22:10   ` Qu Wenruo
  2025-08-19 12:00 ` [PATCH v2 4/4] generic/563: Increase the iosize to to cover for btrfs Nirjhar Roy (IBM)
  3 siblings, 1 reply; 13+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-08-19 12:00 UTC (permalink / raw)
  To: fstests
  Cc: linux-btrfs, ritesh.list, ojaswin, djwong, zlang, fdmanana,
	nirjhar.roy.lists, quwenruo.btrfs

For large block sizes like 64k it failed simply because this
test was written with 4k block size in mind.
The first few lines of the error logs are as follows:

     d3dc847171f9081bd75d7a2d3b53d322  SCRATCH_MNT/snap2/bar

     File snap1/foo fiemap results in the original filesystem:
    -0: [0..7]: data
    +0: [0..127]: data

     File snap1/bar fiemap results in the original filesystem:
    ...

Fix this by making the test choose offsets and block size as 64k
which is aligned with all the underlying supported fs block sizes.

Reported-by: Disha Goel <disgoel@linux.ibm.com>
Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
---
 tests/btrfs/137     | 11 ++++----
 tests/btrfs/137.out | 66 ++++++++++++++++++++++-----------------------
 2 files changed, 39 insertions(+), 38 deletions(-)

diff --git a/tests/btrfs/137 b/tests/btrfs/137
index 7710dc18..c1d498bd 100755
--- a/tests/btrfs/137
+++ b/tests/btrfs/137
@@ -23,6 +23,7 @@ _cleanup()
 _require_test
 _require_scratch
 _require_xfs_io_command "fiemap"
+_require_btrfs_no_compress
 
 send_files_dir=$TEST_DIR/btrfs-test-$seq
 
@@ -33,12 +34,12 @@ _scratch_mkfs >>$seqres.full 2>&1
 _scratch_mount
 
 # Create the first test file.
-$XFS_IO_PROG -f -c "pwrite -S 0xaa 0 4K" $SCRATCH_MNT/foo | _filter_xfs_io
+$XFS_IO_PROG -f -c "pwrite -S 0xaa -b 64k 0 64K" $SCRATCH_MNT/foo | _filter_xfs_io
 
 # Create a second test file with a 1Mb hole.
 $XFS_IO_PROG -f \
-     -c "pwrite -S 0xaa 0 4K" \
-     -c "pwrite -S 0xbb 1028K 4K" \
+     -c "pwrite -S 0xaa -b 64k 0 64K" \
+     -c "pwrite -S 0xbb -b 64k 1088K 64K" \
      $SCRATCH_MNT/bar | _filter_xfs_io
 
 $BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
@@ -46,10 +47,10 @@ $BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
 
 # Now add one new extent to our first test file, increasing its size and leaving
 # a 1Mb hole between the first extent and this new extent.
-$XFS_IO_PROG -c "pwrite -S 0xbb 1028K 4K" $SCRATCH_MNT/foo | _filter_xfs_io
+$XFS_IO_PROG -c "pwrite -S 0xbb -b 64k 1088K 64K" $SCRATCH_MNT/foo | _filter_xfs_io
 
 # Now overwrite the last extent of our second test file.
-$XFS_IO_PROG -c "pwrite -S 0xcc 1028K 4K" $SCRATCH_MNT/bar | _filter_xfs_io
+$XFS_IO_PROG -c "pwrite -S 0xcc -b 64k 1088K 64K" $SCRATCH_MNT/bar | _filter_xfs_io
 
 $BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
 		 $SCRATCH_MNT/snap2 >/dev/null
diff --git a/tests/btrfs/137.out b/tests/btrfs/137.out
index 8554399f..e863dd51 100644
--- a/tests/btrfs/137.out
+++ b/tests/btrfs/137.out
@@ -1,63 +1,63 @@
 QA output created by 137
-wrote 4096/4096 bytes at offset 0
+wrote 65536/65536 bytes at offset 0
 XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 4096/4096 bytes at offset 0
+wrote 65536/65536 bytes at offset 0
 XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 4096/4096 bytes at offset 1052672
+wrote 65536/65536 bytes at offset 1114112
 XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 4096/4096 bytes at offset 1052672
+wrote 65536/65536 bytes at offset 1114112
 XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 4096/4096 bytes at offset 1052672
+wrote 65536/65536 bytes at offset 1114112
 XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 File digests in the original filesystem:
-3e4309c7cc81f23d45e260a8f13ca860  SCRATCH_MNT/snap1/foo
-f3934f0cf164e2efa1bab71f2f164990  SCRATCH_MNT/snap1/bar
-f3934f0cf164e2efa1bab71f2f164990  SCRATCH_MNT/snap2/foo
-d3dc847171f9081bd75d7a2d3b53d322  SCRATCH_MNT/snap2/bar
+9802287a6faa01a1fd0e01732b732fca  SCRATCH_MNT/snap1/foo
+fe93f68ad1d8d5e47feba666ee6d3c47  SCRATCH_MNT/snap1/bar
+fe93f68ad1d8d5e47feba666ee6d3c47  SCRATCH_MNT/snap2/foo
+8d06f9b5841190b586a7526d0dd356f3  SCRATCH_MNT/snap2/bar
 
 File snap1/foo fiemap results in the original filesystem:
-0: [0..7]: data
+0: [0..127]: data
 
 File snap1/bar fiemap results in the original filesystem:
-0: [0..7]: data
-1: [8..2055]: hole
-2: [2056..2063]: data
+0: [0..127]: data
+1: [128..2175]: hole
+2: [2176..2303]: data
 
 File snap2/foo fiemap results in the original filesystem:
-0: [0..7]: data
-1: [8..2055]: hole
-2: [2056..2063]: data
+0: [0..127]: data
+1: [128..2175]: hole
+2: [2176..2303]: data
 
 File snap2/bar fiemap results in the original filesystem:
-0: [0..7]: data
-1: [8..2055]: hole
-2: [2056..2063]: data
+0: [0..127]: data
+1: [128..2175]: hole
+2: [2176..2303]: data
 
 At subvol SCRATCH_MNT/snap1
 At subvol SCRATCH_MNT/snap2
 At subvol snap1
 
 File digests in the new filesystem:
-3e4309c7cc81f23d45e260a8f13ca860  SCRATCH_MNT/snap1/foo
-f3934f0cf164e2efa1bab71f2f164990  SCRATCH_MNT/snap1/bar
-f3934f0cf164e2efa1bab71f2f164990  SCRATCH_MNT/snap2/foo
-d3dc847171f9081bd75d7a2d3b53d322  SCRATCH_MNT/snap2/bar
+9802287a6faa01a1fd0e01732b732fca  SCRATCH_MNT/snap1/foo
+fe93f68ad1d8d5e47feba666ee6d3c47  SCRATCH_MNT/snap1/bar
+fe93f68ad1d8d5e47feba666ee6d3c47  SCRATCH_MNT/snap2/foo
+8d06f9b5841190b586a7526d0dd356f3  SCRATCH_MNT/snap2/bar
 
 File snap1/foo fiemap results in the new filesystem:
-0: [0..7]: data
+0: [0..127]: data
 
 File snap1/bar fiemap results in the new filesystem:
-0: [0..7]: data
-1: [8..2055]: hole
-2: [2056..2063]: data
+0: [0..127]: data
+1: [128..2175]: hole
+2: [2176..2303]: data
 
 File snap2/foo fiemap results in the new filesystem:
-0: [0..7]: data
-1: [8..2055]: hole
-2: [2056..2063]: data
+0: [0..127]: data
+1: [128..2175]: hole
+2: [2176..2303]: data
 
 File snap2/bar fiemap results in the new filesystem:
-0: [0..7]: data
-1: [8..2055]: hole
-2: [2056..2063]: data
+0: [0..127]: data
+1: [128..2175]: hole
+2: [2176..2303]: data
-- 
2.34.1


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

* [PATCH v2 4/4] generic/563: Increase the iosize to to cover for btrfs
  2025-08-19 12:00 [PATCH v2 0/4] btrfs: Misc test fixes for large block/node sizes Nirjhar Roy (IBM)
                   ` (2 preceding siblings ...)
  2025-08-19 12:00 ` [PATCH v2 3/4] btrfs/137: Make this test compatible with all supported block sizes Nirjhar Roy (IBM)
@ 2025-08-19 12:00 ` Nirjhar Roy (IBM)
  2025-08-19 22:29   ` Qu Wenruo
  3 siblings, 1 reply; 13+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-08-19 12:00 UTC (permalink / raw)
  To: fstests
  Cc: linux-btrfs, ritesh.list, ojaswin, djwong, zlang, fdmanana,
	nirjhar.roy.lists, quwenruo.btrfs

When tested with block size/node size 64K on btrfs, then the test fails
with the folllowing error:
     QA output created by 563
     read/write
     read is in range
    -write is in range
    +write has value of 8855552
    +write is NOT in range 7969177.6 .. 8808038.4
     write -> read/write
    ...
The slight increase in the amount of bytes that are written is because
of the increase in the the nodesize(metadata) and hence it exceeds
the tolerance limit slightly. Fix this by increasing the iosize.
Increasing the iosize increases the tolerance range and covers the
tolerance for btrfs higher node sizes.

Reported-by: Disha Goel <disgoel@linux.ibm.com>
Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
---
 tests/generic/563 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/generic/563 b/tests/generic/563
index 89a71aa4..6cb9ddb0 100755
--- a/tests/generic/563
+++ b/tests/generic/563
@@ -43,7 +43,7 @@ _require_block_device $SCRATCH_DEV
 _require_non_zoned_device ${SCRATCH_DEV}
 
 cgdir=$CGROUP2_PATH
-iosize=$((1024 * 1024 * 8))
+iosize=$((1024 * 1024 * 16))
 
 # Check cgroup read/write charges against expected values. Allow for some
 # tolerance as different filesystems seem to account slightly differently.
-- 
2.34.1


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

* Re: [PATCH v2 1/4] btrfs/301: Make the test compatible with all the supported block sizes
  2025-08-19 12:00 ` [PATCH v2 1/4] btrfs/301: Make the test compatible with all the supported block sizes Nirjhar Roy (IBM)
@ 2025-08-19 21:59   ` Qu Wenruo
  0 siblings, 0 replies; 13+ messages in thread
From: Qu Wenruo @ 2025-08-19 21:59 UTC (permalink / raw)
  To: Nirjhar Roy (IBM), fstests
  Cc: linux-btrfs, ritesh.list, ojaswin, djwong, zlang, fdmanana,
	quwenruo.btrfs



在 2025/8/19 21:30, Nirjhar Roy (IBM) 写道:
> With large block sizes like 64k the test failed with the
> following logs:
> 
>       QA output created by 301
>       basic accounting
>      +subvol 256 mismatched usage 33947648 vs 4587520 (expected data 4194304 expected meta 393216 diff 29360128)
>      +subvol 256 mismatched usage 168165376 vs 138805248 (expected data 138412032 expected meta 393216 diff 29360128)
>      +subvol 256 mismatched usage 33947648 vs 4587520 (expected data 4194304 expected meta 393216 diff 29360128)
>      +subvol 256 mismatched usage 33947648 vs 4587520 (expected data 4194304 expected meta 393216 diff 29360128)
>       fallocate: Disk quota exceeded
> 
> The test creates nr_fill files each of size 8k. Now with 64k
> block size, 8k sized files occupy more than the expected sizes (i.e, 8k)
> due to internal fragmentation, since 1 file will occupy at least 1
> fsblock. Fix this by making the file size 64k, which is aligned
> with all the supported block sizes.
> 
> Reported-by: Disha Goel <disgoel@linux.ibm.com>
> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>   tests/btrfs/301 | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/btrfs/301 b/tests/btrfs/301
> index 6b59749d..be346f52 100755
> --- a/tests/btrfs/301
> +++ b/tests/btrfs/301
> @@ -23,7 +23,7 @@ subv=$SCRATCH_MNT/subv
>   nested=$SCRATCH_MNT/subv/nested
>   snap=$SCRATCH_MNT/snap
>   nr_fill=512
> -fill_sz=$((8 * 1024))
> +fill_sz=$((64 * 1024))
>   total_fill=$(($nr_fill * $fill_sz))
>   nodesize=$($BTRFS_UTIL_PROG inspect-internal dump-super $SCRATCH_DEV | \
>   					grep nodesize | $AWK_PROG '{print $2}')


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

* Re: [PATCH v2 2/4] generic/274: Make the pwrite block sizes and offsets to 64k
  2025-08-19 12:00 ` [PATCH v2 2/4] generic/274: Make the pwrite block sizes and offsets to 64k Nirjhar Roy (IBM)
@ 2025-08-19 22:09   ` Qu Wenruo
  2025-08-20  4:36     ` Nirjhar Roy (IBM)
  0 siblings, 1 reply; 13+ messages in thread
From: Qu Wenruo @ 2025-08-19 22:09 UTC (permalink / raw)
  To: Nirjhar Roy (IBM), fstests
  Cc: linux-btrfs, ritesh.list, ojaswin, djwong, zlang, fdmanana,
	quwenruo.btrfs



在 2025/8/19 21:30, Nirjhar Roy (IBM) 写道:
> This test was written with 4k block size in mind and it fails with
> 64k block size when tested with btrfs.
> The test first does pre-allocation, then fills up the
> filesystem. After that it tries to fragment and fill holes at offsets
> of 4k(i.e, 1 fsblock) - which works fine with 4k block size, but with
> 64k block size, the test tries to fragment and fill holes within
> 1 fsblock(of size 64k). This results in overwrite of 64k fsblocks
> and the write fails. The reason for this failure is that during
> overwrite, there is no more space available for COW.
> Fix this by changing the pwrite block size and offsets to 64k
> so that the test never tries to punch holes or overwrite within 1 fsblock
> and the test becomes compatible with all block sizes.
> 
> For non-COW filesystems/files, this test should work even if the
> underlying filesytem block size > 64k.
> 
> Reported-by: Disha Goel <disgoel@linux.ibm.com>
> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>

Overall looks good to me.

Although still a minor concern inlined below.

[...]>
>   # Fill the rest of the fs completely
>   # Note, this will show ENOSPC errors in $seqres.full, that's ok.
>   echo "Fill fs with 1M IOs; ENOSPC expected" >> $seqres.full
>   dd if=/dev/zero of=$SCRATCH_MNT/tmp1 bs=1M >>$seqres.full 2>&1
> -echo "Fill fs with 4K IOs; ENOSPC expected" >> $seqres.full
> -dd if=/dev/zero of=$SCRATCH_MNT/tmp2 bs=4K >>$seqres.full 2>&1
> +echo "Fill fs with 64K IOs; ENOSPC expected" >> $seqres.full
> +dd if=/dev/zero of=$SCRATCH_MNT/tmp2 bs=64K >>$seqres.full 2>&1

Not sure if using 64K block size to fill the fs is the correct way.

For example on a fs with 4K block size, but at end of filling there are 
only 60K left.

This will fail the filling as we can not reserve 64K data space anymore.
But it's not 100% filling the data space either.
This may not matter that much as in the preallocated filling stage, 
every operation is still in 64K block size though.

I'd prefer to keep the old 4K as block size (as it's the minimal support 
one), or use the fs block size for filling.
This will ensure we really use up all the data space.

Thanks,
Qu

>   _scratch_sync
>   # Last effort, use O_SYNC
> -echo "Fill fs with 4K DIOs; ENOSPC expected" >> $seqres.full
> -dd if=/dev/zero of=$SCRATCH_MNT/tmp3 bs=4K oflag=sync >>$seqres.full 2>&1
> +echo "Fill fs with 64K DIOs; ENOSPC expected" >> $seqres.full
> +dd if=/dev/zero of=$SCRATCH_MNT/tmp3 bs=64K oflag=sync >>$seqres.full 2>&1
>   # Save space usage info
>   echo "Post-fill space:" >> $seqres.full
>   df $SCRATCH_MNT >>$seqres.full 2>&1
> @@ -63,7 +63,7 @@ df $SCRATCH_MNT >>$seqres.full 2>&1
>   echo "Fill in prealloc space; fragment at offsets:" >> $seqres.full
>   for i in `seq 1 2 1023`; do
>   	echo -n "$i " >> $seqres.full
> -	dd if=/dev/zero of=$SCRATCH_MNT/test seek=$i bs=4K count=1 conv=notrunc \
> +	dd if=/dev/zero of=$SCRATCH_MNT/test seek=$i bs=64K count=1 conv=notrunc \
>   		>>$seqres.full 2>/dev/null || _fail "failed to write to test file"
>   done
>   _scratch_sync
> @@ -71,7 +71,7 @@ echo >> $seqres.full
>   echo "Fill in prealloc space; fill holes at offsets:" >> $seqres.full
>   for i in `seq 2 2 1023`; do
>   	echo -n "$i " >> $seqres.full
> -	dd if=/dev/zero of=$SCRATCH_MNT/test seek=$i bs=4K count=1 conv=notrunc \
> +	dd if=/dev/zero of=$SCRATCH_MNT/test seek=$i bs=64K count=1 conv=notrunc \
>   		>>$seqres.full 2>/dev/null || _fail "failed to fill test file"
>   done
>   _scratch_sync


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

* Re: [PATCH v2 3/4] btrfs/137: Make this test compatible with all supported block sizes
  2025-08-19 12:00 ` [PATCH v2 3/4] btrfs/137: Make this test compatible with all supported block sizes Nirjhar Roy (IBM)
@ 2025-08-19 22:10   ` Qu Wenruo
  0 siblings, 0 replies; 13+ messages in thread
From: Qu Wenruo @ 2025-08-19 22:10 UTC (permalink / raw)
  To: Nirjhar Roy (IBM), fstests
  Cc: linux-btrfs, ritesh.list, ojaswin, djwong, zlang, fdmanana,
	quwenruo.btrfs



在 2025/8/19 21:30, Nirjhar Roy (IBM) 写道:
> For large block sizes like 64k it failed simply because this
> test was written with 4k block size in mind.
> The first few lines of the error logs are as follows:
> 
>       d3dc847171f9081bd75d7a2d3b53d322  SCRATCH_MNT/snap2/bar
> 
>       File snap1/foo fiemap results in the original filesystem:
>      -0: [0..7]: data
>      +0: [0..127]: data
> 
>       File snap1/bar fiemap results in the original filesystem:
>      ...
> 
> Fix this by making the test choose offsets and block size as 64k
> which is aligned with all the underlying supported fs block sizes.
> 
> Reported-by: Disha Goel <disgoel@linux.ibm.com>
> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>   tests/btrfs/137     | 11 ++++----
>   tests/btrfs/137.out | 66 ++++++++++++++++++++++-----------------------
>   2 files changed, 39 insertions(+), 38 deletions(-)
> 
> diff --git a/tests/btrfs/137 b/tests/btrfs/137
> index 7710dc18..c1d498bd 100755
> --- a/tests/btrfs/137
> +++ b/tests/btrfs/137
> @@ -23,6 +23,7 @@ _cleanup()
>   _require_test
>   _require_scratch
>   _require_xfs_io_command "fiemap"
> +_require_btrfs_no_compress
>   
>   send_files_dir=$TEST_DIR/btrfs-test-$seq
>   
> @@ -33,12 +34,12 @@ _scratch_mkfs >>$seqres.full 2>&1
>   _scratch_mount
>   
>   # Create the first test file.
> -$XFS_IO_PROG -f -c "pwrite -S 0xaa 0 4K" $SCRATCH_MNT/foo | _filter_xfs_io
> +$XFS_IO_PROG -f -c "pwrite -S 0xaa -b 64k 0 64K" $SCRATCH_MNT/foo | _filter_xfs_io
>   
>   # Create a second test file with a 1Mb hole.
>   $XFS_IO_PROG -f \
> -     -c "pwrite -S 0xaa 0 4K" \
> -     -c "pwrite -S 0xbb 1028K 4K" \
> +     -c "pwrite -S 0xaa -b 64k 0 64K" \
> +     -c "pwrite -S 0xbb -b 64k 1088K 64K" \
>        $SCRATCH_MNT/bar | _filter_xfs_io
>   
>   $BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
> @@ -46,10 +47,10 @@ $BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
>   
>   # Now add one new extent to our first test file, increasing its size and leaving
>   # a 1Mb hole between the first extent and this new extent.
> -$XFS_IO_PROG -c "pwrite -S 0xbb 1028K 4K" $SCRATCH_MNT/foo | _filter_xfs_io
> +$XFS_IO_PROG -c "pwrite -S 0xbb -b 64k 1088K 64K" $SCRATCH_MNT/foo | _filter_xfs_io
>   
>   # Now overwrite the last extent of our second test file.
> -$XFS_IO_PROG -c "pwrite -S 0xcc 1028K 4K" $SCRATCH_MNT/bar | _filter_xfs_io
> +$XFS_IO_PROG -c "pwrite -S 0xcc -b 64k 1088K 64K" $SCRATCH_MNT/bar | _filter_xfs_io
>   
>   $BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
>   		 $SCRATCH_MNT/snap2 >/dev/null
> diff --git a/tests/btrfs/137.out b/tests/btrfs/137.out
> index 8554399f..e863dd51 100644
> --- a/tests/btrfs/137.out
> +++ b/tests/btrfs/137.out
> @@ -1,63 +1,63 @@
>   QA output created by 137
> -wrote 4096/4096 bytes at offset 0
> +wrote 65536/65536 bytes at offset 0
>   XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -wrote 4096/4096 bytes at offset 0
> +wrote 65536/65536 bytes at offset 0
>   XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -wrote 4096/4096 bytes at offset 1052672
> +wrote 65536/65536 bytes at offset 1114112
>   XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -wrote 4096/4096 bytes at offset 1052672
> +wrote 65536/65536 bytes at offset 1114112
>   XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -wrote 4096/4096 bytes at offset 1052672
> +wrote 65536/65536 bytes at offset 1114112
>   XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>   
>   File digests in the original filesystem:
> -3e4309c7cc81f23d45e260a8f13ca860  SCRATCH_MNT/snap1/foo
> -f3934f0cf164e2efa1bab71f2f164990  SCRATCH_MNT/snap1/bar
> -f3934f0cf164e2efa1bab71f2f164990  SCRATCH_MNT/snap2/foo
> -d3dc847171f9081bd75d7a2d3b53d322  SCRATCH_MNT/snap2/bar
> +9802287a6faa01a1fd0e01732b732fca  SCRATCH_MNT/snap1/foo
> +fe93f68ad1d8d5e47feba666ee6d3c47  SCRATCH_MNT/snap1/bar
> +fe93f68ad1d8d5e47feba666ee6d3c47  SCRATCH_MNT/snap2/foo
> +8d06f9b5841190b586a7526d0dd356f3  SCRATCH_MNT/snap2/bar
>   
>   File snap1/foo fiemap results in the original filesystem:
> -0: [0..7]: data
> +0: [0..127]: data
>   
>   File snap1/bar fiemap results in the original filesystem:
> -0: [0..7]: data
> -1: [8..2055]: hole
> -2: [2056..2063]: data
> +0: [0..127]: data
> +1: [128..2175]: hole
> +2: [2176..2303]: data
>   
>   File snap2/foo fiemap results in the original filesystem:
> -0: [0..7]: data
> -1: [8..2055]: hole
> -2: [2056..2063]: data
> +0: [0..127]: data
> +1: [128..2175]: hole
> +2: [2176..2303]: data
>   
>   File snap2/bar fiemap results in the original filesystem:
> -0: [0..7]: data
> -1: [8..2055]: hole
> -2: [2056..2063]: data
> +0: [0..127]: data
> +1: [128..2175]: hole
> +2: [2176..2303]: data
>   
>   At subvol SCRATCH_MNT/snap1
>   At subvol SCRATCH_MNT/snap2
>   At subvol snap1
>   
>   File digests in the new filesystem:
> -3e4309c7cc81f23d45e260a8f13ca860  SCRATCH_MNT/snap1/foo
> -f3934f0cf164e2efa1bab71f2f164990  SCRATCH_MNT/snap1/bar
> -f3934f0cf164e2efa1bab71f2f164990  SCRATCH_MNT/snap2/foo
> -d3dc847171f9081bd75d7a2d3b53d322  SCRATCH_MNT/snap2/bar
> +9802287a6faa01a1fd0e01732b732fca  SCRATCH_MNT/snap1/foo
> +fe93f68ad1d8d5e47feba666ee6d3c47  SCRATCH_MNT/snap1/bar
> +fe93f68ad1d8d5e47feba666ee6d3c47  SCRATCH_MNT/snap2/foo
> +8d06f9b5841190b586a7526d0dd356f3  SCRATCH_MNT/snap2/bar
>   
>   File snap1/foo fiemap results in the new filesystem:
> -0: [0..7]: data
> +0: [0..127]: data
>   
>   File snap1/bar fiemap results in the new filesystem:
> -0: [0..7]: data
> -1: [8..2055]: hole
> -2: [2056..2063]: data
> +0: [0..127]: data
> +1: [128..2175]: hole
> +2: [2176..2303]: data
>   
>   File snap2/foo fiemap results in the new filesystem:
> -0: [0..7]: data
> -1: [8..2055]: hole
> -2: [2056..2063]: data
> +0: [0..127]: data
> +1: [128..2175]: hole
> +2: [2176..2303]: data
>   
>   File snap2/bar fiemap results in the new filesystem:
> -0: [0..7]: data
> -1: [8..2055]: hole
> -2: [2056..2063]: data
> +0: [0..127]: data
> +1: [128..2175]: hole
> +2: [2176..2303]: data


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

* Re: [PATCH v2 4/4] generic/563: Increase the iosize to to cover for btrfs
  2025-08-19 12:00 ` [PATCH v2 4/4] generic/563: Increase the iosize to to cover for btrfs Nirjhar Roy (IBM)
@ 2025-08-19 22:29   ` Qu Wenruo
  2025-08-20  5:00     ` Nirjhar Roy (IBM)
  0 siblings, 1 reply; 13+ messages in thread
From: Qu Wenruo @ 2025-08-19 22:29 UTC (permalink / raw)
  To: Nirjhar Roy (IBM), fstests
  Cc: linux-btrfs, ritesh.list, ojaswin, djwong, zlang, fdmanana,
	quwenruo.btrfs



在 2025/8/19 21:30, Nirjhar Roy (IBM) 写道:
> When tested with block size/node size 64K on btrfs, then the test fails
> with the folllowing error:
>       QA output created by 563
>       read/write
>       read is in range
>      -write is in range
>      +write has value of 8855552
>      +write is NOT in range 7969177.6 .. 8808038.4
>       write -> read/write
>      ...
> The slight increase in the amount of bytes that are written is because
> of the increase in the the nodesize(metadata) and hence it exceeds
> the tolerance limit slightly. Fix this by increasing the iosize.
> Increasing the iosize increases the tolerance range and covers the
> tolerance for btrfs higher node sizes.
> 
> Reported-by: Disha Goel <disgoel@linux.ibm.com>
> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>

Looks good to me.

Just want to add some more analyze for the failure case.

For the test it writes 8M data with 5% tolerance (around 408K) with the 
total writes.

With 64K block size (and it implies 64K metadata size) for btrfs, it 
mean we can have at most 7 tree blocks of writes plus two super blocks 
updated.

Considering the default metadata profile is DUP, doubling the metadata 
writes, the real limit is only 3 tree blocks.

And when doing fsync, btrfs will create at least 2 new tree blocks, one 
for the log tree root, and one for the log tree of the subvolume.

This is still inside the tolerance, thus the test case can still pass 
for a lot of cases.

But if a full transaction commit is triggered, btrfs will need to create 
at least 3 new tree blocks for root, extent and subvolume tree.
Depending on the mkfs config, it will increase to 7 tree blocks (free 
space tree, block group tree, csum tree and uuid tree created at mount).

All are exceeding the tolerance limit.

Doubling the io size will make the tolerance to be 8 tree blocks, 
covering the worst case of 64K metadata sized btrfs, at least for now.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>   tests/generic/563 | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/generic/563 b/tests/generic/563
> index 89a71aa4..6cb9ddb0 100755
> --- a/tests/generic/563
> +++ b/tests/generic/563
> @@ -43,7 +43,7 @@ _require_block_device $SCRATCH_DEV
>   _require_non_zoned_device ${SCRATCH_DEV}
>   
>   cgdir=$CGROUP2_PATH
> -iosize=$((1024 * 1024 * 8))
> +iosize=$((1024 * 1024 * 16))
>   
>   # Check cgroup read/write charges against expected values. Allow for some
>   # tolerance as different filesystems seem to account slightly differently.


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

* Re: [PATCH v2 2/4] generic/274: Make the pwrite block sizes and offsets to 64k
  2025-08-19 22:09   ` Qu Wenruo
@ 2025-08-20  4:36     ` Nirjhar Roy (IBM)
  0 siblings, 0 replies; 13+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-08-20  4:36 UTC (permalink / raw)
  To: Qu Wenruo, fstests
  Cc: linux-btrfs, ritesh.list, ojaswin, djwong, zlang, fdmanana,
	quwenruo.btrfs


On 8/20/25 03:39, Qu Wenruo wrote:
>
>
> 在 2025/8/19 21:30, Nirjhar Roy (IBM) 写道:
>> This test was written with 4k block size in mind and it fails with
>> 64k block size when tested with btrfs.
>> The test first does pre-allocation, then fills up the
>> filesystem. After that it tries to fragment and fill holes at offsets
>> of 4k(i.e, 1 fsblock) - which works fine with 4k block size, but with
>> 64k block size, the test tries to fragment and fill holes within
>> 1 fsblock(of size 64k). This results in overwrite of 64k fsblocks
>> and the write fails. The reason for this failure is that during
>> overwrite, there is no more space available for COW.
>> Fix this by changing the pwrite block size and offsets to 64k
>> so that the test never tries to punch holes or overwrite within 1 
>> fsblock
>> and the test becomes compatible with all block sizes.
>>
>> For non-COW filesystems/files, this test should work even if the
>> underlying filesytem block size > 64k.
>>
>> Reported-by: Disha Goel <disgoel@linux.ibm.com>
>> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
>
> Overall looks good to me.
>
> Although still a minor concern inlined below.
>
> [...]>
>>   # Fill the rest of the fs completely
>>   # Note, this will show ENOSPC errors in $seqres.full, that's ok.
>>   echo "Fill fs with 1M IOs; ENOSPC expected" >> $seqres.full
>>   dd if=/dev/zero of=$SCRATCH_MNT/tmp1 bs=1M >>$seqres.full 2>&1
>> -echo "Fill fs with 4K IOs; ENOSPC expected" >> $seqres.full
>> -dd if=/dev/zero of=$SCRATCH_MNT/tmp2 bs=4K >>$seqres.full 2>&1
>> +echo "Fill fs with 64K IOs; ENOSPC expected" >> $seqres.full
>> +dd if=/dev/zero of=$SCRATCH_MNT/tmp2 bs=64K >>$seqres.full 2>&1
>
> Not sure if using 64K block size to fill the fs is the correct way.
>
> For example on a fs with 4K block size, but at end of filling there 
> are only 60K left.
>
> This will fail the filling as we can not reserve 64K data space anymore.
> But it's not 100% filling the data space either.
> This may not matter that much as in the preallocated filling stage, 
> every operation is still in 64K block size though.
>
> I'd prefer to keep the old 4K as block size (as it's the minimal 
> support one), or use the fs block size for filling.
> This will ensure we really use up all the data space.
>
> Thanks,
> Qu

Okay, makes sense. I will revert it to the older 4k size and send an 
updated version.

--NR

>
>>   _scratch_sync
>>   # Last effort, use O_SYNC
>> -echo "Fill fs with 4K DIOs; ENOSPC expected" >> $seqres.full
>> -dd if=/dev/zero of=$SCRATCH_MNT/tmp3 bs=4K oflag=sync >>$seqres.full 
>> 2>&1
>> +echo "Fill fs with 64K DIOs; ENOSPC expected" >> $seqres.full
>> +dd if=/dev/zero of=$SCRATCH_MNT/tmp3 bs=64K oflag=sync 
>> >>$seqres.full 2>&1
>>   # Save space usage info
>>   echo "Post-fill space:" >> $seqres.full
>>   df $SCRATCH_MNT >>$seqres.full 2>&1
>> @@ -63,7 +63,7 @@ df $SCRATCH_MNT >>$seqres.full 2>&1
>>   echo "Fill in prealloc space; fragment at offsets:" >> $seqres.full
>>   for i in `seq 1 2 1023`; do
>>       echo -n "$i " >> $seqres.full
>> -    dd if=/dev/zero of=$SCRATCH_MNT/test seek=$i bs=4K count=1 
>> conv=notrunc \
>> +    dd if=/dev/zero of=$SCRATCH_MNT/test seek=$i bs=64K count=1 
>> conv=notrunc \
>>           >>$seqres.full 2>/dev/null || _fail "failed to write to 
>> test file"
>>   done
>>   _scratch_sync
>> @@ -71,7 +71,7 @@ echo >> $seqres.full
>>   echo "Fill in prealloc space; fill holes at offsets:" >> $seqres.full
>>   for i in `seq 2 2 1023`; do
>>       echo -n "$i " >> $seqres.full
>> -    dd if=/dev/zero of=$SCRATCH_MNT/test seek=$i bs=4K count=1 
>> conv=notrunc \
>> +    dd if=/dev/zero of=$SCRATCH_MNT/test seek=$i bs=64K count=1 
>> conv=notrunc \
>>           >>$seqres.full 2>/dev/null || _fail "failed to fill test file"
>>   done
>>   _scratch_sync
>
-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore


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

* Re: [PATCH v2 4/4] generic/563: Increase the iosize to to cover for btrfs
  2025-08-19 22:29   ` Qu Wenruo
@ 2025-08-20  5:00     ` Nirjhar Roy (IBM)
  2025-08-20  5:45       ` Qu Wenruo
  0 siblings, 1 reply; 13+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-08-20  5:00 UTC (permalink / raw)
  To: Qu Wenruo, fstests
  Cc: linux-btrfs, ritesh.list, ojaswin, djwong, zlang, fdmanana,
	quwenruo.btrfs


On 8/20/25 03:59, Qu Wenruo wrote:
>
>
> 在 2025/8/19 21:30, Nirjhar Roy (IBM) 写道:
>> When tested with block size/node size 64K on btrfs, then the test fails
>> with the folllowing error:
>>       QA output created by 563
>>       read/write
>>       read is in range
>>      -write is in range
>>      +write has value of 8855552
>>      +write is NOT in range 7969177.6 .. 8808038.4
>>       write -> read/write
>>      ...
>> The slight increase in the amount of bytes that are written is because
>> of the increase in the the nodesize(metadata) and hence it exceeds
>> the tolerance limit slightly. Fix this by increasing the iosize.
>> Increasing the iosize increases the tolerance range and covers the
>> tolerance for btrfs higher node sizes.
>>
>> Reported-by: Disha Goel <disgoel@linux.ibm.com>
>> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
>
> Looks good to me.
>
> Just want to add some more analyze for the failure case.
>
> For the test it writes 8M data with 5% tolerance (around 408K) with 
> the total writes.
>
> With 64K block size (and it implies 64K metadata size) for btrfs, it 
> mean we can have at most 7 tree blocks of writes plus two super blocks 
> updated.
>
> Considering the default metadata profile is DUP, doubling the metadata 
> writes, the real limit is only 3 tree blocks.
>
> And when doing fsync, btrfs will create at least 2 new tree blocks, 
> one for the log tree root, and one for the log tree of the subvolume.
>
> This is still inside the tolerance, thus the test case can still pass 
> for a lot of cases.
>
> But if a full transaction commit is triggered, btrfs will need to 
> create at least 3 new tree blocks for root, extent and subvolume tree.
> Depending on the mkfs config, it will increase to 7 tree blocks (free 
> space tree, block group tree, csum tree and uuid tree created at mount).
>
> All are exceeding the tolerance limit.
>
> Doubling the io size will make the tolerance to be 8 tree blocks, 
> covering the worst case of 64K metadata sized btrfs, at least for now.

Thank you for the detailed analysis. I will add this analysis in the 
commit message and address the comment for generic/274[1] and add your 
RBs in the next revision.

A couple of questions for the above explanation:

Doubling the iosize to 16M with 5% tolerance is around 819k. So, with 
64k blocks, it turns out to be 819k/64k = 12. Considering DUP, it should 
be approximately 12/2=6 blocks, but you are saying 8. Can you please 
explain this part a bit?

[1] 
https://lore.kernel.org/all/0a10a9b0-a55c-4607-be0b-7f7f01c2d729@suse.com/

--NR

>
> Reviewed-by: Qu Wenruo <wqu@suse.com>
>
> Thanks,
> Qu
>
>> ---
>>   tests/generic/563 | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/generic/563 b/tests/generic/563
>> index 89a71aa4..6cb9ddb0 100755
>> --- a/tests/generic/563
>> +++ b/tests/generic/563
>> @@ -43,7 +43,7 @@ _require_block_device $SCRATCH_DEV
>>   _require_non_zoned_device ${SCRATCH_DEV}
>>     cgdir=$CGROUP2_PATH
>> -iosize=$((1024 * 1024 * 8))
>> +iosize=$((1024 * 1024 * 16))
>>     # Check cgroup read/write charges against expected values. Allow 
>> for some
>>   # tolerance as different filesystems seem to account slightly 
>> differently.
>
-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore


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

* Re: [PATCH v2 4/4] generic/563: Increase the iosize to to cover for btrfs
  2025-08-20  5:00     ` Nirjhar Roy (IBM)
@ 2025-08-20  5:45       ` Qu Wenruo
  2025-08-20  8:00         ` Nirjhar Roy (IBM)
  0 siblings, 1 reply; 13+ messages in thread
From: Qu Wenruo @ 2025-08-20  5:45 UTC (permalink / raw)
  To: Nirjhar Roy (IBM), Qu Wenruo, fstests
  Cc: linux-btrfs, ritesh.list, ojaswin, djwong, zlang, fdmanana



在 2025/8/20 14:30, Nirjhar Roy (IBM) 写道:
> 
> On 8/20/25 03:59, Qu Wenruo wrote:
>>
>>
>> 在 2025/8/19 21:30, Nirjhar Roy (IBM) 写道:
>>> When tested with block size/node size 64K on btrfs, then the test fails
>>> with the folllowing error:
>>>       QA output created by 563
>>>       read/write
>>>       read is in range
>>>      -write is in range
>>>      +write has value of 8855552
>>>      +write is NOT in range 7969177.6 .. 8808038.4
>>>       write -> read/write
>>>      ...
>>> The slight increase in the amount of bytes that are written is because
>>> of the increase in the the nodesize(metadata) and hence it exceeds
>>> the tolerance limit slightly. Fix this by increasing the iosize.
>>> Increasing the iosize increases the tolerance range and covers the
>>> tolerance for btrfs higher node sizes.
>>>
>>> Reported-by: Disha Goel <disgoel@linux.ibm.com>
>>> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
>>
>> Looks good to me.
>>
>> Just want to add some more analyze for the failure case.
>>
>> For the test it writes 8M data with 5% tolerance (around 408K) with 
>> the total writes.
>>
>> With 64K block size (and it implies 64K metadata size) for btrfs, it 
>> mean we can have at most 7 tree blocks of writes plus two super blocks 
>> updated.
>>
>> Considering the default metadata profile is DUP, doubling the metadata 
>> writes, the real limit is only 3 tree blocks.
>>
>> And when doing fsync, btrfs will create at least 2 new tree blocks, 
>> one for the log tree root, and one for the log tree of the subvolume.
>>
>> This is still inside the tolerance, thus the test case can still pass 
>> for a lot of cases.
>>
>> But if a full transaction commit is triggered, btrfs will need to 
>> create at least 3 new tree blocks for root, extent and subvolume tree.
>> Depending on the mkfs config, it will increase to 7 tree blocks (free 
>> space tree, block group tree, csum tree and uuid tree created at mount).
>>
>> All are exceeding the tolerance limit.
>>
>> Doubling the io size will make the tolerance to be 8 tree blocks, 
>> covering the worst case of 64K metadata sized btrfs, at least for now.
> 
> Thank you for the detailed analysis. I will add this analysis in the 
> commit message and address the comment for generic/274[1] and add your 
> RBs in the next revision.
> 
> A couple of questions for the above explanation:
> 
> Doubling the iosize to 16M with 5% tolerance is around 819k. So, with 
> 64k blocks, it turns out to be 819k/64k = 12. Considering DUP, it should 
> be approximately 12/2=6 blocks, but you are saying 8. Can you please 
> explain this part a bit?

My bad, wrong calculation.

The original 8M tolerance is only for 6 tree blocks, with DUP it reduced 
to 3. Thus a full commit transaction will always fail the tolerance check.

Doubled to 16M iosize, the tolerance is exactly what you said, 12 tree 
blocks not 16, and with DUP into consideration it's 6.

With 6 tree blocks tolerance, it's borderline for a full commit transaction.

The recent default mkfs config means 5 tree blocks (root, extent, 
subvolume, csum and free space), and with incoming new default bgt free 
it will be exactly at the boundary, but should still pass for now.

Thanks,
Qu


> 
> [1] https://lore.kernel.org/all/0a10a9b0-a55c-4607- 
> be0b-7f7f01c2d729@suse.com/
> 
> --NR
> 
>>
>> Reviewed-by: Qu Wenruo <wqu@suse.com>
>>
>> Thanks,
>> Qu
>>
>>> ---
>>>   tests/generic/563 | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tests/generic/563 b/tests/generic/563
>>> index 89a71aa4..6cb9ddb0 100755
>>> --- a/tests/generic/563
>>> +++ b/tests/generic/563
>>> @@ -43,7 +43,7 @@ _require_block_device $SCRATCH_DEV
>>>   _require_non_zoned_device ${SCRATCH_DEV}
>>>     cgdir=$CGROUP2_PATH
>>> -iosize=$((1024 * 1024 * 8))
>>> +iosize=$((1024 * 1024 * 16))
>>>     # Check cgroup read/write charges against expected values. Allow 
>>> for some
>>>   # tolerance as different filesystems seem to account slightly 
>>> differently.
>>


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

* Re: [PATCH v2 4/4] generic/563: Increase the iosize to to cover for btrfs
  2025-08-20  5:45       ` Qu Wenruo
@ 2025-08-20  8:00         ` Nirjhar Roy (IBM)
  0 siblings, 0 replies; 13+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-08-20  8:00 UTC (permalink / raw)
  To: Qu Wenruo, Qu Wenruo, fstests
  Cc: linux-btrfs, ritesh.list, ojaswin, djwong, zlang, fdmanana


On 8/20/25 11:15, Qu Wenruo wrote:
>
>
> 在 2025/8/20 14:30, Nirjhar Roy (IBM) 写道:
>>
>> On 8/20/25 03:59, Qu Wenruo wrote:
>>>
>>>
>>> 在 2025/8/19 21:30, Nirjhar Roy (IBM) 写道:
>>>> When tested with block size/node size 64K on btrfs, then the test 
>>>> fails
>>>> with the folllowing error:
>>>>       QA output created by 563
>>>>       read/write
>>>>       read is in range
>>>>      -write is in range
>>>>      +write has value of 8855552
>>>>      +write is NOT in range 7969177.6 .. 8808038.4
>>>>       write -> read/write
>>>>      ...
>>>> The slight increase in the amount of bytes that are written is because
>>>> of the increase in the the nodesize(metadata) and hence it exceeds
>>>> the tolerance limit slightly. Fix this by increasing the iosize.
>>>> Increasing the iosize increases the tolerance range and covers the
>>>> tolerance for btrfs higher node sizes.
>>>>
>>>> Reported-by: Disha Goel <disgoel@linux.ibm.com>
>>>> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
>>>
>>> Looks good to me.
>>>
>>> Just want to add some more analyze for the failure case.
>>>
>>> For the test it writes 8M data with 5% tolerance (around 408K) with 
>>> the total writes.
>>>
>>> With 64K block size (and it implies 64K metadata size) for btrfs, it 
>>> mean we can have at most 7 tree blocks of writes plus two super 
>>> blocks updated.
>>>
>>> Considering the default metadata profile is DUP, doubling the 
>>> metadata writes, the real limit is only 3 tree blocks.
>>>
>>> And when doing fsync, btrfs will create at least 2 new tree blocks, 
>>> one for the log tree root, and one for the log tree of the subvolume.
>>>
>>> This is still inside the tolerance, thus the test case can still 
>>> pass for a lot of cases.
>>>
>>> But if a full transaction commit is triggered, btrfs will need to 
>>> create at least 3 new tree blocks for root, extent and subvolume tree.
>>> Depending on the mkfs config, it will increase to 7 tree blocks 
>>> (free space tree, block group tree, csum tree and uuid tree created 
>>> at mount).
>>>
>>> All are exceeding the tolerance limit.
>>>
>>> Doubling the io size will make the tolerance to be 8 tree blocks, 
>>> covering the worst case of 64K metadata sized btrfs, at least for now.
>>
>> Thank you for the detailed analysis. I will add this analysis in the 
>> commit message and address the comment for generic/274[1] and add 
>> your RBs in the next revision.
>>
>> A couple of questions for the above explanation:
>>
>> Doubling the iosize to 16M with 5% tolerance is around 819k. So, with 
>> 64k blocks, it turns out to be 819k/64k = 12. Considering DUP, it 
>> should be approximately 12/2=6 blocks, but you are saying 8. Can you 
>> please explain this part a bit?
>
> My bad, wrong calculation.
>
> The original 8M tolerance is only for 6 tree blocks, with DUP it 
> reduced to 3. Thus a full commit transaction will always fail the 
> tolerance check.
>
> Doubled to 16M iosize, the tolerance is exactly what you said, 12 tree 
> blocks not 16, and with DUP into consideration it's 6.
>
> With 6 tree blocks tolerance, it's borderline for a full commit 
> transaction.
>
> The recent default mkfs config means 5 tree blocks (root, extent, 
> subvolume, csum and free space), and with incoming new default bgt 
> free it will be exactly at the boundary, but should still pass for now.
>
Okay, it makes sense now. Thank you for the detailed explanation.

--NR

> Thanks,
> Qu
>
>
>>
>> [1] https://lore.kernel.org/all/0a10a9b0-a55c-4607- 
>> be0b-7f7f01c2d729@suse.com/
>>
>> --NR
>>
>>>
>>> Reviewed-by: Qu Wenruo <wqu@suse.com>
>>>
>>> Thanks,
>>> Qu
>>>
>>>> ---
>>>>   tests/generic/563 | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/tests/generic/563 b/tests/generic/563
>>>> index 89a71aa4..6cb9ddb0 100755
>>>> --- a/tests/generic/563
>>>> +++ b/tests/generic/563
>>>> @@ -43,7 +43,7 @@ _require_block_device $SCRATCH_DEV
>>>>   _require_non_zoned_device ${SCRATCH_DEV}
>>>>     cgdir=$CGROUP2_PATH
>>>> -iosize=$((1024 * 1024 * 8))
>>>> +iosize=$((1024 * 1024 * 16))
>>>>     # Check cgroup read/write charges against expected values. 
>>>> Allow for some
>>>>   # tolerance as different filesystems seem to account slightly 
>>>> differently.
>>>
>
-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore


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

end of thread, other threads:[~2025-08-20  8:00 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-19 12:00 [PATCH v2 0/4] btrfs: Misc test fixes for large block/node sizes Nirjhar Roy (IBM)
2025-08-19 12:00 ` [PATCH v2 1/4] btrfs/301: Make the test compatible with all the supported block sizes Nirjhar Roy (IBM)
2025-08-19 21:59   ` Qu Wenruo
2025-08-19 12:00 ` [PATCH v2 2/4] generic/274: Make the pwrite block sizes and offsets to 64k Nirjhar Roy (IBM)
2025-08-19 22:09   ` Qu Wenruo
2025-08-20  4:36     ` Nirjhar Roy (IBM)
2025-08-19 12:00 ` [PATCH v2 3/4] btrfs/137: Make this test compatible with all supported block sizes Nirjhar Roy (IBM)
2025-08-19 22:10   ` Qu Wenruo
2025-08-19 12:00 ` [PATCH v2 4/4] generic/563: Increase the iosize to to cover for btrfs Nirjhar Roy (IBM)
2025-08-19 22:29   ` Qu Wenruo
2025-08-20  5:00     ` Nirjhar Roy (IBM)
2025-08-20  5:45       ` Qu Wenruo
2025-08-20  8:00         ` Nirjhar Roy (IBM)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox