Re: [PATCH 1/5] [BLOCK] Add 'discard' request handling

[Jens Axboe <jens.axboe@oracle.com>]

On Fri, Aug 08 2008, David Woodhouse wrote:
> On Fri, 2008-08-08 at 14:13 +0200, Jens Axboe wrote:
> > On Fri, Aug 08 2008, David Woodhouse wrote:
> > > On Fri, 2008-08-08 at 13:44 +0200, Jens Axboe wrote:
> > > > 
> > > > Sigh indeed, ->issue_flush_fn() was the actual issuer, not the preparer.
> > > > Let me send a new diff. This adds the ->prepare_discard_fn() to do the
> > > > transformation, and also extends blkdev_issue_flush() to return error if
> > > > the IO was never queued because of some device in the stack not
> > > > supporting it.
> > > 
> > > I think we still want the DISCARD flag in rq->cmd_flags. We can't rely
> > > on rq->cmd_type == REQ_TYPE_DISCARD because that may well be changed to
> > > something else (like REQ_TYPE_BLOCK_PC).
> > 
> > It's getting a bit nasty at this point. Discard requests should be
> > treated as file system requests, if you want merging and sorting on
> > them. 
> 
> Except that they _can't_ be, because file system requests are normal
> reads and writes, and have just a single bit to indicate the direction.
> 
> Although I suppose we could declare that a discard request is just like
> a write but with no buffer attached? I wasn't brave enough to attempt
> that though, because I think it'll get painful and lead to oopses in
> unexpected places.

It was a proposition with two options - one of them being that we treat
them as file system requests, which is a pre-requisite if you want to
have merging supported. If merging isn'ta must, then I would advocate
option 2 as out lined in the previous mail.

> So they're _not_ file system requests; they're different -- and they can
> be marked by the appropriate bit in rq->cmd_flags, just like a flush
> request is. There's no reason that has to prevent merging. The elevator
> needs to learn about discard requests anyway, and it doesn't really
> matter _how_ it recognises them as such. Surely?

Sure, I'm well aware that they are different in nature, I'm talking
about how you treat them in the block layer. You can't have it both
ways - either you use the fs submit path and get the merging, or you
don't. Adding merging on the side for these special requests is not an
option, that would be silly. You would dual path that basically, plus it
would exclude eventually doing overlap detection with real fs requests.

-- 
Jens Axboe