lkml.org 
[lkml]   [2011]   [Aug]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[patch] blk-flush: fix flush policy calculation
Date
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;
}

\
 
 \ /
  Last update: 2011-08-01 22:35    [from the cache]
©2003-2011 Jasper Spaans