Re: [PATCH] btrfs/276: allow a slight increase in the number of extents

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]





On 2023/8/2 18:36, Qu Wenruo wrote:


On 2023/8/2 18:23, Filipe Manana wrote:
On Tue, Aug 1, 2023 at 8:16 AM Qu Wenruo <wqu@xxxxxxxx> wrote:

[BUG]
Sometimes test case btrfs/276 would fail with extra number of extents:

     - output mismatch (see /opt/xfstests/results//btrfs/276.out.bad)
     --- tests/btrfs/276.out     2023-07-19 07:24:07.000000000 +0000
     +++ /opt/xfstests/results//btrfs/276.out.bad        2023-07-28
04:15:06.223985372 +0000
     @@ -1,16 +1,16 @@
      QA output created by 276
      wrote 17179869184/17179869184 bytes at offset 0
      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
     -Number of non-shared extents in the whole file: 131072
     +Number of non-shared extents in the whole file: 131082
      Create a snapshot of 'SCRATCH_MNT' in 'SCRATCH_MNT/snap'
     -Number of shared extents in the whole file: 131072
     ...
     (Run 'diff -u /opt/xfstests/tests/btrfs/276.out
/opt/xfstests/results//btrfs/276.out.bad'  to see the entire diff)

[CAUSE]
The test case uses golden output to record the number of total extents
of a 16G file.

This is not reliable as we can have writeback happen halfway, resulting
smaller extents thus slightly more extents.

With a VM with 4G memory, I have a chance around 1/10 hitting this
false alert.

[FIX]
Instead of using golden output, we allow a slight (5%) float in the
number of extents, and move the 131072 (and 131072 - 16) from golden
output, so even if we have a slightly more extents, we can still pass
the test.

Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
---
  tests/btrfs/276     | 41 ++++++++++++++++++++++++++++++++++++-----
  tests/btrfs/276.out |  4 ----
  2 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/tests/btrfs/276 b/tests/btrfs/276
index 944b0c8f..a63b28bb 100755
--- a/tests/btrfs/276
+++ b/tests/btrfs/276
@@ -65,10 +65,17 @@ count_not_shared_extents()

  # Create a 16G file as that results in 131072 extents, all with a
size of 128K
  # (due to compression), and a fs tree with a height of 3 (root node
at level 2).
+#
+# But due to writeback can happen halfway, we may have slightly more
extents
+# than 128K, so we allow 5% increase in the number of extents.
+#
  # We want to verify later that fiemap correctly reports the
sharedness of each
  # extent, even when it needs to switch from one leaf to the next
one and from a
  # node at level 1 to the next node at level 1.
  #
+nr_extents_lower=$((128 * 1024))
+nr_extents_upper=$((128 * 1024 + 128 * 1024 / 20))
+
  $XFS_IO_PROG -f -c "pwrite -b 8M 0 16G" $SCRATCH_MNT/foo |
_filter_xfs_io

Does adding '-s' (fsync after every write) to the $XFS_IO_PROG fixes
the issue?
On my test vm, it doesn't increase runtime by that much (16 to 23
seconds).

This indeed is much better, without dirty pages pressure we can go the
old golden output.

Unfortunately I still have a very low chance (~1/20) to hit 1~3 more
extent than the golden output.

There are still extra things like the commit intervals to let us to
writeback halfway.

The best situation would be direct IO, but unfortunately direct IO
doesn't support compression, thus the resulted file would lead to merged
fiemap results.

The other solution is to write between two files using direct IO, to
make each extent inside the same file not continuous with each other.

But that would lead to at least 512M * 2, and we also need to do the
same interleaved writes again for the 1M writes.

Any ideas would be appreciated.

Thanks,
Qu

Thanks,
Qu

I'd rather do that so that we can be sure fiemap is working correctly
and not returning more extents than there really are - this approach
of allowing a bit more allows for that type of bug to be unnoticed,
plus that little bit more might not be always enough (depending on
available rm, writeback settings, etc).

Thanks.


  # Sync to flush delalloc and commit the current transaction, so
fiemap will see
@@ -76,13 +83,22 @@ $XFS_IO_PROG -f -c "pwrite -b 8M 0 16G"
$SCRATCH_MNT/foo | _filter_xfs_io
  sync

  # All extents should be reported as non shared (131072 extents).
-echo "Number of non-shared extents in the whole file:
$(count_not_shared_extents)"
+found1=$(count_not_shared_extents)
+echo "Number of non-shared extents in the whole file: ${found1}" >>
$seqres.full
+
+if [ $found1 -lt $nr_extents_lower -o $found1 -gt $nr_extents_upper
]; then
+       echo "unexpected initial number of extents, has $found1
expect [$nr_extents_lower, $nr_extents_upper]"
+fi

  # Creating a snapshot.
  $BTRFS_UTIL_PROG subvolume snapshot $SCRATCH_MNT $SCRATCH_MNT/snap
| _filter_scratch

  # We have a snapshot, so now all extents should be reported as shared.
-echo "Number of shared extents in the whole file:
$(count_shared_extents)"
+found2=$(count_shared_extents)
+echo "Number of shared extents in the whole file: ${found2}" >>
$seqres.full
+if [ $found2 -ne $found1 ]; then
+       echo "unexpected shared extents, has $found2 expect $found1"
+fi

  # Now COW two file ranges, of 1M each, in the snapshot's file.
  # So 16 extents should become non-shared after this.
@@ -97,8 +113,18 @@ sync

  # Now we should have 16 non-shared extents and 131056 (131072 - 16)
shared
  # extents.
-echo "Number of non-shared extents in the whole file:
$(count_not_shared_extents)"
-echo "Number of shared extents in the whole file:
$(count_shared_extents)"
+found3=$(count_not_shared_extents)
+found4=$(count_shared_extents)
+echo "Number of non-shared extents in the whole file: ${found3}"
+echo "Number of shared extents in the whole file: ${found4}" >>
$seqres.full
+
+if [ $found3 != 16 ]; then
+       echo "Unexpected number of non-shared extents, has $found3
expect 16"
+fi
+
+if [ $found4 != $(( $found1 - $found3 )) ]; then
+       echo "Unexpected number of shared extents, has $found4 expect
$(( $found1 - $found3 ))"
+fi

  # Check that the non-shared extents are indeed in the expected file
ranges (each
  # with 8 extents).
@@ -117,7 +143,12 @@ _scratch_remount commit=1
  sleep 1.1

  # Now all extents should be reported as not shared (131072 extents).
-echo "Number of non-shared extents in the whole file:
$(count_not_shared_extents)"
+found5=$(count_not_shared_extents)
+echo "Number of non-shared extents in the whole file: ${found5}" >>
$seqres.full
+
+if [ $found5 != $found1 ]; then
+       echo "Unexpected final number of non-shared extents, has
$found5 expect $found1"
+fi

  # success, all done
  status=0
diff --git a/tests/btrfs/276.out b/tests/btrfs/276.out
index 3bf5a5e6..e318c2e9 100644
--- a/tests/btrfs/276.out
+++ b/tests/btrfs/276.out
@@ -1,16 +1,12 @@
  QA output created by 276
  wrote 17179869184/17179869184 bytes at offset 0
  XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Number of non-shared extents in the whole file: 131072
  Create a snapshot of 'SCRATCH_MNT' in 'SCRATCH_MNT/snap'
-Number of shared extents in the whole file: 131072
  wrote 1048576/1048576 bytes at offset 8388608
  XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
  wrote 1048576/1048576 bytes at offset 12884901888
  XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
  Number of non-shared extents in the whole file: 16
-Number of shared extents in the whole file: 131056
  Number of non-shared extents in range [8M, 9M): 8
  Number of non-shared extents in range [12G, 12G + 1M): 8
  Delete subvolume (commit): 'SCRATCH_MNT/snap'
-Number of non-shared extents in the whole file: 131072
--
2.41.0





[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux