Re: xfs: add FITRIM support

[Christoph Hellwig <hch@infradead.org>]

On Thu, Dec 23, 2010 at 12:44:09PM +1100, Dave Chinner wrote:
> Hmmmm - if we are given a range to trim, wouldn't we do better to
> walk the by-bno btree instead?  i.e, we have two different cases
> here - trim an entire AG, and trim part of an AG given by {start, end}. 
> 
> We only need these range checks on the AGs that are only partially
> trimmed, and it would seem more efficient to me to walk the by-bno
> tree for those rather than walk the by-size tree trying to find
> range matches.

It might be, but I'm not sure it's really worth the complexity.  I can't
really find any good use case for a partially trim anyway.

Ccing Lukas to figure out what his intent with this was.

> Hmmm - so we hold the agf locked for the entire trim? That's a bit
> ugly. Given this is best effort, we could avoid this by changing it
> to something like:
> 
> 	longest = 0;
> 	do {
> 		lock agf
> 		force log
> 		if (!longest)
> 			longest = agf->longest
> 		init cursor
> 		do {
> 			xfs_alloc_lookup_le(longest)
> 			alloc_get_rec(&fbno, &flen)
> 			check flen
> 			busy search
> 			discard
> 			decrement cursor
> 		} while (flen == longest)
> 		destroy cursor
> 		unlock agf
> 		longest = flen;
> 	} while(1)

This doesn't seem overly efficient.  Unless we have lots of extents
with same size we keep having to allocate new cursors all the time.

I'm not too worried about busy systems - FITRIM is explicitly called and
we should expect admins not to call it during the most busy time of the
day.  And even in it's current form it's already much better than
wiper.sh in that respect.

I think adding a periodical break using a modified scheme is fine, but
I'd really like to get the code out into some more testers hands for
now.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs