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 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html