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 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. For file system requests, drivers do the transformation inside the
driver once it receives the request. They don't change types in-flight,
they always stay the same.

So either we do the same for discard requests, or we treat them as we
would a REQ_TYPE_BLOCK_PC on insert. That means doing the transformation
first, then insert the request with elv_add_request(). That would be a
the end of the queue, so it would not either pass or get passed by file
system requests. You would not get merging in this case.

> I was thinking of REQ_TYPE_DISCARD as a special case for some block
> devices, just as REQ_TYPE_FLUSH is a special case for the ps3disk.c
> driver (nobody else uses it). In fact, perhaps both of them should be
> switched to REQ_TYPE_LINUX_BLOCK or REQ_TYPE_SPECIAL?

SPECIAL should go away, it has no definitive meaning. If we choose the
number two option above, it should just be a REQ_TYPE_LINUX_BLOCK with
the "official" cdb of REQ_LB_OP_DISCARD. It would behave the same that
approach.

-- 
Jens Axboe