* [PATCH v4 0/4] xfs: fix a couple sparse chunk alloc problems
@ 2026-09-23 16:15 Brian Foster
2026-09-23 16:15 ` [PATCH v4 1/4] xfs: set minleft correctly for sparse chunk errortag allocation Brian Foster
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Brian Foster @ 2026-09-23 16:15 UTC (permalink / raw)
To: linux-xfs; +Cc: Carlos Maiolino
Hi all,
Here's v4 of the series to fix the sparse chunk alloc shutdown. The
original report is available here[1].
No major changes in this one. The series has been rebased onto for-next
and I collected the review tags from the previous version.
As a brief overview, the idea here is to rework the freelist calculation
helper to calculate a min and a max. The min is the historical min AGFL
requirement for the current allocation. The max is an optional value for
minleft allocations that perform at least one more allocation in the
same transaction and thus are susceptible to seeing the min AGFL
requirement increase due to level increases in the first allocation. The
specific fix is to carry the delta between the max and min into minleft
for such allocations so they are either guaranteed for a given AG or
fail gracefully before dirtying the transaction.
Thoughts, reviews, flames appreciated.
Brian
v4:
- Rebased to for-next and collected review tags.
v3: https://lore.kernel.org/linux-xfs/20260902174025.284387-1-bfoster@redhat.com/
- Added patch 3 to refactor xfs_alloc_min_freelist() and calculate
min/max.
- Updated patch 4 to use the max value for space available and longest
extent checks.
v2: https://lore.kernel.org/linux-xfs/20260814132239.271492-1-bfoster@redhat.com/
- Reworked fix logic into allocator instead of sparse inode alloc
specific.
- Dropped Fixes: tag since this is no longer directly correlated to
sparse inodes.
v1: https://lore.kernel.org/linux-xfs/20260731163337.152522-1-bfoster@redhat.com/
[1] https://lore.kernel.org/linux-xfs/20260717130429.1838767-1-matt@readmodwrite.com/
Brian Foster (4):
xfs: set minleft correctly for sparse chunk errortag allocation
xfs: support additional levels in the agfl minimum calculation
xfs: calculate AGFL max to support multiple-alloc transactions
xfs: incorporate increased AGFL min requirement for minleft allocs
fs/xfs/libxfs/xfs_alloc.c | 106 +++++++++++++++++++++++++++----------
fs/xfs/libxfs/xfs_alloc.h | 7 +--
fs/xfs/libxfs/xfs_bmap.c | 7 ++-
fs/xfs/libxfs/xfs_ialloc.c | 14 ++---
4 files changed, 93 insertions(+), 41 deletions(-)
--
2.55.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v4 1/4] xfs: set minleft correctly for sparse chunk errortag allocation 2026-09-23 16:15 [PATCH v4 0/4] xfs: fix a couple sparse chunk alloc problems Brian Foster @ 2026-09-23 16:15 ` Brian Foster 2026-09-23 16:15 ` [PATCH v4 2/4] xfs: support additional levels in the agfl minimum calculation Brian Foster ` (2 subsequent siblings) 3 siblings, 0 replies; 10+ messages in thread From: Brian Foster @ 2026-09-23 16:15 UTC (permalink / raw) To: linux-xfs; +Cc: Carlos Maiolino The errortag instrumentation for forced sparse chunk allocation jumps straight to the allocation path without setting args.minleft. minleft is unconditionally set to ->inobt_maxlevels for the normal allocation path. Lift the assignment to the initial args setup so it covers all possible paths. Assisted-by: LLM Fixes: 1cdadee11f8d ("xfs: randomly do sparse inode allocations in DEBUG mode") Signed-off-by: Brian Foster <bfoster@redhat.com> Reviewed-by: Mark Tinguely <mark.tinguely@oracle.com> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> --- fs/xfs/libxfs/xfs_ialloc.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c index 19b513b11692..e31611cabd4d 100644 --- a/fs/xfs/libxfs/xfs_ialloc.c +++ b/fs/xfs/libxfs/xfs_ialloc.c @@ -733,6 +733,10 @@ xfs_ialloc_ag_alloc( igeo->maxicount) return -ENOSPC; args.minlen = args.maxlen = igeo->ialloc_blks; + + /* Allow space for the inode btree to split. */ + args.minleft = igeo->inobt_maxlevels; + /* * First try to allocate inodes contiguous with the last-allocated * chunk of inodes. If the filesystem is striped, this will fill @@ -764,8 +768,6 @@ xfs_ialloc_ag_alloc( args.alignment = 1; args.minalignslop = igeo->cluster_align - 1; - /* Allow space for the inode btree to split. */ - args.minleft = igeo->inobt_maxlevels; error = xfs_alloc_vextent_exact_bno(&args, xfs_agbno_to_fsb(pag, args.agbno)); if (error) @@ -804,10 +806,6 @@ xfs_ialloc_ag_alloc( * Allocate a fixed-size extent of inodes. */ args.prod = 1; - /* - * Allow space for the inode btree to split. - */ - args.minleft = igeo->inobt_maxlevels; error = xfs_alloc_vextent_near_bno(&args, xfs_agbno_to_fsb(pag, be32_to_cpu(agi->agi_root))); -- 2.55.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 2/4] xfs: support additional levels in the agfl minimum calculation 2026-09-23 16:15 [PATCH v4 0/4] xfs: fix a couple sparse chunk alloc problems Brian Foster 2026-09-23 16:15 ` [PATCH v4 1/4] xfs: set minleft correctly for sparse chunk errortag allocation Brian Foster @ 2026-09-23 16:15 ` Brian Foster 2026-09-23 16:15 ` [PATCH v4 3/4] xfs: calculate AGFL max to support multiple-alloc transactions Brian Foster 2026-09-23 16:15 ` [PATCH v4 4/4] xfs: incorporate increased AGFL min requirement for minleft allocs Brian Foster 3 siblings, 0 replies; 10+ messages in thread From: Brian Foster @ 2026-09-23 16:15 UTC (permalink / raw) To: linux-xfs; +Cc: Carlos Maiolino xfs_alloc_min_freelist() calculates the worst case AGFL block requirement for a full split plus partial refill for each alloc btree. An upcoming patch needs to calculate the requirement for multiple level increases, so add an optional extra levels parameter and factor out a wrapper function for the common case of a single split. No functional changes. Assisted-by: LLM Signed-off-by: Brian Foster <bfoster@redhat.com> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> Reviewed-by: Mark Tinguely <mark.tinguely@oracle.com> --- fs/xfs/libxfs/xfs_alloc.c | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c index f762dcce8d13..b2525c58051b 100644 --- a/fs/xfs/libxfs/xfs_alloc.c +++ b/fs/xfs/libxfs/xfs_alloc.c @@ -2439,17 +2439,22 @@ xfs_alloc_longest_free_extent( /* * Compute the minimum length of the AGFL in the given AG. If @pag is NULL, - * return the largest possible minimum length. + * return the largest possible minimum length. The base calculation accounts + * for a single full split per btree. @extra_levels adds additional split + * levels to compute the prospective AGFL requirement increase for + * multi-allocation transactions. */ -unsigned int -xfs_alloc_min_freelist( +static unsigned int +__xfs_alloc_min_freelist( struct xfs_mount *mp, - struct xfs_perag *pag) + struct xfs_perag *pag, + unsigned int extra_levels) { /* AG btrees have at least 1 level. */ const unsigned int bno_level = pag ? pag->pagf_bno_level : 1; const unsigned int cnt_level = pag ? pag->pagf_cnt_level : 1; const unsigned int rmap_level = pag ? pag->pagf_rmap_level : 1; + const unsigned int levels = 1 + extra_levels; unsigned int min_free; ASSERT(mp->m_alloc_maxlevels > 0); @@ -2476,15 +2481,25 @@ xfs_alloc_min_freelist( */ /* space needed by-bno freespace btree */ - min_free = min(bno_level + 1, mp->m_alloc_maxlevels) * 2 - 2; + min_free = min(bno_level + levels, mp->m_alloc_maxlevels) * 2 - 2; /* space needed by-size freespace btree */ - min_free += min(cnt_level + 1, mp->m_alloc_maxlevels) * 2 - 2; + min_free += min(cnt_level + levels, mp->m_alloc_maxlevels) * 2 - 2; /* space needed reverse mapping used space btree */ - if (xfs_has_rmapbt(mp)) - min_free += min(rmap_level + 1, mp->m_rmap_maxlevels) * 2 - 2; + if (xfs_has_rmapbt(mp)) { + min_free += min(rmap_level + levels, + mp->m_rmap_maxlevels) * 2 - 2; + } return min_free; } +unsigned int +xfs_alloc_min_freelist( + struct xfs_mount *mp, + struct xfs_perag *pag) +{ + return __xfs_alloc_min_freelist(mp, pag, 0); +} + /* * Check if the operation we are fixing up the freelist for should go ahead or * not. If we are freeing blocks, we always allow it, otherwise the allocation -- 2.55.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 3/4] xfs: calculate AGFL max to support multiple-alloc transactions 2026-09-23 16:15 [PATCH v4 0/4] xfs: fix a couple sparse chunk alloc problems Brian Foster 2026-09-23 16:15 ` [PATCH v4 1/4] xfs: set minleft correctly for sparse chunk errortag allocation Brian Foster 2026-09-23 16:15 ` [PATCH v4 2/4] xfs: support additional levels in the agfl minimum calculation Brian Foster @ 2026-09-23 16:15 ` Brian Foster 2026-09-24 0:38 ` Darrick J. Wong 2026-09-23 16:15 ` [PATCH v4 4/4] xfs: incorporate increased AGFL min requirement for minleft allocs Brian Foster 3 siblings, 1 reply; 10+ messages in thread From: Brian Foster @ 2026-09-23 16:15 UTC (permalink / raw) To: linux-xfs; +Cc: Carlos Maiolino Rework xfs_alloc_min_freelist() to return an (optional) max value along with the historical min freelist value. The max value includes an extra level in the btree based calculation to account for btree splits in the first allocation of a multi-alloc sequence. This value reflects the worst case AGFL requirement that can be expected across multiple allocations within a single AG and single transaction. For example, consider the case of allocating an inode chunk and then an inobt block on inode record insertion. While here, rename the need variable in xfs_alloc_fix_freelist() for clarity with upcoming changes. Note that no callers calculate the max AGFL value as of yet. No functional changes in this patch. Assisted-by: LLM Signed-off-by: Brian Foster <bfoster@redhat.com> Reviewed-by: Mark Tinguely <mark.tinguely@oracle.com> --- fs/xfs/libxfs/xfs_alloc.c | 35 ++++++++++++++++++++++------------- fs/xfs/libxfs/xfs_alloc.h | 4 ++-- fs/xfs/libxfs/xfs_bmap.c | 5 +++-- fs/xfs/libxfs/xfs_ialloc.c | 4 +++- 4 files changed, 30 insertions(+), 18 deletions(-) diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c index b2525c58051b..af63926cc9ee 100644 --- a/fs/xfs/libxfs/xfs_alloc.c +++ b/fs/xfs/libxfs/xfs_alloc.c @@ -2445,7 +2445,7 @@ xfs_alloc_longest_free_extent( * multi-allocation transactions. */ static unsigned int -__xfs_alloc_min_freelist( +__xfs_alloc_freelist( struct xfs_mount *mp, struct xfs_perag *pag, unsigned int extra_levels) @@ -2492,12 +2492,21 @@ __xfs_alloc_min_freelist( return min_free; } -unsigned int -xfs_alloc_min_freelist( +/* + * Compute the minimum and maximum length of the AGFL in the given AG. The max + * value in this context refers to the max requirement the AG might see in a + * multi-alloc transaction. If @pag is NULL, return the largest possible values. + */ +void +xfs_alloc_freelist( struct xfs_mount *mp, - struct xfs_perag *pag) + struct xfs_perag *pag, + unsigned int *min_free, + unsigned int *max_free) { - return __xfs_alloc_min_freelist(mp, pag, 0); + *min_free = __xfs_alloc_freelist(mp, pag, 0); + if (max_free) + *max_free = __xfs_alloc_freelist(mp, pag, 1); } /* @@ -2849,7 +2858,7 @@ xfs_alloc_fix_freelist( struct xfs_buf *agflbp = NULL; struct xfs_alloc_arg targs; /* local allocation arguments */ xfs_agblock_t bno; /* freelist block */ - xfs_extlen_t need; /* total blocks needed in freelist */ + xfs_extlen_t min_free;/* total blocks needed in freelist */ int error = 0; /* deferred ops (AGFL block frees) require permanent transactions */ @@ -2877,8 +2886,8 @@ xfs_alloc_fix_freelist( goto out_agbp_relse; } - need = xfs_alloc_min_freelist(mp, pag); - if (!xfs_alloc_space_available(args, need, alloc_flags | + xfs_alloc_freelist(mp, pag, &min_free, NULL); + if (!xfs_alloc_space_available(args, min_free, alloc_flags | XFS_ALLOC_FLAG_CHECK)) goto out_agbp_relse; @@ -2901,8 +2910,8 @@ xfs_alloc_fix_freelist( xfs_agfl_reset(tp, agbp, pag); /* If there isn't enough total space or single-extent, reject it. */ - need = xfs_alloc_min_freelist(mp, pag); - if (!xfs_alloc_space_available(args, need, alloc_flags)) + xfs_alloc_freelist(mp, pag, &min_free, NULL); + if (!xfs_alloc_space_available(args, min_free, alloc_flags)) goto out_agbp_relse; if (IS_ENABLED(CONFIG_XFS_DEBUG) && args->alloc_minlen_only) { @@ -2944,7 +2953,7 @@ xfs_alloc_fix_freelist( else targs.oinfo = XFS_RMAP_OINFO_AG; while (!(alloc_flags & XFS_ALLOC_FLAG_NOSHRINK) && - pag->pagf_flcount > need) { + pag->pagf_flcount > min_free) { error = xfs_alloc_get_freelist(pag, tp, agbp, &bno, 0); if (error) goto out_agbp_relse; @@ -2978,9 +2987,9 @@ xfs_alloc_fix_freelist( goto out_agbp_relse; /* Make the freelist longer if it's too short. */ - while (pag->pagf_flcount < need) { + while (pag->pagf_flcount < min_free) { targs.agbno = 0; - targs.maxlen = need - pag->pagf_flcount; + targs.maxlen = min_free - pag->pagf_flcount; targs.resv = XFS_AG_RESV_AGFL; /* Allocate as many blocks as possible at once. */ diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h index 50ef79a1ed41..44a10f4a22a2 100644 --- a/fs/xfs/libxfs/xfs_alloc.h +++ b/fs/xfs/libxfs/xfs_alloc.h @@ -71,8 +71,8 @@ unsigned int xfs_alloc_ag_max_usable(struct xfs_mount *mp); xfs_extlen_t xfs_alloc_longest_free_extent(struct xfs_perag *pag, xfs_extlen_t need, xfs_extlen_t reserved); -unsigned int xfs_alloc_min_freelist(struct xfs_mount *mp, - struct xfs_perag *pag); +void xfs_alloc_freelist(struct xfs_mount *mp, struct xfs_perag *pag, + unsigned int *min_free, unsigned int *max_free); int xfs_alloc_get_freelist(struct xfs_perag *pag, struct xfs_trans *tp, struct xfs_buf *agfbp, xfs_agblock_t *bnop, int btreeblk); int xfs_alloc_put_freelist(struct xfs_perag *pag, struct xfs_trans *tp, diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c index ae91f63455c5..d6be6734bd90 100644 --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -3149,6 +3149,7 @@ xfs_bmap_longest_free_extent( xfs_extlen_t *blen) { xfs_extlen_t longest; + unsigned int min_free; int error = 0; if (!xfs_perag_initialised_agf(pag)) { @@ -3158,8 +3159,8 @@ xfs_bmap_longest_free_extent( return error; } - longest = xfs_alloc_longest_free_extent(pag, - xfs_alloc_min_freelist(pag_mount(pag), pag), + xfs_alloc_freelist(pag_mount(pag), pag, &min_free, NULL); + longest = xfs_alloc_longest_free_extent(pag, min_free, xfs_ag_resv_needed(pag, XFS_AG_RESV_NONE)); if (*blen < longest) *blen = longest; diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c index e31611cabd4d..e705f681ed44 100644 --- a/fs/xfs/libxfs/xfs_ialloc.c +++ b/fs/xfs/libxfs/xfs_ialloc.c @@ -3074,6 +3074,7 @@ xfs_ialloc_calc_rootino( { struct xfs_ino_geometry *igeo = M_IGEO(mp); xfs_agblock_t first_bno; + unsigned int min_free; /* * Pre-calculate the geometry of AG 0. We know what it looks like @@ -3092,7 +3093,8 @@ xfs_ialloc_calc_rootino( first_bno += 1; /* ...the initial AGFL... */ - first_bno += xfs_alloc_min_freelist(mp, NULL); + xfs_alloc_freelist(mp, NULL, &min_free, NULL); + first_bno += min_free; /* ...the free inode btree root... */ if (xfs_has_finobt(mp)) -- 2.55.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/4] xfs: calculate AGFL max to support multiple-alloc transactions 2026-09-23 16:15 ` [PATCH v4 3/4] xfs: calculate AGFL max to support multiple-alloc transactions Brian Foster @ 2026-09-24 0:38 ` Darrick J. Wong 0 siblings, 0 replies; 10+ messages in thread From: Darrick J. Wong @ 2026-09-24 0:38 UTC (permalink / raw) To: Brian Foster; +Cc: linux-xfs, Carlos Maiolino On Wed, Sep 23, 2026 at 12:15:23PM -0400, Brian Foster wrote: > Rework xfs_alloc_min_freelist() to return an (optional) max value > along with the historical min freelist value. The max value > includes an extra level in the btree based calculation to account > for btree splits in the first allocation of a multi-alloc sequence. > > This value reflects the worst case AGFL requirement that can be > expected across multiple allocations within a single AG and single > transaction. For example, consider the case of allocating an inode > chunk and then an inobt block on inode record insertion. > > While here, rename the need variable in xfs_alloc_fix_freelist() for > clarity with upcoming changes. > > Note that no callers calculate the max AGFL value as of yet. No > functional changes in this patch. > > Assisted-by: LLM > Signed-off-by: Brian Foster <bfoster@redhat.com> > Reviewed-by: Mark Tinguely <mark.tinguely@oracle.com> Looks fine to me Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> --D > --- > fs/xfs/libxfs/xfs_alloc.c | 35 ++++++++++++++++++++++------------- > fs/xfs/libxfs/xfs_alloc.h | 4 ++-- > fs/xfs/libxfs/xfs_bmap.c | 5 +++-- > fs/xfs/libxfs/xfs_ialloc.c | 4 +++- > 4 files changed, 30 insertions(+), 18 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c > index b2525c58051b..af63926cc9ee 100644 > --- a/fs/xfs/libxfs/xfs_alloc.c > +++ b/fs/xfs/libxfs/xfs_alloc.c > @@ -2445,7 +2445,7 @@ xfs_alloc_longest_free_extent( > * multi-allocation transactions. > */ > static unsigned int > -__xfs_alloc_min_freelist( > +__xfs_alloc_freelist( > struct xfs_mount *mp, > struct xfs_perag *pag, > unsigned int extra_levels) > @@ -2492,12 +2492,21 @@ __xfs_alloc_min_freelist( > return min_free; > } > > -unsigned int > -xfs_alloc_min_freelist( > +/* > + * Compute the minimum and maximum length of the AGFL in the given AG. The max > + * value in this context refers to the max requirement the AG might see in a > + * multi-alloc transaction. If @pag is NULL, return the largest possible values. > + */ > +void > +xfs_alloc_freelist( > struct xfs_mount *mp, > - struct xfs_perag *pag) > + struct xfs_perag *pag, > + unsigned int *min_free, > + unsigned int *max_free) > { > - return __xfs_alloc_min_freelist(mp, pag, 0); > + *min_free = __xfs_alloc_freelist(mp, pag, 0); > + if (max_free) > + *max_free = __xfs_alloc_freelist(mp, pag, 1); > } > > /* > @@ -2849,7 +2858,7 @@ xfs_alloc_fix_freelist( > struct xfs_buf *agflbp = NULL; > struct xfs_alloc_arg targs; /* local allocation arguments */ > xfs_agblock_t bno; /* freelist block */ > - xfs_extlen_t need; /* total blocks needed in freelist */ > + xfs_extlen_t min_free;/* total blocks needed in freelist */ > int error = 0; > > /* deferred ops (AGFL block frees) require permanent transactions */ > @@ -2877,8 +2886,8 @@ xfs_alloc_fix_freelist( > goto out_agbp_relse; > } > > - need = xfs_alloc_min_freelist(mp, pag); > - if (!xfs_alloc_space_available(args, need, alloc_flags | > + xfs_alloc_freelist(mp, pag, &min_free, NULL); > + if (!xfs_alloc_space_available(args, min_free, alloc_flags | > XFS_ALLOC_FLAG_CHECK)) > goto out_agbp_relse; > > @@ -2901,8 +2910,8 @@ xfs_alloc_fix_freelist( > xfs_agfl_reset(tp, agbp, pag); > > /* If there isn't enough total space or single-extent, reject it. */ > - need = xfs_alloc_min_freelist(mp, pag); > - if (!xfs_alloc_space_available(args, need, alloc_flags)) > + xfs_alloc_freelist(mp, pag, &min_free, NULL); > + if (!xfs_alloc_space_available(args, min_free, alloc_flags)) > goto out_agbp_relse; > > if (IS_ENABLED(CONFIG_XFS_DEBUG) && args->alloc_minlen_only) { > @@ -2944,7 +2953,7 @@ xfs_alloc_fix_freelist( > else > targs.oinfo = XFS_RMAP_OINFO_AG; > while (!(alloc_flags & XFS_ALLOC_FLAG_NOSHRINK) && > - pag->pagf_flcount > need) { > + pag->pagf_flcount > min_free) { > error = xfs_alloc_get_freelist(pag, tp, agbp, &bno, 0); > if (error) > goto out_agbp_relse; > @@ -2978,9 +2987,9 @@ xfs_alloc_fix_freelist( > goto out_agbp_relse; > > /* Make the freelist longer if it's too short. */ > - while (pag->pagf_flcount < need) { > + while (pag->pagf_flcount < min_free) { > targs.agbno = 0; > - targs.maxlen = need - pag->pagf_flcount; > + targs.maxlen = min_free - pag->pagf_flcount; > targs.resv = XFS_AG_RESV_AGFL; > > /* Allocate as many blocks as possible at once. */ > diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h > index 50ef79a1ed41..44a10f4a22a2 100644 > --- a/fs/xfs/libxfs/xfs_alloc.h > +++ b/fs/xfs/libxfs/xfs_alloc.h > @@ -71,8 +71,8 @@ unsigned int xfs_alloc_ag_max_usable(struct xfs_mount *mp); > > xfs_extlen_t xfs_alloc_longest_free_extent(struct xfs_perag *pag, > xfs_extlen_t need, xfs_extlen_t reserved); > -unsigned int xfs_alloc_min_freelist(struct xfs_mount *mp, > - struct xfs_perag *pag); > +void xfs_alloc_freelist(struct xfs_mount *mp, struct xfs_perag *pag, > + unsigned int *min_free, unsigned int *max_free); > int xfs_alloc_get_freelist(struct xfs_perag *pag, struct xfs_trans *tp, > struct xfs_buf *agfbp, xfs_agblock_t *bnop, int btreeblk); > int xfs_alloc_put_freelist(struct xfs_perag *pag, struct xfs_trans *tp, > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > index ae91f63455c5..d6be6734bd90 100644 > --- a/fs/xfs/libxfs/xfs_bmap.c > +++ b/fs/xfs/libxfs/xfs_bmap.c > @@ -3149,6 +3149,7 @@ xfs_bmap_longest_free_extent( > xfs_extlen_t *blen) > { > xfs_extlen_t longest; > + unsigned int min_free; > int error = 0; > > if (!xfs_perag_initialised_agf(pag)) { > @@ -3158,8 +3159,8 @@ xfs_bmap_longest_free_extent( > return error; > } > > - longest = xfs_alloc_longest_free_extent(pag, > - xfs_alloc_min_freelist(pag_mount(pag), pag), > + xfs_alloc_freelist(pag_mount(pag), pag, &min_free, NULL); > + longest = xfs_alloc_longest_free_extent(pag, min_free, > xfs_ag_resv_needed(pag, XFS_AG_RESV_NONE)); > if (*blen < longest) > *blen = longest; > diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c > index e31611cabd4d..e705f681ed44 100644 > --- a/fs/xfs/libxfs/xfs_ialloc.c > +++ b/fs/xfs/libxfs/xfs_ialloc.c > @@ -3074,6 +3074,7 @@ xfs_ialloc_calc_rootino( > { > struct xfs_ino_geometry *igeo = M_IGEO(mp); > xfs_agblock_t first_bno; > + unsigned int min_free; > > /* > * Pre-calculate the geometry of AG 0. We know what it looks like > @@ -3092,7 +3093,8 @@ xfs_ialloc_calc_rootino( > first_bno += 1; > > /* ...the initial AGFL... */ > - first_bno += xfs_alloc_min_freelist(mp, NULL); > + xfs_alloc_freelist(mp, NULL, &min_free, NULL); > + first_bno += min_free; > > /* ...the free inode btree root... */ > if (xfs_has_finobt(mp)) > -- > 2.55.0 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 4/4] xfs: incorporate increased AGFL min requirement for minleft allocs 2026-09-23 16:15 [PATCH v4 0/4] xfs: fix a couple sparse chunk alloc problems Brian Foster ` (2 preceding siblings ...) 2026-09-23 16:15 ` [PATCH v4 3/4] xfs: calculate AGFL max to support multiple-alloc transactions Brian Foster @ 2026-09-23 16:15 ` Brian Foster 2026-09-24 1:15 ` Darrick J. Wong 3 siblings, 1 reply; 10+ messages in thread From: Brian Foster @ 2026-09-23 16:15 UTC (permalink / raw) To: linux-xfs; +Cc: Carlos Maiolino Matt Fleming reports a filesystem shutdown due to inobt block allocation failure during sparse chunk allocation. Inode creation can involve multiple allocations in a transaction: the initial chunk allocation and inode btree blocks via inobt record insertion. This is expected to be safe by using the minleft parameter on the chunk allocation to guarantee the selected AG has blocks available for a followup inobt insertion. The sequence that leads to this failure is that the alloc and inode btrees are all full (require a full split on next insertion) and the AG has just enough free space to satisfy a sparse chunk allocation with minleft set (i.e. 7 blocks in this example). The chunk allocation splits a free extent, triggers full allocbt splits, and consumes 4 free blocks for the chunk and 4 AGFL blocks for the btrees. Next, the inobt record insertion triggers an inobt split. The AG has enough free blocks, but the allocbt splits caused by the chunk allocation have increased the min AGFL requirement for the AG due to btree level increases. The AGFL requirement as calculated by xfs_alloc_fix_freelist() is: free + AGFL - res - minfree - minleft = avail This evaluates to the following on initial chunk allocation: 2514 + 8 - 2505 - 8 - 2 = 7 ... and then after the chunk allocation but before the inobt block allocation: 2510 + 4 - 2505 - 12 - 0 = -3 This causes the inobt alloc to fail despite minleft being set in the first allocation. The error path cancels the dirty transaction and shuts down the fs. The problem here is that while minleft ensures free blocks are available for the inobt insert, it is not sufficient to cover the increase of the AGFL min free requirement. To address this, first have xfs_alloc_freelist() return both min and max freelist values. The min value is the current AGFL requirement and remains used for actual AGFL sizing. The max value calculates the worst case AGFL requirement after potential allocbt splits during the current allocation. Incorporate the max value into space availability checks for AG selection and the longest free extent calculation. The latter is necessary because callers like the bmap layer can size allocation requests based on the longest free extent. Without this, aligned allocs can end up oversized, prematurely fail, and fall back to non-aligned to make up the difference. This ensures the selected AG has enough blocks for both the caller's minleft value and the worst case AGFL increase. In the example above, the initial calculation now evaluates to 3 blocks available instead of 7 and the inode allocation fails gracefully with -ENOSPC. Assisted-by: LLM Reported-by: Matt Fleming <matt@readmodwrite.com> Signed-off-by: Brian Foster <bfoster@redhat.com> Reviewed-by: Mark Tinguely <mark.tinguely@oracle.com> --- fs/xfs/libxfs/xfs_alloc.c | 58 +++++++++++++++++++++++++++------------ fs/xfs/libxfs/xfs_alloc.h | 3 +- fs/xfs/libxfs/xfs_bmap.c | 6 ++-- 3 files changed, 47 insertions(+), 20 deletions(-) diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c index af63926cc9ee..86f74a2dfa10 100644 --- a/fs/xfs/libxfs/xfs_alloc.c +++ b/fs/xfs/libxfs/xfs_alloc.c @@ -2397,31 +2397,36 @@ xfs_alloc_compute_maxlevels( } /* - * Find the length of the longest extent in an AG. The 'need' parameter - * specifies how much space we're going to need for the AGFL and the - * 'reserved' parameter tells us how many blocks in this AG are reserved for + * Find the length of the longest extent in an AG. The @min_free and @max_free + * parameters specify how much space we're going to need for the AGFL and the + * @reserved parameter tells us how many blocks in this AG are reserved for * other callers. */ xfs_extlen_t xfs_alloc_longest_free_extent( struct xfs_perag *pag, - xfs_extlen_t need, + xfs_extlen_t min_free, + xfs_extlen_t max_free, xfs_extlen_t reserved) { xfs_extlen_t delta = 0; /* - * If the AGFL needs a recharge, we'll have to subtract that from the - * longest extent. + * If the AGFL needs a recharge, subtract that from the longest extent + * because AGFL refill happens before the alloc. */ - if (need > pag->pagf_flcount) - delta = need - pag->pagf_flcount; + if (min_free > pag->pagf_flcount) + delta = min_free - pag->pagf_flcount; /* - * If we cannot maintain others' reservations with space from the - * not-longest freesp extents, we'll have to subtract /that/ from - * the longest extent too. + * Extra AGFL blocks beyond the min are reserved by ->minleft during + * allocation. Similar to reserved, these blocks are not available to + * this allocation. Check if we can preserve the combined total without + * the longest extent. If not, deduct the necessary blocks from the + * longest extent. */ + if (max_free > min_free) + reserved += max_free - min_free; if (pag->pagf_freeblks - pag->pagf_longest < reserved) delta += reserved - (pag->pagf_freeblks - pag->pagf_longest); @@ -2519,6 +2524,7 @@ static bool xfs_alloc_space_available( struct xfs_alloc_arg *args, xfs_extlen_t min_free, + xfs_extlen_t max_free, int flags) { struct xfs_perag *pag = args->pag; @@ -2526,15 +2532,32 @@ xfs_alloc_space_available( xfs_extlen_t reservation; /* blocks that are still reserved */ int available; xfs_extlen_t agflcount; + xfs_extlen_t minleft; if (flags & XFS_ALLOC_FLAG_FREEING) return true; reservation = xfs_ag_resv_needed(pag, args->resv); + /* + * minleft implies a multi-alloc transaction. If set, the first alloc + * might cause btree splits that increase the AGFL requirement for the + * next. This worst case requirement is calculated in max_free. + * + * We don't prepopulate the AGFL because we don't know in advance if + * splits will occur. Instead, add the delta to minleft so it is + * accounted for in AG selection. This ensures the AG has enough space + * for the caller's minleft plus that needed to repopulate the AGFL on + * the next alloc if splits do occur. + */ + minleft = args->minleft; + if (minleft) + minleft += max_free - min_free; + /* do we have enough contiguous free space for the allocation? */ alloc_len = args->minlen + (args->alignment - 1) + args->minalignslop; - longest = xfs_alloc_longest_free_extent(pag, min_free, reservation); + longest = xfs_alloc_longest_free_extent(pag, min_free, + minleft ? max_free : min_free, reservation); if (longest < alloc_len) return false; @@ -2545,7 +2568,7 @@ xfs_alloc_space_available( */ agflcount = min_t(xfs_extlen_t, pag->pagf_flcount, min_free); available = (int)(pag->pagf_freeblks + agflcount - - reservation - min_free - args->minleft); + reservation - min_free - minleft); if (available < (int)max(args->total, alloc_len)) return false; @@ -2859,6 +2882,7 @@ xfs_alloc_fix_freelist( struct xfs_alloc_arg targs; /* local allocation arguments */ xfs_agblock_t bno; /* freelist block */ xfs_extlen_t min_free;/* total blocks needed in freelist */ + xfs_extlen_t max_free; /* max freelist requirement */ int error = 0; /* deferred ops (AGFL block frees) require permanent transactions */ @@ -2886,8 +2910,8 @@ xfs_alloc_fix_freelist( goto out_agbp_relse; } - xfs_alloc_freelist(mp, pag, &min_free, NULL); - if (!xfs_alloc_space_available(args, min_free, alloc_flags | + xfs_alloc_freelist(mp, pag, &min_free, &max_free); + if (!xfs_alloc_space_available(args, min_free, max_free, alloc_flags | XFS_ALLOC_FLAG_CHECK)) goto out_agbp_relse; @@ -2910,8 +2934,8 @@ xfs_alloc_fix_freelist( xfs_agfl_reset(tp, agbp, pag); /* If there isn't enough total space or single-extent, reject it. */ - xfs_alloc_freelist(mp, pag, &min_free, NULL); - if (!xfs_alloc_space_available(args, min_free, alloc_flags)) + xfs_alloc_freelist(mp, pag, &min_free, &max_free); + if (!xfs_alloc_space_available(args, min_free, max_free, alloc_flags)) goto out_agbp_relse; if (IS_ENABLED(CONFIG_XFS_DEBUG) && args->alloc_minlen_only) { diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h index 44a10f4a22a2..5812c9b5e609 100644 --- a/fs/xfs/libxfs/xfs_alloc.h +++ b/fs/xfs/libxfs/xfs_alloc.h @@ -70,7 +70,8 @@ unsigned int xfs_alloc_set_aside(struct xfs_mount *mp); unsigned int xfs_alloc_ag_max_usable(struct xfs_mount *mp); xfs_extlen_t xfs_alloc_longest_free_extent(struct xfs_perag *pag, - xfs_extlen_t need, xfs_extlen_t reserved); + xfs_extlen_t min_free, xfs_extlen_t max_free, + xfs_extlen_t reserved); void xfs_alloc_freelist(struct xfs_mount *mp, struct xfs_perag *pag, unsigned int *min_free, unsigned int *max_free); int xfs_alloc_get_freelist(struct xfs_perag *pag, struct xfs_trans *tp, diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c index d6be6734bd90..9bffde1484a7 100644 --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -3150,6 +3150,7 @@ xfs_bmap_longest_free_extent( { xfs_extlen_t longest; unsigned int min_free; + unsigned int max_free; int error = 0; if (!xfs_perag_initialised_agf(pag)) { @@ -3159,8 +3160,9 @@ xfs_bmap_longest_free_extent( return error; } - xfs_alloc_freelist(pag_mount(pag), pag, &min_free, NULL); - longest = xfs_alloc_longest_free_extent(pag, min_free, + /* bmap allocs always have minleft set, so account for max_free */ + xfs_alloc_freelist(pag_mount(pag), pag, &min_free, &max_free); + longest = xfs_alloc_longest_free_extent(pag, min_free, max_free, xfs_ag_resv_needed(pag, XFS_AG_RESV_NONE)); if (*blen < longest) *blen = longest; -- 2.55.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 4/4] xfs: incorporate increased AGFL min requirement for minleft allocs 2026-09-23 16:15 ` [PATCH v4 4/4] xfs: incorporate increased AGFL min requirement for minleft allocs Brian Foster @ 2026-09-24 1:15 ` Darrick J. Wong 2026-09-24 13:14 ` Brian Foster 0 siblings, 1 reply; 10+ messages in thread From: Darrick J. Wong @ 2026-09-24 1:15 UTC (permalink / raw) To: Brian Foster; +Cc: linux-xfs, Carlos Maiolino On Wed, Sep 23, 2026 at 12:15:24PM -0400, Brian Foster wrote: > Matt Fleming reports a filesystem shutdown due to inobt block > allocation failure during sparse chunk allocation. Inode creation > can involve multiple allocations in a transaction: the initial chunk > allocation and inode btree blocks via inobt record insertion. This > is expected to be safe by using the minleft parameter on the chunk > allocation to guarantee the selected AG has blocks available for > a followup inobt insertion. > > The sequence that leads to this failure is that the alloc and inode > btrees are all full (require a full split on next insertion) and the > AG has just enough free space to satisfy a sparse chunk allocation > with minleft set (i.e. 7 blocks in this example). The chunk > allocation splits a free extent, triggers full allocbt splits, and > consumes 4 free blocks for the chunk and 4 AGFL blocks for the > btrees. > > Next, the inobt record insertion triggers an inobt split. The AG has > enough free blocks, but the allocbt splits caused by the chunk > allocation have increased the min AGFL requirement for the AG due to > btree level increases. The AGFL requirement as calculated by > xfs_alloc_fix_freelist() is: > > free + AGFL - res - minfree - minleft = avail > > This evaluates to the following on initial chunk allocation: > > 2514 + 8 - 2505 - 8 - 2 = 7 > > ... and then after the chunk allocation but before the inobt block > allocation: > > 2510 + 4 - 2505 - 12 - 0 = -3 > > This causes the inobt alloc to fail despite minleft being set in the > first allocation. The error path cancels the dirty transaction and > shuts down the fs. The problem here is that while minleft ensures > free blocks are available for the inobt insert, it is not sufficient > to cover the increase of the AGFL min free requirement. > > To address this, first have xfs_alloc_freelist() return both min and > max freelist values. The min value is the current AGFL requirement > and remains used for actual AGFL sizing. The max value calculates > the worst case AGFL requirement after potential allocbt splits > during the current allocation. Incorporate the max value into space > availability checks for AG selection and the longest free extent > calculation. The latter is necessary because callers like the bmap > layer can size allocation requests based on the longest free extent. > Without this, aligned allocs can end up oversized, prematurely fail, > and fall back to non-aligned to make up the difference. > > This ensures the selected AG has enough blocks for both the caller's > minleft value and the worst case AGFL increase. In the example > above, the initial calculation now evaluates to 3 blocks available > instead of 7 and the inode allocation fails gracefully with -ENOSPC. > > Assisted-by: LLM > Reported-by: Matt Fleming <matt@readmodwrite.com> > Signed-off-by: Brian Foster <bfoster@redhat.com> > Reviewed-by: Mark Tinguely <mark.tinguely@oracle.com> > --- > fs/xfs/libxfs/xfs_alloc.c | 58 +++++++++++++++++++++++++++------------ > fs/xfs/libxfs/xfs_alloc.h | 3 +- > fs/xfs/libxfs/xfs_bmap.c | 6 ++-- > 3 files changed, 47 insertions(+), 20 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c > index af63926cc9ee..86f74a2dfa10 100644 > --- a/fs/xfs/libxfs/xfs_alloc.c > +++ b/fs/xfs/libxfs/xfs_alloc.c > @@ -2397,31 +2397,36 @@ xfs_alloc_compute_maxlevels( > } > > /* > - * Find the length of the longest extent in an AG. The 'need' parameter > - * specifies how much space we're going to need for the AGFL and the > - * 'reserved' parameter tells us how many blocks in this AG are reserved for > + * Find the length of the longest extent in an AG. The @min_free and @max_free > + * parameters specify how much space we're going to need for the AGFL and the > + * @reserved parameter tells us how many blocks in this AG are reserved for > * other callers. > */ > xfs_extlen_t > xfs_alloc_longest_free_extent( > struct xfs_perag *pag, > - xfs_extlen_t need, > + xfs_extlen_t min_free, > + xfs_extlen_t max_free, > xfs_extlen_t reserved) > { > xfs_extlen_t delta = 0; > > /* > - * If the AGFL needs a recharge, we'll have to subtract that from the > - * longest extent. > + * If the AGFL needs a recharge, subtract that from the longest extent > + * because AGFL refill happens before the alloc. > */ > - if (need > pag->pagf_flcount) > - delta = need - pag->pagf_flcount; > + if (min_free > pag->pagf_flcount) > + delta = min_free - pag->pagf_flcount; > > /* > - * If we cannot maintain others' reservations with space from the > - * not-longest freesp extents, we'll have to subtract /that/ from > - * the longest extent too. > + * Extra AGFL blocks beyond the min are reserved by ->minleft during > + * allocation. Similar to reserved, these blocks are not available to > + * this allocation. Check if we can preserve the combined total without > + * the longest extent. If not, deduct the necessary blocks from the > + * longest extent. > */ > + if (max_free > min_free) > + reserved += max_free - min_free; Hmm. So @min_free here is the minimum number of blocks that we have to keep on the AGFL to handle bnobt/cntbt/rmapbt btree expansions, right? And @max_free is the same, but assuming that they all increase one level in height, right? So we're adding to @reserved the quantity of fsblocks needed to handle adding that new layer and then making the "Can this AG handle this much allocation?" decision? /me wonders if they should be called min_agfl and max_agfl, respectively, but that only makes sense if the answers to the above are all 'yes'. > if (pag->pagf_freeblks - pag->pagf_longest < reserved) > delta += reserved - (pag->pagf_freeblks - pag->pagf_longest); > > @@ -2519,6 +2524,7 @@ static bool > xfs_alloc_space_available( > struct xfs_alloc_arg *args, > xfs_extlen_t min_free, > + xfs_extlen_t max_free, > int flags) > { > struct xfs_perag *pag = args->pag; > @@ -2526,15 +2532,32 @@ xfs_alloc_space_available( > xfs_extlen_t reservation; /* blocks that are still reserved */ > int available; > xfs_extlen_t agflcount; > + xfs_extlen_t minleft; > > if (flags & XFS_ALLOC_FLAG_FREEING) > return true; > > reservation = xfs_ag_resv_needed(pag, args->resv); > > + /* > + * minleft implies a multi-alloc transaction. If set, the first alloc > + * might cause btree splits that increase the AGFL requirement for the > + * next. This worst case requirement is calculated in max_free. > + * > + * We don't prepopulate the AGFL because we don't know in advance if > + * splits will occur. Instead, add the delta to minleft so it is > + * accounted for in AG selection. This ensures the AG has enough space > + * for the caller's minleft plus that needed to repopulate the AGFL on > + * the next alloc if splits do occur. > + */ > + minleft = args->minleft; > + if (minleft) > + minleft += max_free - min_free; > + > /* do we have enough contiguous free space for the allocation? */ > alloc_len = args->minlen + (args->alignment - 1) + args->minalignslop; > - longest = xfs_alloc_longest_free_extent(pag, min_free, reservation); > + longest = xfs_alloc_longest_free_extent(pag, min_free, > + minleft ? max_free : min_free, reservation); My first thought was "Why do we only supply max_free if minleft>0?" but I think that's the part that provides "...plus that needed to repopulate the AGFL on the next alloc...", right? (the important phrase here being "next alloc") If the answers to all my questions are yes then I think I've understood this well enough to say Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> --D > if (longest < alloc_len) > return false; > > @@ -2545,7 +2568,7 @@ xfs_alloc_space_available( > */ > agflcount = min_t(xfs_extlen_t, pag->pagf_flcount, min_free); available = (int)(pag->pagf_freeblks + agflcount - > - reservation - min_free - args->minleft); > + reservation - min_free - minleft); > if (available < (int)max(args->total, alloc_len)) > return false; > > @@ -2859,6 +2882,7 @@ xfs_alloc_fix_freelist( > struct xfs_alloc_arg targs; /* local allocation arguments */ > xfs_agblock_t bno; /* freelist block */ > xfs_extlen_t min_free;/* total blocks needed in freelist */ > + xfs_extlen_t max_free; /* max freelist requirement */ > int error = 0; > > /* deferred ops (AGFL block frees) require permanent transactions */ > @@ -2886,8 +2910,8 @@ xfs_alloc_fix_freelist( > goto out_agbp_relse; > } > > - xfs_alloc_freelist(mp, pag, &min_free, NULL); > - if (!xfs_alloc_space_available(args, min_free, alloc_flags | > + xfs_alloc_freelist(mp, pag, &min_free, &max_free); > + if (!xfs_alloc_space_available(args, min_free, max_free, alloc_flags | > XFS_ALLOC_FLAG_CHECK)) > goto out_agbp_relse; > > @@ -2910,8 +2934,8 @@ xfs_alloc_fix_freelist( > xfs_agfl_reset(tp, agbp, pag); > > /* If there isn't enough total space or single-extent, reject it. */ > - xfs_alloc_freelist(mp, pag, &min_free, NULL); > - if (!xfs_alloc_space_available(args, min_free, alloc_flags)) > + xfs_alloc_freelist(mp, pag, &min_free, &max_free); > + if (!xfs_alloc_space_available(args, min_free, max_free, alloc_flags)) > goto out_agbp_relse; > > if (IS_ENABLED(CONFIG_XFS_DEBUG) && args->alloc_minlen_only) { > diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h > index 44a10f4a22a2..5812c9b5e609 100644 > --- a/fs/xfs/libxfs/xfs_alloc.h > +++ b/fs/xfs/libxfs/xfs_alloc.h > @@ -70,7 +70,8 @@ unsigned int xfs_alloc_set_aside(struct xfs_mount *mp); > unsigned int xfs_alloc_ag_max_usable(struct xfs_mount *mp); > > xfs_extlen_t xfs_alloc_longest_free_extent(struct xfs_perag *pag, > - xfs_extlen_t need, xfs_extlen_t reserved); > + xfs_extlen_t min_free, xfs_extlen_t max_free, > + xfs_extlen_t reserved); > void xfs_alloc_freelist(struct xfs_mount *mp, struct xfs_perag *pag, > unsigned int *min_free, unsigned int *max_free); > int xfs_alloc_get_freelist(struct xfs_perag *pag, struct xfs_trans *tp, > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > index d6be6734bd90..9bffde1484a7 100644 > --- a/fs/xfs/libxfs/xfs_bmap.c > +++ b/fs/xfs/libxfs/xfs_bmap.c > @@ -3150,6 +3150,7 @@ xfs_bmap_longest_free_extent( > { > xfs_extlen_t longest; > unsigned int min_free; > + unsigned int max_free; > int error = 0; > > if (!xfs_perag_initialised_agf(pag)) { > @@ -3159,8 +3160,9 @@ xfs_bmap_longest_free_extent( > return error; > } > > - xfs_alloc_freelist(pag_mount(pag), pag, &min_free, NULL); > - longest = xfs_alloc_longest_free_extent(pag, min_free, > + /* bmap allocs always have minleft set, so account for max_free */ > + xfs_alloc_freelist(pag_mount(pag), pag, &min_free, &max_free); > + longest = xfs_alloc_longest_free_extent(pag, min_free, max_free, > xfs_ag_resv_needed(pag, XFS_AG_RESV_NONE)); > if (*blen < longest) > *blen = longest; > -- > 2.55.0 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 4/4] xfs: incorporate increased AGFL min requirement for minleft allocs 2026-09-24 1:15 ` Darrick J. Wong @ 2026-09-24 13:14 ` Brian Foster 2026-09-24 18:56 ` Darrick J. Wong 0 siblings, 1 reply; 10+ messages in thread From: Brian Foster @ 2026-09-24 13:14 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-xfs, Carlos Maiolino On Wed, Sep 23, 2026 at 06:15:42PM -0700, Darrick J. Wong wrote: > On Wed, Sep 23, 2026 at 12:15:24PM -0400, Brian Foster wrote: > > Matt Fleming reports a filesystem shutdown due to inobt block > > allocation failure during sparse chunk allocation. Inode creation > > can involve multiple allocations in a transaction: the initial chunk > > allocation and inode btree blocks via inobt record insertion. This > > is expected to be safe by using the minleft parameter on the chunk > > allocation to guarantee the selected AG has blocks available for > > a followup inobt insertion. > > > > The sequence that leads to this failure is that the alloc and inode > > btrees are all full (require a full split on next insertion) and the > > AG has just enough free space to satisfy a sparse chunk allocation > > with minleft set (i.e. 7 blocks in this example). The chunk > > allocation splits a free extent, triggers full allocbt splits, and > > consumes 4 free blocks for the chunk and 4 AGFL blocks for the > > btrees. > > > > Next, the inobt record insertion triggers an inobt split. The AG has > > enough free blocks, but the allocbt splits caused by the chunk > > allocation have increased the min AGFL requirement for the AG due to > > btree level increases. The AGFL requirement as calculated by > > xfs_alloc_fix_freelist() is: > > > > free + AGFL - res - minfree - minleft = avail > > > > This evaluates to the following on initial chunk allocation: > > > > 2514 + 8 - 2505 - 8 - 2 = 7 > > > > ... and then after the chunk allocation but before the inobt block > > allocation: > > > > 2510 + 4 - 2505 - 12 - 0 = -3 > > > > This causes the inobt alloc to fail despite minleft being set in the > > first allocation. The error path cancels the dirty transaction and > > shuts down the fs. The problem here is that while minleft ensures > > free blocks are available for the inobt insert, it is not sufficient > > to cover the increase of the AGFL min free requirement. > > > > To address this, first have xfs_alloc_freelist() return both min and > > max freelist values. The min value is the current AGFL requirement > > and remains used for actual AGFL sizing. The max value calculates > > the worst case AGFL requirement after potential allocbt splits > > during the current allocation. Incorporate the max value into space > > availability checks for AG selection and the longest free extent > > calculation. The latter is necessary because callers like the bmap > > layer can size allocation requests based on the longest free extent. > > Without this, aligned allocs can end up oversized, prematurely fail, > > and fall back to non-aligned to make up the difference. > > > > This ensures the selected AG has enough blocks for both the caller's > > minleft value and the worst case AGFL increase. In the example > > above, the initial calculation now evaluates to 3 blocks available > > instead of 7 and the inode allocation fails gracefully with -ENOSPC. > > > > Assisted-by: LLM > > Reported-by: Matt Fleming <matt@readmodwrite.com> > > Signed-off-by: Brian Foster <bfoster@redhat.com> > > Reviewed-by: Mark Tinguely <mark.tinguely@oracle.com> > > --- > > fs/xfs/libxfs/xfs_alloc.c | 58 +++++++++++++++++++++++++++------------ > > fs/xfs/libxfs/xfs_alloc.h | 3 +- > > fs/xfs/libxfs/xfs_bmap.c | 6 ++-- > > 3 files changed, 47 insertions(+), 20 deletions(-) > > > > diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c > > index af63926cc9ee..86f74a2dfa10 100644 > > --- a/fs/xfs/libxfs/xfs_alloc.c > > +++ b/fs/xfs/libxfs/xfs_alloc.c > > @@ -2397,31 +2397,36 @@ xfs_alloc_compute_maxlevels( > > } > > > > /* > > - * Find the length of the longest extent in an AG. The 'need' parameter > > - * specifies how much space we're going to need for the AGFL and the > > - * 'reserved' parameter tells us how many blocks in this AG are reserved for > > + * Find the length of the longest extent in an AG. The @min_free and @max_free > > + * parameters specify how much space we're going to need for the AGFL and the > > + * @reserved parameter tells us how many blocks in this AG are reserved for > > * other callers. > > */ > > xfs_extlen_t > > xfs_alloc_longest_free_extent( > > struct xfs_perag *pag, > > - xfs_extlen_t need, > > + xfs_extlen_t min_free, > > + xfs_extlen_t max_free, > > xfs_extlen_t reserved) > > { > > xfs_extlen_t delta = 0; > > > > /* > > - * If the AGFL needs a recharge, we'll have to subtract that from the > > - * longest extent. > > + * If the AGFL needs a recharge, subtract that from the longest extent > > + * because AGFL refill happens before the alloc. > > */ > > - if (need > pag->pagf_flcount) > > - delta = need - pag->pagf_flcount; > > + if (min_free > pag->pagf_flcount) > > + delta = min_free - pag->pagf_flcount; > > > > /* > > - * If we cannot maintain others' reservations with space from the > > - * not-longest freesp extents, we'll have to subtract /that/ from > > - * the longest extent too. > > + * Extra AGFL blocks beyond the min are reserved by ->minleft during > > + * allocation. Similar to reserved, these blocks are not available to > > + * this allocation. Check if we can preserve the combined total without > > + * the longest extent. If not, deduct the necessary blocks from the > > + * longest extent. > > */ > > + if (max_free > min_free) > > + reserved += max_free - min_free; > > Hmm. So @min_free here is the minimum number of blocks that we have to > keep on the AGFL to handle bnobt/cntbt/rmapbt btree expansions, right? > And @max_free is the same, but assuming that they all increase one level > in height, right? So we're adding to @reserved the quantity of fsblocks > needed to handle adding that new layer and then making the "Can this AG > handle this much allocation?" decision? > Yep. > /me wonders if they should be called min_agfl and max_agfl, > respectively, but that only makes sense if the answers to the above are > all 'yes'. > Do you mean within this function, or across the board? IIRC here I was generally just trying to keep things consistent wrt naming (i.e. I found the 'need' naming here annoyingly confusing) across the various function calls, but I'm not opposed to just renaming them all to min/max_agfl or whatever.. > > if (pag->pagf_freeblks - pag->pagf_longest < reserved) > > delta += reserved - (pag->pagf_freeblks - pag->pagf_longest); > > > > @@ -2519,6 +2524,7 @@ static bool > > xfs_alloc_space_available( > > struct xfs_alloc_arg *args, > > xfs_extlen_t min_free, > > + xfs_extlen_t max_free, > > int flags) > > { > > struct xfs_perag *pag = args->pag; > > @@ -2526,15 +2532,32 @@ xfs_alloc_space_available( > > xfs_extlen_t reservation; /* blocks that are still reserved */ > > int available; > > xfs_extlen_t agflcount; > > + xfs_extlen_t minleft; > > > > if (flags & XFS_ALLOC_FLAG_FREEING) > > return true; > > > > reservation = xfs_ag_resv_needed(pag, args->resv); > > > > + /* > > + * minleft implies a multi-alloc transaction. If set, the first alloc > > + * might cause btree splits that increase the AGFL requirement for the > > + * next. This worst case requirement is calculated in max_free. > > + * > > + * We don't prepopulate the AGFL because we don't know in advance if > > + * splits will occur. Instead, add the delta to minleft so it is > > + * accounted for in AG selection. This ensures the AG has enough space > > + * for the caller's minleft plus that needed to repopulate the AGFL on > > + * the next alloc if splits do occur. > > + */ > > + minleft = args->minleft; > > + if (minleft) > > + minleft += max_free - min_free; > > + > > /* do we have enough contiguous free space for the allocation? */ > > alloc_len = args->minlen + (args->alignment - 1) + args->minalignslop; > > - longest = xfs_alloc_longest_free_extent(pag, min_free, reservation); > > + longest = xfs_alloc_longest_free_extent(pag, min_free, > > + minleft ? max_free : min_free, reservation); > > My first thought was "Why do we only supply max_free if minleft>0?" but > I think that's the part that provides "...plus that needed to repopulate > the AGFL on the next alloc...", right? (the important phrase here being > "next alloc") > Yeah.. minleft is a dual purpose thing here. First, it indicates a "multi-allocation" case (as Dave coined in a prior thread) as minleft implies there will be a followup allocation with minleft reset back to zero under the same transaction/agf lock. Second, in that multi-alloc case, we need to make sure that the first allocation requires not only that the pure minleft value set by the caller remains available after the allocation, but also enough to satisfy the potentially increased AGFL requirement that the associated gatekeeping logic will enforce on the followup allocation. The main reason for the separate min/max fields and using minleft/reservation for this extra space is that just bumping min_free (or min_agfl) would spuriously populate and depopulate the AGFL across these allocations for the uncommon worst case. > If the answers to all my questions are yes then I think I've understood > this well enough to say > Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> > Thanks. Let me know what you were looking for on the naming thing and I'll either tack on a full rename patch or respin this with more selective changes.. Brian > --D > > > if (longest < alloc_len) > > return false; > > > > @@ -2545,7 +2568,7 @@ xfs_alloc_space_available( > > */ > > agflcount = min_t(xfs_extlen_t, pag->pagf_flcount, min_free); > available = (int)(pag->pagf_freeblks + agflcount - > > - reservation - min_free - args->minleft); > > + reservation - min_free - minleft); > > if (available < (int)max(args->total, alloc_len)) > > return false; > > > > @@ -2859,6 +2882,7 @@ xfs_alloc_fix_freelist( > > struct xfs_alloc_arg targs; /* local allocation arguments */ > > xfs_agblock_t bno; /* freelist block */ > > xfs_extlen_t min_free;/* total blocks needed in freelist */ > > + xfs_extlen_t max_free; /* max freelist requirement */ > > int error = 0; > > > > /* deferred ops (AGFL block frees) require permanent transactions */ > > @@ -2886,8 +2910,8 @@ xfs_alloc_fix_freelist( > > goto out_agbp_relse; > > } > > > > - xfs_alloc_freelist(mp, pag, &min_free, NULL); > > - if (!xfs_alloc_space_available(args, min_free, alloc_flags | > > + xfs_alloc_freelist(mp, pag, &min_free, &max_free); > > + if (!xfs_alloc_space_available(args, min_free, max_free, alloc_flags | > > XFS_ALLOC_FLAG_CHECK)) > > goto out_agbp_relse; > > > > @@ -2910,8 +2934,8 @@ xfs_alloc_fix_freelist( > > xfs_agfl_reset(tp, agbp, pag); > > > > /* If there isn't enough total space or single-extent, reject it. */ > > - xfs_alloc_freelist(mp, pag, &min_free, NULL); > > - if (!xfs_alloc_space_available(args, min_free, alloc_flags)) > > + xfs_alloc_freelist(mp, pag, &min_free, &max_free); > > + if (!xfs_alloc_space_available(args, min_free, max_free, alloc_flags)) > > goto out_agbp_relse; > > > > if (IS_ENABLED(CONFIG_XFS_DEBUG) && args->alloc_minlen_only) { > > diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h > > index 44a10f4a22a2..5812c9b5e609 100644 > > --- a/fs/xfs/libxfs/xfs_alloc.h > > +++ b/fs/xfs/libxfs/xfs_alloc.h > > @@ -70,7 +70,8 @@ unsigned int xfs_alloc_set_aside(struct xfs_mount *mp); > > unsigned int xfs_alloc_ag_max_usable(struct xfs_mount *mp); > > > > xfs_extlen_t xfs_alloc_longest_free_extent(struct xfs_perag *pag, > > - xfs_extlen_t need, xfs_extlen_t reserved); > > + xfs_extlen_t min_free, xfs_extlen_t max_free, > > + xfs_extlen_t reserved); > > void xfs_alloc_freelist(struct xfs_mount *mp, struct xfs_perag *pag, > > unsigned int *min_free, unsigned int *max_free); > > int xfs_alloc_get_freelist(struct xfs_perag *pag, struct xfs_trans *tp, > > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > > index d6be6734bd90..9bffde1484a7 100644 > > --- a/fs/xfs/libxfs/xfs_bmap.c > > +++ b/fs/xfs/libxfs/xfs_bmap.c > > @@ -3150,6 +3150,7 @@ xfs_bmap_longest_free_extent( > > { > > xfs_extlen_t longest; > > unsigned int min_free; > > + unsigned int max_free; > > int error = 0; > > > > if (!xfs_perag_initialised_agf(pag)) { > > @@ -3159,8 +3160,9 @@ xfs_bmap_longest_free_extent( > > return error; > > } > > > > - xfs_alloc_freelist(pag_mount(pag), pag, &min_free, NULL); > > - longest = xfs_alloc_longest_free_extent(pag, min_free, > > + /* bmap allocs always have minleft set, so account for max_free */ > > + xfs_alloc_freelist(pag_mount(pag), pag, &min_free, &max_free); > > + longest = xfs_alloc_longest_free_extent(pag, min_free, max_free, > > xfs_ag_resv_needed(pag, XFS_AG_RESV_NONE)); > > if (*blen < longest) > > *blen = longest; > > -- > > 2.55.0 > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 4/4] xfs: incorporate increased AGFL min requirement for minleft allocs 2026-09-24 13:14 ` Brian Foster @ 2026-09-24 18:56 ` Darrick J. Wong 2026-09-25 18:48 ` Brian Foster 0 siblings, 1 reply; 10+ messages in thread From: Darrick J. Wong @ 2026-09-24 18:56 UTC (permalink / raw) To: Brian Foster; +Cc: linux-xfs, Carlos Maiolino On Thu, Sep 24, 2026 at 09:14:11AM -0400, Brian Foster wrote: > On Wed, Sep 23, 2026 at 06:15:42PM -0700, Darrick J. Wong wrote: > > On Wed, Sep 23, 2026 at 12:15:24PM -0400, Brian Foster wrote: > > > Matt Fleming reports a filesystem shutdown due to inobt block > > > allocation failure during sparse chunk allocation. Inode creation > > > can involve multiple allocations in a transaction: the initial chunk > > > allocation and inode btree blocks via inobt record insertion. This > > > is expected to be safe by using the minleft parameter on the chunk > > > allocation to guarantee the selected AG has blocks available for > > > a followup inobt insertion. > > > > > > The sequence that leads to this failure is that the alloc and inode > > > btrees are all full (require a full split on next insertion) and the > > > AG has just enough free space to satisfy a sparse chunk allocation > > > with minleft set (i.e. 7 blocks in this example). The chunk > > > allocation splits a free extent, triggers full allocbt splits, and > > > consumes 4 free blocks for the chunk and 4 AGFL blocks for the > > > btrees. > > > > > > Next, the inobt record insertion triggers an inobt split. The AG has > > > enough free blocks, but the allocbt splits caused by the chunk > > > allocation have increased the min AGFL requirement for the AG due to > > > btree level increases. The AGFL requirement as calculated by > > > xfs_alloc_fix_freelist() is: > > > > > > free + AGFL - res - minfree - minleft = avail > > > > > > This evaluates to the following on initial chunk allocation: > > > > > > 2514 + 8 - 2505 - 8 - 2 = 7 > > > > > > ... and then after the chunk allocation but before the inobt block > > > allocation: > > > > > > 2510 + 4 - 2505 - 12 - 0 = -3 > > > > > > This causes the inobt alloc to fail despite minleft being set in the > > > first allocation. The error path cancels the dirty transaction and > > > shuts down the fs. The problem here is that while minleft ensures > > > free blocks are available for the inobt insert, it is not sufficient > > > to cover the increase of the AGFL min free requirement. > > > > > > To address this, first have xfs_alloc_freelist() return both min and > > > max freelist values. The min value is the current AGFL requirement > > > and remains used for actual AGFL sizing. The max value calculates > > > the worst case AGFL requirement after potential allocbt splits > > > during the current allocation. Incorporate the max value into space > > > availability checks for AG selection and the longest free extent > > > calculation. The latter is necessary because callers like the bmap > > > layer can size allocation requests based on the longest free extent. > > > Without this, aligned allocs can end up oversized, prematurely fail, > > > and fall back to non-aligned to make up the difference. > > > > > > This ensures the selected AG has enough blocks for both the caller's > > > minleft value and the worst case AGFL increase. In the example > > > above, the initial calculation now evaluates to 3 blocks available > > > instead of 7 and the inode allocation fails gracefully with -ENOSPC. > > > > > > Assisted-by: LLM > > > Reported-by: Matt Fleming <matt@readmodwrite.com> > > > Signed-off-by: Brian Foster <bfoster@redhat.com> > > > Reviewed-by: Mark Tinguely <mark.tinguely@oracle.com> > > > --- > > > fs/xfs/libxfs/xfs_alloc.c | 58 +++++++++++++++++++++++++++------------ > > > fs/xfs/libxfs/xfs_alloc.h | 3 +- > > > fs/xfs/libxfs/xfs_bmap.c | 6 ++-- > > > 3 files changed, 47 insertions(+), 20 deletions(-) > > > > > > diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c > > > index af63926cc9ee..86f74a2dfa10 100644 > > > --- a/fs/xfs/libxfs/xfs_alloc.c > > > +++ b/fs/xfs/libxfs/xfs_alloc.c > > > @@ -2397,31 +2397,36 @@ xfs_alloc_compute_maxlevels( > > > } > > > > > > /* > > > - * Find the length of the longest extent in an AG. The 'need' parameter > > > - * specifies how much space we're going to need for the AGFL and the > > > - * 'reserved' parameter tells us how many blocks in this AG are reserved for > > > + * Find the length of the longest extent in an AG. The @min_free and @max_free > > > + * parameters specify how much space we're going to need for the AGFL and the > > > + * @reserved parameter tells us how many blocks in this AG are reserved for > > > * other callers. > > > */ > > > xfs_extlen_t > > > xfs_alloc_longest_free_extent( > > > struct xfs_perag *pag, > > > - xfs_extlen_t need, > > > + xfs_extlen_t min_free, > > > + xfs_extlen_t max_free, > > > xfs_extlen_t reserved) > > > { > > > xfs_extlen_t delta = 0; > > > > > > /* > > > - * If the AGFL needs a recharge, we'll have to subtract that from the > > > - * longest extent. > > > + * If the AGFL needs a recharge, subtract that from the longest extent > > > + * because AGFL refill happens before the alloc. > > > */ > > > - if (need > pag->pagf_flcount) > > > - delta = need - pag->pagf_flcount; > > > + if (min_free > pag->pagf_flcount) > > > + delta = min_free - pag->pagf_flcount; > > > > > > /* > > > - * If we cannot maintain others' reservations with space from the > > > - * not-longest freesp extents, we'll have to subtract /that/ from > > > - * the longest extent too. > > > + * Extra AGFL blocks beyond the min are reserved by ->minleft during > > > + * allocation. Similar to reserved, these blocks are not available to > > > + * this allocation. Check if we can preserve the combined total without > > > + * the longest extent. If not, deduct the necessary blocks from the > > > + * longest extent. > > > */ > > > + if (max_free > min_free) > > > + reserved += max_free - min_free; > > > > Hmm. So @min_free here is the minimum number of blocks that we have to > > keep on the AGFL to handle bnobt/cntbt/rmapbt btree expansions, right? > > And @max_free is the same, but assuming that they all increase one level > > in height, right? So we're adding to @reserved the quantity of fsblocks > > needed to handle adding that new layer and then making the "Can this AG > > handle this much allocation?" decision? > > > > Yep. > > > /me wonders if they should be called min_agfl and max_agfl, > > respectively, but that only makes sense if the answers to the above are > > all 'yes'. > > > > Do you mean within this function, or across the board? IIRC here I was > generally just trying to keep things consistent wrt naming (i.e. I found > the 'need' naming here annoyingly confusing) across the various function > calls, but I'm not opposed to just renaming them all to min/max_agfl or > whatever.. No, just here in this function. > > > if (pag->pagf_freeblks - pag->pagf_longest < reserved) > > > delta += reserved - (pag->pagf_freeblks - pag->pagf_longest); > > > > > > @@ -2519,6 +2524,7 @@ static bool > > > xfs_alloc_space_available( > > > struct xfs_alloc_arg *args, > > > xfs_extlen_t min_free, > > > + xfs_extlen_t max_free, > > > int flags) > > > { > > > struct xfs_perag *pag = args->pag; > > > @@ -2526,15 +2532,32 @@ xfs_alloc_space_available( > > > xfs_extlen_t reservation; /* blocks that are still reserved */ > > > int available; > > > xfs_extlen_t agflcount; > > > + xfs_extlen_t minleft; > > > > > > if (flags & XFS_ALLOC_FLAG_FREEING) > > > return true; > > > > > > reservation = xfs_ag_resv_needed(pag, args->resv); > > > > > > + /* > > > + * minleft implies a multi-alloc transaction. If set, the first alloc > > > + * might cause btree splits that increase the AGFL requirement for the > > > + * next. This worst case requirement is calculated in max_free. > > > + * > > > + * We don't prepopulate the AGFL because we don't know in advance if > > > + * splits will occur. Instead, add the delta to minleft so it is > > > + * accounted for in AG selection. This ensures the AG has enough space > > > + * for the caller's minleft plus that needed to repopulate the AGFL on > > > + * the next alloc if splits do occur. > > > + */ > > > + minleft = args->minleft; > > > + if (minleft) > > > + minleft += max_free - min_free; > > > + > > > /* do we have enough contiguous free space for the allocation? */ > > > alloc_len = args->minlen + (args->alignment - 1) + args->minalignslop; > > > - longest = xfs_alloc_longest_free_extent(pag, min_free, reservation); > > > + longest = xfs_alloc_longest_free_extent(pag, min_free, > > > + minleft ? max_free : min_free, reservation); > > > > My first thought was "Why do we only supply max_free if minleft>0?" but > > I think that's the part that provides "...plus that needed to repopulate > > the AGFL on the next alloc...", right? (the important phrase here being > > "next alloc") > > > > Yeah.. minleft is a dual purpose thing here. First, it indicates a > "multi-allocation" case (as Dave coined in a prior thread) as minleft > implies there will be a followup allocation with minleft reset back to > zero under the same transaction/agf lock. > > Second, in that multi-alloc case, we need to make sure that the first > allocation requires not only that the pure minleft value set by the > caller remains available after the allocation, but also enough to > satisfy the potentially increased AGFL requirement that the associated > gatekeeping logic will enforce on the followup allocation. > > The main reason for the separate min/max fields and using > minleft/reservation for this extra space is that just bumping min_free > (or min_agfl) would spuriously populate and depopulate the AGFL across > these allocations for the uncommon worst case. > > > If the answers to all my questions are yes then I think I've understood > > this well enough to say > > Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> > > > > Thanks. Let me know what you were looking for on the naming thing and > I'll either tack on a full rename patch or respin this with more > selective changes.. Nah, the naming thing is specific to xfs_alloc_longest_free_extent. I'd just fold in any name changes that you decide to make. --D > > Brian > > > --D > > > > > if (longest < alloc_len) > > > return false; > > > > > > @@ -2545,7 +2568,7 @@ xfs_alloc_space_available( > > > */ > > > agflcount = min_t(xfs_extlen_t, pag->pagf_flcount, min_free); > > available = (int)(pag->pagf_freeblks + agflcount - > > > - reservation - min_free - args->minleft); > > > + reservation - min_free - minleft); > > > if (available < (int)max(args->total, alloc_len)) > > > return false; > > > > > > @@ -2859,6 +2882,7 @@ xfs_alloc_fix_freelist( > > > struct xfs_alloc_arg targs; /* local allocation arguments */ > > > xfs_agblock_t bno; /* freelist block */ > > > xfs_extlen_t min_free;/* total blocks needed in freelist */ > > > + xfs_extlen_t max_free; /* max freelist requirement */ > > > int error = 0; > > > > > > /* deferred ops (AGFL block frees) require permanent transactions */ > > > @@ -2886,8 +2910,8 @@ xfs_alloc_fix_freelist( > > > goto out_agbp_relse; > > > } > > > > > > - xfs_alloc_freelist(mp, pag, &min_free, NULL); > > > - if (!xfs_alloc_space_available(args, min_free, alloc_flags | > > > + xfs_alloc_freelist(mp, pag, &min_free, &max_free); > > > + if (!xfs_alloc_space_available(args, min_free, max_free, alloc_flags | > > > XFS_ALLOC_FLAG_CHECK)) > > > goto out_agbp_relse; > > > > > > @@ -2910,8 +2934,8 @@ xfs_alloc_fix_freelist( > > > xfs_agfl_reset(tp, agbp, pag); > > > > > > /* If there isn't enough total space or single-extent, reject it. */ > > > - xfs_alloc_freelist(mp, pag, &min_free, NULL); > > > - if (!xfs_alloc_space_available(args, min_free, alloc_flags)) > > > + xfs_alloc_freelist(mp, pag, &min_free, &max_free); > > > + if (!xfs_alloc_space_available(args, min_free, max_free, alloc_flags)) > > > goto out_agbp_relse; > > > > > > if (IS_ENABLED(CONFIG_XFS_DEBUG) && args->alloc_minlen_only) { > > > diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h > > > index 44a10f4a22a2..5812c9b5e609 100644 > > > --- a/fs/xfs/libxfs/xfs_alloc.h > > > +++ b/fs/xfs/libxfs/xfs_alloc.h > > > @@ -70,7 +70,8 @@ unsigned int xfs_alloc_set_aside(struct xfs_mount *mp); > > > unsigned int xfs_alloc_ag_max_usable(struct xfs_mount *mp); > > > > > > xfs_extlen_t xfs_alloc_longest_free_extent(struct xfs_perag *pag, > > > - xfs_extlen_t need, xfs_extlen_t reserved); > > > + xfs_extlen_t min_free, xfs_extlen_t max_free, > > > + xfs_extlen_t reserved); > > > void xfs_alloc_freelist(struct xfs_mount *mp, struct xfs_perag *pag, > > > unsigned int *min_free, unsigned int *max_free); > > > int xfs_alloc_get_freelist(struct xfs_perag *pag, struct xfs_trans *tp, > > > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > > > index d6be6734bd90..9bffde1484a7 100644 > > > --- a/fs/xfs/libxfs/xfs_bmap.c > > > +++ b/fs/xfs/libxfs/xfs_bmap.c > > > @@ -3150,6 +3150,7 @@ xfs_bmap_longest_free_extent( > > > { > > > xfs_extlen_t longest; > > > unsigned int min_free; > > > + unsigned int max_free; > > > int error = 0; > > > > > > if (!xfs_perag_initialised_agf(pag)) { > > > @@ -3159,8 +3160,9 @@ xfs_bmap_longest_free_extent( > > > return error; > > > } > > > > > > - xfs_alloc_freelist(pag_mount(pag), pag, &min_free, NULL); > > > - longest = xfs_alloc_longest_free_extent(pag, min_free, > > > + /* bmap allocs always have minleft set, so account for max_free */ > > > + xfs_alloc_freelist(pag_mount(pag), pag, &min_free, &max_free); > > > + longest = xfs_alloc_longest_free_extent(pag, min_free, max_free, > > > xfs_ag_resv_needed(pag, XFS_AG_RESV_NONE)); > > > if (*blen < longest) > > > *blen = longest; > > > -- > > > 2.55.0 > > > > > > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 4/4] xfs: incorporate increased AGFL min requirement for minleft allocs 2026-09-24 18:56 ` Darrick J. Wong @ 2026-09-25 18:48 ` Brian Foster 0 siblings, 0 replies; 10+ messages in thread From: Brian Foster @ 2026-09-25 18:48 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-xfs, Carlos Maiolino On Thu, Sep 24, 2026 at 11:56:20AM -0700, Darrick J. Wong wrote: > On Thu, Sep 24, 2026 at 09:14:11AM -0400, Brian Foster wrote: > > On Wed, Sep 23, 2026 at 06:15:42PM -0700, Darrick J. Wong wrote: > > > On Wed, Sep 23, 2026 at 12:15:24PM -0400, Brian Foster wrote: > > > > Matt Fleming reports a filesystem shutdown due to inobt block > > > > allocation failure during sparse chunk allocation. Inode creation > > > > can involve multiple allocations in a transaction: the initial chunk > > > > allocation and inode btree blocks via inobt record insertion. This > > > > is expected to be safe by using the minleft parameter on the chunk > > > > allocation to guarantee the selected AG has blocks available for > > > > a followup inobt insertion. > > > > > > > > The sequence that leads to this failure is that the alloc and inode > > > > btrees are all full (require a full split on next insertion) and the > > > > AG has just enough free space to satisfy a sparse chunk allocation > > > > with minleft set (i.e. 7 blocks in this example). The chunk > > > > allocation splits a free extent, triggers full allocbt splits, and > > > > consumes 4 free blocks for the chunk and 4 AGFL blocks for the > > > > btrees. > > > > > > > > Next, the inobt record insertion triggers an inobt split. The AG has > > > > enough free blocks, but the allocbt splits caused by the chunk > > > > allocation have increased the min AGFL requirement for the AG due to > > > > btree level increases. The AGFL requirement as calculated by > > > > xfs_alloc_fix_freelist() is: > > > > > > > > free + AGFL - res - minfree - minleft = avail > > > > > > > > This evaluates to the following on initial chunk allocation: > > > > > > > > 2514 + 8 - 2505 - 8 - 2 = 7 > > > > > > > > ... and then after the chunk allocation but before the inobt block > > > > allocation: > > > > > > > > 2510 + 4 - 2505 - 12 - 0 = -3 > > > > > > > > This causes the inobt alloc to fail despite minleft being set in the > > > > first allocation. The error path cancels the dirty transaction and > > > > shuts down the fs. The problem here is that while minleft ensures > > > > free blocks are available for the inobt insert, it is not sufficient > > > > to cover the increase of the AGFL min free requirement. > > > > > > > > To address this, first have xfs_alloc_freelist() return both min and > > > > max freelist values. The min value is the current AGFL requirement > > > > and remains used for actual AGFL sizing. The max value calculates > > > > the worst case AGFL requirement after potential allocbt splits > > > > during the current allocation. Incorporate the max value into space > > > > availability checks for AG selection and the longest free extent > > > > calculation. The latter is necessary because callers like the bmap > > > > layer can size allocation requests based on the longest free extent. > > > > Without this, aligned allocs can end up oversized, prematurely fail, > > > > and fall back to non-aligned to make up the difference. > > > > > > > > This ensures the selected AG has enough blocks for both the caller's > > > > minleft value and the worst case AGFL increase. In the example > > > > above, the initial calculation now evaluates to 3 blocks available > > > > instead of 7 and the inode allocation fails gracefully with -ENOSPC. > > > > > > > > Assisted-by: LLM > > > > Reported-by: Matt Fleming <matt@readmodwrite.com> > > > > Signed-off-by: Brian Foster <bfoster@redhat.com> > > > > Reviewed-by: Mark Tinguely <mark.tinguely@oracle.com> > > > > --- > > > > fs/xfs/libxfs/xfs_alloc.c | 58 +++++++++++++++++++++++++++------------ > > > > fs/xfs/libxfs/xfs_alloc.h | 3 +- > > > > fs/xfs/libxfs/xfs_bmap.c | 6 ++-- > > > > 3 files changed, 47 insertions(+), 20 deletions(-) > > > > > > > > diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c > > > > index af63926cc9ee..86f74a2dfa10 100644 > > > > --- a/fs/xfs/libxfs/xfs_alloc.c > > > > +++ b/fs/xfs/libxfs/xfs_alloc.c > > > > @@ -2397,31 +2397,36 @@ xfs_alloc_compute_maxlevels( > > > > } > > > > > > > > /* > > > > - * Find the length of the longest extent in an AG. The 'need' parameter > > > > - * specifies how much space we're going to need for the AGFL and the > > > > - * 'reserved' parameter tells us how many blocks in this AG are reserved for > > > > + * Find the length of the longest extent in an AG. The @min_free and @max_free > > > > + * parameters specify how much space we're going to need for the AGFL and the > > > > + * @reserved parameter tells us how many blocks in this AG are reserved for > > > > * other callers. > > > > */ > > > > xfs_extlen_t > > > > xfs_alloc_longest_free_extent( > > > > struct xfs_perag *pag, > > > > - xfs_extlen_t need, > > > > + xfs_extlen_t min_free, > > > > + xfs_extlen_t max_free, > > > > xfs_extlen_t reserved) > > > > { > > > > xfs_extlen_t delta = 0; > > > > > > > > /* > > > > - * If the AGFL needs a recharge, we'll have to subtract that from the > > > > - * longest extent. > > > > + * If the AGFL needs a recharge, subtract that from the longest extent > > > > + * because AGFL refill happens before the alloc. > > > > */ > > > > - if (need > pag->pagf_flcount) > > > > - delta = need - pag->pagf_flcount; > > > > + if (min_free > pag->pagf_flcount) > > > > + delta = min_free - pag->pagf_flcount; > > > > > > > > /* > > > > - * If we cannot maintain others' reservations with space from the > > > > - * not-longest freesp extents, we'll have to subtract /that/ from > > > > - * the longest extent too. > > > > + * Extra AGFL blocks beyond the min are reserved by ->minleft during > > > > + * allocation. Similar to reserved, these blocks are not available to > > > > + * this allocation. Check if we can preserve the combined total without > > > > + * the longest extent. If not, deduct the necessary blocks from the > > > > + * longest extent. > > > > */ > > > > + if (max_free > min_free) > > > > + reserved += max_free - min_free; > > > > > > Hmm. So @min_free here is the minimum number of blocks that we have to > > > keep on the AGFL to handle bnobt/cntbt/rmapbt btree expansions, right? > > > And @max_free is the same, but assuming that they all increase one level > > > in height, right? So we're adding to @reserved the quantity of fsblocks > > > needed to handle adding that new layer and then making the "Can this AG > > > handle this much allocation?" decision? > > > > > > > Yep. > > > > > /me wonders if they should be called min_agfl and max_agfl, > > > respectively, but that only makes sense if the answers to the above are > > > all 'yes'. > > > > > > > Do you mean within this function, or across the board? IIRC here I was > > generally just trying to keep things consistent wrt naming (i.e. I found > > the 'need' naming here annoyingly confusing) across the various function > > calls, but I'm not opposed to just renaming them all to min/max_agfl or > > whatever.. > > No, just here in this function. > Hi Carlos, Any chance you want to just fold in the diff below into this patch 4? If not, let me know and I'll respin a v5 of the series. Thanks! Brian diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c index 86f74a2dfa10..ae221a158764 100644 --- a/fs/xfs/libxfs/xfs_alloc.c +++ b/fs/xfs/libxfs/xfs_alloc.c @@ -2397,7 +2397,7 @@ xfs_alloc_compute_maxlevels( } /* - * Find the length of the longest extent in an AG. The @min_free and @max_free + * Find the length of the longest extent in an AG. The @min_agfl and @max_agfl * parameters specify how much space we're going to need for the AGFL and the * @reserved parameter tells us how many blocks in this AG are reserved for * other callers. @@ -2405,8 +2405,8 @@ xfs_alloc_compute_maxlevels( xfs_extlen_t xfs_alloc_longest_free_extent( struct xfs_perag *pag, - xfs_extlen_t min_free, - xfs_extlen_t max_free, + xfs_extlen_t min_agfl, + xfs_extlen_t max_agfl, xfs_extlen_t reserved) { xfs_extlen_t delta = 0; @@ -2415,8 +2415,8 @@ xfs_alloc_longest_free_extent( * If the AGFL needs a recharge, subtract that from the longest extent * because AGFL refill happens before the alloc. */ - if (min_free > pag->pagf_flcount) - delta = min_free - pag->pagf_flcount; + if (min_agfl > pag->pagf_flcount) + delta = min_agfl - pag->pagf_flcount; /* * Extra AGFL blocks beyond the min are reserved by ->minleft during @@ -2425,8 +2425,8 @@ xfs_alloc_longest_free_extent( * the longest extent. If not, deduct the necessary blocks from the * longest extent. */ - if (max_free > min_free) - reserved += max_free - min_free; + if (max_agfl > min_agfl) + reserved += max_agfl - min_agfl; if (pag->pagf_freeblks - pag->pagf_longest < reserved) delta += reserved - (pag->pagf_freeblks - pag->pagf_longest); ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-09-25 18:48 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-23 16:15 [PATCH v4 0/4] xfs: fix a couple sparse chunk alloc problems Brian Foster 2026-09-23 16:15 ` [PATCH v4 1/4] xfs: set minleft correctly for sparse chunk errortag allocation Brian Foster 2026-09-23 16:15 ` [PATCH v4 2/4] xfs: support additional levels in the agfl minimum calculation Brian Foster 2026-09-23 16:15 ` [PATCH v4 3/4] xfs: calculate AGFL max to support multiple-alloc transactions Brian Foster 2026-09-24 0:38 ` Darrick J. Wong 2026-09-23 16:15 ` [PATCH v4 4/4] xfs: incorporate increased AGFL min requirement for minleft allocs Brian Foster 2026-09-24 1:15 ` Darrick J. Wong 2026-09-24 13:14 ` Brian Foster 2026-09-24 18:56 ` Darrick J. Wong 2026-09-25 18:48 ` Brian Foster
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox