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

[David Woodhouse <dwmw2@infradead.org>]

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).

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?

I was actually tempted to leave REQ_TYPE_DISCARD out of the original
patch 1/5 completely, and add it only as part of the FTL patch. I don't
think it's something for the core code to be caring about -- let the
flag in rq->cmd_flags do that.

> Until we have overlap detection, I think we should make the discard
> request a explicit barrier. Otherwise we could have problems with a
> discard being passed by a write request and such.

I think that making it an explicit barrier is a reasonable thing to do
for now. We can work on merges as a next step.

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