From: Jeff Moyer <jmoyer@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: Tejun Heo <tj@kernle.org>, Jens Axboe <jaxboe@fusionio.com>
Subject: [patch] blk-flush: fix flush policy calculation
Date: Mon, 01 Aug 2011 16:32:17 -0400 [thread overview]
Message-ID: <x49d3go98ku.fsf@segfault.boston.devel.redhat.com> (raw)
Hi,
Reading through the code in blk-flush.c, it appears that there is an
oversight in the policy returned from blk_flush_policy:
if (fflags & REQ_FLUSH) {
if (rq->cmd_flags & REQ_FLUSH)
policy |= REQ_FSEQ_PREFLUSH;
if (blk_rq_sectors(rq))
policy |= REQ_FSEQ_DATA;
if (!(fflags & REQ_FUA) && (rq->cmd_flags & REQ_FUA))
policy |= REQ_FSEQ_POSTFLUSH;
}
return policy;
This means that REQ_FSEQ_DATA can only be set if the queue flush_flags
include FLUSH and/or FUA. However, the short-circuit for not issuing
flushes when the device doesn't need/support them depends on
REQ_FSEQ_DATA being set while the other two bits are clear:
/*
* If there's data but flush is not necessary, the request can be
* processed directly without going through flush machinery. Queue
* for normal execution.
*/
if ((policy & REQ_FSEQ_DATA) &&
!(policy & (REQ_FSEQ_PREFLUSH | REQ_FSEQ_POSTFLUSH))) {
list_add_tail(&rq->queuelist, &q->queue_head);
return;
}
Given the code as it stands, I don't think the body of this if statement
will ever be executed. I've attached a fix for this below. It seems
like this could be both a performance and a correctness issue, though
I've not run into any problems I can directly attribute to this (perhaps
due to file systems not issuing flushes when support is not advertised?).
Comments are appreciated.
Cheers,
Jeff
Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
diff --git a/block/blk-flush.c b/block/blk-flush.c
index bb21e4c..3a06118 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -95,11 +95,11 @@ static unsigned int blk_flush_policy(unsigned int fflags, struct request *rq)
{
unsigned int policy = 0;
+ if (blk_rq_sectors(rq))
+ policy |= REQ_FSEQ_DATA;
if (fflags & REQ_FLUSH) {
if (rq->cmd_flags & REQ_FLUSH)
policy |= REQ_FSEQ_PREFLUSH;
- if (blk_rq_sectors(rq))
- policy |= REQ_FSEQ_DATA;
if (!(fflags & REQ_FUA) && (rq->cmd_flags & REQ_FUA))
policy |= REQ_FSEQ_POSTFLUSH;
}
next reply other threads:[~2011-08-01 20:32 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-01 20:32 Jeff Moyer [this message]
2011-08-02 1:20 ` Shaohua Li
2011-08-02 15:28 ` Jeff Moyer
2011-08-02 17:39 ` Jeff Moyer
2011-08-02 18:17 ` Vivek Goyal
2011-08-02 18:31 ` Jeff Moyer
2011-08-02 18:41 ` Vivek Goyal
2011-08-02 19:46 ` Jeff Moyer
2011-08-03 1:19 ` Shaohua Li
2011-08-09 17:13 ` Vivek Goyal
2011-08-09 17:29 ` Jeff Moyer
2011-08-02 18:40 ` Mike Snitzer
2011-08-04 10:20 ` Tejun Heo
2011-08-08 17:31 ` Jeff Moyer
2011-08-04 10:16 ` Tejun Heo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=x49d3go98ku.fsf@segfault.boston.devel.redhat.com \
--to=jmoyer@redhat.com \
--cc=jaxboe@fusionio.com \
--cc=linux-kernel@vger.kernel.org \
--cc=tj@kernle.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®