All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: Dave Chinner <dgc@kernel.org>
Cc: linux-xfs@vger.kernel.org, Matt Fleming <matt@readmodwrite.com>
Subject: Re: [PATCH v2 1/3] xfs: set minleft correctly for sparse chunk errortag allocation
Date: Wed, 19 Aug 2026 16:11:36 -0400	[thread overview]
Message-ID: <aoYN-HbYzFXNfXXY@bfoster> (raw)
In-Reply-To: <aoOKYRBTK7ocCBrY@dread>

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 <bfoster@redhat.com>
> > Reviewed-by: Mark Tinguely <mark.tinguely@oracle.com>
> > ---
> >  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
> dgc@kernel.org
> 


  reply	other threads:[~2026-08-19 20:11 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-14 13:22 [PATCH v2 0/3] xfs: fix a couple sparse chunk alloc problems Brian Foster
2026-08-14 13:22 ` [PATCH v2 1/3] xfs: set minleft correctly for sparse chunk errortag allocation Brian Foster
2026-08-14 18:54   ` Darrick J. Wong
2026-08-17 22:25   ` Dave Chinner
2026-08-19 20:11     ` Brian Foster [this message]
2026-08-14 13:22 ` [PATCH v2 2/3] xfs: support additional levels in the agfl minimum calculation Brian Foster
2026-08-14 18:58   ` Darrick J. Wong
2026-08-14 13:22 ` [PATCH v2 3/3] xfs: incorporate increased AGFL min requirement for minleft allocs Brian Foster
2026-08-14 13:57   ` Brian Foster
2026-08-14 14:15     ` [External] : " Mark Tinguely
2026-08-14 14:47       ` Brian Foster
2026-08-14 15:18         ` Mark Tinguely
2026-08-14 16:24           ` Brian Foster
2026-08-14 23:57   ` Darrick J. Wong
2026-08-17 13:13     ` Brian Foster
2026-08-17 22:55   ` Dave Chinner
2026-08-19 20:13     ` Brian Foster

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=aoYN-HbYzFXNfXXY@bfoster \
    --to=bfoster@redhat.com \
    --cc=dgc@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.