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    [W:0.020 / U:34.396 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site