Re: xfs: add FITRIM support

[Christoph Hellwig <hch@infradead.org>]

On Wed, Dec 22, 2010 at 03:41:13PM -0600, Alex Elder wrote:
> > +	error = xfs_alloc_read_agf(mp, NULL, agno,
> > +				   XFS_ALLOC_FLAG_TRYLOCK, &agbp);
> > +	if (error || !agbp) {
> > +		if (error == EAGAIN)
> > +			error = 0;
> 
> EAGAIN is ignored because it's an advisory interface, right?
> How hard are we expected to try?  What I really mean is,
> is the benefit of FITRIM enough that we should try again
> later when we can get a buffer or lock on it?

That was the idea when I wrote this code.  But back then we called it
regularly from a kernel thread.  For FITRIM it makes more sense to just
remove the trylock.

> I don't know where (or if) FITRIM is precisely documented.
> But I question whether truncating down the start offset is
> the correct thing to do.  If the starting byte offset given
> were not block-aligned, it seems like you should not assume
> that the caller wanted the bytes below unmapped.  (This is
> a broader question, not related directly to your change.)
> 
> Similarly, on the length it is probably best to truncate
> it, because it avoids any bytes beyond the specified range
> getting unmapped.  (I.e., in my mind what you did is the
> right way to do it.)  But these interpretations are
> dependent on the specific interpretation of FITRIM...

Good question.  Adding Lukas to the Cc.  I tried to talk him into
writing a manpage to document the interface better, but that's only
been a few days before the holidays.  This is something we should
documented.  I don't quite understand the need for the range interface
anyway.

> You don't update range anywhere, so the copyout below
> is not really doing anything useful.  However I think
> it should stay, and the number of bytes actually
> trimmed should be updated and returned to the user.
> That seems to be what ext4 does (the only reference
> I found at the moment for what FITRIM is supposed
> to return).

Yes, I guess I should update the range.

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