Re: [PATCH v2 1/3] xfs: set minleft correctly for sparse chunk errortag allocation
Brian Foster <[email protected]>
| Newsgroups | org.kernel.vger.linux-xfs |
|---|---|
| Message-ID | <aoYN-HbYzFXNfXXY@bfoster> |
On Tue, Aug 18, 2026 at 08:25:37AM +1000, Dave Chinner wrote:
> On Fri, Aug 14, 2026 at 09:22:37AM -0400, Brian Foster wrote:
> > 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 <[email protected]>
> > Reviewed-by: Mark Tinguely <[email protected]>
> > ---
> > 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 ffcdd1f691fd..633b2d6e42c5 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;
>
> As an extra question: is that reservation even correct? We can split
> both the inobt and the finobt on insert, and this only reserves
> space for a inobt split....
>
I think I mentioned in one of the earlier postings that finobt is
covered by the metadata perag res, so minleft isn't necessary. One of
the longer term ideas worth considering is to perhaps revisit why inobt
blocks aren't reserved and see if it's worth doing that.
> Secondly, why always reserve space for a max level split? The amount
> we need is depedent on the current level of the btree, and for all
> other btree types our reservations are based on the current level.
> i.e. we know the math needed to make this dependent on current btree
> levels, and we know that we are inserting into multiple btrees of
> different heights here, so maybe we should fix this reservation
> whilst we are here, too?
>
I thought the reason we do this historically is because we haven't
selected an AG at this point, but that could be wrong. The current code
looks like it has the AGI buffer at this point at least, so technically
I think we could tailor minleft to the current AG if we wanted to.
That said, I'm skeptical of the value for one.. how many blocks would we
expect to make available in the best case? Enough for an extra chunk or
two? Second, I really don't want to turn this into a squeeze more blocks
out of -ENOSPC exercise so if we do want to dig into something like
this, I'd prefer to defer it to a follow on series.
Brian
> -Dave.
> --
> Dave Chinner
> [email protected]
>