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

2021-02-25 Thread Paolo Valente



> Il giorno 26 gen 2021, alle ore 17:17, Jens Axboe  ha 
> scritto:
> 
> On 1/26/21 3:50 AM, Paolo Valente wrote:
>> 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(>woken_list) ?
>> +container_of(bfqq->woken_list.first,
>> + struct bfq_queue,
>> + woken_list_node)
>> +: NULL;
> 
> hlist_first_entry_or_null?
> 

I didn't find any such function.  There is a list_first_entry_or_null,
but it's for circular doubly linked lists.

I'll wait a little bit for your reply, then send a V2 with this patch
unchanged.

Thanks,
Paolo

> -- 
> Jens Axboe
> 



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

2021-01-26 Thread Jens Axboe
On 1/26/21 3:50 AM, Paolo Valente wrote:
> 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(>woken_list) ?
> + container_of(bfqq->woken_list.first,
> +  struct bfq_queue,
> +  woken_list_node)
> + : NULL;

hlist_first_entry_or_null?

-- 
Jens Axboe



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

2021-01-26 Thread Paolo Valente
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 
Signed-off-by: Paolo Valente 
---
 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(>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)
+