On Fri, 2008-08-08 at 14:37 +0200, Jens Axboe wrote: > 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. It would be nice to have merging and sorting on them. Mostly, they could be handled just like writes. That includes the fact that you can discard writes from the queue if they're going to be obsoleted by a subsequent write. Can we get away with representing discards as 'writes without buffers'? > > 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. We definitely want to use the fs submit path and get merging. The only reason I didn't do that from the beginning is because I wanted to keep things simple to start with. My point is that we can use the fs submit path whether we use REQ_TYPE_DISCARD or whether we use a DISCARD flag in rq->cmd_flags. That detail really shouldn't matter, surely? -- David Woodhouse Open Source Technology Centre David.Woodhouse@xxxxxxxxx Intel Corporation -- 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
