* [PATCH 1/8] xfs: remove redundant stat checks after xfs_btree_insert()
2026-10-16 19:29 [PATCH 0/8] xfs: consolidate btree insert/delete checks Eric Sandeen
@ 2026-10-16 19:29 ` Eric Sandeen
2026-10-16 19:44 ` Darrick J. Wong
2026-10-16 19:29 ` [PATCH 2/8] xfs: remove redundant stat checks after xfs_refcount_insert() Eric Sandeen
` (6 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Eric Sandeen @ 2026-10-16 19:29 UTC (permalink / raw)
To: linux-xfs; +Cc: cem, hch, dgc, Eric Sandeen
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>
---
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 1/8] xfs: remove redundant stat checks after xfs_btree_insert()
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
0 siblings, 0 replies; 17+ messages in thread
From: Darrick J. Wong @ 2026-10-16 19:44 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs, cem, hch, dgc
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
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/8] xfs: remove redundant stat checks after xfs_refcount_insert()
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:29 ` 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
` (5 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Eric Sandeen @ 2026-10-16 19:29 UTC (permalink / raw)
To: linux-xfs; +Cc: cem, hch, dgc, Eric Sandeen
xfs_refcount_insert() simply returns the error and status from
xfs_btree_insert, which has already validated the insert or returned
an error.
This makes every instance of the 2nd test in this idiom:
if ((error = xfs_refcount_insert(cnt_cur, &i)))
return error;
if (XFS_IS_CORRUPT(mp, i != 1)) {
xfs_btree_mark_sick(cnt_cur);
return -EFSCORRUPTED;
}
unnecessary/unreachable code. Remove them.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/libxfs/xfs_refcount.c | 19 ++-----------------
1 file changed, 2 insertions(+), 17 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
index 49e9993ce360..aec6c839220a 100644
--- a/fs/xfs/libxfs/xfs_refcount.c
+++ b/fs/xfs/libxfs/xfs_refcount.c
@@ -457,12 +457,8 @@ xfs_refcount_split_extent(
error = xfs_refcount_insert(cur, &tmp, &found_rec);
if (error)
goto out_error;
- if (XFS_IS_CORRUPT(cur->bc_mp, found_rec != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto out_error;
- }
- return error;
+
+ return 0;
out_error:
trace_xfs_refcount_split_extent_error(cur, error, _RET_IP_);
@@ -1191,12 +1187,6 @@ xfs_refcount_adjust_extents(
&found_tmp);
if (error)
goto out_error;
- if (XFS_IS_CORRUPT(cur->bc_mp,
- found_tmp != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto out_error;
- }
} else {
error = xrefc_free_extent(cur, &tmp);
if (error)
@@ -1841,11 +1831,6 @@ xfs_refcount_adjust_cow_extents(
&found_tmp);
if (error)
goto out_error;
- if (XFS_IS_CORRUPT(cur->bc_mp, found_tmp != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto out_error;
- }
break;
case XFS_REFCOUNT_ADJUST_COW_FREE:
/* Removing a CoW reservation, there should be one extent. */
--
2.55.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 2/8] xfs: remove redundant stat checks after xfs_refcount_insert()
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
0 siblings, 0 replies; 17+ messages in thread
From: Darrick J. Wong @ 2026-10-16 19:44 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs, cem, hch, dgc
On Fri, Oct 16, 2026 at 02:29:02PM -0500, Eric Sandeen wrote:
> xfs_refcount_insert() simply returns the error and status from
> xfs_btree_insert, which has already validated the insert or returned
> an error.
>
> This makes every instance of the 2nd test in this idiom:
>
> if ((error = xfs_refcount_insert(cnt_cur, &i)))
> return error;
> if (XFS_IS_CORRUPT(mp, i != 1)) {
> xfs_btree_mark_sick(cnt_cur);
> return -EFSCORRUPTED;
> }
>
> unnecessary/unreachable code. Remove them.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Looks ok,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> ---
> fs/xfs/libxfs/xfs_refcount.c | 19 ++-----------------
> 1 file changed, 2 insertions(+), 17 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
> index 49e9993ce360..aec6c839220a 100644
> --- a/fs/xfs/libxfs/xfs_refcount.c
> +++ b/fs/xfs/libxfs/xfs_refcount.c
> @@ -457,12 +457,8 @@ xfs_refcount_split_extent(
> error = xfs_refcount_insert(cur, &tmp, &found_rec);
> if (error)
> goto out_error;
> - if (XFS_IS_CORRUPT(cur->bc_mp, found_rec != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto out_error;
> - }
> - return error;
> +
> + return 0;
>
> out_error:
> trace_xfs_refcount_split_extent_error(cur, error, _RET_IP_);
> @@ -1191,12 +1187,6 @@ xfs_refcount_adjust_extents(
> &found_tmp);
> if (error)
> goto out_error;
> - if (XFS_IS_CORRUPT(cur->bc_mp,
> - found_tmp != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto out_error;
> - }
> } else {
> error = xrefc_free_extent(cur, &tmp);
> if (error)
> @@ -1841,11 +1831,6 @@ xfs_refcount_adjust_cow_extents(
> &found_tmp);
> if (error)
> goto out_error;
> - if (XFS_IS_CORRUPT(cur->bc_mp, found_tmp != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto out_error;
> - }
> break;
> case XFS_REFCOUNT_ADJUST_COW_FREE:
> /* Removing a CoW reservation, there should be one extent. */
> --
> 2.55.0
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/8] xfs: remove redundant stat checks after xfs_inobt_insert_rec()
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:29 ` [PATCH 2/8] xfs: remove redundant stat checks after xfs_refcount_insert() Eric Sandeen
@ 2026-10-16 19:29 ` 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
` (4 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Eric Sandeen @ 2026-10-16 19:29 UTC (permalink / raw)
To: linux-xfs; +Cc: cem, hch, dgc, Eric Sandeen
xfs_inobt_insert_rec() simply returns the error and status from
xfs_btree_insert, which has already validated the insert or returned
an error.
This makes every instance of the 2nd test in this idiom:
if ((error = xfs_inobt_insert_rec(cnt_cur, &i)))
return error;
if (XFS_IS_CORRUPT(mp, i != 1)) {
xfs_btree_mark_sick(cnt_cur);
return -EFSCORRUPTED;
}
unnecessary/unreachable code. Remove them.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/libxfs/xfs_ialloc.c | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index bb6e684f5467..fd916a2a005b 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -236,7 +236,6 @@ xfs_inobt_insert(
xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
return error;
}
- ASSERT(i == 1);
}
xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
@@ -572,12 +571,6 @@ xfs_inobt_insert_sprec(
nrec->ir_free, &i);
if (error)
goto error;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto error;
- }
-
goto out;
}
@@ -647,7 +640,6 @@ xfs_finobt_insert_sprec(
struct xfs_buf *agbp,
struct xfs_inobt_rec_incore *nrec) /* in/out: new rec. */
{
- struct xfs_mount *mp = pag_mount(pag);
struct xfs_btree_cur *cur;
int error;
int i;
@@ -665,11 +657,6 @@ xfs_finobt_insert_sprec(
nrec->ir_free, &i);
if (error)
goto error;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto error;
- }
} else {
error = xfs_inobt_update(cur, nrec);
if (error)
@@ -2254,8 +2241,6 @@ xfs_difree_finobt(
ibtrec->ir_free, &i);
if (error)
goto error;
- ASSERT(i == 1);
-
goto out;
}
--
2.55.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 3/8] xfs: remove redundant stat checks after xfs_inobt_insert_rec()
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
0 siblings, 0 replies; 17+ messages in thread
From: Darrick J. Wong @ 2026-10-16 19:45 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs, cem, hch, dgc
On Fri, Oct 16, 2026 at 02:29:03PM -0500, Eric Sandeen wrote:
> xfs_inobt_insert_rec() simply returns the error and status from
> xfs_btree_insert, which has already validated the insert or returned
> an error.
>
> This makes every instance of the 2nd test in this idiom:
>
> if ((error = xfs_inobt_insert_rec(cnt_cur, &i)))
> return error;
> if (XFS_IS_CORRUPT(mp, i != 1)) {
> xfs_btree_mark_sick(cnt_cur);
> return -EFSCORRUPTED;
> }
>
> unnecessary/unreachable code. Remove them.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Looks good!
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> ---
> fs/xfs/libxfs/xfs_ialloc.c | 15 ---------------
> 1 file changed, 15 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index bb6e684f5467..fd916a2a005b 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -236,7 +236,6 @@ xfs_inobt_insert(
> xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
> return error;
> }
> - ASSERT(i == 1);
> }
>
> xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
> @@ -572,12 +571,6 @@ xfs_inobt_insert_sprec(
> nrec->ir_free, &i);
> if (error)
> goto error;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto error;
> - }
> -
> goto out;
> }
>
> @@ -647,7 +640,6 @@ xfs_finobt_insert_sprec(
> struct xfs_buf *agbp,
> struct xfs_inobt_rec_incore *nrec) /* in/out: new rec. */
> {
> - struct xfs_mount *mp = pag_mount(pag);
> struct xfs_btree_cur *cur;
> int error;
> int i;
> @@ -665,11 +657,6 @@ xfs_finobt_insert_sprec(
> nrec->ir_free, &i);
> if (error)
> goto error;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto error;
> - }
> } else {
> error = xfs_inobt_update(cur, nrec);
> if (error)
> @@ -2254,8 +2241,6 @@ xfs_difree_finobt(
> ibtrec->ir_free, &i);
> if (error)
> goto error;
> - ASSERT(i == 1);
> -
> goto out;
> }
>
> --
> 2.55.0
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/8] xfs: remove redundant stat check after rcbagbt_insert()
2026-10-16 19:29 [PATCH 0/8] xfs: consolidate btree insert/delete checks Eric Sandeen
` (2 preceding siblings ...)
2026-10-16 19:29 ` [PATCH 3/8] xfs: remove redundant stat checks after xfs_inobt_insert_rec() Eric Sandeen
@ 2026-10-16 19:29 ` 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
` (3 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Eric Sandeen @ 2026-10-16 19:29 UTC (permalink / raw)
To: linux-xfs; +Cc: cem, hch, dgc, Eric Sandeen
rcbagbt_insert() simply returns the error and status from
xfs_btree_insert, which has already validated the insert
or returned an error.
This makes this instance of this idiom:
error = rcbagbt_insert(cur, &bagrec, &has);
if (error)
goto out_cur;
if (!has) {
error = -EFSCORRUPTED;
goto out_cur;
}
unnecessary/unreachable code. Remove it.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/scrub/rcbag.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/fs/xfs/scrub/rcbag.c b/fs/xfs/scrub/rcbag.c
index 8f3a10f7c908..fd9ccb205725 100644
--- a/fs/xfs/scrub/rcbag.c
+++ b/fs/xfs/scrub/rcbag.c
@@ -105,10 +105,6 @@ rcbag_add(
error = rcbagbt_insert(cur, &bagrec, &has);
if (error)
goto out_cur;
- if (!has) {
- error = -EFSCORRUPTED;
- goto out_cur;
- }
}
xfs_btree_del_cursor(cur, 0);
--
2.55.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 4/8] xfs: remove redundant stat check after rcbagbt_insert()
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
0 siblings, 0 replies; 17+ messages in thread
From: Darrick J. Wong @ 2026-10-16 19:45 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs, cem, hch, dgc
On Fri, Oct 16, 2026 at 02:29:04PM -0500, Eric Sandeen wrote:
> rcbagbt_insert() simply returns the error and status from
> xfs_btree_insert, which has already validated the insert
> or returned an error.
>
> This makes this instance of this idiom:
>
> error = rcbagbt_insert(cur, &bagrec, &has);
> if (error)
> goto out_cur;
> if (!has) {
> error = -EFSCORRUPTED;
> goto out_cur;
> }
>
> unnecessary/unreachable code. Remove it.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> ---
> fs/xfs/scrub/rcbag.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/fs/xfs/scrub/rcbag.c b/fs/xfs/scrub/rcbag.c
> index 8f3a10f7c908..fd9ccb205725 100644
> --- a/fs/xfs/scrub/rcbag.c
> +++ b/fs/xfs/scrub/rcbag.c
> @@ -105,10 +105,6 @@ rcbag_add(
> error = rcbagbt_insert(cur, &bagrec, &has);
> if (error)
> goto out_cur;
> - if (!has) {
> - error = -EFSCORRUPTED;
> - goto out_cur;
> - }
> }
>
> xfs_btree_del_cursor(cur, 0);
> --
> 2.55.0
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 5/8] xfs: remove status parameter from several btree insert functions
2026-10-16 19:29 [PATCH 0/8] xfs: consolidate btree insert/delete checks Eric Sandeen
` (3 preceding siblings ...)
2026-10-16 19:29 ` [PATCH 4/8] xfs: remove redundant stat check after rcbagbt_insert() Eric Sandeen
@ 2026-10-16 19:29 ` 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
` (2 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Eric Sandeen @ 2026-10-16 19:29 UTC (permalink / raw)
To: linux-xfs; +Cc: cem, hch, dgc, Eric Sandeen
xfs_btree_insert and its callers now have no need for the status
parameter; either they return error = 0 / status = 1, or nonzero error
and status 0. No caller evaluates the status variable, so remove that
parameter from these functions.
As requested, convert all of the compound tests/assignments to discrete
lines.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/libxfs/xfs_alloc.c | 15 ++++++++++-----
fs/xfs/libxfs/xfs_bmap.c | 26 +++++++++++++++-----------
fs/xfs/libxfs/xfs_btree.c | 4 +---
fs/xfs/libxfs/xfs_btree.h | 2 +-
fs/xfs/libxfs/xfs_ialloc.c | 13 ++++++-------
fs/xfs/libxfs/xfs_ialloc.h | 3 +--
fs/xfs/libxfs/xfs_refcount.c | 17 +++++++----------
fs/xfs/libxfs/xfs_refcount.h | 2 +-
fs/xfs/libxfs/xfs_rmap.c | 14 +++++++-------
fs/xfs/scrub/rcbag.c | 2 +-
fs/xfs/scrub/rcbag_btree.c | 5 ++---
fs/xfs/scrub/rcbag_btree.h | 3 +--
12 files changed, 53 insertions(+), 53 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index b40c51a0f651..acd8f4521c4c 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -692,7 +692,8 @@ xfs_alloc_fixup_trees(
xfs_btree_mark_sick(cnt_cur);
return -EFSCORRUPTED;
}
- if ((error = xfs_btree_insert(cnt_cur, &i)))
+ error = xfs_btree_insert(cnt_cur);
+ if (error)
return error;
}
if (nfbno2 != NULLAGBLOCK) {
@@ -702,7 +703,8 @@ xfs_alloc_fixup_trees(
xfs_btree_mark_sick(cnt_cur);
return -EFSCORRUPTED;
}
- if ((error = xfs_btree_insert(cnt_cur, &i)))
+ error = xfs_btree_insert(cnt_cur);
+ if (error)
return error;
}
/*
@@ -735,7 +737,8 @@ xfs_alloc_fixup_trees(
xfs_btree_mark_sick(bno_cur);
return -EFSCORRUPTED;
}
- if ((error = xfs_btree_insert(bno_cur, &i)))
+ error = xfs_btree_insert(bno_cur);
+ if (error)
return error;
}
@@ -2300,7 +2303,8 @@ xfs_free_ag_extent(
else {
nbno = bno;
nlen = len;
- if ((error = xfs_btree_insert(bno_cur, &i)))
+ error = xfs_btree_insert(bno_cur);
+ if (error)
goto error0;
}
xfs_btree_del_cursor(bno_cur, XFS_BTREE_NOERROR);
@@ -2322,7 +2326,8 @@ xfs_free_ag_extent(
}
if (xfs_alloc_cursor_at_lastrec(cnt_cur))
fixup_longest = true;
- if ((error = xfs_btree_insert(cnt_cur, &i)))
+ error = xfs_btree_insert(cnt_cur);
+ if (error)
goto error0;
if (fixup_longest) {
error = xfs_alloc_fixup_longest(cnt_cur);
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 073c11e631ea..09d6792e42be 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1671,7 +1671,7 @@ xfs_bmap_add_extent_delay_real(
error = -EFSCORRUPTED;
goto done;
}
- error = xfs_btree_insert(bma->cur, &i);
+ error = xfs_btree_insert(bma->cur);
if (error)
goto done;
}
@@ -1737,7 +1737,7 @@ xfs_bmap_add_extent_delay_real(
error = -EFSCORRUPTED;
goto done;
}
- error = xfs_btree_insert(bma->cur, &i);
+ error = xfs_btree_insert(bma->cur);
if (error)
goto done;
}
@@ -1823,7 +1823,7 @@ xfs_bmap_add_extent_delay_real(
error = -EFSCORRUPTED;
goto done;
}
- error = xfs_btree_insert(bma->cur, &i);
+ error = xfs_btree_insert(bma->cur);
if (error)
goto done;
}
@@ -1907,7 +1907,7 @@ xfs_bmap_add_extent_delay_real(
error = -EFSCORRUPTED;
goto done;
}
- error = xfs_btree_insert(bma->cur, &i);
+ error = xfs_btree_insert(bma->cur);
if (error)
goto done;
}
@@ -2328,7 +2328,8 @@ xfs_bmap_add_extent_unwritten_real(
if (error)
goto done;
cur->bc_rec.b = *new;
- if ((error = xfs_btree_insert(cur, &i)))
+ error = xfs_btree_insert(cur);
+ if (error)
goto done;
}
break;
@@ -2409,7 +2410,8 @@ xfs_bmap_add_extent_unwritten_real(
error = -EFSCORRUPTED;
goto done;
}
- if ((error = xfs_btree_insert(cur, &i)))
+ error = xfs_btree_insert(cur);
+ if (error)
goto done;
}
break;
@@ -2454,7 +2456,8 @@ xfs_bmap_add_extent_unwritten_real(
goto done;
/* new left extent - oldext */
cur->bc_rec.b = PREV;
- if ((error = xfs_btree_insert(cur, &i)))
+ error = xfs_btree_insert(cur);
+ if (error)
goto done;
/*
* Reset the cursor to the position of the new extent
@@ -2470,7 +2473,8 @@ xfs_bmap_add_extent_unwritten_real(
goto done;
}
/* new middle extent - newext */
- if ((error = xfs_btree_insert(cur, &i)))
+ error = xfs_btree_insert(cur);
+ if (error)
goto done;
}
break;
@@ -2724,7 +2728,7 @@ xfs_bmap_add_extent_hole_real(
error = -EFSCORRUPTED;
goto done;
}
- error = xfs_btree_insert(cur, &i);
+ error = xfs_btree_insert(cur);
if (error)
goto done;
}
@@ -5013,7 +5017,7 @@ xfs_bmap_del_extent_real(
if (error)
return error;
cur->bc_rec.b = new;
- error = xfs_btree_insert(cur, &i);
+ error = xfs_btree_insert(cur);
if (error && error != -ENOSPC)
return error;
/*
@@ -5906,7 +5910,7 @@ xfs_bmap_split_extent(
error = -EFSCORRUPTED;
goto del_cursor;
}
- error = xfs_btree_insert(cur, &i);
+ error = xfs_btree_insert(cur);
if (error)
goto del_cursor;
}
diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index 7effebe297ea..e2e698596c21 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -3728,8 +3728,7 @@ xfs_btree_insrec(
*/
int
xfs_btree_insert(
- struct xfs_btree_cur *cur,
- int *stat)
+ struct xfs_btree_cur *cur)
{
int error; /* error return value */
int i; /* result value, 0 for failure */
@@ -3798,7 +3797,6 @@ xfs_btree_insert(
}
} while (!xfs_btree_ptr_is_null(cur, &nptr));
- *stat = i;
return 0;
error0:
return error;
diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
index 60e78572e725..0f82f3106bac 100644
--- a/fs/xfs/libxfs/xfs_btree.h
+++ b/fs/xfs/libxfs/xfs_btree.h
@@ -405,7 +405,7 @@ int xfs_btree_decrement(struct xfs_btree_cur *, int, int *);
int xfs_btree_lookup(struct xfs_btree_cur *, xfs_lookup_t, int *);
int xfs_btree_update(struct xfs_btree_cur *, union xfs_btree_rec *);
int xfs_btree_new_iroot(struct xfs_btree_cur *, int *, int *);
-int xfs_btree_insert(struct xfs_btree_cur *, int *);
+int xfs_btree_insert(struct xfs_btree_cur *);
int xfs_btree_delete(struct xfs_btree_cur *, int *);
int xfs_btree_get_rec(struct xfs_btree_cur *, union xfs_btree_rec **, int *);
int xfs_btree_change_owner(struct xfs_btree_cur *cur, uint64_t new_owner,
diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index fd916a2a005b..bd7b0a24225f 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -186,14 +186,13 @@ xfs_inobt_insert_rec(
uint16_t holemask,
uint8_t count,
int32_t freecount,
- xfs_inofree_t free,
- int *stat)
+ xfs_inofree_t free)
{
cur->bc_rec.i.ir_holemask = holemask;
cur->bc_rec.i.ir_count = count;
cur->bc_rec.i.ir_freecount = freecount;
cur->bc_rec.i.ir_free = free;
- return xfs_btree_insert(cur, stat);
+ return xfs_btree_insert(cur);
}
/*
@@ -231,7 +230,7 @@ xfs_inobt_insert(
error = xfs_inobt_insert_rec(cur, XFS_INOBT_HOLEMASK_FULL,
XFS_INODES_PER_CHUNK,
XFS_INODES_PER_CHUNK,
- XFS_INOBT_ALL_FREE, &i);
+ XFS_INOBT_ALL_FREE);
if (error) {
xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
return error;
@@ -568,7 +567,7 @@ xfs_inobt_insert_sprec(
if (i == 0) {
error = xfs_inobt_insert_rec(cur, nrec->ir_holemask,
nrec->ir_count, nrec->ir_freecount,
- nrec->ir_free, &i);
+ nrec->ir_free);
if (error)
goto error;
goto out;
@@ -654,7 +653,7 @@ xfs_finobt_insert_sprec(
if (i == 0) {
error = xfs_inobt_insert_rec(cur, nrec->ir_holemask,
nrec->ir_count, nrec->ir_freecount,
- nrec->ir_free, &i);
+ nrec->ir_free);
if (error)
goto error;
} else {
@@ -2238,7 +2237,7 @@ xfs_difree_finobt(
error = xfs_inobt_insert_rec(cur, ibtrec->ir_holemask,
ibtrec->ir_count,
ibtrec->ir_freecount,
- ibtrec->ir_free, &i);
+ ibtrec->ir_free);
if (error)
goto error;
goto out;
diff --git a/fs/xfs/libxfs/xfs_ialloc.h b/fs/xfs/libxfs/xfs_ialloc.h
index 3a1323155a45..49c3d4c513c9 100644
--- a/fs/xfs/libxfs/xfs_ialloc.h
+++ b/fs/xfs/libxfs/xfs_ialloc.h
@@ -105,8 +105,7 @@ int xfs_ialloc_has_inodes_at_extent(struct xfs_btree_cur *cur,
int xfs_ialloc_count_inodes(struct xfs_btree_cur *cur, xfs_agino_t *count,
xfs_agino_t *freecount);
int xfs_inobt_insert_rec(struct xfs_btree_cur *cur, uint16_t holemask,
- uint8_t count, int32_t freecount, xfs_inofree_t free,
- int *stat);
+ uint8_t count, int32_t freecount, xfs_inofree_t free);
int xfs_ialloc_cluster_alignment(struct xfs_mount *mp);
void xfs_ialloc_setup_geometry(struct xfs_mount *mp);
diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
index aec6c839220a..574a5e7c0cdf 100644
--- a/fs/xfs/libxfs/xfs_refcount.c
+++ b/fs/xfs/libxfs/xfs_refcount.c
@@ -263,8 +263,7 @@ xfs_refcount_update(
int
xfs_refcount_insert(
struct xfs_btree_cur *cur,
- struct xfs_refcount_irec *irec,
- int *i)
+ struct xfs_refcount_irec *irec)
{
int error;
@@ -275,7 +274,7 @@ xfs_refcount_insert(
cur->bc_rec.rc.rc_refcount = irec->rc_refcount;
cur->bc_rec.rc.rc_domain = irec->rc_domain;
- error = xfs_btree_insert(cur, i);
+ error = xfs_btree_insert(cur);
if (error)
trace_xfs_refcount_insert_error(cur, error, _RET_IP_);
return error;
@@ -454,7 +453,7 @@ xfs_refcount_split_extent(
/* Insert the left extent. */
tmp = rcext;
tmp.rc_blockcount = agbno - rcext.rc_startblock;
- error = xfs_refcount_insert(cur, &tmp, &found_rec);
+ error = xfs_refcount_insert(cur, &tmp);
if (error)
goto out_error;
@@ -1141,7 +1140,7 @@ xfs_refcount_adjust_extents(
{
struct xfs_refcount_irec ext, tmp;
int error;
- int found_rec, found_tmp;
+ int found_rec;
/* Merging did all the work already. */
if (*aglen == 0)
@@ -1183,8 +1182,7 @@ xfs_refcount_adjust_extents(
*/
cur->bc_refc.nr_ops++;
if (tmp.rc_refcount) {
- error = xfs_refcount_insert(cur, &tmp,
- &found_tmp);
+ error = xfs_refcount_insert(cur, &tmp);
if (error)
goto out_error;
} else {
@@ -1784,7 +1782,7 @@ xfs_refcount_adjust_cow_extents(
{
struct xfs_refcount_irec ext, tmp;
int error;
- int found_rec, found_tmp;
+ int found_rec;
if (aglen == 0)
return 0;
@@ -1827,8 +1825,7 @@ xfs_refcount_adjust_cow_extents(
trace_xfs_refcount_modify_extent(cur, &tmp);
- error = xfs_refcount_insert(cur, &tmp,
- &found_tmp);
+ error = xfs_refcount_insert(cur, &tmp);
if (error)
goto out_error;
break;
diff --git a/fs/xfs/libxfs/xfs_refcount.h b/fs/xfs/libxfs/xfs_refcount.h
index f2e299a716a4..cf300532bf04 100644
--- a/fs/xfs/libxfs/xfs_refcount.h
+++ b/fs/xfs/libxfs/xfs_refcount.h
@@ -126,7 +126,7 @@ xfs_failaddr_t xfs_refcount_check_irec(struct xfs_perag *pag,
xfs_failaddr_t xfs_rtrefcount_check_irec(struct xfs_rtgroup *rtg,
const struct xfs_refcount_irec *irec);
extern int xfs_refcount_insert(struct xfs_btree_cur *cur,
- struct xfs_refcount_irec *irec, int *stat);
+ struct xfs_refcount_irec *irec);
extern struct kmem_cache *xfs_refcount_intent_cache;
diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
index ff4aef8d30a7..99575ae419a7 100644
--- a/fs/xfs/libxfs/xfs_rmap.c
+++ b/fs/xfs/libxfs/xfs_rmap.c
@@ -145,7 +145,7 @@ xfs_rmap_insert(
rcur->bc_rec.r.rm_owner = owner;
rcur->bc_rec.r.rm_offset = offset;
rcur->bc_rec.r.rm_flags = flags;
- error = xfs_btree_insert(rcur, &i);
+ error = xfs_btree_insert(rcur);
done:
if (error)
trace_xfs_rmap_insert_error(rcur, error, _RET_IP_);
@@ -855,7 +855,7 @@ xfs_rmap_unmap(
cur->bc_rec.r.rm_owner,
cur->bc_rec.r.rm_offset,
cur->bc_rec.r.rm_flags);
- error = xfs_btree_insert(cur, &i);
+ error = xfs_btree_insert(cur);
if (error)
goto out_error;
}
@@ -1175,7 +1175,7 @@ xfs_rmap_map(
cur->bc_rec.r.rm_offset = offset;
cur->bc_rec.r.rm_flags = flags;
trace_xfs_rmap_insert(cur, bno, len, owner, offset, flags);
- error = xfs_btree_insert(cur, &i);
+ error = xfs_btree_insert(cur);
if (error)
goto out_error;
}
@@ -1577,7 +1577,7 @@ xfs_rmap_convert(
NEW.rm_flags = newext;
cur->bc_rec.r = NEW;
trace_xfs_rmap_insert(cur, bno, len, owner, offset, newext);
- error = xfs_btree_insert(cur, &i);
+ error = xfs_btree_insert(cur);
if (error)
goto done;
break;
@@ -1630,7 +1630,7 @@ xfs_rmap_convert(
NEW.rm_flags = newext;
cur->bc_rec.r = NEW;
trace_xfs_rmap_insert(cur, bno, len, owner, offset, newext);
- error = xfs_btree_insert(cur, &i);
+ error = xfs_btree_insert(cur);
if (error)
goto done;
break;
@@ -1658,7 +1658,7 @@ xfs_rmap_convert(
trace_xfs_rmap_insert(cur, NEW.rm_startblock,
NEW.rm_blockcount, NEW.rm_owner, NEW.rm_offset,
NEW.rm_flags);
- error = xfs_btree_insert(cur, &i);
+ error = xfs_btree_insert(cur);
if (error)
goto done;
/*
@@ -1679,7 +1679,7 @@ xfs_rmap_convert(
cur->bc_rec.r.rm_flags &= ~XFS_RMAP_UNWRITTEN;
cur->bc_rec.r.rm_flags |= newext;
trace_xfs_rmap_insert(cur, bno, len, owner, offset, newext);
- error = xfs_btree_insert(cur, &i);
+ error = xfs_btree_insert(cur);
if (error)
goto done;
break;
diff --git a/fs/xfs/scrub/rcbag.c b/fs/xfs/scrub/rcbag.c
index fd9ccb205725..41e798f65c67 100644
--- a/fs/xfs/scrub/rcbag.c
+++ b/fs/xfs/scrub/rcbag.c
@@ -102,7 +102,7 @@ rcbag_add(
bagrec.rbg_blockcount = rmap->rm_blockcount;
bagrec.rbg_refcount = 1;
- error = rcbagbt_insert(cur, &bagrec, &has);
+ error = rcbagbt_insert(cur, &bagrec);
if (error)
goto out_cur;
}
diff --git a/fs/xfs/scrub/rcbag_btree.c b/fs/xfs/scrub/rcbag_btree.c
index 367f8ccf55c4..790931fa8f06 100644
--- a/fs/xfs/scrub/rcbag_btree.c
+++ b/fs/xfs/scrub/rcbag_btree.c
@@ -342,11 +342,10 @@ rcbagbt_update(
int
rcbagbt_insert(
struct xfs_btree_cur *cur,
- const struct rcbag_rec *rec,
- int *success)
+ const struct rcbag_rec *rec)
{
struct rcbag_rec *btrec = (struct rcbag_rec *)&cur->bc_rec;
memcpy(btrec, rec, sizeof(struct rcbag_rec));
- return xfs_btree_insert(cur, success);
+ return xfs_btree_insert(cur);
}
diff --git a/fs/xfs/scrub/rcbag_btree.h b/fs/xfs/scrub/rcbag_btree.h
index 03cadb032552..da45d9e885f8 100644
--- a/fs/xfs/scrub/rcbag_btree.h
+++ b/fs/xfs/scrub/rcbag_btree.h
@@ -70,8 +70,7 @@ int rcbagbt_lookup_eq(struct xfs_btree_cur *cur,
const struct xfs_rmap_irec *rmap, int *success);
int rcbagbt_get_rec(struct xfs_btree_cur *cur, struct rcbag_rec *rec, int *has);
int rcbagbt_update(struct xfs_btree_cur *cur, const struct rcbag_rec *rec);
-int rcbagbt_insert(struct xfs_btree_cur *cur, const struct rcbag_rec *rec,
- int *success);
+int rcbagbt_insert(struct xfs_btree_cur *cur, const struct rcbag_rec *rec);
#else
# define rcbagbt_init_cur_cache() 0
--
2.55.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 5/8] xfs: remove status parameter from several btree insert functions
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
0 siblings, 0 replies; 17+ messages in thread
From: Darrick J. Wong @ 2026-10-16 19:46 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs, cem, hch, dgc
On Fri, Oct 16, 2026 at 02:29:05PM -0500, Eric Sandeen wrote:
> xfs_btree_insert and its callers now have no need for the status
> parameter; either they return error = 0 / status = 1, or nonzero error
> and status 0. No caller evaluates the status variable, so remove that
> parameter from these functions.
>
> As requested, convert all of the compound tests/assignments to discrete
> lines.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> fs/xfs/libxfs/xfs_alloc.c | 15 ++++++++++-----
> fs/xfs/libxfs/xfs_bmap.c | 26 +++++++++++++++-----------
> fs/xfs/libxfs/xfs_btree.c | 4 +---
> fs/xfs/libxfs/xfs_btree.h | 2 +-
> fs/xfs/libxfs/xfs_ialloc.c | 13 ++++++-------
> fs/xfs/libxfs/xfs_ialloc.h | 3 +--
> fs/xfs/libxfs/xfs_refcount.c | 17 +++++++----------
> fs/xfs/libxfs/xfs_refcount.h | 2 +-
> fs/xfs/libxfs/xfs_rmap.c | 14 +++++++-------
> fs/xfs/scrub/rcbag.c | 2 +-
> fs/xfs/scrub/rcbag_btree.c | 5 ++---
> fs/xfs/scrub/rcbag_btree.h | 3 +--
> 12 files changed, 53 insertions(+), 53 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index b40c51a0f651..acd8f4521c4c 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -692,7 +692,8 @@ xfs_alloc_fixup_trees(
> xfs_btree_mark_sick(cnt_cur);
> return -EFSCORRUPTED;
> }
> - if ((error = xfs_btree_insert(cnt_cur, &i)))
> + error = xfs_btree_insert(cnt_cur);
> + if (error)
> return error;
> }
> if (nfbno2 != NULLAGBLOCK) {
> @@ -702,7 +703,8 @@ xfs_alloc_fixup_trees(
> xfs_btree_mark_sick(cnt_cur);
> return -EFSCORRUPTED;
> }
> - if ((error = xfs_btree_insert(cnt_cur, &i)))
> + error = xfs_btree_insert(cnt_cur);
> + if (error)
> return error;
> }
> /*
> @@ -735,7 +737,8 @@ xfs_alloc_fixup_trees(
> xfs_btree_mark_sick(bno_cur);
> return -EFSCORRUPTED;
> }
> - if ((error = xfs_btree_insert(bno_cur, &i)))
> + error = xfs_btree_insert(bno_cur);
> + if (error)
> return error;
> }
>
> @@ -2300,7 +2303,8 @@ xfs_free_ag_extent(
> else {
> nbno = bno;
> nlen = len;
> - if ((error = xfs_btree_insert(bno_cur, &i)))
> + error = xfs_btree_insert(bno_cur);
> + if (error)
> goto error0;
> }
> xfs_btree_del_cursor(bno_cur, XFS_BTREE_NOERROR);
> @@ -2322,7 +2326,8 @@ xfs_free_ag_extent(
> }
> if (xfs_alloc_cursor_at_lastrec(cnt_cur))
> fixup_longest = true;
> - if ((error = xfs_btree_insert(cnt_cur, &i)))
> + error = xfs_btree_insert(cnt_cur);
> + if (error)
> goto error0;
> if (fixup_longest) {
> error = xfs_alloc_fixup_longest(cnt_cur);
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 073c11e631ea..09d6792e42be 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1671,7 +1671,7 @@ xfs_bmap_add_extent_delay_real(
> error = -EFSCORRUPTED;
> goto done;
> }
> - error = xfs_btree_insert(bma->cur, &i);
> + error = xfs_btree_insert(bma->cur);
> if (error)
> goto done;
> }
> @@ -1737,7 +1737,7 @@ xfs_bmap_add_extent_delay_real(
> error = -EFSCORRUPTED;
> goto done;
> }
> - error = xfs_btree_insert(bma->cur, &i);
> + error = xfs_btree_insert(bma->cur);
> if (error)
> goto done;
> }
> @@ -1823,7 +1823,7 @@ xfs_bmap_add_extent_delay_real(
> error = -EFSCORRUPTED;
> goto done;
> }
> - error = xfs_btree_insert(bma->cur, &i);
> + error = xfs_btree_insert(bma->cur);
> if (error)
> goto done;
> }
> @@ -1907,7 +1907,7 @@ xfs_bmap_add_extent_delay_real(
> error = -EFSCORRUPTED;
> goto done;
> }
> - error = xfs_btree_insert(bma->cur, &i);
> + error = xfs_btree_insert(bma->cur);
> if (error)
> goto done;
> }
> @@ -2328,7 +2328,8 @@ xfs_bmap_add_extent_unwritten_real(
> if (error)
> goto done;
> cur->bc_rec.b = *new;
> - if ((error = xfs_btree_insert(cur, &i)))
> + error = xfs_btree_insert(cur);
> + if (error)
> goto done;
> }
> break;
> @@ -2409,7 +2410,8 @@ xfs_bmap_add_extent_unwritten_real(
> error = -EFSCORRUPTED;
> goto done;
> }
> - if ((error = xfs_btree_insert(cur, &i)))
> + error = xfs_btree_insert(cur);
> + if (error)
> goto done;
> }
> break;
> @@ -2454,7 +2456,8 @@ xfs_bmap_add_extent_unwritten_real(
> goto done;
> /* new left extent - oldext */
> cur->bc_rec.b = PREV;
> - if ((error = xfs_btree_insert(cur, &i)))
> + error = xfs_btree_insert(cur);
> + if (error)
> goto done;
> /*
> * Reset the cursor to the position of the new extent
> @@ -2470,7 +2473,8 @@ xfs_bmap_add_extent_unwritten_real(
> goto done;
> }
> /* new middle extent - newext */
> - if ((error = xfs_btree_insert(cur, &i)))
> + error = xfs_btree_insert(cur);
> + if (error)
> goto done;
> }
> break;
> @@ -2724,7 +2728,7 @@ xfs_bmap_add_extent_hole_real(
> error = -EFSCORRUPTED;
> goto done;
> }
> - error = xfs_btree_insert(cur, &i);
> + error = xfs_btree_insert(cur);
> if (error)
> goto done;
> }
> @@ -5013,7 +5017,7 @@ xfs_bmap_del_extent_real(
> if (error)
> return error;
> cur->bc_rec.b = new;
> - error = xfs_btree_insert(cur, &i);
> + error = xfs_btree_insert(cur);
> if (error && error != -ENOSPC)
> return error;
> /*
> @@ -5906,7 +5910,7 @@ xfs_bmap_split_extent(
> error = -EFSCORRUPTED;
> goto del_cursor;
> }
> - error = xfs_btree_insert(cur, &i);
> + error = xfs_btree_insert(cur);
> if (error)
> goto del_cursor;
> }
> diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
> index 7effebe297ea..e2e698596c21 100644
> --- a/fs/xfs/libxfs/xfs_btree.c
> +++ b/fs/xfs/libxfs/xfs_btree.c
> @@ -3728,8 +3728,7 @@ xfs_btree_insrec(
> */
> int
> xfs_btree_insert(
> - struct xfs_btree_cur *cur,
> - int *stat)
> + struct xfs_btree_cur *cur)
> {
> int error; /* error return value */
> int i; /* result value, 0 for failure */
> @@ -3798,7 +3797,6 @@ xfs_btree_insert(
> }
> } while (!xfs_btree_ptr_is_null(cur, &nptr));
>
> - *stat = i;
> return 0;
> error0:
> return error;
> diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
> index 60e78572e725..0f82f3106bac 100644
> --- a/fs/xfs/libxfs/xfs_btree.h
> +++ b/fs/xfs/libxfs/xfs_btree.h
> @@ -405,7 +405,7 @@ int xfs_btree_decrement(struct xfs_btree_cur *, int, int *);
> int xfs_btree_lookup(struct xfs_btree_cur *, xfs_lookup_t, int *);
> int xfs_btree_update(struct xfs_btree_cur *, union xfs_btree_rec *);
> int xfs_btree_new_iroot(struct xfs_btree_cur *, int *, int *);
> -int xfs_btree_insert(struct xfs_btree_cur *, int *);
> +int xfs_btree_insert(struct xfs_btree_cur *);
Nitpick: parameters should have names... even though it's pretty obvious
what the one parameter is supposed to be here.
Everything else in here looks good, so
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> int xfs_btree_delete(struct xfs_btree_cur *, int *);
> int xfs_btree_get_rec(struct xfs_btree_cur *, union xfs_btree_rec **, int *);
> int xfs_btree_change_owner(struct xfs_btree_cur *cur, uint64_t new_owner,
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index fd916a2a005b..bd7b0a24225f 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -186,14 +186,13 @@ xfs_inobt_insert_rec(
> uint16_t holemask,
> uint8_t count,
> int32_t freecount,
> - xfs_inofree_t free,
> - int *stat)
> + xfs_inofree_t free)
> {
> cur->bc_rec.i.ir_holemask = holemask;
> cur->bc_rec.i.ir_count = count;
> cur->bc_rec.i.ir_freecount = freecount;
> cur->bc_rec.i.ir_free = free;
> - return xfs_btree_insert(cur, stat);
> + return xfs_btree_insert(cur);
> }
>
> /*
> @@ -231,7 +230,7 @@ xfs_inobt_insert(
> error = xfs_inobt_insert_rec(cur, XFS_INOBT_HOLEMASK_FULL,
> XFS_INODES_PER_CHUNK,
> XFS_INODES_PER_CHUNK,
> - XFS_INOBT_ALL_FREE, &i);
> + XFS_INOBT_ALL_FREE);
> if (error) {
> xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
> return error;
> @@ -568,7 +567,7 @@ xfs_inobt_insert_sprec(
> if (i == 0) {
> error = xfs_inobt_insert_rec(cur, nrec->ir_holemask,
> nrec->ir_count, nrec->ir_freecount,
> - nrec->ir_free, &i);
> + nrec->ir_free);
> if (error)
> goto error;
> goto out;
> @@ -654,7 +653,7 @@ xfs_finobt_insert_sprec(
> if (i == 0) {
> error = xfs_inobt_insert_rec(cur, nrec->ir_holemask,
> nrec->ir_count, nrec->ir_freecount,
> - nrec->ir_free, &i);
> + nrec->ir_free);
> if (error)
> goto error;
> } else {
> @@ -2238,7 +2237,7 @@ xfs_difree_finobt(
> error = xfs_inobt_insert_rec(cur, ibtrec->ir_holemask,
> ibtrec->ir_count,
> ibtrec->ir_freecount,
> - ibtrec->ir_free, &i);
> + ibtrec->ir_free);
> if (error)
> goto error;
> goto out;
> diff --git a/fs/xfs/libxfs/xfs_ialloc.h b/fs/xfs/libxfs/xfs_ialloc.h
> index 3a1323155a45..49c3d4c513c9 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.h
> +++ b/fs/xfs/libxfs/xfs_ialloc.h
> @@ -105,8 +105,7 @@ int xfs_ialloc_has_inodes_at_extent(struct xfs_btree_cur *cur,
> int xfs_ialloc_count_inodes(struct xfs_btree_cur *cur, xfs_agino_t *count,
> xfs_agino_t *freecount);
> int xfs_inobt_insert_rec(struct xfs_btree_cur *cur, uint16_t holemask,
> - uint8_t count, int32_t freecount, xfs_inofree_t free,
> - int *stat);
> + uint8_t count, int32_t freecount, xfs_inofree_t free);
>
> int xfs_ialloc_cluster_alignment(struct xfs_mount *mp);
> void xfs_ialloc_setup_geometry(struct xfs_mount *mp);
> diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
> index aec6c839220a..574a5e7c0cdf 100644
> --- a/fs/xfs/libxfs/xfs_refcount.c
> +++ b/fs/xfs/libxfs/xfs_refcount.c
> @@ -263,8 +263,7 @@ xfs_refcount_update(
> int
> xfs_refcount_insert(
> struct xfs_btree_cur *cur,
> - struct xfs_refcount_irec *irec,
> - int *i)
> + struct xfs_refcount_irec *irec)
> {
> int error;
>
> @@ -275,7 +274,7 @@ xfs_refcount_insert(
> cur->bc_rec.rc.rc_refcount = irec->rc_refcount;
> cur->bc_rec.rc.rc_domain = irec->rc_domain;
>
> - error = xfs_btree_insert(cur, i);
> + error = xfs_btree_insert(cur);
> if (error)
> trace_xfs_refcount_insert_error(cur, error, _RET_IP_);
> return error;
> @@ -454,7 +453,7 @@ xfs_refcount_split_extent(
> /* Insert the left extent. */
> tmp = rcext;
> tmp.rc_blockcount = agbno - rcext.rc_startblock;
> - error = xfs_refcount_insert(cur, &tmp, &found_rec);
> + error = xfs_refcount_insert(cur, &tmp);
> if (error)
> goto out_error;
>
> @@ -1141,7 +1140,7 @@ xfs_refcount_adjust_extents(
> {
> struct xfs_refcount_irec ext, tmp;
> int error;
> - int found_rec, found_tmp;
> + int found_rec;
>
> /* Merging did all the work already. */
> if (*aglen == 0)
> @@ -1183,8 +1182,7 @@ xfs_refcount_adjust_extents(
> */
> cur->bc_refc.nr_ops++;
> if (tmp.rc_refcount) {
> - error = xfs_refcount_insert(cur, &tmp,
> - &found_tmp);
> + error = xfs_refcount_insert(cur, &tmp);
> if (error)
> goto out_error;
> } else {
> @@ -1784,7 +1782,7 @@ xfs_refcount_adjust_cow_extents(
> {
> struct xfs_refcount_irec ext, tmp;
> int error;
> - int found_rec, found_tmp;
> + int found_rec;
>
> if (aglen == 0)
> return 0;
> @@ -1827,8 +1825,7 @@ xfs_refcount_adjust_cow_extents(
>
> trace_xfs_refcount_modify_extent(cur, &tmp);
>
> - error = xfs_refcount_insert(cur, &tmp,
> - &found_tmp);
> + error = xfs_refcount_insert(cur, &tmp);
> if (error)
> goto out_error;
> break;
> diff --git a/fs/xfs/libxfs/xfs_refcount.h b/fs/xfs/libxfs/xfs_refcount.h
> index f2e299a716a4..cf300532bf04 100644
> --- a/fs/xfs/libxfs/xfs_refcount.h
> +++ b/fs/xfs/libxfs/xfs_refcount.h
> @@ -126,7 +126,7 @@ xfs_failaddr_t xfs_refcount_check_irec(struct xfs_perag *pag,
> xfs_failaddr_t xfs_rtrefcount_check_irec(struct xfs_rtgroup *rtg,
> const struct xfs_refcount_irec *irec);
> extern int xfs_refcount_insert(struct xfs_btree_cur *cur,
> - struct xfs_refcount_irec *irec, int *stat);
> + struct xfs_refcount_irec *irec);
>
> extern struct kmem_cache *xfs_refcount_intent_cache;
>
> diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
> index ff4aef8d30a7..99575ae419a7 100644
> --- a/fs/xfs/libxfs/xfs_rmap.c
> +++ b/fs/xfs/libxfs/xfs_rmap.c
> @@ -145,7 +145,7 @@ xfs_rmap_insert(
> rcur->bc_rec.r.rm_owner = owner;
> rcur->bc_rec.r.rm_offset = offset;
> rcur->bc_rec.r.rm_flags = flags;
> - error = xfs_btree_insert(rcur, &i);
> + error = xfs_btree_insert(rcur);
> done:
> if (error)
> trace_xfs_rmap_insert_error(rcur, error, _RET_IP_);
> @@ -855,7 +855,7 @@ xfs_rmap_unmap(
> cur->bc_rec.r.rm_owner,
> cur->bc_rec.r.rm_offset,
> cur->bc_rec.r.rm_flags);
> - error = xfs_btree_insert(cur, &i);
> + error = xfs_btree_insert(cur);
> if (error)
> goto out_error;
> }
> @@ -1175,7 +1175,7 @@ xfs_rmap_map(
> cur->bc_rec.r.rm_offset = offset;
> cur->bc_rec.r.rm_flags = flags;
> trace_xfs_rmap_insert(cur, bno, len, owner, offset, flags);
> - error = xfs_btree_insert(cur, &i);
> + error = xfs_btree_insert(cur);
> if (error)
> goto out_error;
> }
> @@ -1577,7 +1577,7 @@ xfs_rmap_convert(
> NEW.rm_flags = newext;
> cur->bc_rec.r = NEW;
> trace_xfs_rmap_insert(cur, bno, len, owner, offset, newext);
> - error = xfs_btree_insert(cur, &i);
> + error = xfs_btree_insert(cur);
> if (error)
> goto done;
> break;
> @@ -1630,7 +1630,7 @@ xfs_rmap_convert(
> NEW.rm_flags = newext;
> cur->bc_rec.r = NEW;
> trace_xfs_rmap_insert(cur, bno, len, owner, offset, newext);
> - error = xfs_btree_insert(cur, &i);
> + error = xfs_btree_insert(cur);
> if (error)
> goto done;
> break;
> @@ -1658,7 +1658,7 @@ xfs_rmap_convert(
> trace_xfs_rmap_insert(cur, NEW.rm_startblock,
> NEW.rm_blockcount, NEW.rm_owner, NEW.rm_offset,
> NEW.rm_flags);
> - error = xfs_btree_insert(cur, &i);
> + error = xfs_btree_insert(cur);
> if (error)
> goto done;
> /*
> @@ -1679,7 +1679,7 @@ xfs_rmap_convert(
> cur->bc_rec.r.rm_flags &= ~XFS_RMAP_UNWRITTEN;
> cur->bc_rec.r.rm_flags |= newext;
> trace_xfs_rmap_insert(cur, bno, len, owner, offset, newext);
> - error = xfs_btree_insert(cur, &i);
> + error = xfs_btree_insert(cur);
> if (error)
> goto done;
> break;
> diff --git a/fs/xfs/scrub/rcbag.c b/fs/xfs/scrub/rcbag.c
> index fd9ccb205725..41e798f65c67 100644
> --- a/fs/xfs/scrub/rcbag.c
> +++ b/fs/xfs/scrub/rcbag.c
> @@ -102,7 +102,7 @@ rcbag_add(
> bagrec.rbg_blockcount = rmap->rm_blockcount;
> bagrec.rbg_refcount = 1;
>
> - error = rcbagbt_insert(cur, &bagrec, &has);
> + error = rcbagbt_insert(cur, &bagrec);
> if (error)
> goto out_cur;
> }
> diff --git a/fs/xfs/scrub/rcbag_btree.c b/fs/xfs/scrub/rcbag_btree.c
> index 367f8ccf55c4..790931fa8f06 100644
> --- a/fs/xfs/scrub/rcbag_btree.c
> +++ b/fs/xfs/scrub/rcbag_btree.c
> @@ -342,11 +342,10 @@ rcbagbt_update(
> int
> rcbagbt_insert(
> struct xfs_btree_cur *cur,
> - const struct rcbag_rec *rec,
> - int *success)
> + const struct rcbag_rec *rec)
> {
> struct rcbag_rec *btrec = (struct rcbag_rec *)&cur->bc_rec;
>
> memcpy(btrec, rec, sizeof(struct rcbag_rec));
> - return xfs_btree_insert(cur, success);
> + return xfs_btree_insert(cur);
> }
> diff --git a/fs/xfs/scrub/rcbag_btree.h b/fs/xfs/scrub/rcbag_btree.h
> index 03cadb032552..da45d9e885f8 100644
> --- a/fs/xfs/scrub/rcbag_btree.h
> +++ b/fs/xfs/scrub/rcbag_btree.h
> @@ -70,8 +70,7 @@ int rcbagbt_lookup_eq(struct xfs_btree_cur *cur,
> const struct xfs_rmap_irec *rmap, int *success);
> int rcbagbt_get_rec(struct xfs_btree_cur *cur, struct rcbag_rec *rec, int *has);
> int rcbagbt_update(struct xfs_btree_cur *cur, const struct rcbag_rec *rec);
> -int rcbagbt_insert(struct xfs_btree_cur *cur, const struct rcbag_rec *rec,
> - int *success);
> +int rcbagbt_insert(struct xfs_btree_cur *cur, const struct rcbag_rec *rec);
>
> #else
> # define rcbagbt_init_cur_cache() 0
> --
> 2.55.0
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 6/8] xfs: add stat check to xfs_btree_delete and remove caller checks
2026-10-16 19:29 [PATCH 0/8] xfs: consolidate btree insert/delete checks Eric Sandeen
` (4 preceding siblings ...)
2026-10-16 19:29 ` [PATCH 5/8] xfs: remove status parameter from several btree insert functions Eric Sandeen
@ 2026-10-16 19:29 ` 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:29 ` [PATCH 8/8] xfs: remove status parameter from two btree delete functions Eric Sandeen
7 siblings, 1 reply; 17+ messages in thread
From: Eric Sandeen @ 2026-10-16 19:29 UTC (permalink / raw)
To: linux-xfs; +Cc: cem, hch, dgc, Eric Sandeen
Unlike xfs_btree_insert(), xfs_btree_delete() does not validate its own
status return internally, so every caller open-codes the same check that
a record was actually deleted:
error = xfs_btree_delete(cur, &i);
if (error)
return error;
if (XFS_IS_CORRUPT(mp, i != 1)) {
xfs_btree_mark_sick(cur);
return -EFSCORRUPTED;
}
No caller expects or accepts i == 0; that is considered corruption in
every case.
Move this check into xfs_btree_delete, to mirror xfs_btree_insert's
behavior, so that xfs_btree_delete either returns error = 0 / status = 1,
or a nonzero error.
With that in place, remove the now-redundant i != 1 checks from all
callers, parallel to what was done earlier for xfs_btree_insert().
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/libxfs/xfs_alloc.c | 33 -------------------------------
fs/xfs/libxfs/xfs_bmap.c | 38 ------------------------------------
fs/xfs/libxfs/xfs_btree.c | 6 ++++++
fs/xfs/libxfs/xfs_ialloc.c | 15 --------------
fs/xfs/libxfs/xfs_refcount.c | 5 -----
fs/xfs/libxfs/xfs_rmap.c | 35 ---------------------------------
fs/xfs/scrub/rcbag.c | 4 ----
7 files changed, 6 insertions(+), 130 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index acd8f4521c4c..bc43afff33bc 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -678,10 +678,6 @@ xfs_alloc_fixup_trees(
*/
if ((error = xfs_btree_delete(cnt_cur, &i)))
return error;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cnt_cur);
- return -EFSCORRUPTED;
- }
/*
* Add new by-size btree entry(s).
*/
@@ -716,10 +712,6 @@ xfs_alloc_fixup_trees(
*/
if ((error = xfs_btree_delete(bno_cur, &i)))
return error;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(bno_cur);
- return -EFSCORRUPTED;
- }
} else {
/*
* Update the by-block entry to start later|be shorter.
@@ -2155,11 +2147,6 @@ xfs_free_ag_extent(
}
if ((error = xfs_btree_delete(cnt_cur, &i)))
goto error0;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cnt_cur);
- error = -EFSCORRUPTED;
- goto error0;
- }
/*
* Delete the old by-size entry on the right.
*/
@@ -2172,21 +2159,11 @@ xfs_free_ag_extent(
}
if ((error = xfs_btree_delete(cnt_cur, &i)))
goto error0;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cnt_cur);
- error = -EFSCORRUPTED;
- goto error0;
- }
/*
* Delete the old by-block entry for the right block.
*/
if ((error = xfs_btree_delete(bno_cur, &i)))
goto error0;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(bno_cur);
- error = -EFSCORRUPTED;
- goto error0;
- }
/*
* Move the by-block cursor back to the left neighbor.
*/
@@ -2244,11 +2221,6 @@ xfs_free_ag_extent(
}
if ((error = xfs_btree_delete(cnt_cur, &i)))
goto error0;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cnt_cur);
- error = -EFSCORRUPTED;
- goto error0;
- }
/*
* Back up the by-block cursor to the left neighbor, and
* update its length.
@@ -2282,11 +2254,6 @@ xfs_free_ag_extent(
}
if ((error = xfs_btree_delete(cnt_cur, &i)))
goto error0;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cnt_cur);
- error = -EFSCORRUPTED;
- goto error0;
- }
/*
* Update the starting block and length of the right
* neighbor in the by-block tree.
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 09d6792e42be..cf7360adf1a8 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1562,11 +1562,6 @@ xfs_bmap_add_extent_delay_real(
error = xfs_btree_delete(bma->cur, &i);
if (error)
goto done;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(bma->cur);
- error = -EFSCORRUPTED;
- goto done;
- }
error = xfs_btree_decrement(bma->cur, 0, &i);
if (error)
goto done;
@@ -2110,11 +2105,6 @@ xfs_bmap_add_extent_unwritten_real(
}
if ((error = xfs_btree_delete(cur, &i)))
goto done;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto done;
- }
if ((error = xfs_btree_decrement(cur, 0, &i)))
goto done;
if (XFS_IS_CORRUPT(mp, i != 1)) {
@@ -2124,11 +2114,6 @@ xfs_bmap_add_extent_unwritten_real(
}
if ((error = xfs_btree_delete(cur, &i)))
goto done;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto done;
- }
if ((error = xfs_btree_decrement(cur, 0, &i)))
goto done;
if (XFS_IS_CORRUPT(mp, i != 1)) {
@@ -2167,11 +2152,6 @@ xfs_bmap_add_extent_unwritten_real(
}
if ((error = xfs_btree_delete(cur, &i)))
goto done;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto done;
- }
if ((error = xfs_btree_decrement(cur, 0, &i)))
goto done;
if (XFS_IS_CORRUPT(mp, i != 1)) {
@@ -2213,11 +2193,6 @@ xfs_bmap_add_extent_unwritten_real(
}
if ((error = xfs_btree_delete(cur, &i)))
goto done;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto done;
- }
if ((error = xfs_btree_decrement(cur, 0, &i)))
goto done;
if (XFS_IS_CORRUPT(mp, i != 1)) {
@@ -2627,11 +2602,6 @@ xfs_bmap_add_extent_hole_real(
error = xfs_btree_delete(cur, &i);
if (error)
goto done;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto done;
- }
error = xfs_btree_decrement(cur, 0, &i);
if (error)
goto done;
@@ -4958,10 +4928,6 @@ xfs_bmap_del_extent_real(
}
if ((error = xfs_btree_delete(cur, &i)))
return error;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cur);
- return -EFSCORRUPTED;
- }
break;
case BMAP_LEFT_FILLING:
/*
@@ -5525,10 +5491,6 @@ xfs_bmse_merge(
error = xfs_btree_delete(cur, &i);
if (error)
return error;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cur);
- return -EFSCORRUPTED;
- }
/* lookup and update size of the previous extent */
error = xfs_bmbt_lookup_eq(cur, left, &i);
diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index e2e698596c21..367c14e4dec5 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -4598,6 +4598,12 @@ xfs_btree_delete(
}
}
+ if (XFS_IS_CORRUPT(cur->bc_mp, i != 1)) {
+ xfs_btree_mark_sick(cur);
+ error = -EFSCORRUPTED;
+ goto error0;
+ }
+
*stat = i;
return 0;
error0:
diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index bd7b0a24225f..7a9cc0b9e206 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -1620,11 +1620,6 @@ xfs_dialloc_ag(
error = xfs_btree_delete(cur, &i);
if (error)
goto error_cur;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto error_cur;
- }
}
/*
@@ -2158,11 +2153,6 @@ xfs_difree_inobt(
__func__, error);
goto error0;
}
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto error0;
- }
error = xfs_difree_inode_chunk(tp, pag, &rec);
if (error)
@@ -2287,11 +2277,6 @@ xfs_difree_finobt(
error = xfs_btree_delete(cur, &i);
if (error)
goto error;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto error;
- }
} else {
error = xfs_inobt_update(cur, &rec);
if (error)
diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
index 574a5e7c0cdf..bf7526621681 100644
--- a/fs/xfs/libxfs/xfs_refcount.c
+++ b/fs/xfs/libxfs/xfs_refcount.c
@@ -305,11 +305,6 @@ xfs_refcount_delete(
}
trace_xfs_refcount_delete(cur, &irec);
error = xfs_btree_delete(cur, i);
- if (XFS_IS_CORRUPT(cur->bc_mp, *i != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto out_error;
- }
if (error)
goto out_error;
error = xfs_refcount_lookup_ge(cur, irec.rc_domain, irec.rc_startblock,
diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
index 99575ae419a7..31c41e9bcb3f 100644
--- a/fs/xfs/libxfs/xfs_rmap.c
+++ b/fs/xfs/libxfs/xfs_rmap.c
@@ -178,11 +178,6 @@ xfs_rmap_delete(
error = xfs_btree_delete(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_delete_error(rcur, error, _RET_IP_);
@@ -778,11 +773,6 @@ xfs_rmap_unmap(
error = xfs_btree_delete(cur, &i);
if (error)
goto out_error;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto out_error;
- }
} else if (ltrec.rm_startblock == bno) {
/*
* overlap left hand side of extent: move the start, trim the
@@ -1131,11 +1121,6 @@ xfs_rmap_map(
error = xfs_btree_delete(cur, &i);
if (error)
goto out_error;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto out_error;
- }
}
/* point the cursor back to the left record and update */
@@ -1413,11 +1398,6 @@ xfs_rmap_convert(
error = xfs_btree_delete(cur, &i);
if (error)
goto done;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto done;
- }
error = xfs_btree_decrement(cur, 0, &i);
if (error)
goto done;
@@ -1432,11 +1412,6 @@ xfs_rmap_convert(
error = xfs_btree_delete(cur, &i);
if (error)
goto done;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto done;
- }
error = xfs_btree_decrement(cur, 0, &i);
if (error)
goto done;
@@ -1463,11 +1438,6 @@ xfs_rmap_convert(
error = xfs_btree_delete(cur, &i);
if (error)
goto done;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto done;
- }
error = xfs_btree_decrement(cur, 0, &i);
if (error)
goto done;
@@ -1502,11 +1472,6 @@ xfs_rmap_convert(
error = xfs_btree_delete(cur, &i);
if (error)
goto done;
- if (XFS_IS_CORRUPT(mp, i != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto done;
- }
error = xfs_btree_decrement(cur, 0, &i);
if (error)
goto done;
diff --git a/fs/xfs/scrub/rcbag.c b/fs/xfs/scrub/rcbag.c
index 41e798f65c67..5a390e3c7777 100644
--- a/fs/xfs/scrub/rcbag.c
+++ b/fs/xfs/scrub/rcbag.c
@@ -242,10 +242,6 @@ rcbag_remove_ending_at(
error = xfs_btree_delete(cur, &has);
if (error)
goto out_cur;
- if (!has) {
- error = -EFSCORRUPTED;
- goto out_cur;
- }
bag->nr_items -= bagrec.rbg_refcount;
}
--
2.55.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 6/8] xfs: add stat check to xfs_btree_delete and remove caller checks
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
0 siblings, 0 replies; 17+ messages in thread
From: Darrick J. Wong @ 2026-10-16 19:47 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs, cem, hch, dgc
On Fri, Oct 16, 2026 at 02:29:06PM -0500, Eric Sandeen wrote:
> Unlike xfs_btree_insert(), xfs_btree_delete() does not validate its own
> status return internally, so every caller open-codes the same check that
> a record was actually deleted:
>
> error = xfs_btree_delete(cur, &i);
> if (error)
> return error;
> if (XFS_IS_CORRUPT(mp, i != 1)) {
> xfs_btree_mark_sick(cur);
> return -EFSCORRUPTED;
> }
>
> No caller expects or accepts i == 0; that is considered corruption in
> every case.
>
> Move this check into xfs_btree_delete, to mirror xfs_btree_insert's
> behavior, so that xfs_btree_delete either returns error = 0 / status = 1,
> or a nonzero error.
>
> With that in place, remove the now-redundant i != 1 checks from all
> callers, parallel to what was done earlier for xfs_btree_insert().
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> fs/xfs/libxfs/xfs_alloc.c | 33 -------------------------------
> fs/xfs/libxfs/xfs_bmap.c | 38 ------------------------------------
> fs/xfs/libxfs/xfs_btree.c | 6 ++++++
> fs/xfs/libxfs/xfs_ialloc.c | 15 --------------
> fs/xfs/libxfs/xfs_refcount.c | 5 -----
> fs/xfs/libxfs/xfs_rmap.c | 35 ---------------------------------
> fs/xfs/scrub/rcbag.c | 4 ----
> 7 files changed, 6 insertions(+), 130 deletions(-)
Excellent! Very nice cleanup!
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
>
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index acd8f4521c4c..bc43afff33bc 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -678,10 +678,6 @@ xfs_alloc_fixup_trees(
> */
> if ((error = xfs_btree_delete(cnt_cur, &i)))
> return error;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cnt_cur);
> - return -EFSCORRUPTED;
> - }
> /*
> * Add new by-size btree entry(s).
> */
> @@ -716,10 +712,6 @@ xfs_alloc_fixup_trees(
> */
> if ((error = xfs_btree_delete(bno_cur, &i)))
> return error;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(bno_cur);
> - return -EFSCORRUPTED;
> - }
> } else {
> /*
> * Update the by-block entry to start later|be shorter.
> @@ -2155,11 +2147,6 @@ xfs_free_ag_extent(
> }
> if ((error = xfs_btree_delete(cnt_cur, &i)))
> goto error0;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cnt_cur);
> - error = -EFSCORRUPTED;
> - goto error0;
> - }
> /*
> * Delete the old by-size entry on the right.
> */
> @@ -2172,21 +2159,11 @@ xfs_free_ag_extent(
> }
> if ((error = xfs_btree_delete(cnt_cur, &i)))
> goto error0;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cnt_cur);
> - error = -EFSCORRUPTED;
> - goto error0;
> - }
> /*
> * Delete the old by-block entry for the right block.
> */
> if ((error = xfs_btree_delete(bno_cur, &i)))
> goto error0;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(bno_cur);
> - error = -EFSCORRUPTED;
> - goto error0;
> - }
> /*
> * Move the by-block cursor back to the left neighbor.
> */
> @@ -2244,11 +2221,6 @@ xfs_free_ag_extent(
> }
> if ((error = xfs_btree_delete(cnt_cur, &i)))
> goto error0;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cnt_cur);
> - error = -EFSCORRUPTED;
> - goto error0;
> - }
> /*
> * Back up the by-block cursor to the left neighbor, and
> * update its length.
> @@ -2282,11 +2254,6 @@ xfs_free_ag_extent(
> }
> if ((error = xfs_btree_delete(cnt_cur, &i)))
> goto error0;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cnt_cur);
> - error = -EFSCORRUPTED;
> - goto error0;
> - }
> /*
> * Update the starting block and length of the right
> * neighbor in the by-block tree.
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 09d6792e42be..cf7360adf1a8 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1562,11 +1562,6 @@ xfs_bmap_add_extent_delay_real(
> error = xfs_btree_delete(bma->cur, &i);
> if (error)
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(bma->cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> error = xfs_btree_decrement(bma->cur, 0, &i);
> if (error)
> goto done;
> @@ -2110,11 +2105,6 @@ xfs_bmap_add_extent_unwritten_real(
> }
> if ((error = xfs_btree_delete(cur, &i)))
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> if ((error = xfs_btree_decrement(cur, 0, &i)))
> goto done;
> if (XFS_IS_CORRUPT(mp, i != 1)) {
> @@ -2124,11 +2114,6 @@ xfs_bmap_add_extent_unwritten_real(
> }
> if ((error = xfs_btree_delete(cur, &i)))
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> if ((error = xfs_btree_decrement(cur, 0, &i)))
> goto done;
> if (XFS_IS_CORRUPT(mp, i != 1)) {
> @@ -2167,11 +2152,6 @@ xfs_bmap_add_extent_unwritten_real(
> }
> if ((error = xfs_btree_delete(cur, &i)))
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> if ((error = xfs_btree_decrement(cur, 0, &i)))
> goto done;
> if (XFS_IS_CORRUPT(mp, i != 1)) {
> @@ -2213,11 +2193,6 @@ xfs_bmap_add_extent_unwritten_real(
> }
> if ((error = xfs_btree_delete(cur, &i)))
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> if ((error = xfs_btree_decrement(cur, 0, &i)))
> goto done;
> if (XFS_IS_CORRUPT(mp, i != 1)) {
> @@ -2627,11 +2602,6 @@ xfs_bmap_add_extent_hole_real(
> error = xfs_btree_delete(cur, &i);
> if (error)
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> error = xfs_btree_decrement(cur, 0, &i);
> if (error)
> goto done;
> @@ -4958,10 +4928,6 @@ xfs_bmap_del_extent_real(
> }
> if ((error = xfs_btree_delete(cur, &i)))
> return error;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - return -EFSCORRUPTED;
> - }
> break;
> case BMAP_LEFT_FILLING:
> /*
> @@ -5525,10 +5491,6 @@ xfs_bmse_merge(
> error = xfs_btree_delete(cur, &i);
> if (error)
> return error;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - return -EFSCORRUPTED;
> - }
>
> /* lookup and update size of the previous extent */
> error = xfs_bmbt_lookup_eq(cur, left, &i);
> diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
> index e2e698596c21..367c14e4dec5 100644
> --- a/fs/xfs/libxfs/xfs_btree.c
> +++ b/fs/xfs/libxfs/xfs_btree.c
> @@ -4598,6 +4598,12 @@ xfs_btree_delete(
> }
> }
>
> + if (XFS_IS_CORRUPT(cur->bc_mp, i != 1)) {
> + xfs_btree_mark_sick(cur);
> + error = -EFSCORRUPTED;
> + goto error0;
> + }
> +
> *stat = i;
> return 0;
> error0:
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index bd7b0a24225f..7a9cc0b9e206 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -1620,11 +1620,6 @@ xfs_dialloc_ag(
> error = xfs_btree_delete(cur, &i);
> if (error)
> goto error_cur;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto error_cur;
> - }
> }
>
> /*
> @@ -2158,11 +2153,6 @@ xfs_difree_inobt(
> __func__, error);
> goto error0;
> }
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto error0;
> - }
>
> error = xfs_difree_inode_chunk(tp, pag, &rec);
> if (error)
> @@ -2287,11 +2277,6 @@ xfs_difree_finobt(
> error = xfs_btree_delete(cur, &i);
> if (error)
> goto error;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto error;
> - }
> } else {
> error = xfs_inobt_update(cur, &rec);
> if (error)
> diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
> index 574a5e7c0cdf..bf7526621681 100644
> --- a/fs/xfs/libxfs/xfs_refcount.c
> +++ b/fs/xfs/libxfs/xfs_refcount.c
> @@ -305,11 +305,6 @@ xfs_refcount_delete(
> }
> trace_xfs_refcount_delete(cur, &irec);
> error = xfs_btree_delete(cur, i);
> - if (XFS_IS_CORRUPT(cur->bc_mp, *i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto out_error;
> - }
> if (error)
> goto out_error;
> error = xfs_refcount_lookup_ge(cur, irec.rc_domain, irec.rc_startblock,
> diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
> index 99575ae419a7..31c41e9bcb3f 100644
> --- a/fs/xfs/libxfs/xfs_rmap.c
> +++ b/fs/xfs/libxfs/xfs_rmap.c
> @@ -178,11 +178,6 @@ xfs_rmap_delete(
> error = xfs_btree_delete(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_delete_error(rcur, error, _RET_IP_);
> @@ -778,11 +773,6 @@ xfs_rmap_unmap(
> error = xfs_btree_delete(cur, &i);
> if (error)
> goto out_error;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto out_error;
> - }
> } else if (ltrec.rm_startblock == bno) {
> /*
> * overlap left hand side of extent: move the start, trim the
> @@ -1131,11 +1121,6 @@ xfs_rmap_map(
> error = xfs_btree_delete(cur, &i);
> if (error)
> goto out_error;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto out_error;
> - }
> }
>
> /* point the cursor back to the left record and update */
> @@ -1413,11 +1398,6 @@ xfs_rmap_convert(
> error = xfs_btree_delete(cur, &i);
> if (error)
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> error = xfs_btree_decrement(cur, 0, &i);
> if (error)
> goto done;
> @@ -1432,11 +1412,6 @@ xfs_rmap_convert(
> error = xfs_btree_delete(cur, &i);
> if (error)
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> error = xfs_btree_decrement(cur, 0, &i);
> if (error)
> goto done;
> @@ -1463,11 +1438,6 @@ xfs_rmap_convert(
> error = xfs_btree_delete(cur, &i);
> if (error)
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> error = xfs_btree_decrement(cur, 0, &i);
> if (error)
> goto done;
> @@ -1502,11 +1472,6 @@ xfs_rmap_convert(
> error = xfs_btree_delete(cur, &i);
> if (error)
> goto done;
> - if (XFS_IS_CORRUPT(mp, i != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto done;
> - }
> error = xfs_btree_decrement(cur, 0, &i);
> if (error)
> goto done;
> diff --git a/fs/xfs/scrub/rcbag.c b/fs/xfs/scrub/rcbag.c
> index 41e798f65c67..5a390e3c7777 100644
> --- a/fs/xfs/scrub/rcbag.c
> +++ b/fs/xfs/scrub/rcbag.c
> @@ -242,10 +242,6 @@ rcbag_remove_ending_at(
> error = xfs_btree_delete(cur, &has);
> if (error)
> goto out_cur;
> - if (!has) {
> - error = -EFSCORRUPTED;
> - goto out_cur;
> - }
>
> bag->nr_items -= bagrec.rbg_refcount;
> }
> --
> 2.55.0
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 7/8] xfs: remove redundant stat checks after xfs_refcount_delete()
2026-10-16 19:29 [PATCH 0/8] xfs: consolidate btree insert/delete checks Eric Sandeen
` (5 preceding siblings ...)
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:29 ` 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
7 siblings, 1 reply; 17+ messages in thread
From: Eric Sandeen @ 2026-10-16 19:29 UTC (permalink / raw)
To: linux-xfs; +Cc: cem, hch, dgc, Eric Sandeen
xfs_refcount_delete() simply returns the error and status from
xfs_btree_delete, which now validates the delete or returns an error.
This makes every instance of the 2nd test in this idiom:
error = xfs_refcount_delete(cur, &found_rec);
if (error)
goto out_error;
if (XFS_IS_CORRUPT(cur->bc_mp, found_rec != 1)) {
xfs_btree_mark_sick(cur);
error = -EFSCORRUPTED;
goto out_error;
}
unnecessary/unreachable code. Remove them.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/libxfs/xfs_refcount.c | 30 ------------------------------
1 file changed, 30 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
index bf7526621681..924f839b35a0 100644
--- a/fs/xfs/libxfs/xfs_refcount.c
+++ b/fs/xfs/libxfs/xfs_refcount.c
@@ -500,21 +500,11 @@ xfs_refcount_merge_center_extents(
error = xfs_refcount_delete(cur, &found_rec);
if (error)
goto out_error;
- if (XFS_IS_CORRUPT(cur->bc_mp, found_rec != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto out_error;
- }
if (center->rc_refcount > 1) {
error = xfs_refcount_delete(cur, &found_rec);
if (error)
goto out_error;
- if (XFS_IS_CORRUPT(cur->bc_mp, found_rec != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto out_error;
- }
}
/* Enlarge the left extent. */
@@ -574,11 +564,6 @@ xfs_refcount_merge_left_extent(
error = xfs_refcount_delete(cur, &found_rec);
if (error)
goto out_error;
- if (XFS_IS_CORRUPT(cur->bc_mp, found_rec != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto out_error;
- }
}
/* Enlarge the left extent. */
@@ -641,11 +626,6 @@ xfs_refcount_merge_right_extent(
error = xfs_refcount_delete(cur, &found_rec);
if (error)
goto out_error;
- if (XFS_IS_CORRUPT(cur->bc_mp, found_rec != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto out_error;
- }
}
/* Enlarge the right extent. */
@@ -1232,11 +1212,6 @@ xfs_refcount_adjust_extents(
error = xfs_refcount_delete(cur, &found_rec);
if (error)
goto out_error;
- if (XFS_IS_CORRUPT(cur->bc_mp, found_rec != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto out_error;
- }
goto advloop;
} else {
error = xrefc_free_extent(cur, &ext);
@@ -1847,11 +1822,6 @@ xfs_refcount_adjust_cow_extents(
error = xfs_refcount_delete(cur, &found_rec);
if (error)
goto out_error;
- if (XFS_IS_CORRUPT(cur->bc_mp, found_rec != 1)) {
- xfs_btree_mark_sick(cur);
- error = -EFSCORRUPTED;
- goto out_error;
- }
break;
default:
ASSERT(0);
--
2.55.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 7/8] xfs: remove redundant stat checks after xfs_refcount_delete()
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
0 siblings, 0 replies; 17+ messages in thread
From: Darrick J. Wong @ 2026-10-16 19:47 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs, cem, hch, dgc
On Fri, Oct 16, 2026 at 02:29:07PM -0500, Eric Sandeen wrote:
> xfs_refcount_delete() simply returns the error and status from
> xfs_btree_delete, which now validates the delete or returns an error.
>
> This makes every instance of the 2nd test in this idiom:
>
> error = xfs_refcount_delete(cur, &found_rec);
> if (error)
> goto out_error;
> if (XFS_IS_CORRUPT(cur->bc_mp, found_rec != 1)) {
> xfs_btree_mark_sick(cur);
> error = -EFSCORRUPTED;
> goto out_error;
> }
>
> unnecessary/unreachable code. Remove them.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Looks good,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> ---
> fs/xfs/libxfs/xfs_refcount.c | 30 ------------------------------
> 1 file changed, 30 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
> index bf7526621681..924f839b35a0 100644
> --- a/fs/xfs/libxfs/xfs_refcount.c
> +++ b/fs/xfs/libxfs/xfs_refcount.c
> @@ -500,21 +500,11 @@ xfs_refcount_merge_center_extents(
> error = xfs_refcount_delete(cur, &found_rec);
> if (error)
> goto out_error;
> - if (XFS_IS_CORRUPT(cur->bc_mp, found_rec != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto out_error;
> - }
>
> if (center->rc_refcount > 1) {
> error = xfs_refcount_delete(cur, &found_rec);
> if (error)
> goto out_error;
> - if (XFS_IS_CORRUPT(cur->bc_mp, found_rec != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto out_error;
> - }
> }
>
> /* Enlarge the left extent. */
> @@ -574,11 +564,6 @@ xfs_refcount_merge_left_extent(
> error = xfs_refcount_delete(cur, &found_rec);
> if (error)
> goto out_error;
> - if (XFS_IS_CORRUPT(cur->bc_mp, found_rec != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto out_error;
> - }
> }
>
> /* Enlarge the left extent. */
> @@ -641,11 +626,6 @@ xfs_refcount_merge_right_extent(
> error = xfs_refcount_delete(cur, &found_rec);
> if (error)
> goto out_error;
> - if (XFS_IS_CORRUPT(cur->bc_mp, found_rec != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto out_error;
> - }
> }
>
> /* Enlarge the right extent. */
> @@ -1232,11 +1212,6 @@ xfs_refcount_adjust_extents(
> error = xfs_refcount_delete(cur, &found_rec);
> if (error)
> goto out_error;
> - if (XFS_IS_CORRUPT(cur->bc_mp, found_rec != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto out_error;
> - }
> goto advloop;
> } else {
> error = xrefc_free_extent(cur, &ext);
> @@ -1847,11 +1822,6 @@ xfs_refcount_adjust_cow_extents(
> error = xfs_refcount_delete(cur, &found_rec);
> if (error)
> goto out_error;
> - if (XFS_IS_CORRUPT(cur->bc_mp, found_rec != 1)) {
> - xfs_btree_mark_sick(cur);
> - error = -EFSCORRUPTED;
> - goto out_error;
> - }
> break;
> default:
> ASSERT(0);
> --
> 2.55.0
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 8/8] xfs: remove status parameter from two btree delete functions
2026-10-16 19:29 [PATCH 0/8] xfs: consolidate btree insert/delete checks Eric Sandeen
` (6 preceding siblings ...)
2026-10-16 19:29 ` [PATCH 7/8] xfs: remove redundant stat checks after xfs_refcount_delete() Eric Sandeen
@ 2026-10-16 19:29 ` Eric Sandeen
2026-10-16 19:47 ` Darrick J. Wong
7 siblings, 1 reply; 17+ messages in thread
From: Eric Sandeen @ 2026-10-16 19:29 UTC (permalink / raw)
To: linux-xfs; +Cc: cem, hch, dgc, Eric Sandeen
xfs_btree_delete and its callers now have no need for the status
parameter; either they return error = 0 / status = 1, or nonzero error
and status 0. No caller evaluates the status variable, so remove that
parameter from these functions.
As requested, convert all of the compound tests/assignments to discrete
lines.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/libxfs/xfs_alloc.c | 21 ++++++++++++++-------
fs/xfs/libxfs/xfs_bmap.c | 21 +++++++++++++--------
fs/xfs/libxfs/xfs_btree.c | 4 +---
fs/xfs/libxfs/xfs_btree.h | 2 +-
fs/xfs/libxfs/xfs_ialloc.c | 8 ++++----
fs/xfs/libxfs/xfs_refcount.c | 17 ++++++++---------
fs/xfs/libxfs/xfs_rmap.c | 15 +++++++--------
fs/xfs/scrub/rcbag.c | 2 +-
8 files changed, 49 insertions(+), 41 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index bc43afff33bc..c32816390217 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -676,7 +676,8 @@ xfs_alloc_fixup_trees(
/*
* Delete the entry from the by-size btree.
*/
- if ((error = xfs_btree_delete(cnt_cur, &i)))
+ error = xfs_btree_delete(cnt_cur);
+ if (error)
return error;
/*
* Add new by-size btree entry(s).
@@ -710,7 +711,8 @@ xfs_alloc_fixup_trees(
/*
* No remaining freespace, just delete the by-block tree entry.
*/
- if ((error = xfs_btree_delete(bno_cur, &i)))
+ error = xfs_btree_delete(bno_cur);
+ if (error)
return error;
} else {
/*
@@ -2145,7 +2147,8 @@ xfs_free_ag_extent(
error = -EFSCORRUPTED;
goto error0;
}
- if ((error = xfs_btree_delete(cnt_cur, &i)))
+ error = xfs_btree_delete(cnt_cur);
+ if (error)
goto error0;
/*
* Delete the old by-size entry on the right.
@@ -2157,12 +2160,14 @@ xfs_free_ag_extent(
error = -EFSCORRUPTED;
goto error0;
}
- if ((error = xfs_btree_delete(cnt_cur, &i)))
+ error = xfs_btree_delete(cnt_cur);
+ if (error)
goto error0;
/*
* Delete the old by-block entry for the right block.
*/
- if ((error = xfs_btree_delete(bno_cur, &i)))
+ error = xfs_btree_delete(bno_cur);
+ if (error)
goto error0;
/*
* Move the by-block cursor back to the left neighbor.
@@ -2219,7 +2224,8 @@ xfs_free_ag_extent(
error = -EFSCORRUPTED;
goto error0;
}
- if ((error = xfs_btree_delete(cnt_cur, &i)))
+ error = xfs_btree_delete(cnt_cur);
+ if (error)
goto error0;
/*
* Back up the by-block cursor to the left neighbor, and
@@ -2252,7 +2258,8 @@ xfs_free_ag_extent(
error = -EFSCORRUPTED;
goto error0;
}
- if ((error = xfs_btree_delete(cnt_cur, &i)))
+ error = xfs_btree_delete(cnt_cur);
+ if (error)
goto error0;
/*
* Update the starting block and length of the right
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index cf7360adf1a8..beaa92ea8015 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1559,7 +1559,7 @@ xfs_bmap_add_extent_delay_real(
error = -EFSCORRUPTED;
goto done;
}
- error = xfs_btree_delete(bma->cur, &i);
+ error = xfs_btree_delete(bma->cur);
if (error)
goto done;
error = xfs_btree_decrement(bma->cur, 0, &i);
@@ -2103,7 +2103,8 @@ xfs_bmap_add_extent_unwritten_real(
error = -EFSCORRUPTED;
goto done;
}
- if ((error = xfs_btree_delete(cur, &i)))
+ error = xfs_btree_delete(cur);
+ if (error)
goto done;
if ((error = xfs_btree_decrement(cur, 0, &i)))
goto done;
@@ -2112,7 +2113,8 @@ xfs_bmap_add_extent_unwritten_real(
error = -EFSCORRUPTED;
goto done;
}
- if ((error = xfs_btree_delete(cur, &i)))
+ error = xfs_btree_delete(cur);
+ if (error)
goto done;
if ((error = xfs_btree_decrement(cur, 0, &i)))
goto done;
@@ -2150,7 +2152,8 @@ xfs_bmap_add_extent_unwritten_real(
error = -EFSCORRUPTED;
goto done;
}
- if ((error = xfs_btree_delete(cur, &i)))
+ error = xfs_btree_delete(cur);
+ if (error)
goto done;
if ((error = xfs_btree_decrement(cur, 0, &i)))
goto done;
@@ -2191,7 +2194,8 @@ xfs_bmap_add_extent_unwritten_real(
error = -EFSCORRUPTED;
goto done;
}
- if ((error = xfs_btree_delete(cur, &i)))
+ error = xfs_btree_delete(cur);
+ if (error)
goto done;
if ((error = xfs_btree_decrement(cur, 0, &i)))
goto done;
@@ -2599,7 +2603,7 @@ xfs_bmap_add_extent_hole_real(
error = -EFSCORRUPTED;
goto done;
}
- error = xfs_btree_delete(cur, &i);
+ error = xfs_btree_delete(cur);
if (error)
goto done;
error = xfs_btree_decrement(cur, 0, &i);
@@ -4926,7 +4930,8 @@ xfs_bmap_del_extent_real(
*logflagsp |= xfs_ilog_fext(whichfork);
break;
}
- if ((error = xfs_btree_delete(cur, &i)))
+ error = xfs_btree_delete(cur);
+ if (error)
return error;
break;
case BMAP_LEFT_FILLING:
@@ -5488,7 +5493,7 @@ xfs_bmse_merge(
return -EFSCORRUPTED;
}
- error = xfs_btree_delete(cur, &i);
+ error = xfs_btree_delete(cur);
if (error)
return error;
diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index 367c14e4dec5..91211d212c74 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -4555,8 +4555,7 @@ xfs_btree_delrec(
*/
int /* error */
xfs_btree_delete(
- struct xfs_btree_cur *cur,
- int *stat) /* success/failure */
+ struct xfs_btree_cur *cur)
{
int error; /* error return value */
int level;
@@ -4604,7 +4603,6 @@ xfs_btree_delete(
goto error0;
}
- *stat = i;
return 0;
error0:
return error;
diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
index 0f82f3106bac..c4337ab244f2 100644
--- a/fs/xfs/libxfs/xfs_btree.h
+++ b/fs/xfs/libxfs/xfs_btree.h
@@ -406,7 +406,7 @@ int xfs_btree_lookup(struct xfs_btree_cur *, xfs_lookup_t, int *);
int xfs_btree_update(struct xfs_btree_cur *, union xfs_btree_rec *);
int xfs_btree_new_iroot(struct xfs_btree_cur *, int *, int *);
int xfs_btree_insert(struct xfs_btree_cur *);
-int xfs_btree_delete(struct xfs_btree_cur *, int *);
+int xfs_btree_delete(struct xfs_btree_cur *);
int xfs_btree_get_rec(struct xfs_btree_cur *, union xfs_btree_rec **, int *);
int xfs_btree_change_owner(struct xfs_btree_cur *cur, uint64_t new_owner,
struct list_head *buffer_list);
diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index 7a9cc0b9e206..ec0a37499f28 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -1564,7 +1564,6 @@ xfs_dialloc_ag(
xfs_ino_t ino;
int error;
int offset;
- int i;
if (!xfs_has_finobt(mp))
return xfs_dialloc_ag_inobt(pag, tp, agbp, parent, inop);
@@ -1617,7 +1616,7 @@ xfs_dialloc_ag(
if (error)
goto error_cur;
} else {
- error = xfs_btree_delete(cur, &i);
+ error = xfs_btree_delete(cur);
if (error)
goto error_cur;
}
@@ -2148,7 +2147,8 @@ xfs_difree_inobt(
xfs_trans_mod_sb(tp, XFS_TRANS_SB_ICOUNT, -ilen);
xfs_trans_mod_sb(tp, XFS_TRANS_SB_IFREE, -(ilen - 1));
- if ((error = xfs_btree_delete(cur, &i))) {
+ error = xfs_btree_delete(cur);
+ if (error) {
xfs_warn(mp, "%s: xfs_btree_delete returned error %d.",
__func__, error);
goto error0;
@@ -2274,7 +2274,7 @@ xfs_difree_finobt(
*/
if (rec.ir_free == XFS_INOBT_ALL_FREE &&
mp->m_sb.sb_inopblock <= XFS_INODES_PER_CHUNK) {
- error = xfs_btree_delete(cur, &i);
+ error = xfs_btree_delete(cur);
if (error)
goto error;
} else {
diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
index 924f839b35a0..75e469576a8b 100644
--- a/fs/xfs/libxfs/xfs_refcount.c
+++ b/fs/xfs/libxfs/xfs_refcount.c
@@ -288,8 +288,7 @@ xfs_refcount_insert(
*/
STATIC int
xfs_refcount_delete(
- struct xfs_btree_cur *cur,
- int *i)
+ struct xfs_btree_cur *cur)
{
struct xfs_refcount_irec irec;
int found_rec;
@@ -304,7 +303,7 @@ xfs_refcount_delete(
goto out_error;
}
trace_xfs_refcount_delete(cur, &irec);
- error = xfs_btree_delete(cur, i);
+ error = xfs_btree_delete(cur);
if (error)
goto out_error;
error = xfs_refcount_lookup_ge(cur, irec.rc_domain, irec.rc_startblock,
@@ -497,12 +496,12 @@ xfs_refcount_merge_center_extents(
goto out_error;
}
- error = xfs_refcount_delete(cur, &found_rec);
+ error = xfs_refcount_delete(cur);
if (error)
goto out_error;
if (center->rc_refcount > 1) {
- error = xfs_refcount_delete(cur, &found_rec);
+ error = xfs_refcount_delete(cur);
if (error)
goto out_error;
}
@@ -561,7 +560,7 @@ xfs_refcount_merge_left_extent(
goto out_error;
}
- error = xfs_refcount_delete(cur, &found_rec);
+ error = xfs_refcount_delete(cur);
if (error)
goto out_error;
}
@@ -623,7 +622,7 @@ xfs_refcount_merge_right_extent(
goto out_error;
}
- error = xfs_refcount_delete(cur, &found_rec);
+ error = xfs_refcount_delete(cur);
if (error)
goto out_error;
}
@@ -1209,7 +1208,7 @@ xfs_refcount_adjust_extents(
if (error)
goto out_error;
} else if (ext.rc_refcount == 1) {
- error = xfs_refcount_delete(cur, &found_rec);
+ error = xfs_refcount_delete(cur);
if (error)
goto out_error;
goto advloop;
@@ -1819,7 +1818,7 @@ xfs_refcount_adjust_cow_extents(
ext.rc_refcount = 0;
trace_xfs_refcount_modify_extent(cur, &ext);
- error = xfs_refcount_delete(cur, &found_rec);
+ error = xfs_refcount_delete(cur);
if (error)
goto out_error;
break;
diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
index 31c41e9bcb3f..6fe06612eac2 100644
--- a/fs/xfs/libxfs/xfs_rmap.c
+++ b/fs/xfs/libxfs/xfs_rmap.c
@@ -175,7 +175,7 @@ xfs_rmap_delete(
goto done;
}
- error = xfs_btree_delete(rcur, &i);
+ error = xfs_btree_delete(rcur);
if (error)
goto done;
done:
@@ -770,7 +770,7 @@ xfs_rmap_unmap(
trace_xfs_rmap_delete(cur, ltrec.rm_startblock,
ltrec.rm_blockcount, ltrec.rm_owner,
ltrec.rm_offset, ltrec.rm_flags);
- error = xfs_btree_delete(cur, &i);
+ error = xfs_btree_delete(cur);
if (error)
goto out_error;
} else if (ltrec.rm_startblock == bno) {
@@ -1012,7 +1012,6 @@ xfs_rmap_map(
int have_gt;
int have_lt;
int error = 0;
- int i;
uint64_t owner;
uint64_t offset;
unsigned int flags = 0;
@@ -1118,7 +1117,7 @@ xfs_rmap_map(
trace_xfs_rmap_delete(cur, gtrec.rm_startblock,
gtrec.rm_blockcount, gtrec.rm_owner,
gtrec.rm_offset, gtrec.rm_flags);
- error = xfs_btree_delete(cur, &i);
+ error = xfs_btree_delete(cur);
if (error)
goto out_error;
}
@@ -1395,7 +1394,7 @@ xfs_rmap_convert(
trace_xfs_rmap_delete(cur, RIGHT.rm_startblock,
RIGHT.rm_blockcount, RIGHT.rm_owner,
RIGHT.rm_offset, RIGHT.rm_flags);
- error = xfs_btree_delete(cur, &i);
+ error = xfs_btree_delete(cur);
if (error)
goto done;
error = xfs_btree_decrement(cur, 0, &i);
@@ -1409,7 +1408,7 @@ xfs_rmap_convert(
trace_xfs_rmap_delete(cur, PREV.rm_startblock,
PREV.rm_blockcount, PREV.rm_owner,
PREV.rm_offset, PREV.rm_flags);
- error = xfs_btree_delete(cur, &i);
+ error = xfs_btree_delete(cur);
if (error)
goto done;
error = xfs_btree_decrement(cur, 0, &i);
@@ -1435,7 +1434,7 @@ xfs_rmap_convert(
trace_xfs_rmap_delete(cur, PREV.rm_startblock,
PREV.rm_blockcount, PREV.rm_owner,
PREV.rm_offset, PREV.rm_flags);
- error = xfs_btree_delete(cur, &i);
+ error = xfs_btree_delete(cur);
if (error)
goto done;
error = xfs_btree_decrement(cur, 0, &i);
@@ -1469,7 +1468,7 @@ xfs_rmap_convert(
trace_xfs_rmap_delete(cur, RIGHT.rm_startblock,
RIGHT.rm_blockcount, RIGHT.rm_owner,
RIGHT.rm_offset, RIGHT.rm_flags);
- error = xfs_btree_delete(cur, &i);
+ error = xfs_btree_delete(cur);
if (error)
goto done;
error = xfs_btree_decrement(cur, 0, &i);
diff --git a/fs/xfs/scrub/rcbag.c b/fs/xfs/scrub/rcbag.c
index 5a390e3c7777..bd1aa55a8910 100644
--- a/fs/xfs/scrub/rcbag.c
+++ b/fs/xfs/scrub/rcbag.c
@@ -239,7 +239,7 @@ rcbag_remove_ending_at(
if (rcbag_rec_next_bno(&bagrec) != next_bno)
continue;
- error = xfs_btree_delete(cur, &has);
+ error = xfs_btree_delete(cur);
if (error)
goto out_cur;
--
2.55.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 8/8] xfs: remove status parameter from two btree delete functions
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
0 siblings, 0 replies; 17+ messages in thread
From: Darrick J. Wong @ 2026-10-16 19:47 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs, cem, hch, dgc
On Fri, Oct 16, 2026 at 02:29:08PM -0500, Eric Sandeen wrote:
> xfs_btree_delete and its callers now have no need for the status
> parameter; either they return error = 0 / status = 1, or nonzero error
> and status 0. No caller evaluates the status variable, so remove that
> parameter from these functions.
>
> As requested, convert all of the compound tests/assignments to discrete
> lines.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> fs/xfs/libxfs/xfs_alloc.c | 21 ++++++++++++++-------
> fs/xfs/libxfs/xfs_bmap.c | 21 +++++++++++++--------
> fs/xfs/libxfs/xfs_btree.c | 4 +---
> fs/xfs/libxfs/xfs_btree.h | 2 +-
> fs/xfs/libxfs/xfs_ialloc.c | 8 ++++----
> fs/xfs/libxfs/xfs_refcount.c | 17 ++++++++---------
> fs/xfs/libxfs/xfs_rmap.c | 15 +++++++--------
> fs/xfs/scrub/rcbag.c | 2 +-
> 8 files changed, 49 insertions(+), 41 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index bc43afff33bc..c32816390217 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -676,7 +676,8 @@ xfs_alloc_fixup_trees(
> /*
> * Delete the entry from the by-size btree.
> */
> - if ((error = xfs_btree_delete(cnt_cur, &i)))
> + error = xfs_btree_delete(cnt_cur);
> + if (error)
> return error;
> /*
> * Add new by-size btree entry(s).
> @@ -710,7 +711,8 @@ xfs_alloc_fixup_trees(
> /*
> * No remaining freespace, just delete the by-block tree entry.
> */
> - if ((error = xfs_btree_delete(bno_cur, &i)))
> + error = xfs_btree_delete(bno_cur);
> + if (error)
> return error;
> } else {
> /*
> @@ -2145,7 +2147,8 @@ xfs_free_ag_extent(
> error = -EFSCORRUPTED;
> goto error0;
> }
> - if ((error = xfs_btree_delete(cnt_cur, &i)))
> + error = xfs_btree_delete(cnt_cur);
> + if (error)
> goto error0;
> /*
> * Delete the old by-size entry on the right.
> @@ -2157,12 +2160,14 @@ xfs_free_ag_extent(
> error = -EFSCORRUPTED;
> goto error0;
> }
> - if ((error = xfs_btree_delete(cnt_cur, &i)))
> + error = xfs_btree_delete(cnt_cur);
> + if (error)
> goto error0;
> /*
> * Delete the old by-block entry for the right block.
> */
> - if ((error = xfs_btree_delete(bno_cur, &i)))
> + error = xfs_btree_delete(bno_cur);
> + if (error)
> goto error0;
> /*
> * Move the by-block cursor back to the left neighbor.
> @@ -2219,7 +2224,8 @@ xfs_free_ag_extent(
> error = -EFSCORRUPTED;
> goto error0;
> }
> - if ((error = xfs_btree_delete(cnt_cur, &i)))
> + error = xfs_btree_delete(cnt_cur);
> + if (error)
> goto error0;
> /*
> * Back up the by-block cursor to the left neighbor, and
> @@ -2252,7 +2258,8 @@ xfs_free_ag_extent(
> error = -EFSCORRUPTED;
> goto error0;
> }
> - if ((error = xfs_btree_delete(cnt_cur, &i)))
> + error = xfs_btree_delete(cnt_cur);
> + if (error)
> goto error0;
> /*
> * Update the starting block and length of the right
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index cf7360adf1a8..beaa92ea8015 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1559,7 +1559,7 @@ xfs_bmap_add_extent_delay_real(
> error = -EFSCORRUPTED;
> goto done;
> }
> - error = xfs_btree_delete(bma->cur, &i);
> + error = xfs_btree_delete(bma->cur);
> if (error)
> goto done;
> error = xfs_btree_decrement(bma->cur, 0, &i);
> @@ -2103,7 +2103,8 @@ xfs_bmap_add_extent_unwritten_real(
> error = -EFSCORRUPTED;
> goto done;
> }
> - if ((error = xfs_btree_delete(cur, &i)))
> + error = xfs_btree_delete(cur);
> + if (error)
> goto done;
> if ((error = xfs_btree_decrement(cur, 0, &i)))
> goto done;
> @@ -2112,7 +2113,8 @@ xfs_bmap_add_extent_unwritten_real(
> error = -EFSCORRUPTED;
> goto done;
> }
> - if ((error = xfs_btree_delete(cur, &i)))
> + error = xfs_btree_delete(cur);
> + if (error)
> goto done;
> if ((error = xfs_btree_decrement(cur, 0, &i)))
> goto done;
> @@ -2150,7 +2152,8 @@ xfs_bmap_add_extent_unwritten_real(
> error = -EFSCORRUPTED;
> goto done;
> }
> - if ((error = xfs_btree_delete(cur, &i)))
> + error = xfs_btree_delete(cur);
> + if (error)
> goto done;
> if ((error = xfs_btree_decrement(cur, 0, &i)))
> goto done;
> @@ -2191,7 +2194,8 @@ xfs_bmap_add_extent_unwritten_real(
> error = -EFSCORRUPTED;
> goto done;
> }
> - if ((error = xfs_btree_delete(cur, &i)))
> + error = xfs_btree_delete(cur);
> + if (error)
> goto done;
> if ((error = xfs_btree_decrement(cur, 0, &i)))
> goto done;
> @@ -2599,7 +2603,7 @@ xfs_bmap_add_extent_hole_real(
> error = -EFSCORRUPTED;
> goto done;
> }
> - error = xfs_btree_delete(cur, &i);
> + error = xfs_btree_delete(cur);
> if (error)
> goto done;
> error = xfs_btree_decrement(cur, 0, &i);
> @@ -4926,7 +4930,8 @@ xfs_bmap_del_extent_real(
> *logflagsp |= xfs_ilog_fext(whichfork);
> break;
> }
> - if ((error = xfs_btree_delete(cur, &i)))
> + error = xfs_btree_delete(cur);
> + if (error)
> return error;
> break;
> case BMAP_LEFT_FILLING:
> @@ -5488,7 +5493,7 @@ xfs_bmse_merge(
> return -EFSCORRUPTED;
> }
>
> - error = xfs_btree_delete(cur, &i);
> + error = xfs_btree_delete(cur);
> if (error)
> return error;
>
> diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
> index 367c14e4dec5..91211d212c74 100644
> --- a/fs/xfs/libxfs/xfs_btree.c
> +++ b/fs/xfs/libxfs/xfs_btree.c
> @@ -4555,8 +4555,7 @@ xfs_btree_delrec(
> */
> int /* error */
> xfs_btree_delete(
> - struct xfs_btree_cur *cur,
> - int *stat) /* success/failure */
> + struct xfs_btree_cur *cur)
> {
> int error; /* error return value */
> int level;
> @@ -4604,7 +4603,6 @@ xfs_btree_delete(
> goto error0;
> }
>
> - *stat = i;
> return 0;
> error0:
> return error;
> diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
> index 0f82f3106bac..c4337ab244f2 100644
> --- a/fs/xfs/libxfs/xfs_btree.h
> +++ b/fs/xfs/libxfs/xfs_btree.h
> @@ -406,7 +406,7 @@ int xfs_btree_lookup(struct xfs_btree_cur *, xfs_lookup_t, int *);
> int xfs_btree_update(struct xfs_btree_cur *, union xfs_btree_rec *);
> int xfs_btree_new_iroot(struct xfs_btree_cur *, int *, int *);
> int xfs_btree_insert(struct xfs_btree_cur *);
> -int xfs_btree_delete(struct xfs_btree_cur *, int *);
> +int xfs_btree_delete(struct xfs_btree_cur *);
Same nitpick about parameter names, but otherwise I like this, so
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> int xfs_btree_get_rec(struct xfs_btree_cur *, union xfs_btree_rec **, int *);
> int xfs_btree_change_owner(struct xfs_btree_cur *cur, uint64_t new_owner,
> struct list_head *buffer_list);
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index 7a9cc0b9e206..ec0a37499f28 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -1564,7 +1564,6 @@ xfs_dialloc_ag(
> xfs_ino_t ino;
> int error;
> int offset;
> - int i;
>
> if (!xfs_has_finobt(mp))
> return xfs_dialloc_ag_inobt(pag, tp, agbp, parent, inop);
> @@ -1617,7 +1616,7 @@ xfs_dialloc_ag(
> if (error)
> goto error_cur;
> } else {
> - error = xfs_btree_delete(cur, &i);
> + error = xfs_btree_delete(cur);
> if (error)
> goto error_cur;
> }
> @@ -2148,7 +2147,8 @@ xfs_difree_inobt(
> xfs_trans_mod_sb(tp, XFS_TRANS_SB_ICOUNT, -ilen);
> xfs_trans_mod_sb(tp, XFS_TRANS_SB_IFREE, -(ilen - 1));
>
> - if ((error = xfs_btree_delete(cur, &i))) {
> + error = xfs_btree_delete(cur);
> + if (error) {
> xfs_warn(mp, "%s: xfs_btree_delete returned error %d.",
> __func__, error);
> goto error0;
> @@ -2274,7 +2274,7 @@ xfs_difree_finobt(
> */
> if (rec.ir_free == XFS_INOBT_ALL_FREE &&
> mp->m_sb.sb_inopblock <= XFS_INODES_PER_CHUNK) {
> - error = xfs_btree_delete(cur, &i);
> + error = xfs_btree_delete(cur);
> if (error)
> goto error;
> } else {
> diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
> index 924f839b35a0..75e469576a8b 100644
> --- a/fs/xfs/libxfs/xfs_refcount.c
> +++ b/fs/xfs/libxfs/xfs_refcount.c
> @@ -288,8 +288,7 @@ xfs_refcount_insert(
> */
> STATIC int
> xfs_refcount_delete(
> - struct xfs_btree_cur *cur,
> - int *i)
> + struct xfs_btree_cur *cur)
> {
> struct xfs_refcount_irec irec;
> int found_rec;
> @@ -304,7 +303,7 @@ xfs_refcount_delete(
> goto out_error;
> }
> trace_xfs_refcount_delete(cur, &irec);
> - error = xfs_btree_delete(cur, i);
> + error = xfs_btree_delete(cur);
> if (error)
> goto out_error;
> error = xfs_refcount_lookup_ge(cur, irec.rc_domain, irec.rc_startblock,
> @@ -497,12 +496,12 @@ xfs_refcount_merge_center_extents(
> goto out_error;
> }
>
> - error = xfs_refcount_delete(cur, &found_rec);
> + error = xfs_refcount_delete(cur);
> if (error)
> goto out_error;
>
> if (center->rc_refcount > 1) {
> - error = xfs_refcount_delete(cur, &found_rec);
> + error = xfs_refcount_delete(cur);
> if (error)
> goto out_error;
> }
> @@ -561,7 +560,7 @@ xfs_refcount_merge_left_extent(
> goto out_error;
> }
>
> - error = xfs_refcount_delete(cur, &found_rec);
> + error = xfs_refcount_delete(cur);
> if (error)
> goto out_error;
> }
> @@ -623,7 +622,7 @@ xfs_refcount_merge_right_extent(
> goto out_error;
> }
>
> - error = xfs_refcount_delete(cur, &found_rec);
> + error = xfs_refcount_delete(cur);
> if (error)
> goto out_error;
> }
> @@ -1209,7 +1208,7 @@ xfs_refcount_adjust_extents(
> if (error)
> goto out_error;
> } else if (ext.rc_refcount == 1) {
> - error = xfs_refcount_delete(cur, &found_rec);
> + error = xfs_refcount_delete(cur);
> if (error)
> goto out_error;
> goto advloop;
> @@ -1819,7 +1818,7 @@ xfs_refcount_adjust_cow_extents(
>
> ext.rc_refcount = 0;
> trace_xfs_refcount_modify_extent(cur, &ext);
> - error = xfs_refcount_delete(cur, &found_rec);
> + error = xfs_refcount_delete(cur);
> if (error)
> goto out_error;
> break;
> diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
> index 31c41e9bcb3f..6fe06612eac2 100644
> --- a/fs/xfs/libxfs/xfs_rmap.c
> +++ b/fs/xfs/libxfs/xfs_rmap.c
> @@ -175,7 +175,7 @@ xfs_rmap_delete(
> goto done;
> }
>
> - error = xfs_btree_delete(rcur, &i);
> + error = xfs_btree_delete(rcur);
> if (error)
> goto done;
> done:
> @@ -770,7 +770,7 @@ xfs_rmap_unmap(
> trace_xfs_rmap_delete(cur, ltrec.rm_startblock,
> ltrec.rm_blockcount, ltrec.rm_owner,
> ltrec.rm_offset, ltrec.rm_flags);
> - error = xfs_btree_delete(cur, &i);
> + error = xfs_btree_delete(cur);
> if (error)
> goto out_error;
> } else if (ltrec.rm_startblock == bno) {
> @@ -1012,7 +1012,6 @@ xfs_rmap_map(
> int have_gt;
> int have_lt;
> int error = 0;
> - int i;
> uint64_t owner;
> uint64_t offset;
> unsigned int flags = 0;
> @@ -1118,7 +1117,7 @@ xfs_rmap_map(
> trace_xfs_rmap_delete(cur, gtrec.rm_startblock,
> gtrec.rm_blockcount, gtrec.rm_owner,
> gtrec.rm_offset, gtrec.rm_flags);
> - error = xfs_btree_delete(cur, &i);
> + error = xfs_btree_delete(cur);
> if (error)
> goto out_error;
> }
> @@ -1395,7 +1394,7 @@ xfs_rmap_convert(
> trace_xfs_rmap_delete(cur, RIGHT.rm_startblock,
> RIGHT.rm_blockcount, RIGHT.rm_owner,
> RIGHT.rm_offset, RIGHT.rm_flags);
> - error = xfs_btree_delete(cur, &i);
> + error = xfs_btree_delete(cur);
> if (error)
> goto done;
> error = xfs_btree_decrement(cur, 0, &i);
> @@ -1409,7 +1408,7 @@ xfs_rmap_convert(
> trace_xfs_rmap_delete(cur, PREV.rm_startblock,
> PREV.rm_blockcount, PREV.rm_owner,
> PREV.rm_offset, PREV.rm_flags);
> - error = xfs_btree_delete(cur, &i);
> + error = xfs_btree_delete(cur);
> if (error)
> goto done;
> error = xfs_btree_decrement(cur, 0, &i);
> @@ -1435,7 +1434,7 @@ xfs_rmap_convert(
> trace_xfs_rmap_delete(cur, PREV.rm_startblock,
> PREV.rm_blockcount, PREV.rm_owner,
> PREV.rm_offset, PREV.rm_flags);
> - error = xfs_btree_delete(cur, &i);
> + error = xfs_btree_delete(cur);
> if (error)
> goto done;
> error = xfs_btree_decrement(cur, 0, &i);
> @@ -1469,7 +1468,7 @@ xfs_rmap_convert(
> trace_xfs_rmap_delete(cur, RIGHT.rm_startblock,
> RIGHT.rm_blockcount, RIGHT.rm_owner,
> RIGHT.rm_offset, RIGHT.rm_flags);
> - error = xfs_btree_delete(cur, &i);
> + error = xfs_btree_delete(cur);
> if (error)
> goto done;
> error = xfs_btree_decrement(cur, 0, &i);
> diff --git a/fs/xfs/scrub/rcbag.c b/fs/xfs/scrub/rcbag.c
> index 5a390e3c7777..bd1aa55a8910 100644
> --- a/fs/xfs/scrub/rcbag.c
> +++ b/fs/xfs/scrub/rcbag.c
> @@ -239,7 +239,7 @@ rcbag_remove_ending_at(
> if (rcbag_rec_next_bno(&bagrec) != next_bno)
> continue;
>
> - error = xfs_btree_delete(cur, &has);
> + error = xfs_btree_delete(cur);
> if (error)
> goto out_cur;
>
> --
> 2.55.0
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread