difference. It doesn't make more flush requests merge. Avoiding
unnecessary requeue is a gain for fast devices, but my test doesn't
show.
Subject: block: hold queue if flush is running for non-queueable flush drive
Commit 53d63e6b0dfb9(block: make the flush insertion use the tail of
the dispatch list) causes about 20% regression running a sysbench fileio
workload. Let's consider the following scenario:
- flush1 is dispatched with write1 in the elevator.
- Driver dispatches write1 and requeues it.
- flush2 is issued and appended to dispatch queue after the requeued write1.
As write1 has been requeued flush2 can't be put in front of it.
- When flush1 finishes, the driver has to process write1 before flush2 even
though there's no fundamental reason flush2 can't be processed first and,
when two flushes are issued back-to-back without intervening writes, the
second one essentially becomes noop.
Without the commit, flush2 is inserted before write1, so the issue is hiden.
But the commit itself makes sense, because flush request isn't a preempt
request, there is no reason to add it to queue head.
The regression is exposed in a SATA device. In SATA, flush requests are
non-queueable. When flush request is running, normal read/write requests
can't run. If block layer dispatches such request, driver can't handle it
and requeue it. Tejun suggested we can hold the queue when flush is running.
This can avoid unnecessary requeue.
And also this can improve performance and solve the regression. In above
scenario, when flush1 is running, queue is hold, so write1 isn't dispatched.
flush2 will be the only request in the queue. After flush1 is finished, flush2
will be dispatched soon. Since there is no write between flush1 and flush2,
flush2 essentially becomes noop.
Signed-off-by: Shaohua Li <shaohua.li@intel.com>
Acked-by: Tejun Heo <tj@kernel.org>
---
block/blk-flush.c | 19 ++++++++++++++-----
block/blk.h | 35 ++++++++++++++++++++++++++++++++++-
include/linux/blkdev.h | 1 +
3 files changed, 49 insertions(+), 6 deletions(-)
===================================================================
@@ -212,13 +212,22 @@ static void flush_end_io(struct request
}
/*
- * Moving a request silently to empty queue_head may stall the
- * queue. Kick the queue in those cases. This function is called
- * from request completion path and calling directly into
- * request_fn may confuse the driver. Always use kblockd.
+ * After flush sequencing, the following two cases may lead to
+ * queue stall.
+ *
+ * 1. Moving a request silently to empty queue_head.
+ *
+ * 2. If flush request was non-queueable, request dispatching may
+ * have been blocked while flush was in progress.
+ *
+ * Make sure queue processing is restarted by kicking the queue.
+ * As this function is called from request completion path,
+ * calling directly into request_fn may confuse the driver. Always
+ * use kblockd.
*/
- if (queued)
+ if (queued || q->flush_queue_delayed)
blk_run_queue_async(q);
+ q->flush_queue_delayed = 0;
}
/**
===================================================================
@@ -365,6 +365,7 @@ struct request_queue
*/
unsigned int flush_flags;
unsigned int flush_not_queueable:1;
+ unsigned int flush_queue_delayed:1;
unsigned int flush_pending_idx:1;
unsigned int flush_running_idx:1;
unsigned long flush_pending_since;
===================================================================
@@ -61,7 +61,40 @@ static inline struct request *__elv_next
rq = list_entry_rq(q->queue_head.next);
return rq;
}
-
+ /*
+ * Hold dispatching of regular requests if non-queueable
+ * flush is in progress; otherwise, the low level driver
+ * would keep dispatching IO requests just to requeue them
+ * until the flush finishes, which not only adds
+ * dispatching / requeueing overhead but may also
+ * significantly affect throughput when multiple flushes
+ * are issued back-to-back. Please consider the following
+ * scenario.
+ *
+ * - flush1 is dispatched with write1 in the elevator.
+ *
+ * - Driver dispatches write1 and requeues it.
+ *
+ * - flush2 is issued and appended to dispatch queue after
+ * the requeued write1. As write1 has been requeued
+ * flush2 can't be put in front of it.
+ *
+ * - When flush1 finishes, the driver has to process write1
+ * before flush2 even though there's no fundamental
+ * reason flush2 can't be processed first and, when two
+ * flushes are issued back-to-back without intervening
+ * writes, the second one essentially becomes noop.
+ *
+ * This phenomena becomes quite visible under heavy
+ * concurrent fsync workload and holding the queue while
+ * flush is in progress leads to significant throughput
+ * gain.
+ */
+ if (q->flush_pending_idx != q->flush_running_idx &&
+ !queue_flush_queueable(q)) {
+ q->flush_queue_delayed = 1;
+ return NULL;
+ }
if (!q->elevator->ops->elevator_dispatch_fn(q, 0))
return NULL;
}