[PATCH BUGFIX/IMPROVEMENT 1/6] block, bfq: always inject I/O of queues blocked by wakers

From: Paolo Valente
Date: Tue Jan 26 2021 - 06:40:43 EST


Suppose that I/O dispatch is plugged, to wait for new I/O for the
in-service bfq-queue, say bfqq. Suppose then that there is a further
bfq_queue woken by bfqq, and that this woken queue has pending I/O. A
woken queue does not steal bandwidth from bfqq, because it remains
soon without I/O if bfqq is not served. So there is virtually no risk
of loss of bandwidth for bfqq if this woken queue has I/O dispatched
while bfqq is waiting for new I/O. In contrast, this extra I/O
injection boosts throughput. This commit performs this extra
injection.

Tested-by: Jan Kara <jack@xxxxxxx>
Signed-off-by: Paolo Valente <paolo.valente@xxxxxxxxxx>
---
block/bfq-iosched.c | 32 +++++++++++++++++++++++++++-----
block/bfq-wf2q.c | 8 ++++++++
2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 445cef9c0bb9..a83149407336 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -4487,9 +4487,15 @@ static struct bfq_queue *bfq_select_queue(struct bfq_data *bfqd)
bfq_bfqq_busy(bfqq->bic->bfqq[0]) &&
bfqq->bic->bfqq[0]->next_rq ?
bfqq->bic->bfqq[0] : NULL;
+ struct bfq_queue *blocked_bfqq =
+ !hlist_empty(&bfqq->woken_list) ?
+ container_of(bfqq->woken_list.first,
+ struct bfq_queue,
+ woken_list_node)
+ : NULL;

/*
- * The next three mutually-exclusive ifs decide
+ * The next four mutually-exclusive ifs decide
* whether to try injection, and choose the queue to
* pick an I/O request from.
*
@@ -4522,7 +4528,15 @@ static struct bfq_queue *bfq_select_queue(struct bfq_data *bfqd)
* next bfqq's I/O is brought forward dramatically,
* for it is not blocked for milliseconds.
*
- * The third if checks whether bfqq is a queue for
+ * The third if checks whether there is a queue woken
+ * by bfqq, and currently with pending I/O. Such a
+ * woken queue does not steal bandwidth from bfqq,
+ * because it remains soon without I/O if bfqq is not
+ * served. So there is virtually no risk of loss of
+ * bandwidth for bfqq if this woken queue has I/O
+ * dispatched while bfqq is waiting for new I/O.
+ *
+ * The fourth if checks whether bfqq is a queue for
* which it is better to avoid injection. It is so if
* bfqq delivers more throughput when served without
* any further I/O from other queues in the middle, or
@@ -4542,11 +4556,11 @@ static struct bfq_queue *bfq_select_queue(struct bfq_data *bfqd)
* bfq_update_has_short_ttime(), it is rather likely
* that, if I/O is being plugged for bfqq and the
* waker queue has pending I/O requests that are
- * blocking bfqq's I/O, then the third alternative
+ * blocking bfqq's I/O, then the fourth alternative
* above lets the waker queue get served before the
* I/O-plugging timeout fires. So one may deem the
* second alternative superfluous. It is not, because
- * the third alternative may be way less effective in
+ * the fourth alternative may be way less effective in
* case of a synchronization. For two main
* reasons. First, throughput may be low because the
* inject limit may be too low to guarantee the same
@@ -4555,7 +4569,7 @@ static struct bfq_queue *bfq_select_queue(struct bfq_data *bfqd)
* guarantees (the second alternative unconditionally
* injects a pending I/O request of the waker queue
* for each bfq_dispatch_request()). Second, with the
- * third alternative, the duration of the plugging,
+ * fourth alternative, the duration of the plugging,
* i.e., the time before bfqq finally receives new I/O,
* may not be minimized, because the waker queue may
* happen to be served only after other queues.
@@ -4573,6 +4587,14 @@ static struct bfq_queue *bfq_select_queue(struct bfq_data *bfqd)
bfq_bfqq_budget_left(bfqq->waker_bfqq)
)
bfqq = bfqq->waker_bfqq;
+ else if (blocked_bfqq &&
+ bfq_bfqq_busy(blocked_bfqq) &&
+ blocked_bfqq->next_rq &&
+ bfq_serv_to_charge(blocked_bfqq->next_rq,
+ blocked_bfqq) <=
+ bfq_bfqq_budget_left(blocked_bfqq)
+ )
+ bfqq = blocked_bfqq;
else if (!idling_boosts_thr_without_issues(bfqd, bfqq) &&
(bfqq->wr_coeff == 1 || bfqd->wr_busy_queues > 1 ||
!bfq_bfqq_has_short_ttime(bfqq)))
diff --git a/block/bfq-wf2q.c b/block/bfq-wf2q.c
index 26776bdbdf36..02e59931d897 100644
--- a/block/bfq-wf2q.c
+++ b/block/bfq-wf2q.c
@@ -1709,4 +1709,12 @@ void bfq_add_bfqq_busy(struct bfq_data *bfqd, struct bfq_queue *bfqq)

if (bfqq->wr_coeff > 1)
bfqd->wr_busy_queues++;
+
+ /* Move bfqq to the head of the woken list of its waker */
+ if (!hlist_unhashed(&bfqq->woken_list_node) &&
+ &bfqq->woken_list_node != bfqq->waker_bfqq->woken_list.first) {
+ hlist_del_init(&bfqq->woken_list_node);
+ hlist_add_head(&bfqq->woken_list_node,
+ &bfqq->waker_bfqq->woken_list);
+ }
}
--
2.20.1