xfs
[Top] [All Lists]

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

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 1/4 v2] xfs: add online discard support
From: Alex Elder <aelder@xxxxxxx>
Date: Fri, 20 May 2011 10:42:23 -0500
Cc: <xfs@xxxxxxxxxxx>
In-reply-to: <20110520134531.GA30013@xxxxxxxxxxxxx>
References: <20110504185513.136746538@xxxxxxxxxxxxxxxxxxxxxx> <20110504190011.156999943@xxxxxxxxxxxxxxxxxxxxxx> <20110520134531.GA30013@xxxxxxxxxxxxx>
Reply-to: <aelder@xxxxxxx>
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@xxxxxx>

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


<Prev in Thread] Current Thread [Next in Thread>