diff mbox series

[3/4] xfs/420: fix occasional test failures due to pagecache readahead

Message ID 164740142033.3371628.11850774504699213977.stgit@magnolia (mailing list archive)
State New, archived
Headers show
Series fstests: random fixes | expand

Commit Message

Darrick J. Wong March 16, 2022, 3:30 a.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

Every now and then, this test fails with this golden output:

Comments

Zorro Lang March 22, 2022, 5:13 a.m. UTC | #1 
On Tue, Mar 15, 2022 at 08:30:20PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Every now and then, this test fails with this golden output:
> 
> --- xfs/420.out
> +++ xfs/420.out.bad
> @@ -29,7 +29,7 @@
>  Whence Result
>  DATA   0
>  HOLE   131072
> -DATA   196608
> +DATA   192512
>  HOLE   262144
>  Compare files
>  c2803804acc9936eef8aab42c119bfac  SCRATCH_MNT/test-420/file1

Looks like this part easy to cause `git am` misunderstanding[1], Hmm...
any method to deal with that?

[1]
Applying: xfs/420: fix occasional test failures due to pagecache readahead
error: 420.out: does not exist in index
Patch failed at 0001 xfs/420: fix occasional test failures due to pagecache readahead
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

> 
> Curiously, the file checksums always match, and it's not *forbidden* for
> the page cache to have a page backing an unwritten extent that hasn't
> been written.
> 
> The condition that this test cares about is that block 3 (192k-256k) are
> reported by SEEK_DATA as data even if the data fork has a hole and the
> COW fork has an unwritten extent.  Matthew Wilcox thinks this is a side
> effect of readahead.
> 
> To fix this occasional false failure, call SEEK_DATA and SEEK_HOLE only
> on the offsets that we care about.
> 
> Suggested-by: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  tests/xfs/420 |   33 +++++++++++++++++++++------------
>  1 file changed, 21 insertions(+), 12 deletions(-)
> 
> 
> diff --git a/tests/xfs/420 b/tests/xfs/420
> index 12b17588..d38772c9 100755
> --- a/tests/xfs/420
> +++ b/tests/xfs/420
> @@ -50,6 +50,24 @@ _scratch_mount >> $seqres.full 2>&1
>  testdir=$SCRATCH_MNT/test-$seq
>  mkdir $testdir
>  
> +# pagecache readahead can sometimes cause extra pages to be inserted into the
> +# file mapping where we have an unwritten extent in the COW fork.  Call lseek
> +# on each $blksz offset that interests us (as opposed to the whole file) so
> +# that these extra pages are not disclosed.
> +#
> +# The important thing we're testing is that SEEK_DATA reports block 3 as data
> +# when the COW fork has an unwritten mapping and the data fork has a hole.
> +exercise_lseek() {
> +	echo "Seek holes and data in file1"
> +	$XFS_IO_PROG -c "seek -d 0" $testdir/file1
> +	$XFS_IO_PROG -c "seek -h $((2 * blksz))" $testdir/file1 | sed -e '/Whence/d'
> +	echo "Seek holes and data in file2"
> +	$XFS_IO_PROG -c "seek -d 0" $testdir/file2
> +	$XFS_IO_PROG -c "seek -h $((2 * blksz))" $testdir/file2 | sed -e '/Whence/d'
> +	$XFS_IO_PROG -c "seek -d $((3 * blksz))" $testdir/file2 | sed -e '/Whence/d'
> +	$XFS_IO_PROG -c "seek -h $((4 * blksz))" $testdir/file2 | sed -e '/Whence/d'
> +}
> +
>  blksz=65536
>  nr=8
>  filesize=$((blksz * nr))
> @@ -83,10 +101,7 @@ $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file1 >> $seqres.full 2>&1
>  $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file2 >> $seqres.full 2>&1
>  $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file3 >> $seqres.full 2>&1
>  
> -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
> +exercise_lseek
>  
>  echo "Compare files"
>  md5sum $testdir/file1 | _filter_scratch
> @@ -102,10 +117,7 @@ $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file1 >> $seqres.full 2>&1
>  $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file2 >> $seqres.full 2>&1
>  $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file3 >> $seqres.full 2>&1
>  
> -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
> +exercise_lseek
>  
>  echo "Compare files"
>  md5sum $testdir/file1 | _filter_scratch
> @@ -121,10 +133,7 @@ $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file1 >> $seqres.full 2>&1
>  $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file2 >> $seqres.full 2>&1
>  $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file3 >> $seqres.full 2>&1
>  
> -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
> +exercise_lseek
>  
>  echo "Compare files"
>  md5sum $testdir/file1 | _filter_scratch
>
Darrick J. Wong March 22, 2022, 4:10 p.m. UTC | #2 
On Tue, Mar 22, 2022 at 01:13:54PM +0800, Zorro Lang wrote:
> On Tue, Mar 15, 2022 at 08:30:20PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Every now and then, this test fails with this golden output:
> > 
> > --- xfs/420.out
> > +++ xfs/420.out.bad
> > @@ -29,7 +29,7 @@
> >  Whence Result
> >  DATA   0
> >  HOLE   131072
> > -DATA   196608
> > +DATA   192512
> >  HOLE   262144
> >  Compare files
> >  c2803804acc9936eef8aab42c119bfac  SCRATCH_MNT/test-420/file1
> 
> Looks like this part easy to cause `git am` misunderstanding[1], Hmm...
> any method to deal with that?

LOL.  Um.  I guess I'll edit the commit message to omit the diff header.

--D

> [1]
> Applying: xfs/420: fix occasional test failures due to pagecache readahead
> error: 420.out: does not exist in index
> Patch failed at 0001 xfs/420: fix occasional test failures due to pagecache readahead
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> 
> > 
> > Curiously, the file checksums always match, and it's not *forbidden* for
> > the page cache to have a page backing an unwritten extent that hasn't
> > been written.
> > 
> > The condition that this test cares about is that block 3 (192k-256k) are
> > reported by SEEK_DATA as data even if the data fork has a hole and the
> > COW fork has an unwritten extent.  Matthew Wilcox thinks this is a side
> > effect of readahead.
> > 
> > To fix this occasional false failure, call SEEK_DATA and SEEK_HOLE only
> > on the offsets that we care about.
> > 
> > Suggested-by: Matthew Wilcox <willy@infradead.org>
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  tests/xfs/420 |   33 +++++++++++++++++++++------------
> >  1 file changed, 21 insertions(+), 12 deletions(-)
> > 
> > 
> > diff --git a/tests/xfs/420 b/tests/xfs/420
> > index 12b17588..d38772c9 100755
> > --- a/tests/xfs/420
> > +++ b/tests/xfs/420
> > @@ -50,6 +50,24 @@ _scratch_mount >> $seqres.full 2>&1
> >  testdir=$SCRATCH_MNT/test-$seq
> >  mkdir $testdir
> >  
> > +# pagecache readahead can sometimes cause extra pages to be inserted into the
> > +# file mapping where we have an unwritten extent in the COW fork.  Call lseek
> > +# on each $blksz offset that interests us (as opposed to the whole file) so
> > +# that these extra pages are not disclosed.
> > +#
> > +# The important thing we're testing is that SEEK_DATA reports block 3 as data
> > +# when the COW fork has an unwritten mapping and the data fork has a hole.
> > +exercise_lseek() {
> > +	echo "Seek holes and data in file1"
> > +	$XFS_IO_PROG -c "seek -d 0" $testdir/file1
> > +	$XFS_IO_PROG -c "seek -h $((2 * blksz))" $testdir/file1 | sed -e '/Whence/d'
> > +	echo "Seek holes and data in file2"
> > +	$XFS_IO_PROG -c "seek -d 0" $testdir/file2
> > +	$XFS_IO_PROG -c "seek -h $((2 * blksz))" $testdir/file2 | sed -e '/Whence/d'
> > +	$XFS_IO_PROG -c "seek -d $((3 * blksz))" $testdir/file2 | sed -e '/Whence/d'
> > +	$XFS_IO_PROG -c "seek -h $((4 * blksz))" $testdir/file2 | sed -e '/Whence/d'
> > +}
> > +
> >  blksz=65536
> >  nr=8
> >  filesize=$((blksz * nr))
> > @@ -83,10 +101,7 @@ $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file1 >> $seqres.full 2>&1
> >  $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file2 >> $seqres.full 2>&1
> >  $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file3 >> $seqres.full 2>&1
> >  
> > -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
> > +exercise_lseek
> >  
> >  echo "Compare files"
> >  md5sum $testdir/file1 | _filter_scratch
> > @@ -102,10 +117,7 @@ $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file1 >> $seqres.full 2>&1
> >  $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file2 >> $seqres.full 2>&1
> >  $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file3 >> $seqres.full 2>&1
> >  
> > -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
> > +exercise_lseek
> >  
> >  echo "Compare files"
> >  md5sum $testdir/file1 | _filter_scratch
> > @@ -121,10 +133,7 @@ $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file1 >> $seqres.full 2>&1
> >  $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file2 >> $seqres.full 2>&1
> >  $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file3 >> $seqres.full 2>&1
> >  
> > -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
> > +exercise_lseek
> >  
> >  echo "Compare files"
> >  md5sum $testdir/file1 | _filter_scratch
> > 
>
diff mbox series

Patch

--- xfs/420.out
+++ xfs/420.out.bad
@@ -29,7 +29,7 @@ 
 Whence Result
 DATA   0
 HOLE   131072
-DATA   196608
+DATA   192512
 HOLE   262144
 Compare files
 c2803804acc9936eef8aab42c119bfac  SCRATCH_MNT/test-420/file1

Curiously, the file checksums always match, and it's not *forbidden* for
the page cache to have a page backing an unwritten extent that hasn't
been written.

The condition that this test cares about is that block 3 (192k-256k) are
reported by SEEK_DATA as data even if the data fork has a hole and the
COW fork has an unwritten extent.  Matthew Wilcox thinks this is a side
effect of readahead.

To fix this occasional false failure, call SEEK_DATA and SEEK_HOLE only
on the offsets that we care about.

Suggested-by: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/xfs/420 |   33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)


diff --git a/tests/xfs/420 b/tests/xfs/420
index 12b17588..d38772c9 100755
--- a/tests/xfs/420
+++ b/tests/xfs/420
@@ -50,6 +50,24 @@  _scratch_mount >> $seqres.full 2>&1
 testdir=$SCRATCH_MNT/test-$seq
 mkdir $testdir
 
+# pagecache readahead can sometimes cause extra pages to be inserted into the
+# file mapping where we have an unwritten extent in the COW fork.  Call lseek
+# on each $blksz offset that interests us (as opposed to the whole file) so
+# that these extra pages are not disclosed.
+#
+# The important thing we're testing is that SEEK_DATA reports block 3 as data
+# when the COW fork has an unwritten mapping and the data fork has a hole.
+exercise_lseek() {
+	echo "Seek holes and data in file1"
+	$XFS_IO_PROG -c "seek -d 0" $testdir/file1
+	$XFS_IO_PROG -c "seek -h $((2 * blksz))" $testdir/file1 | sed -e '/Whence/d'
+	echo "Seek holes and data in file2"
+	$XFS_IO_PROG -c "seek -d 0" $testdir/file2
+	$XFS_IO_PROG -c "seek -h $((2 * blksz))" $testdir/file2 | sed -e '/Whence/d'
+	$XFS_IO_PROG -c "seek -d $((3 * blksz))" $testdir/file2 | sed -e '/Whence/d'
+	$XFS_IO_PROG -c "seek -h $((4 * blksz))" $testdir/file2 | sed -e '/Whence/d'
+}
+
 blksz=65536
 nr=8
 filesize=$((blksz * nr))
@@ -83,10 +101,7 @@  $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file1 >> $seqres.full 2>&1
 $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file2 >> $seqres.full 2>&1
 $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file3 >> $seqres.full 2>&1
 
-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
+exercise_lseek
 
 echo "Compare files"
 md5sum $testdir/file1 | _filter_scratch
@@ -102,10 +117,7 @@  $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file1 >> $seqres.full 2>&1
 $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file2 >> $seqres.full 2>&1
 $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file3 >> $seqres.full 2>&1
 
-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
+exercise_lseek
 
 echo "Compare files"
 md5sum $testdir/file1 | _filter_scratch
@@ -121,10 +133,7 @@  $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file1 >> $seqres.full 2>&1
 $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file2 >> $seqres.full 2>&1
 $XFS_IO_PROG -c "bmap -ev" -c "bmap -cv" $testdir/file3 >> $seqres.full 2>&1
 
-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
+exercise_lseek
 
 echo "Compare files"
 md5sum $testdir/file1 | _filter_scratch