* xfs/420 and xfs/421: don't disturb unwritten status with md5sum [not found] <20190218091827.12619-1-hch@lst.de> @ 2019-02-18 9:19 ` Christoph Hellwig 2019-03-09 10:32 ` Eryu Guan 0 siblings, 1 reply; 3+ messages in thread From: Christoph Hellwig @ 2019-02-18 9:19 UTC (permalink / raw) To: linux-xfs, fstests 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@lst.de> --- 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 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: xfs/420 and xfs/421: don't disturb unwritten status with md5sum 2019-02-18 9:19 ` xfs/420 and xfs/421: don't disturb unwritten status with md5sum Christoph Hellwig @ 2019-03-09 10:32 ` Eryu Guan 2019-03-09 17:34 ` Darrick J. Wong 0 siblings, 1 reply; 3+ messages in thread From: Eryu Guan @ 2019-03-09 10:32 UTC (permalink / raw) To: Christoph Hellwig, Darrick J. Wong; +Cc: linux-xfs, fstests 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@lst.de> 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! 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 > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: xfs/420 and xfs/421: don't disturb unwritten status with md5sum 2019-03-09 10:32 ` Eryu Guan @ 2019-03-09 17:34 ` Darrick J. Wong 0 siblings, 0 replies; 3+ messages in thread From: Darrick J. Wong @ 2019-03-09 17:34 UTC (permalink / raw) To: Eryu Guan; +Cc: Christoph Hellwig, linux-xfs, fstests 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@lst.de> > > 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@oracle.com> --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 > > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-03-09 17:34 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20190218091827.12619-1-hch@lst.de>
2019-02-18 9:19 ` xfs/420 and xfs/421: don't disturb unwritten status with md5sum Christoph Hellwig
2019-03-09 10:32 ` Eryu Guan
2019-03-09 17:34 ` Darrick J. Wong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox