From: "Darrick J. Wong" <djwong@kernel.org>
To: Eric Sandeen <sandeen@redhat.com>
Cc: linux-xfs@vger.kernel.org, cem@kernel.org, hch@lst.de, dgc@kernel.org
Subject: Re: [PATCH 1/8] xfs: remove redundant stat checks after xfs_btree_insert()
Date: Fri, 16 Oct 2026 12:44:41 -0700 [thread overview]
Message-ID: <20261016194441.GC107986@frogsfrogsfrogs> (raw)
In-Reply-To: <20261016193422.1630907-2-sandeen@redhat.com>
On Fri, Oct 16, 2026 at 02:29:01PM -0500, Eric Sandeen wrote:
> xfs_btree_insert() validates its own status internally via the result
> of xfs_btree_insrec(). When it returns 0 it guarantees the status value
> is 1; on any error it returns a nonzero error and does not update the
> status value, so on error the status value is meaningless and on success
> the status value does not need to be tested.
>
> This makes every instance of the 2nd test in this idiom:
>
> if ((error = xfs_btree_insert(cnt_cur, &i)))
> return error;
> if (XFS_IS_CORRUPT(mp, i != 1)) {
> xfs_btree_mark_sick(cnt_cur);
> return -EFSCORRUPTED;
> }
>
> unreachable on an error, and always false on success. The mark_sick and
> error return cannot be reached, so remove all that.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Oh hey, I hadn't even realized there were all these redundant checks!
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> ---
> fs/xfs/libxfs/xfs_alloc.c | 22 ---------------
> fs/xfs/libxfs/xfs_bmap.c | 54 ------------------------------------
> fs/xfs/libxfs/xfs_refcount.c | 9 ------
> fs/xfs/libxfs/xfs_rmap.c | 32 ---------------------
> 4 files changed, 117 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index ae221a158764..b40c51a0f651 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -694,10 +694,6 @@ xfs_alloc_fixup_trees(
> }
> if ((error = xfs_btree_insert(cnt_cur, &i)))
> return error;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cnt_cur);
> - return -EFSCORRUPTED;
> - }
> }
> if (nfbno2 != NULLAGBLOCK) {
> if ((error = xfs_alloc_lookup_eq(cnt_cur, nfbno2, nflen2, &i)))
> @@ -708,10 +704,6 @@ xfs_alloc_fixup_trees(
> }
> if ((error = xfs_btree_insert(cnt_cur, &i)))
> return error;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cnt_cur);
> - return -EFSCORRUPTED;
> - }
> }
> /*
> * Fix up the by-block btree entry(s).
> @@ -745,10 +737,6 @@ xfs_alloc_fixup_trees(
> }
> if ((error = xfs_btree_insert(bno_cur, &i)))
> return error;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(bno_cur);
> - return -EFSCORRUPTED;
> - }
> }
>
> if (fixup_longest)
> @@ -2314,11 +2302,6 @@ xfs_free_ag_extent(
> nlen = len;
> if ((error = xfs_btree_insert(bno_cur, &i)))
> goto error0;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(bno_cur);
> - error = -EFSCORRUPTED;
> - goto error0;
> - }
> }
> xfs_btree_del_cursor(bno_cur, XFS_BTREE_NOERROR);
> bno_cur = NULL;
> @@ -2341,11 +2324,6 @@ xfs_free_ag_extent(
> fixup_longest = true;
> if ((error = xfs_btree_insert(cnt_cur, &i)))
> goto error0;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cnt_cur);
> - error = -EFSCORRUPTED;
> - goto error0;
> - }
> if (fixup_longest) {
> error = xfs_alloc_fixup_longest(cnt_cur);
> if (error)
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index ca49bf4a50c5..073c11e631ea 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1674,11 +1674,6 @@ xfs_bmap_add_extent_delay_real(
> error = xfs_btree_insert(bma->cur, &i);
> if (error)
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(bma->cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> }
> ASSERT(da_new <= da_old);
> break;
> @@ -1745,11 +1740,6 @@ xfs_bmap_add_extent_delay_real(
> error = xfs_btree_insert(bma->cur, &i);
> if (error)
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(bma->cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> }
>
> if (xfs_bmap_needs_btree(bma->ip, whichfork)) {
> @@ -1836,11 +1826,6 @@ xfs_bmap_add_extent_delay_real(
> error = xfs_btree_insert(bma->cur, &i);
> if (error)
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(bma->cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> }
>
> if (xfs_bmap_needs_btree(bma->ip, whichfork)) {
> @@ -1925,11 +1910,6 @@ xfs_bmap_add_extent_delay_real(
> error = xfs_btree_insert(bma->cur, &i);
> if (error)
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(bma->cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> }
>
> if (xfs_bmap_needs_btree(bma->ip, whichfork)) {
> @@ -2350,11 +2330,6 @@ xfs_bmap_add_extent_unwritten_real(
> cur->bc_rec.b = *new;
> if ((error = xfs_btree_insert(cur, &i)))
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> }
> break;
>
> @@ -2436,11 +2411,6 @@ xfs_bmap_add_extent_unwritten_real(
> }
> if ((error = xfs_btree_insert(cur, &i)))
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> }
> break;
>
> @@ -2486,11 +2456,6 @@ xfs_bmap_add_extent_unwritten_real(
> cur->bc_rec.b = PREV;
> if ((error = xfs_btree_insert(cur, &i)))
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> /*
> * Reset the cursor to the position of the new extent
> * we are about to insert as we can't trust it after
> @@ -2507,11 +2472,6 @@ xfs_bmap_add_extent_unwritten_real(
> /* new middle extent - newext */
> if ((error = xfs_btree_insert(cur, &i)))
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> }
> break;
>
> @@ -2767,11 +2727,6 @@ xfs_bmap_add_extent_hole_real(
> error = xfs_btree_insert(cur, &i);
> if (error)
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> }
> break;
> }
> @@ -5093,10 +5048,6 @@ xfs_bmap_del_extent_real(
> *logflagsp = 0;
> return -ENOSPC;
> }
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - return -EFSCORRUPTED;
> - }
> } else
> *logflagsp |= xfs_ilog_fext(whichfork);
>
> @@ -5958,11 +5909,6 @@ xfs_bmap_split_extent(
> error = xfs_btree_insert(cur, &i);
> if (error)
> goto del_cursor;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto del_cursor;
> - }
> }
>
> /*
> diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
> index 6b67308fa594..49e9993ce360 100644
> --- a/fs/xfs/libxfs/xfs_refcount.c
> +++ b/fs/xfs/libxfs/xfs_refcount.c
> @@ -276,15 +276,6 @@ xfs_refcount_insert(
> cur->bc_rec.rc.rc_domain = irec->rc_domain;
>
> error = xfs_btree_insert(cur, i);
> - if (error)
> - goto out_error;
> - if (XFS_IS_CORRUPT(cur->bc_mp, *i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto out_error;
> - }
> -
> -out_error:
> if (error)
> trace_xfs_refcount_insert_error(cur, error, _RET_IP_);
> return error;
> diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
> index 14aef87837a9..ff4aef8d30a7 100644
> --- a/fs/xfs/libxfs/xfs_rmap.c
> +++ b/fs/xfs/libxfs/xfs_rmap.c
> @@ -146,13 +146,6 @@ xfs_rmap_insert(
> rcur->bc_rec.r.rm_offset = offset;
> rcur->bc_rec.r.rm_flags = flags;
> error = xfs_btree_insert(rcur, &i);
> - if (error)
> - goto done;
> - if (XFS_IS_CORRUPT(rcur->bc_mp, i != 1)) {
> - xfs_btree_mark_sick(rcur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> done:
> if (error)
> trace_xfs_rmap_insert_error(rcur, error, _RET_IP_);
> @@ -1185,11 +1178,6 @@ xfs_rmap_map(
> error = xfs_btree_insert(cur, &i);
> if (error)
> goto out_error;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto out_error;
> - }
> }
>
> trace_xfs_rmap_map_done(cur, bno, len, unwritten, oinfo);
> @@ -1592,11 +1580,6 @@ xfs_rmap_convert(
> error = xfs_btree_insert(cur, &i);
> if (error)
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> break;
>
> case RMAP_RIGHT_FILLING | RMAP_RIGHT_CONTIG:
> @@ -1650,11 +1633,6 @@ xfs_rmap_convert(
> error = xfs_btree_insert(cur, &i);
> if (error)
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> break;
>
> case 0:
> @@ -1683,11 +1661,6 @@ xfs_rmap_convert(
> error = xfs_btree_insert(cur, &i);
> if (error)
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> /*
> * Reset the cursor to the position of the new extent
> * we are about to insert as we can't trust it after
> @@ -1709,11 +1682,6 @@ xfs_rmap_convert(
> error = xfs_btree_insert(cur, &i);
> if (error)
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> break;
>
> case RMAP_LEFT_FILLING | RMAP_LEFT_CONTIG | RMAP_RIGHT_CONTIG:
> --
> 2.55.0
>
>
next prev parent reply other threads:[~2026-10-16 19:44 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-16 19:29 [PATCH 0/8] xfs: consolidate btree insert/delete checks Eric Sandeen
2026-10-16 19:29 ` [PATCH 1/8] xfs: remove redundant stat checks after xfs_btree_insert() Eric Sandeen
2026-10-16 19:44 ` Darrick J. Wong [this message]
2026-10-16 19:29 ` [PATCH 2/8] xfs: remove redundant stat checks after xfs_refcount_insert() Eric Sandeen
2026-10-16 19:44 ` Darrick J. Wong
2026-10-16 19:29 ` [PATCH 3/8] xfs: remove redundant stat checks after xfs_inobt_insert_rec() Eric Sandeen
2026-10-16 19:45 ` Darrick J. Wong
2026-10-16 19:29 ` [PATCH 4/8] xfs: remove redundant stat check after rcbagbt_insert() Eric Sandeen
2026-10-16 19:45 ` Darrick J. Wong
2026-10-16 19:29 ` [PATCH 5/8] xfs: remove status parameter from several btree insert functions Eric Sandeen
2026-10-16 19:46 ` Darrick J. Wong
2026-10-16 19:29 ` [PATCH 6/8] xfs: add stat check to xfs_btree_delete and remove caller checks Eric Sandeen
2026-10-16 19:47 ` Darrick J. Wong
2026-10-16 19:29 ` [PATCH 7/8] xfs: remove redundant stat checks after xfs_refcount_delete() Eric Sandeen
2026-10-16 19:47 ` Darrick J. Wong
2026-10-16 19:29 ` [PATCH 8/8] xfs: remove status parameter from two btree delete functions Eric Sandeen
2026-10-16 19:47 ` Darrick J. Wong
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=20261016194441.GC107986@frogsfrogsfrogs \
--to=djwong@kernel.org \
--cc=cem@kernel.org \
--cc=dgc@kernel.org \
--cc=hch@lst.de \
--cc=linux-xfs@vger.kernel.org \
--cc=sandeen@redhat.com \
/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.