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

[David Woodhouse <dwmw2@infradead.org>]

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.

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?

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation