Linux XFS filesystem development
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: Matt Fleming <matt@readmodwrite.com>
Cc: linux-xfs@vger.kernel.org, Carlos Maiolino <cem@kernel.org>,
	"Darrick J . Wong" <djwong@kernel.org>,
	Dave Chinner <dchinner@redhat.com>,
	Christoph Hellwig <hch@lst.de>,
	linux-kernel@vger.kernel.org, kernel-team@cloudflare.com
Subject: Re: [BUG] xfs: sparse inode allocation can trip i != 1 after AGFL growth
Date: Wed, 29 Jul 2026 14:50:13 -0400	[thread overview]
Message-ID: <ampLZcB3CAzLPKV5@bfoster> (raw)
In-Reply-To: <al33dUXS_8Yr5vEu@matt-Precision-5490>

On Mon, Jul 20, 2026 at 11:37:25AM +0100, Matt Fleming wrote:
> On Fri, Jul 17, 2026 at 01:55:42PM -0400, Brian Foster wrote:
> > On Fri, Jul 17, 2026 at 02:04:29PM +0100, Matt Fleming wrote:
> > > 
> > > The failure sequence seems to be:
> > > 
> > > 1. AG0 has no free inodes, so inode allocation needs a new chunk.
> > > 2. Full chunk allocation cannot fit.
> > > 3. Sparse chunk allocation can fit and passes the old AGFL minimum check.
> > > 4. Removing the sparse extent from free space grows both bnobt and cntbt.
> > 
> > I assume this means we happen to alloc a sparse chunk out of the middle
> > of a free extent, causing an additional record and thus splits in both
> > trees.
>  
> Yep, exactly.
> 
> > It also looks like we set args.minleft = igeo->inobt_maxlevels in the
> > chunk alloc path, presumably with the intent to leave enough blocks
> > around for inode record insertion. I assume the above available value is
> > prior to the sparse chunk alloc. It would be interesting to see what the
> > same values/calculations are after the chunk alloc at the time of the
> > inobt block alloc that fails.
>  
> Before sparse inode allocation:
> 
>    pagf_freeblks:    2514
>    pagf_flcount:     8
>    levels:           bno/cnt/rmap = 1/1/2
>    reservation:      2505
>    minleft:          2
>    min_freelist:     8
>    AGFL credit:      min(8, 8) = 8
> 
>    available:
>      2514 + 8 - 2505 - 8 - 2 = 7
> 
>    request:
>      minlen=4 align=4 slop=0
>      alloc_len = 4 + (4 - 1) + 0 = 7
> 
> So the sparse inode allocation only just passes the allocator check.
> 
> After sparse inode allocation, at the failing inobt split allocation:
> 
>    pagf_freeblks:    2510
>    pagf_flcount:     4
>    levels:           bno/cnt/rmap = 2/2/2
>    reservation:      2505
>    minleft:          0
>    min_freelist:     12
>    AGFL credit:      min(4, 12) = 4
> 
>    available:
>      2510 + 4 - 2505 - 12 - 0 = -3
> 
>    request:
>      minlen=1
> 
> So after removing the 4-block sparse extent, bnobt/cntbt have grown,
> AGFL has dropped, min_freelist has increased from 8 to 12, and the
> later single-block inobt allocation has no available space.
> 

Ok, thanks for the additional data. So the first allocation proceeds
based on meeting the minleft and agfl requirements at the time. That
allocation causes a full split and level increase on both alloc btrees
in the AG, so as you've pointed out we consume the 4 free blocks and 4
blocks from the agfl. We turn around in the same transaction and insert
the inobt records, which attempts an alloc for the inobt that fails
because the AG no longer meets minimal allocation criteria, despite
having minleft set to 2 to ensure that some blocks remain available for
this very purpose after the first allocation.

I think the immediate issue here is that the allocbt min free
requirements change within the same (transaction + agf buf) instance.
IOW, since the btrees run a full split on the first allocation and the
agfl requirements are based on the requirements for the next full split,
we can be reasonably certain another full split will not occur in the
lifecycle of this particular trans/agf combination.

Having an LLM run through a hypothetical situation where we held the
agfl requirements constant across these two allocations suggested that
indeed, this would allow us to allocate the final available block for
record insertion.

That said, I'm not totally convinced this is the best solution. The
purpose of sparse inodes was not necessarily to facilitate allocation of
every last block in the AG, but moreso to improve the ability to
allocate inodes in AGs with high levels of free space fragmentation. I'm
a little concerned that trying to do the former here would just lead us
into other future traps by trying to extract precision out of gating
logic that isn't really designed for it.

With that in mind, the other possible way to handle this is to be more
conservative and just make sure the allocation fails gracefully up
front. There are probably multiple different ways to do that as well.
The LLM hacked up something to bump minfree within the allocator when
minleft is set. That seems to work, but I don't really like it.

Another option that seems a bit more simple/logical to me is to let the
sparse allocation attempt bump minleft by the number of blocks that the
length is reduced by when we fall into the sparse allocation path. The
idea here is just to maintain consistent enospc behavior between sparse
and non-sparse chunk allocation. That's a simple one line change, also
prevents the problem, and (IMO) is more straightforward to document and
reason about.

I want to think about that a little more and need to run more tests, but
unless something better comes along I'll try to post that soonish. BTW,
I had to make some tweaks to the reproducer test to make it work on my
fedora based test vm. Is this something you plan to post for upstream
inclusion?

Brian

--- 8< ---

diff --git a/tests/xfs/842 b/tests/xfs/842
index 9e694a56..5455998f 100755
--- a/tests/xfs/842
+++ b/tests/xfs/842
@@ -293,9 +293,9 @@ inspect_unmounted
 read -r free reserved used <<<"$(ag0_reservation_state)"
 reservation=$((reserved - used))
 outside=$((free - reservation))
-bno_level=$(agf_field 'levels[0]')
-cnt_level=$(agf_field 'levels[1]')
-rmap_level=$(agf_field 'levels[2]')
+bno_level=$(agf_field bnolevel)
+cnt_level=$(agf_field cntlevel)
+rmap_level=$(agf_field rmaplevel)
 flcount=$(agf_field flcount)
 bno_recs=$(alloc_root_field bnoroot numrecs)
 cnt_recs=$(alloc_root_field cntroot numrecs)


      reply	other threads:[~2026-07-29 18:50 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-17 13:04 [BUG] xfs: sparse inode allocation can trip i != 1 after AGFL growth Matt Fleming
2026-07-17 17:55 ` Brian Foster
2026-07-20 10:37   ` Matt Fleming
2026-07-29 18:50     ` Brian Foster [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ampLZcB3CAzLPKV5@bfoster \
    --to=bfoster@redhat.com \
    --cc=cem@kernel.org \
    --cc=dchinner@redhat.com \
    --cc=djwong@kernel.org \
    --cc=hch@lst.de \
    --cc=kernel-team@cloudflare.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=matt@readmodwrite.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox