On Tue, Feb 12, 2019 at 08:00:22PM +0100, Christoph Hellwig wrote: > On Mon, Feb 11, 2019 at 03:51:44PM -0800, Darrick J. Wong wrote: > > > The comparism pass before the sync might see an "error" if we use COW > > > fork speculative preallocations for non-overwrites, which is useful to > > > reduce fragmentation. > > > > What error do you see? > > -- /root/xfstests/tests/xfs/420.out 2019-02-12 15:41:12.202606228 +0000 > +++ /root/xfstests/results//xfs/420.out.bad 2019-02-12 18:58:06.158426573 > +0000 > @@ -14,8 +14,6 @@ > Whence Result > DATA 0 > HOLE 131072 > -DATA 196608 > -HOLE 262144 > Compare files > c2803804acc9936eef8aab42c119bfac SCRATCH_MNT/test-420/file1 > 017c08a9320aad844ce86aa9631afb98 SCRATCH_MNT/test-420/file2 > > > > > -echo "Seek holes and data in file1" > > > -$XFS_IO_PROG -c "seek -a -r 0" $testdir/file1 > > > -echo "Seek holes and data in file2" > > > -$XFS_IO_PROG -c "seek -a -r 0" $testdir/file2 > > > > This removed code tests that the earlier write of 64k of data into file2 > > between 192k and 256k can be found by SEEK_DATA before file2 gets > > sync'd to disk. > > Well, and it might not be able to be found if it is in the COW fork.. But why would data only be able to be found in the COW fork? Does this test failure happen for you on a regular xfs filesystem, or does it only happen with the alwayscow patchset? If yes, are unfsync'd O_ATOMIC writes to holes invisible to seek_data until fsync finishes? > > And this removed code checks that the page cache contents remain stable > > and correct even for a write that goes through the COW mechanism. > > > > I don't see why it's advantageous to remove this part of the test? > > Last time I send a patch to just add a sync and got the recommendation > to just remove the double tests before and after the sync.. Yes, and I think Eryu is wrong. We wrote data into a hole between 192k and 256k, and SEEK_DATA should be able to find that data (it's readable from the page cache, after all) even if we haven't yet fsync'd the file. --D