All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: linux-xfs@vger.kernel.org, david@fromorbit.com
Subject: [RFC PATCH] xfs/179: modify test to trigger refcount update bugs
Date: Tue, 29 Nov 2022 14:06:39 -0800	[thread overview]
Message-ID: <Y4aCb+y2ej1TBE/R@magnolia> (raw)
In-Reply-To: <166975928548.3768925.15141817742859398250.stgit@magnolia>

From: Darrick J. Wong <djwong@kernel.org>

Upon enabling fsdax + reflink for XFS, this test began to report
refcount metadata corruptions after being run.  Specifically, xfs_repair
noticed single-block refcount records that could be combined but had not
been.

The root cause of this is improper MAXREFCOUNT edge case handling in
xfs_refcount_merge_extents.  When we're trying to find candidates for a
record merge, we compute the refcount of the merged record, but without
accounting for the fact that once a record hits rc_refcount ==
MAXREFCOUNT, it is pinned that way forever.

Adjust this test to use a sub-filesize write for one of the COW writes,
because this is how we force the extent merge code to run.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/xfs/179 |   28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/tests/xfs/179 b/tests/xfs/179
index ec0cb7e5b4..214558f694 100755
--- a/tests/xfs/179
+++ b/tests/xfs/179
@@ -21,17 +21,28 @@ _require_scratch_nocheck
 _require_cp_reflink
 _require_test_program "punch-alternating"
 
+_fixed_by_kernel_commit XXXXXXXXXXXX "xfs: estimate post-merge refcounts correctly"
+
 echo "Format and mount"
 _scratch_mkfs -d agcount=1 > $seqres.full 2>&1
 _scratch_mount >> $seqres.full 2>&1
 
+# This test modifies the refcount btree on the data device, so we must force
+# rtinherit off so that the test files are created there.
+_xfs_force_bdev data $SCRATCH_MNT
+
 testdir=$SCRATCH_MNT/test-$seq
 mkdir $testdir
 
+# Set the file size to 10x the block size to guarantee that the COW writes will
+# touch multiple blocks and exercise the refcount extent merging code.  This is
+# necessary to catch a bug in the refcount extent merging code that handles
+# MAXREFCOUNT edge cases.
 blksz=65536
+filesz=$((blksz * 10))
 
 echo "Create original files"
-_pwrite_byte 0x61 0 $blksz $testdir/file1 >> $seqres.full
+_pwrite_byte 0x61 0 $filesz $testdir/file1 >> $seqres.full
 _cp_reflink $testdir/file1 $testdir/file2 >> $seqres.full
 
 echo "Change reference count"
@@ -56,9 +67,20 @@ _scratch_xfs_db -c 'agf 0' -c 'addr refcntroot' -c 'p recs[1]' >> $seqres.full
 _scratch_mount >> $seqres.full
 
 echo "CoW a couple files"
-_pwrite_byte 0x62 0 $blksz $testdir/file3 >> $seqres.full
-_pwrite_byte 0x62 0 $blksz $testdir/file5 >> $seqres.full
+_pwrite_byte 0x62 0 $filesz $testdir/file3 >> $seqres.full
+_pwrite_byte 0x62 0 $filesz $testdir/file5 >> $seqres.full
+
+# For the last COW test, write single blocks at the start, middle, and end of
+# the shared file to exercise a refcount btree update that targets a single
+# block of the multiblock refcount record that we just modified.
+#
+# This trips a bug where XFS didn't correctly identify refcount record merge
+# candidates when any of the records are pinned at MAXREFCOUNT.  The bug was
+# originally discovered by enabling fsdax + reflink, but the bug can be
+# triggered by any COW that doesn't target the entire extent.
 _pwrite_byte 0x62 0 $blksz $testdir/file7 >> $seqres.full
+_pwrite_byte 0x62 $((blksz * 4)) $blksz $testdir/file7 >> $seqres.full
+_pwrite_byte 0x62 $((filesz - blksz)) $blksz $testdir/file7 >> $seqres.full
 
 echo "Check scratch fs"
 _scratch_unmount

  parent reply	other threads:[~2022-11-29 22:06 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-29 22:01 [PATCHSET 0/2] xfs: fix broken MAXREFCOUNT handling Darrick J. Wong
2022-11-29 22:01 ` [PATCH 1/2] xfs: hoist refcount record merge predicates Darrick J. Wong
2022-11-29 22:35   ` Dave Chinner
2022-11-30  0:13     ` Darrick J. Wong
2022-11-30  9:24   ` Yang, Xiao/杨 晓
2022-11-29 22:01 ` [PATCH 2/2] xfs: estimate post-merge refcounts correctly Darrick J. Wong
2022-11-29 22:37   ` Dave Chinner
2022-11-30  9:32   ` Yang, Xiao/杨 晓
2022-11-30 18:49     ` Darrick J. Wong
2022-11-29 22:06 ` Darrick J. Wong [this message]
2022-11-29 22:42   ` [RFC PATCH] xfs/179: modify test to trigger refcount update bugs Dave Chinner
2022-11-30  0:19     ` Darrick J. Wong
2022-11-30 10:07   ` Yang, Xiao/杨 晓

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y4aCb+y2ej1TBE/R@magnolia \
    --to=djwong@kernel.org \
    --cc=david@fromorbit.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.