Re: [PATCH v4 4/4] xfs: incorporate increased AGFL min requirement for minleft allocs

[Brian Foster <bfoster@redhat.com>]

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);