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@xxxxxxxxxxxxx
