Linux kernel -stable discussions
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: djwong@kernel.org
Cc: stable@vger.kernel.org, linux-xfs@vger.kernel.org
Subject: [PATCH 11/21] xfs: update btree keys correctly when _insrec splits an inode root block
Date: Mon, 25 Nov 2024 17:27:34 -0800	[thread overview]
Message-ID: <173258397991.4032920.4586526854197814179.stgit@frogsfrogsfrogs> (raw)
In-Reply-To: <173258397748.4032920.4159079744952779287.stgit@frogsfrogsfrogs>

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

In commit 2c813ad66a72, I partially fixed a bug wherein xfs_btree_insrec
would erroneously try to update the parent's key for a block that had
been split if we decided to insert the new record into the new block.
The solution was to detect this situation and update the in-core key
value that we pass up to the caller so that the caller will (eventually)
add the new block to the parent level of the tree with the correct key.

However, I missed a subtlety about the way inode-rooted btrees work.  If
the full block was a maximally sized inode root block, we'll solve that
fullness by moving the root block's records to a new block, resizing the
root block, and updating the root to point to the new block.  We don't
pass a pointer to the new block to the caller because that work has
already been done.  The new record will /always/ land in the new block,
so in this case we need to use xfs_btree_update_keys to update the keys.

This bug can theoretically manifest itself in the very rare case that we
split a bmbt root block and the new record lands in the very first slot
of the new block, though I've never managed to trigger it in practice.
However, it is very easy to reproduce by running generic/522 with the
realtime rmapbt patchset if rtinherit=1.

Cc: <stable@vger.kernel.org> # v4.8
Fixes: 2c813ad66a7218 ("xfs: support btrees with overlapping intervals for keys")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_btree.c |   29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index c748866ef92368..68ee1c299c25fd 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -3557,14 +3557,31 @@ xfs_btree_insrec(
 	xfs_btree_log_block(cur, bp, XFS_BB_NUMRECS);
 
 	/*
-	 * If we just inserted into a new tree block, we have to
-	 * recalculate nkey here because nkey is out of date.
+	 * Update btree keys to reflect the newly added record or keyptr.
+	 * There are three cases here to be aware of.  Normally, all we have to
+	 * do is walk towards the root, updating keys as necessary.
 	 *
-	 * Otherwise we're just updating an existing block (having shoved
-	 * some records into the new tree block), so use the regular key
-	 * update mechanism.
+	 * If the caller had us target a full block for the insertion, we dealt
+	 * with that by calling the _make_block_unfull function.  If the
+	 * "make unfull" function splits the block, it'll hand us back the key
+	 * and pointer of the new block.  We haven't yet added the new block to
+	 * the next level up, so if we decide to add the new record to the new
+	 * block (bp->b_bn != old_bn), we have to update the caller's pointer
+	 * so that the caller adds the new block with the correct key.
+	 *
+	 * However, there is a third possibility-- if the selected block is the
+	 * root block of an inode-rooted btree and cannot be expanded further,
+	 * the "make unfull" function moves the root block contents to a new
+	 * block and updates the root block to point to the new block.  In this
+	 * case, no block pointer is passed back because the block has already
+	 * been added to the btree.  In this case, we need to use the regular
+	 * key update function, just like the first case.  This is critical for
+	 * overlapping btrees, because the high key must be updated to reflect
+	 * the entire tree, not just the subtree accessible through the first
+	 * child of the root (which is now two levels down from the root).
 	 */
-	if (bp && xfs_buf_daddr(bp) != old_bn) {
+	if (!xfs_btree_ptr_is_null(cur, &nptr) &&
+	    bp && xfs_buf_daddr(bp) != old_bn) {
 		xfs_btree_get_keys(cur, block, lkey);
 	} else if (xfs_btree_needs_key_update(cur, optr)) {
 		error = xfs_btree_update_keys(cur, level);


  parent reply	other threads:[~2024-11-26  1:27 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20241126011838.GI9438@frogsfrogsfrogs>
2024-11-26  1:20 ` [PATCHSET] xfs: bug fixes for 6.13 Darrick J. Wong
2024-11-26  1:24   ` [PATCH 01/21] xfs: fix off-by-one error in fsmap's end_daddr usage Darrick J. Wong
2024-11-26  1:25   ` [PATCH 04/21] xfs: return a 64-bit block count from xfs_btree_count_blocks Darrick J. Wong
2024-11-26  1:26   ` [PATCH 05/21] xfs: don't drop errno values when we fail to ficlone the entire range Darrick J. Wong
2024-11-26  1:26   ` [PATCH 06/21] xfs: separate healthy clearing mask during repair Darrick J. Wong
2024-11-26  1:26   ` [PATCH 07/21] xfs: set XFS_SICK_INO_SYMLINK_ZAPPED explicitly when zapping a symlink Darrick J. Wong
2024-11-26  1:27   ` [PATCH 09/21] xfs: fix null bno_hint handling in xfs_rtallocate_rtg Darrick J. Wong
2024-11-26  1:27   ` Darrick J. Wong [this message]
2024-11-26  5:01     ` [PATCH 11/21] xfs: update btree keys correctly when _insrec splits an inode root block Christoph Hellwig
2024-11-26  1:27   ` [PATCH 12/21] xfs: fix scrub tracepoints when inode-rooted btrees are involved Darrick J. Wong
2024-11-26  5:01     ` Christoph Hellwig
2024-11-26  1:28   ` [PATCH 13/21] xfs: unlock inodes when erroring out of xfs_trans_alloc_dir Darrick J. Wong
2024-11-26  5:03     ` Christoph Hellwig
2024-11-26  1:28   ` [PATCH 14/21] xfs: only run precommits once per transaction object Darrick J. Wong
2024-11-26  5:09     ` Christoph Hellwig
2024-11-26  1:28   ` [PATCH 16/21] xfs: don't lose solo superblock counter update transactions Darrick J. Wong
2024-11-26  5:14     ` Christoph Hellwig
2024-11-26 18:23       ` Darrick J. Wong
2024-11-26  1:29   ` [PATCH 17/21] xfs: don't lose solo dquot " Darrick J. Wong
2024-11-26  5:18     ` Christoph Hellwig
2024-11-26 18:23       ` Darrick J. Wong
2024-11-26  1:29   ` [PATCH 20/21] xfs: attach dquot buffer to dquot log item buffer Darrick J. Wong
2024-11-26  5:42     ` Christoph Hellwig
2024-11-26  1:30   ` [PATCH 21/21] xfs: convert quotacheck to attach dquot buffers Darrick J. Wong
2024-11-26  5:42     ` Christoph Hellwig
2024-12-17  2:06     ` Lai, Yi

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=173258397991.4032920.4586526854197814179.stgit@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=stable@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox