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

Darrick J. Wong
2024-03-31T22:44:01Z

On Sun, Mar 31, 2024 at 08:15:49AM +1100, Dave Chinner wrote:
> 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....

Ooooh, that's a /very/ good point. xfs_scrub also now constructs a
histogram of free space extent lengths to see if it can reduce the
runtime of phase 8 (FITRIM) by setting minlen to something large enough
to reduce runtime by 4-5x while not missing more than a few percent of
the free space.

--D