Re: [External] : Re: [PATCH v2 3/3] xfs: incorporate increased AGFL min requirement for minleft allocs

On 8/14/26 8:57 AM, Brian Foster wrote:
> On Fri, Aug 14, 2026 at 09:22:39AM -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, create a
>> variant of the AGFL min free calculation for minleft allocations
>> that incorporates an additional allocbt level increase.
>>
>> We do not add the additional blocks directly to min_free because
>> this would lead to spurious AGFL block allocations and frees in the
>> common case (i.e. no btree splits). Instead, add the surplus block
>> requirement to the minleft value used to select the AG. This ensures
>> the AG has enough blocks for the caller's minleft value plus the
>> worst case increase in the AGFL. In the example above, the initial
>> calculation now evaluates to 3 blocks available instead of 7 and the
>> sparse inode allocation fails gracefully with -ENOSPC.
>>
>> Reported-by: Matt Fleming <[email protected]>
>> Assisted-by: LLM
>> Signed-off-by: Brian Foster <[email protected]>
>> ---
>>   fs/xfs/libxfs/xfs_alloc.c | 29 ++++++++++++++++++++++++++++-
>>   fs/xfs/libxfs/xfs_alloc.h |  2 ++
>>   fs/xfs/libxfs/xfs_bmap.c  |  2 +-
>>   3 files changed, 31 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
>> index dbb85fb6314b..74c5b587c87b 100644
>> --- a/fs/xfs/libxfs/xfs_alloc.c
>> +++ b/fs/xfs/libxfs/xfs_alloc.c
>> @@ -2500,6 +2500,20 @@ xfs_alloc_min_freelist(
>>   	return __xfs_alloc_min_freelist(mp, pag, 0);
>>   }
>>   
>> +/*
>> + * Return the minimum freelist requirement considering a potential allocbt split
>> + * from the current allocation. Use this when computing longest free extent for
>> + * allocations with minleft set to ensure that the available extent length
>> + * accounts for the subsequent allocation's increased AGFL requirement.
>> + */
>> +unsigned int
>> +xfs_alloc_min_freelist_minleft(
>> +	struct xfs_mount	*mp,
>> +	struct xfs_perag	*pag)
>> +{
>> +	return __xfs_alloc_min_freelist(mp, pag, 1);
>> +}
>> +
>>   /*
>>    * 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
>> @@ -2517,6 +2531,7 @@ 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;
>> @@ -2533,10 +2548,22 @@ xfs_alloc_space_available(
>>   	 * Do we have enough free space remaining for the allocation? Don't
>>   	 * account extra agfl blocks because we are about to defer free them,
>>   	 * making them unavailable until the current transaction commits.
>> +	 *
>> +	 * If minleft is set, this allocation might cause an allocbt split that
>> +	 * increases the AGFL minimum for the next allocation in the
>> +	 * transaction. Reserve that space from the available block count
>> +	 * (without prematurely growing the AGFL) to prevent the subsequent
>> +	 * allocation from failing due to an increased min_free requirement.
>>   	 */
>> +	minleft = args->minleft;
>> +	if (minleft) {
>> +		minleft += xfs_alloc_min_freelist_minleft(args->mp, pag) -
>> +					min_free;
>> +	}
>> +
>>   	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;
>>   
>> diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
>> index 50ef79a1ed41..026b61a63994 100644
>> --- a/fs/xfs/libxfs/xfs_alloc.h
>> +++ b/fs/xfs/libxfs/xfs_alloc.h
>> @@ -73,6 +73,8 @@ 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);
>> +unsigned int xfs_alloc_min_freelist_minleft(struct xfs_mount *mp,
>> +		struct xfs_perag *pag);
>>   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 d64defeda645..f396df864cf4 100644
>> --- a/fs/xfs/libxfs/xfs_bmap.c
>> +++ b/fs/xfs/libxfs/xfs_bmap.c
>> @@ -3160,7 +3160,7 @@ xfs_bmap_longest_free_extent(
>>   	}
>>   
> 
> I intended to add a comment here but lost track. I.e., something like:
> 
>          /*
>           * Use the minleft freelist helper because minleft can be set for bmbt
>           * allocs. If we don't factor it in here, the alloc can be sized
>           * incorrectly and fail.
>           */
> 
> I'll wait for any further feedback before reposting.
> 
> Brian
> 
>>   	longest = xfs_alloc_longest_free_extent(pag,
>> -				xfs_alloc_min_freelist(pag_mount(pag), pag),
>> +				xfs_alloc_min_freelist_minleft(pag_mount(pag), pag),
>>   				xfs_ag_resv_needed(pag, XFS_AG_RESV_NONE));
>>   	if (*blen < longest)
>>   		*blen = longest;
>> -- 
>> 2.55.0
>>
>>


thank you for the comment clarification.
I was confused by that change.

Could you CC stable. We saw a version of this problem in Linux 5.15