Re: [PATCH 1/1] xfs: fix severe performance problems when fstrimming a subset of an AG

[Dave Chinner <david@fromorbit.com>]

On Fri, Mar 29, 2024 at 02:35:20PM -0700, Darrick J. Wong wrote:
> On Wed, Mar 27, 2024 at 04:35:12AM -0700, Christoph Hellwig wrote:
> > On Tue, Mar 26, 2024 at 07:07:58PM -0700, Darrick J. Wong wrote:
> > > periodically to allow other threads to do work.  This implementation
> > > avoids the worst problems of the original code, though it lacks the
> > > desirable attribute of freeing the biggest chunks first.
> > 
> > Do we really care much about freeing larger area first?  I don't think
> > it really matters for FITRIM at all.
> > 
> > In other words, I suspect we're better off with only the by-bno
> > implementation.
> 
> Welll... you could argue that if the underlying "thin provisioning" is
> actually just an xfs file, that punching tons of tiny holes in that file
> could increase the height of the bmbt.  In that case, you'd want to
> reduce the chances of the punch failing with ENOSPC by punching out
> larger ranges to free up more blocks.

That's true, but it's a consideration for dm-thinp, too. I've always
considered FITRIM as an optimisation for dm-thinp systems (rather
than a hardware device optimisation), and so "large extents first"
make a whole lot more sense from that perspective.

That said, there's another reason for by-size searches being the
default behaviour: FITRIM has a minimum length control parameter.
Hence for fragmented filesystems where there are few extents larger
than the minimum length, we needed to avoid doing an exhaustive
search of the by-bno tree to find extents longer than the minimum
length....

-Dave.
-- 
Dave Chinner
david@fromorbit.com