lkml.org 
[lkml]   [2011]   [Aug]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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


\
 
 \ /
  Last update: 2011-08-09 17:41    [W:0.066 / U:0.120 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site