On 2023/8/24 06:27, Darrick J. Wong wrote:
On Wed, Aug 23, 2023 at 05:18:02PM -0500, Bill O'Donnell wrote:
On Wed, Aug 23, 2023 at 09:46:41AM -0700, Darrick J. Wong wrote:
On Wed, Aug 23, 2023 at 10:43:50AM -0500, Bill O'Donnell wrote:
xfs_io pwrite issues a series of block size writes, but there is no guarantee
that the resulting extent(s) will be singular or contiguous.
However this doesn't make much difference, at least for btrfs.
Btrfs would do the merging emitting the fiemap entry, thus even if the
write didn't result a singular extent, as long as they are contiguous
(under most cases they are) the fiemap result would still be a single one.
This behavior is
acceptable, but the test is flawed in that it expects a single extent for a
pwrite.
I'm more interested in if you're hitting any test failure?
Modify test to accept any layout for the reflinked logical range.
Signed-off-by: Bill O'Donnell <bodonnel@xxxxxxxxxx>
---
tests/generic/352 | 16 +++++++++++-----
tests/generic/352.out | 2 --
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/tests/generic/352 b/tests/generic/352
index 52ec4850..c4ee8a44 100755
--- a/tests/generic/352
+++ b/tests/generic/352
@@ -48,19 +48,25 @@ _pwrite_byte 0xcdcdcdcd 0 $blocksize $file | _filter_xfs_io
# use reflink to create the rest of the file, whose all extents are all
# pointing to the first extent
for i in $(seq 1 $nr); do
- _reflink_range $file 0 $file $(($i * $blocksize)) $blocksize > /dev/null
+ _reflink_range $file 0 $file $(($i * $blocksize)) $blocksize > $tmp1.out
$tmp1 isnt defined anywhere.
done
# then call fiemap on that file to test both the shared flag and if
# reserved extent mapping search will cause soft lockup
-$XFS_IO_PROG -c "fiemap -v" $file | _filter_fiemap_flags > $tmp.out
-cat $tmp.out >> $seqres.full
+$XFS_IO_PROG -c "fiemap -v" $file | _filter_fiemap_flags > $tmp2.out
+cat $tmp2.out >> $seqres.full
Nor is $tmp2
# refact the $LOAD_FACTOR to 1 to match the golden output
sed -i -e "s/$(($last_extent - 1))/$(($orig_last_extent - 1))/" \
-e "s/$last_extent/$orig_last_extent/" \
- -e "s/$end/$orig_end/" $tmp.out
-cat $tmp.out
+ -e "s/$end/$orig_end/" $tmp2.out
+
+cat $tmp1.out > tmp.1
+cat $tmp2.out > tmp.2
Not sure why you didn't make the _reflink_range and the fiemap above
output to $tmp.out1 and $tmp.out2, respectively. If you had, then the
default _cleanup would delete $tmp.* automatically...
+
+diff tmp.[12]
+rm tmp.1
+rm tmp.2
...and the rm here wouldn't be necessary.
Ok. Nitpicking over. Moving on to the weirder design questions of the
original test:
[add original test author to cc]
Emails to quwenruo@xxxxxxxxxxxxxx seem to be undeliverable. Maybe Joseph
would know what btrfs intent was?
...or I guess I could have used the current email addr instead of the
one on the commit. :(
Qu: Question for you:
Thanks a lot for referring it to me.
I don't know why $blocksize is set to 128k above. If this test needs to
guarantee that there would only be *one* extent (and the golden output
implies this as you note), then it should have been written to say:
blocksize=$(_get_file_block_size $SCRATCH_MNT)
But I don't know if the "btrfs soft lock up and return wrong shared
flag" behavior required sharing a (probably multi-block) 128k range, or
if that was simply what the author selected because it reproduced the
problem.
It's quite sometime ago, thus my memory may not be reliable, but IIRC
the blocksize has no specific requirement other than allowing all
possible blocksize (4K to 64K).
And at that time, at least I was preferring to use golden output to
detect errors, thus I choose a larger blocksize to allow all blocksizes
to work.
Any thoughts?
--D
# success, all done
status=0
diff --git a/tests/generic/352.out b/tests/generic/352.out
index 4ff66c21..ad90ae0d 100644
--- a/tests/generic/352.out
+++ b/tests/generic/352.out
@@ -1,5 +1,3 @@
QA output created by 352
wrote 131072/131072 bytes at offset 0
XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-0: [0..2097151]: shared
-1: [2097152..2097407]: shared|last
Also I suspect from the test description that the goal here was to
detect the golden output failing because the shared flag does not get
reported correctly.
Could explain more on why the shared flag detection is not correct here?
If a file extent is shared, no matter if it's shared by another inode or
not, shouldn't it be marked with SHARED flag?
Thanks,
Qu
--D
--
2.41.0
