Linux-Block Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/4] block, bfq: series of improvements and small fixes of the injection mechanism
@ 2019-08-22 15:20 Paolo Valente
  2019-08-22 15:20 ` [PATCH 1/4] block, bfq: update inject limit only after injection occurred Paolo Valente
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Paolo Valente @ 2019-08-22 15:20 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-block, linux-kernel, ulf.hansson, linus.walleij,
	bfq-iosched, oleksandr, Paolo Valente

Hi Jens,
this patch series makes the injection mechanism better at preserving
control on I/O.

Thanks,
Paolo

Paolo Valente (4):
  block, bfq: update inject limit only after injection occurred
  block, bfq: reduce upper bound for inject limit to max_rq_in_driver+1
  block, bfq: increase update frequency of inject limit
  block, bfq: push up injection only after setting service time

 block/bfq-iosched.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

--
2.20.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] block, bfq: update inject limit only after injection occurred
  2019-08-22 15:20 [PATCH 0/4] block, bfq: series of improvements and small fixes of the injection mechanism Paolo Valente
@ 2019-08-22 15:20 ` Paolo Valente
  2019-08-22 15:20 ` [PATCH 2/4] block, bfq: reduce upper bound for inject limit to max_rq_in_driver+1 Paolo Valente
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Paolo Valente @ 2019-08-22 15:20 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-block, linux-kernel, ulf.hansson, linus.walleij,
	bfq-iosched, oleksandr, Paolo Valente

BFQ updates the injection limit of each bfq_queue as a function of how
much the limit inflates the service times experienced by the I/O
requests of the queue. So only service times affected by injection
must be taken into account. Unfortunately, in the current
implementation of this update scheme, the service time of an I/O
request rq not affected by injection may happen to be considered in
the following case: there is no I/O request in service when rq
arrives.

This commit fixes this issue by making sure that only service times
affected by injection are considered for updating the injection
limit. In particular, the service time of an I/O request rq is now
considered only if at least one of the following two conditions holds:
- the destination bfq_queue for rq underwent injection before rq
arrival, and there is still I/O in service in the drive on rq arrival
(the service of such unfinished I/O may delay the service of rq);
- injection occurs between the arrival and the completion time of rq.

Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
---
 block/bfq-iosched.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index b33be928d164..5a2bbd8613a8 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -2025,7 +2025,21 @@ static void bfq_add_request(struct request *rq)
 			 * be set when rq will be dispatched.
 			 */
 			bfqd->wait_dispatch = true;
-			bfqd->rqs_injected = false;
+			/*
+			 * If there is no I/O in service in the drive,
+			 * then possible injection occurred before the
+			 * arrival of rq will not affect the total
+			 * service time of rq. So the injection limit
+			 * must not be updated as a function of such
+			 * total service time, unless new injection
+			 * occurs before rq is completed. To have the
+			 * injection limit updated only in the latter
+			 * case, reset rqs_injected here (rqs_injected
+			 * will be set in case injection is performed
+			 * on bfqq before rq is completed).
+			 */
+			if (bfqd->rq_in_driver == 0)
+				bfqd->rqs_injected = false;
 		}
 	}
 
@@ -5784,7 +5798,7 @@ static void bfq_update_inject_limit(struct bfq_data *bfqd,
 	u64 tot_time_ns = ktime_get_ns() - bfqd->last_empty_occupied_ns;
 	unsigned int old_limit = bfqq->inject_limit;
 
-	if (bfqq->last_serv_time_ns > 0) {
+	if (bfqq->last_serv_time_ns > 0 && bfqd->rqs_injected) {
 		u64 threshold = (bfqq->last_serv_time_ns * 3)>>1;
 
 		if (tot_time_ns >= threshold && old_limit > 0) {
@@ -5830,6 +5844,7 @@ static void bfq_update_inject_limit(struct bfq_data *bfqd,
 
 	/* update complete, not waiting for any request completion any longer */
 	bfqd->waited_rq = NULL;
+	bfqd->rqs_injected = false;
 }
 
 /*
-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/4] block, bfq: reduce upper bound for inject limit to max_rq_in_driver+1
  2019-08-22 15:20 [PATCH 0/4] block, bfq: series of improvements and small fixes of the injection mechanism Paolo Valente
  2019-08-22 15:20 ` [PATCH 1/4] block, bfq: update inject limit only after injection occurred Paolo Valente
@ 2019-08-22 15:20 ` Paolo Valente
  2019-08-22 15:20 ` [PATCH 3/4] block, bfq: increase update frequency of inject limit Paolo Valente
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Paolo Valente @ 2019-08-22 15:20 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-block, linux-kernel, ulf.hansson, linus.walleij,
	bfq-iosched, oleksandr, Paolo Valente

Upon an increment attempt of the injection limit, the latter is
constrained not to become higher than twice the maximum number
max_rq_in_driver of I/O requests that have happened to be in service
in the drive. This high bound allows the injection limit to grow
beyond max_rq_in_driver, which may then cause max_rq_in_driver itself
to grow.

However, since the limit is incremented by only one unit at a time,
there is no need for such a high bound, and just max_rq_in_driver+1 is
enough.

Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
---
 block/bfq-iosched.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 5a2bbd8613a8..e114282204f6 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -5805,7 +5805,7 @@ static void bfq_update_inject_limit(struct bfq_data *bfqd,
 			bfqq->inject_limit--;
 			bfqq->decrease_time_jif = jiffies;
 		} else if (tot_time_ns < threshold &&
-			   old_limit < bfqd->max_rq_in_driver<<1)
+			   old_limit <= bfqd->max_rq_in_driver)
 			bfqq->inject_limit++;
 	}
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/4] block, bfq: increase update frequency of inject limit
  2019-08-22 15:20 [PATCH 0/4] block, bfq: series of improvements and small fixes of the injection mechanism Paolo Valente
  2019-08-22 15:20 ` [PATCH 1/4] block, bfq: update inject limit only after injection occurred Paolo Valente
  2019-08-22 15:20 ` [PATCH 2/4] block, bfq: reduce upper bound for inject limit to max_rq_in_driver+1 Paolo Valente
@ 2019-08-22 15:20 ` Paolo Valente
  2019-08-22 15:20 ` [PATCH 4/4] block, bfq: push up injection only after setting service time Paolo Valente
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Paolo Valente @ 2019-08-22 15:20 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-block, linux-kernel, ulf.hansson, linus.walleij,
	bfq-iosched, oleksandr, Paolo Valente

The update period of the injection limit has been tentatively set to
100 ms, to reduce fluctuations. This value however proved to cause,
occasionally, the limit to be decremented for some bfq_queue only
after the queue underwent excessive injection for a lot of time. This
commit reduces the period to 10 ms.

Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
---
 block/bfq-iosched.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index e114282204f6..ddac93e910fa 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -2016,7 +2016,7 @@ static void bfq_add_request(struct request *rq)
 		     (bfqq->last_serv_time_ns > 0 &&
 		      bfqd->rqs_injected && bfqd->rq_in_driver > 0)) &&
 		    time_is_before_eq_jiffies(bfqq->decrease_time_jif +
-					      msecs_to_jiffies(100))) {
+					      msecs_to_jiffies(10))) {
 			bfqd->last_empty_occupied_ns = ktime_get_ns();
 			/*
 			 * Start the state machine for measuring the
-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/4] block, bfq: push up injection only after setting service time
  2019-08-22 15:20 [PATCH 0/4] block, bfq: series of improvements and small fixes of the injection mechanism Paolo Valente
                   ` (2 preceding siblings ...)
  2019-08-22 15:20 ` [PATCH 3/4] block, bfq: increase update frequency of inject limit Paolo Valente
@ 2019-08-22 15:20 ` Paolo Valente
  2019-09-09  5:57 ` [PATCH 0/4] block, bfq: series of improvements and small fixes of the injection mechanism Paolo Valente
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Paolo Valente @ 2019-08-22 15:20 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-block, linux-kernel, ulf.hansson, linus.walleij,
	bfq-iosched, oleksandr, Paolo Valente

If equal to 0, the injection limit for a bfq_queue is pushed to 1
after a first sample of the total service time of the I/O requests of
the queue is computed (to allow injection to start). Yet, because of a
mistake in the branch that performs this action, the push may happen
also in some other case. This commit fixes this issue.

Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
---
 block/bfq-iosched.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index ddac93e910fa..0319d6339822 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -5823,12 +5823,14 @@ static void bfq_update_inject_limit(struct bfq_data *bfqd,
 	 */
 	if ((bfqq->last_serv_time_ns == 0 && bfqd->rq_in_driver == 1) ||
 	    tot_time_ns < bfqq->last_serv_time_ns) {
+		if (bfqq->last_serv_time_ns == 0) {
+			/*
+			 * Now we certainly have a base value: make sure we
+			 * start trying injection.
+			 */
+			bfqq->inject_limit = max_t(unsigned int, 1, old_limit);
+		}
 		bfqq->last_serv_time_ns = tot_time_ns;
-		/*
-		 * Now we certainly have a base value: make sure we
-		 * start trying injection.
-		 */
-		bfqq->inject_limit = max_t(unsigned int, 1, old_limit);
 	} else if (!bfqd->rqs_injected && bfqd->rq_in_driver == 1)
 		/*
 		 * No I/O injected and no request still in service in
-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/4] block, bfq: series of improvements and small fixes of the injection mechanism
  2019-08-22 15:20 [PATCH 0/4] block, bfq: series of improvements and small fixes of the injection mechanism Paolo Valente
                   ` (3 preceding siblings ...)
  2019-08-22 15:20 ` [PATCH 4/4] block, bfq: push up injection only after setting service time Paolo Valente
@ 2019-09-09  5:57 ` Paolo Valente
  2019-09-09  6:03 ` Oleksandr Natalenko
  2019-09-16 16:17 ` Jens Axboe
  6 siblings, 0 replies; 9+ messages in thread
From: Paolo Valente @ 2019-09-09  5:57 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-block, linux-kernel, ulf.hansson, linus.walleij,
	bfq-iosched, oleksandr

Hi Jens,
have you looked into this?

Thanks,
Paolo

> Il giorno 22 ago 2019, alle ore 17:20, Paolo Valente <paolo.valente@linaro.org> ha scritto:
> 
> Hi Jens,
> this patch series makes the injection mechanism better at preserving
> control on I/O.
> 
> Thanks,
> Paolo
> 
> Paolo Valente (4):
>  block, bfq: update inject limit only after injection occurred
>  block, bfq: reduce upper bound for inject limit to max_rq_in_driver+1
>  block, bfq: increase update frequency of inject limit
>  block, bfq: push up injection only after setting service time
> 
> block/bfq-iosched.c | 35 ++++++++++++++++++++++++++---------
> 1 file changed, 26 insertions(+), 9 deletions(-)
> 
> --
> 2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/4] block, bfq: series of improvements and small fixes of the injection mechanism
  2019-08-22 15:20 [PATCH 0/4] block, bfq: series of improvements and small fixes of the injection mechanism Paolo Valente
                   ` (4 preceding siblings ...)
  2019-09-09  5:57 ` [PATCH 0/4] block, bfq: series of improvements and small fixes of the injection mechanism Paolo Valente
@ 2019-09-09  6:03 ` Oleksandr Natalenko
  2019-09-16 15:32   ` Paolo Valente
  2019-09-16 16:17 ` Jens Axboe
  6 siblings, 1 reply; 9+ messages in thread
From: Oleksandr Natalenko @ 2019-09-09  6:03 UTC (permalink / raw)
  To: Paolo Valente
  Cc: Jens Axboe, linux-block, linux-kernel, ulf.hansson,
	linus.walleij, bfq-iosched

On 22.08.2019 17:20, Paolo Valente wrote:
> Hi Jens,
> this patch series makes the injection mechanism better at preserving
> control on I/O.
> 
> Thanks,
> Paolo
> 
> Paolo Valente (4):
>   block, bfq: update inject limit only after injection occurred
>   block, bfq: reduce upper bound for inject limit to max_rq_in_driver+1
>   block, bfq: increase update frequency of inject limit
>   block, bfq: push up injection only after setting service time
> 
>  block/bfq-iosched.c | 35 ++++++++++++++++++++++++++---------
>  1 file changed, 26 insertions(+), 9 deletions(-)
> 
> --
> 2.20.1

FWIW, Tested-by: Oleksandr Natalenko <oleksandr@natalenko.name> (I run 
it for quite some time already).

-- 
   Oleksandr Natalenko (post-factum)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/4] block, bfq: series of improvements and small fixes of the injection mechanism
  2019-09-09  6:03 ` Oleksandr Natalenko
@ 2019-09-16 15:32   ` Paolo Valente
  0 siblings, 0 replies; 9+ messages in thread
From: Paolo Valente @ 2019-09-16 15:32 UTC (permalink / raw)
  To: Oleksandr Natalenko
  Cc: Jens Axboe, linux-block, linux-kernel, ulf.hansson,
	linus.walleij, bfq-iosched

Hi Jens,
can these be considered for 5.4 too?

Thanks,
Paolo

> Il giorno 9 set 2019, alle ore 08:03, Oleksandr Natalenko <oleksandr@natalenko.name> ha scritto:
> 
> On 22.08.2019 17:20, Paolo Valente wrote:
>> Hi Jens,
>> this patch series makes the injection mechanism better at preserving
>> control on I/O.
>> Thanks,
>> Paolo
>> Paolo Valente (4):
>>  block, bfq: update inject limit only after injection occurred
>>  block, bfq: reduce upper bound for inject limit to max_rq_in_driver+1
>>  block, bfq: increase update frequency of inject limit
>>  block, bfq: push up injection only after setting service time
>> block/bfq-iosched.c | 35 ++++++++++++++++++++++++++---------
>> 1 file changed, 26 insertions(+), 9 deletions(-)
>> --
>> 2.20.1
> 
> FWIW, Tested-by: Oleksandr Natalenko <oleksandr@natalenko.name> (I run it for quite some time already).
> 
> -- 
>  Oleksandr Natalenko (post-factum)


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/4] block, bfq: series of improvements and small fixes of the injection mechanism
  2019-08-22 15:20 [PATCH 0/4] block, bfq: series of improvements and small fixes of the injection mechanism Paolo Valente
                   ` (5 preceding siblings ...)
  2019-09-09  6:03 ` Oleksandr Natalenko
@ 2019-09-16 16:17 ` Jens Axboe
  6 siblings, 0 replies; 9+ messages in thread
From: Jens Axboe @ 2019-09-16 16:17 UTC (permalink / raw)
  To: Paolo Valente
  Cc: linux-block, linux-kernel, ulf.hansson, linus.walleij,
	bfq-iosched, oleksandr

On 8/22/19 9:20 AM, Paolo Valente wrote:
> Hi Jens,
> this patch series makes the injection mechanism better at preserving
> control on I/O.

Applied, thanks.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-22 15:20 [PATCH 0/4] block, bfq: series of improvements and small fixes of the injection mechanism Paolo Valente
2019-08-22 15:20 ` [PATCH 1/4] block, bfq: update inject limit only after injection occurred Paolo Valente
2019-08-22 15:20 ` [PATCH 2/4] block, bfq: reduce upper bound for inject limit to max_rq_in_driver+1 Paolo Valente
2019-08-22 15:20 ` [PATCH 3/4] block, bfq: increase update frequency of inject limit Paolo Valente
2019-08-22 15:20 ` [PATCH 4/4] block, bfq: push up injection only after setting service time Paolo Valente
2019-09-09  5:57 ` [PATCH 0/4] block, bfq: series of improvements and small fixes of the injection mechanism Paolo Valente
2019-09-09  6:03 ` Oleksandr Natalenko
2019-09-16 15:32   ` Paolo Valente
2019-09-16 16:17 ` Jens Axboe

Linux-Block Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-block/0 linux-block/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-block linux-block/ https://lore.kernel.org/linux-block \
		linux-block@vger.kernel.org linux-block@archiver.kernel.org
	public-inbox-index linux-block


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-block


AGPL code for this site: git clone https://public-inbox.org/ public-inbox