Re: [PATCH 1/4 v2] xfs: add online discard support

Alex Elder
2011-05-20T15:42:27Z

On Fri, 2011-05-20 at 09:45 -0400, Christoph Hellwig wrote:
> Now that we have reliably tracking of deleted extents in a transaction
> we can easily implement "online" discard support which calls
> blkdev_issue_discard once a transaction commits.
>
> The actual discard is a two stage operation as we first have to mark
> the busy extent as not available for reuse before we can start the
> actual discard. Note that we don't bother supporting discard for
> the non-delaylog mode.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good.

Given Lukas' note about EOPNOTSUPP getting reported only
if 100% of the underlying storage does not (at the moment)
support discard:
- You might as well drop out of the loop in xfs_discard_extents()
once EOPNOTSUPP gets reported for any extent.
- It might be nice to report something the first time that
happens on a particular mount point, record that fact,
then report again if it begins to *not* happen at some
point thereafter (due to the underlying support being
changed via dmsetup while still mounted).

But I'm not going to wait for that. I'll take in this
patch as-is and if you care to act on those suggestions
will rely on you to do so by sending a new patch.

Reviewed-by: Alex Elder <aelder@sgi.com>