Messages in this thread | | | Date | Tue, 9 Aug 2011 17:38:26 +0200 | From | Tejun Heo <> | Subject | Re: [patch] block: properly handle flush/fua requests in blk_insert_cloned_request |
| |
Hello,
On Tue, Aug 09, 2011 at 11:05:34AM -0400, Jeff Moyer wrote: > @@ -1708,6 +1710,21 @@ int blk_insert_cloned_request(struct request_queue *q, struct request *rq) > should_fail_request(&rq->rq_disk->part0, blk_rq_bytes(rq))) > return -EIO; > > + /* > + * Check the cmd_flags against the flush flags of the underlying > + * request_queue and resolve any differences. > + */ > + if (rq->cmd_flags & (REQ_FLUSH|REQ_FUA)) { > + if (!(q->flush_flags & REQ_FLUSH)) > + rq->cmd_flags &= ~REQ_FLUSH; > + if (!(q->flush_flags & REQ_FUA)) > + rq->cmd_flags &= ~REQ_FUA; > + if (!(rq->cmd_flags & REQ_FLUSH) && !blk_rq_sectors(rq)) { > + blk_end_bidi_request(rq, 0, 0, 0); > + return 0; > + } > + } > +
I'm a bit confused. We still need ELEVATOR_INSERT_FLUSH fix for insertion paths, right? Or is blk_insert_cloned_request() supposed to used only by request based dm which lives under the elevator? If so, it would be great to make that explicit in the comment. Maybe just renaming it to blk_insert_dm_cloned_request() would be better as it wouldn't be safe for other cases anyway.
> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h > index 6395692..4fe753f 100644 > --- a/include/linux/blk_types.h > +++ b/include/linux/blk_types.h > @@ -168,7 +168,7 @@ enum rq_flag_bits { > #define REQ_COMMON_MASK \ > (REQ_WRITE | REQ_FAILFAST_MASK | REQ_SYNC | REQ_META | REQ_DISCARD | \ > REQ_NOIDLE | REQ_FLUSH | REQ_FUA | REQ_SECURE) > -#define REQ_CLONE_MASK REQ_COMMON_MASK > +#define REQ_CLONE_MASK (REQ_COMMON_MASK | REQ_FLUSH_SEQ)
Given the weirdness, I think it deserves fat comment on why REQ_FLUSH_SEQ is necessary.
Thanks.
-- tejun
| |