On Sat, Mar 09, 2019 at 06:32:11PM +0800, Eryu Guan wrote: > On Mon, Feb 18, 2019 at 10:19:51AM +0100, Christoph Hellwig wrote: > > The way we decided if an unwritten extent is considered a hole or data > > is by checking if the page and/or blocks are marked uptodate, that is > > contain valid data in the page cache. > > > > xfs/420 and xfs/421 try to exercise SEEK_HOLE / SEEK_DATA in the > > presence of cowextsize preallocations over holes in the data fork. The > > current XFS code never actually uses those for buffer writes, but a > > pending patch changes that. For SEEK_HOLE / SEEK_DATA to work properly > > in that case we also need to look at the COW fork in their > > implementations and thus have to rely on the unwritten extent page cache > > probing. But the tests for it ensure we do have valid data in the > > pagecache by calling md5sum on the test files, and thus reading their > > contents (including the zero-filled holes) in, and thus making them > > all valid data. > > > > Fix that by dropping the page cache content after the md5sum calls. > > > > Signed-off-by: Christoph Hellwig <hch@xxxxxx> > > Hi Darrick, would you please help review this version of the fix as > well? I basically have no idea about the implementation of the "pending > patch" and what it changes.. Thanks a lot! /me wonders if the "md5sum and drop caches" could be refactored a bit, but as a strict bugfix for always_cow mode this looks ok to me: Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> --D > Eryu > > > --- > > tests/xfs/420 | 6 ++++++ > > tests/xfs/421 | 6 ++++++ > > 2 files changed, 12 insertions(+) > > > > diff --git a/tests/xfs/420 b/tests/xfs/420 > > index a083a12b..34199637 100755 > > --- a/tests/xfs/420 > > +++ b/tests/xfs/420 > > @@ -83,6 +83,8 @@ echo "Compare files" > > md5sum $testdir/file1 | _filter_scratch > > md5sum $testdir/file2 | _filter_scratch > > md5sum $testdir/file3 | _filter_scratch > > +# drop caches to make sure the page cache for the unwritten extents is clean > > +echo 1 > /proc/sys/vm/drop_caches > > > > echo "CoW the shared part then write into the empty part" | tee -a $seqres.full > > $XFS_IO_PROG -c "cowextsize" $testdir/file1 >> $seqres.full > > @@ -106,6 +108,8 @@ echo "Compare files" > > md5sum $testdir/file1 | _filter_scratch > > md5sum $testdir/file2 | _filter_scratch > > md5sum $testdir/file3 | _filter_scratch > > +# drop caches to make sure the page cache for the unwritten extents is clean > > +echo 1 > /proc/sys/vm/drop_caches > > > > echo "sync filesystem" | tee -a $seqres.full > > sync > > @@ -123,6 +127,8 @@ echo "Compare files" > > md5sum $testdir/file1 | _filter_scratch > > md5sum $testdir/file2 | _filter_scratch > > md5sum $testdir/file3 | _filter_scratch > > +# drop caches to make sure the page cache for the unwritten extents is clean > > +echo 1 > /proc/sys/vm/drop_caches > > > > echo "Remount" | tee -a $seqres.full > > _scratch_cycle_mount > > diff --git a/tests/xfs/421 b/tests/xfs/421 > > index a2734aba..374389bd 100755 > > --- a/tests/xfs/421 > > +++ b/tests/xfs/421 > > @@ -83,6 +83,8 @@ echo "Compare files" > > md5sum $testdir/file1 | _filter_scratch > > md5sum $testdir/file2 | _filter_scratch > > md5sum $testdir/file3 | _filter_scratch > > +# drop caches to make sure the page cache for the unwritten extents is clean > > +echo 1 > /proc/sys/vm/drop_caches > > > > echo "CoW the shared part then write into the empty part" | tee -a $seqres.full > > $XFS_IO_PROG -c "cowextsize" $testdir/file1 >> $seqres.full > > @@ -106,6 +108,8 @@ echo "Compare files" > > md5sum $testdir/file1 | _filter_scratch > > md5sum $testdir/file2 | _filter_scratch > > md5sum $testdir/file3 | _filter_scratch > > +# drop caches to make sure the page cache for the unwritten extents is clean > > +echo 1 > /proc/sys/vm/drop_caches > > > > echo "sync filesystem" | tee -a $seqres.full > > sync > > @@ -123,6 +127,8 @@ echo "Compare files" > > md5sum $testdir/file1 | _filter_scratch > > md5sum $testdir/file2 | _filter_scratch > > md5sum $testdir/file3 | _filter_scratch > > +# drop caches to make sure the page cache for the unwritten extents is clean > > +echo 1 > /proc/sys/vm/drop_caches > > > > echo "Remount" | tee -a $seqres.full > > _scratch_cycle_mount > > -- > > 2.20.1 > >
