Message-ID: <ZrLA/46GudqcYx4K@dread.disaster.area>
Date: Wed, 7 Aug 2024 10:34:07 +1000
From: Dave Chinner <david@...morbit.com>
To: "Darrick J. Wong" <djwong@...nel.org>
Cc: John Garry <john.g.garry@...cle.com>, chandan.babu@...cle.com,
	dchinner@...hat.com, hch@....de, viro@...iv.linux.org.uk,
	brauner@...nel.org, jack@...e.cz, linux-xfs@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	catherine.hoang@...cle.com, martin.petersen@...cle.com
Subject: Re: [PATCH v3 03/14] xfs: simplify extent allocation alignment

On Tue, Aug 06, 2024 at 05:23:58PM -0700, Darrick J. Wong wrote:
> On Wed, Aug 07, 2024 at 09:52:06AM +1000, Dave Chinner wrote:
> > On Tue, Aug 06, 2024 at 11:56:51AM -0700, Darrick J. Wong wrote:
> > > On Thu, Aug 01, 2024 at 04:30:46PM +0000, John Garry wrote:
> > > > From: Dave Chinner <dchinner@...hat.com>
> > > > 
> > > > We currently align extent allocation to stripe unit or stripe width.
> > > > That is specified by an external parameter to the allocation code,
> > > > which then manipulates the xfs_alloc_args alignment configuration in
> > > > interesting ways.
> > > > 
> > > > The args->alignment field specifies extent start alignment, but
> > > > because we may be attempting non-aligned allocation first there are
> > > > also slop variables that allow for those allocation attempts to
> > > > account for aligned allocation if they fail.
> > > > 
> > > > This gets much more complex as we introduce forced allocation
> > > > alignment, where extent size hints are used to generate the extent
> > > > start alignment. extent size hints currently only affect extent
> > > > lengths (via args->prod and args->mod) and so with this change we
> > > > will have two different start alignment conditions.
> > > > 
> > > > Avoid this complexity by always using args->alignment to indicate
> > > > extent start alignment, and always using args->prod/mod to indicate
> > > > extent length adjustment.
> > > > 
> > > > Signed-off-by: Dave Chinner <dchinner@...hat.com>
> > > > [jpg: fixup alignslop references in xfs_trace.h and xfs_ialloc.c]
> > > > Signed-off-by: John Garry <john.g.garry@...cle.com>
> > > 
> > > Going back to the 6/21 posting[1], what were the answers to the
> > > questions I posted?  Did I correctly figure out what alignslop refers
> > > to?
> > 
> > Hard to say.
> > 
> > alignslop is basically an temporary accounting mechanism used to
> > prevent filesystem shutdowns when the AG is near ENOSPC and exact
> > BNO allocation is attempted and fails because there isn't an exact
> > free space available. This exact bno allocation attempt can dirty
> > the AGFL, and before we dirty the transaction *we must guarantee the
> > allocation will succeed*. If the allocation fails after we've
> > started modifying metadata (for whatever reason) we will cancel a
> > dirty transaction and shut down the filesystem.
> > 
> > Hence the first allocation done from the xfs_bmap_btalloc() context
> > needs to account for every block the specific allocation and all the
> > failure fallback attempts *may require* before it starts modifying
> > metadata. The contiguous exact bno allocation case isn't an aligned
> > allocation, but it will be followed by an aligned allocation attempt
> > if it fails and so it must take into account the space requirements
> > of aligned allocation even though it is not an aligned allocation
> > itself.
> > 
> > args->alignslop allows xfs_alloc_space_available() to take this
> > space requirement into account for any allocation that has lesser
> > alignment requirements than any subsequent allocation attempt that
> > may follow if this specific allocation attempt fails.
> > 
> > IOWs, args->alignslop is similar to args->minleft and args->total in
> > purpose, but it only affects the accounting for this specific
> > allocation attempt rather than defining the amount of space
> > that needs to remain available at the successful completion of this
> > allocation for future allocations within this transaction context.
> 
> Oh, okay.  IOWs, "slop" is a means for alloc callers to communicate that
> they need to perform an aligned allocation, but that right now they want
> to try an exact allocation (with looser alignment) so that they might
> get better locality.  However, they don't want the exact allocation scan
> to commit to a certain AG unless the aligned allocation will be able to
> find an aligned space *somewhere* in that AG if the exact scan fails.
> For any other kind of allocation situation, slop should be zero.
> 
> If the above statement is correct,

It is, except for a small nit: alignslop isn't exact bno allocation
specific, it allows any sort of unaligned -> fail -> aligned
allocation fallback pattern to select an AG where the fallback
aligned allocation will have space to succeed.

> could we paste that into the
> definition of struct xfs_alloc_arg so that I don't have to recollect all
> this the next time I see something involving alignslop?

Sure.

> After which
> I'm ok saying:
> 
> Reviewed-by: Darrick J. Wong <djwong@...nel.org>

Thanks!

-Dave.
-- 
Dave Chinner
david@...morbit.com

Powered by blists - more mailing lists