[RFC]block: don't mark flush request as SOFTBARRIER

[RFC]block: don't mark flush request as SOFTBARRIER

[Shaohua Li]

I'm wondering why flush request should be marked as SOFTBARRIER. Current flush
sequence guarantees the correctness of flush even the request is reordered by
ioscheduler.

Signed-off-by: Shaohua Li <shaohua.li@intel.com>

diff --git a/block/elevator.c b/block/elevator.c
index a3b64bc..e589b76 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -715,7 +715,6 @@ void __elv_add_request(struct request_queue *q, struct request *rq, int where)
 		break;
 
 	case ELEVATOR_INSERT_FLUSH:
-		rq->cmd_flags |= REQ_SOFTBARRIER;
 		blk_insert_flush(rq);
 		break;
 	default:

Re: [RFC]block: don't mark flush request as SOFTBARRIER

[Tejun Heo]

Hello,

On Thu, Sep 08, 2011 at 09:29:54AM +0800, Shaohua Li wrote:
> I'm wondering why flush request should be marked as
> SOFTBARRIER. Current flush sequence guarantees the correctness of
> flush even the request is reordered by ioscheduler.

The primary reason would be because when those flush data requests are
put on the dispatch queue, it doesn't go through elv_dispatch_sort().
SOFTBARRIERs function as dispatch sort boundaries and queueing flush
data request at the head w/o SOFTBARRIER would confuse dispatch
sorting, and we want to put it at the head of dispatch queue to
decrease latency for flush sequence.

Thanks.

-- 
tejun

Re: [RFC]block: don't mark flush request as SOFTBARRIER

[Shaohua Li]

On Sun, 2011-09-11 at 10:51 +0800, Tejun Heo wrote:
> Hello,
> 
> On Thu, Sep 08, 2011 at 09:29:54AM +0800, Shaohua Li wrote:
> > I'm wondering why flush request should be marked as
> > SOFTBARRIER. Current flush sequence guarantees the correctness of
> > flush even the request is reordered by ioscheduler.
> 
> The primary reason would be because when those flush data requests are
> put on the dispatch queue, it doesn't go through elv_dispatch_sort().
> SOFTBARRIERs function as dispatch sort boundaries and queueing flush
> data request at the head w/o SOFTBARRIER would confuse dispatch
> sorting, and we want to put it at the head of dispatch queue to
> decrease latency for flush sequence.
we do the flush first and then dispatch the data. The flush is already
delayed a lot, so if latency is a problem, we already saw it. Why not
just remove SOFTBARRIER and use elv_dispatch_sort() for flush data so
drive can better arrange requests?

Thanks,
Shaohua

Re: [RFC]block: don't mark flush request as SOFTBARRIER

[Tejun Heo]

Hello, Shaohua.

On Tue, Sep 13, 2011 at 09:04:01AM +0800, Shaohua Li wrote:
> we do the flush first and then dispatch the data. The flush is
> already delayed a lot, so if latency is a problem, we already saw
> it.

I don't necessarily agree with the above.  We don't induce any extra
latency for flushes which don't overlap.

> Why not just remove SOFTBARRIER and use elv_dispatch_sort() for
> flush data so drive can better arrange requests?

Maybe, I don't know.  Elevator sorting on writes is likely to be much
less important to begin with.  Combined with the fact that sorting
flush data would require more writes to be flushed by the following
flush, I don't think it would be clear which way would be better.  If
it can be shown that sorting flush data is better, why not?

Thanks.

-- 
tejun

Re: [RFC]block: don't mark flush request as SOFTBARRIER

[Shaohua Li]

On Tue, 2011-09-13 at 15:46 +0800, Tejun Heo wrote:
> Hello, Shaohua.
> 
> On Tue, Sep 13, 2011 at 09:04:01AM +0800, Shaohua Li wrote:
> > we do the flush first and then dispatch the data. The flush is
> > already delayed a lot, so if latency is a problem, we already saw
> > it.
> 
> I don't necessarily agree with the above.  We don't induce any extra
> latency for flushes which don't overlap.
> 
> > Why not just remove SOFTBARRIER and use elv_dispatch_sort() for
> > flush data so drive can better arrange requests?
> 
> Maybe, I don't know.  Elevator sorting on writes is likely to be much
> less important to begin with.  Combined with the fact that sorting
> flush data would require more writes to be flushed by the following
> flush, I don't think it would be clear which way would be better.  If
> it can be shown that sorting flush data is better, why not?
Just found request_queue->queue_head list usually has only one entry,
because drive only takes one extra request from elevator. so either
dispatch_sort() or dispatch_add_tail() has no difference.

Thanks,
Shaohua