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

[Mark Tinguely <mark.tinguely@oracle.com>]

On 9/2/26 12:40 PM, 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>
> ---
>   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 1377c65d694e..2ed269080bbb 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);




The only concern that I have is we have to assume the caller calculated the max_free.
But ....

Reviewed-by: Mark Tinguely <mark.tinguely@oracle.com>