Re: [PATCH] xfs/420: only check the extent layout after syncing

[Christoph Hellwig <hch@lst.de>]

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..

> 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..