[PATCH BUGFIX/IMPROVEMENT 0/4] block, bfq: series of fixes of bugs affecting service guarantees

[PATCH BUGFIX/IMPROVEMENT 0/4] block, bfq: series of fixes of bugs affecting service guarantees

[Paolo Valente]

Hi,
the first patch in this series fixes a bug that causes bfq to fail to
guarantee a high responsiveness on some drives, if there is heavy
random read+write I/O in the background. More precisely, such a
failure allowed this bug to be found [1], but the bug may well cause
other yet unreported anomalies.

This fix uncovered other bugs that were concealed by the fixed bug,
for rather subtle reasons. These further bugs caused similar
responsiveness failures, but with sequential reaad+write workloads in
the background. The remaining three patches fix these further bugs.

The sum of these fixes makes responsiveness much stabler with BFQ. In
the presence of write hogs, it is however still impossible for an I/O
scheduler to guarantee perfect responsiveness in any circustance,
because of throttling issues in the virtual-memory management
subsystem, and in other higher-level components.

Thanks,
Paolo

[1] Background I/O Type: Random - Background I/O mix: Reads and writes
- Application to start: LibreOffice Writer in
http://www.phoronix.com/scan.php?page=news_item&px=Linux-4.13-IO-Laptop


Paolo Valente (4):
  block, bfq: fix wrong init of saved start time for weight raising
  block, bfq: check and switch back to interactive wr also on queue
    split
  block, bfq: let early-merged queues be weight-raised on split too
  block, bfq: decrease burst size when queues in burst exit

 block/bfq-iosched.c | 169 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 102 insertions(+), 67 deletions(-)

--
2.10.0

[PATCH BUGFIX/IMPROVEMENT 1/4] block, bfq: fix wrong init of saved start time for weight raising

[Paolo Valente]

This commit fixes a bug that causes bfq to fail to guarantee a high
responsiveness on some drives, if there is heavy random read+write I/O
in the background. More precisely, such a failure allowed this bug to
be found [1], but the bug may well cause other yet unreported
anomalies.

BFQ raises the weight of the bfq_queues associated with soft real-time
applications, to privilege the I/O, and thus reduce latency, for these
applications. This mechanism is named soft-real-time weight raising in
BFQ. A soft real-time period may happen to be nested into an
interactive weight raising period, i.e., it may happen that, when a
bfq_queue switches to a soft real-time weight-raised state, the
bfq_queue is already being weight-raised because deemed interactive
too. In this case, BFQ saves in a special variable
wr_start_at_switch_to_srt, the time instant when the interactive
weight-raising period started for the bfq_queue, i.e., the time
instant when BFQ started to deem the bfq_queue interactive. This value
is then used to check whether the interactive weight-raising period
would still be in progress when the soft real-time weight-raising
period ends.  If so, interactive weight raising is restored for the
bfq_queue. This restore is useful, in particular, because it prevents
bfq_queues from losing their interactive weight raising prematurely,
as a consequence of spurious, short-lived soft real-time
weight-raising periods caused by wrong detections as soft real-time.

If, instead, a bfq_queue switches to soft-real-time weight raising
while it *is not* already in an interactive weight-raising period,
then the variable wr_start_at_switch_to_srt has no meaning during the
following soft real-time weight-raising period. Unfortunately the
handling of this case is wrong in BFQ: not only the variable is not
flagged somehow as meaningless, but it is also set to the time when
the switch to soft real-time weight-raising occurs. This may cause an
interactive weight-raising period to be considered mistakenly as still
in progress, and thus a spurious interactive weight-raising period to
start for the bfq_queue, at the end of the soft-real-time
weight-raising period. In particular the spurious interactive
weight-raising period will be considered as still in progress, if the
soft-real-time weight-raising period does not last very long. The
bfq_queue will then be wrongly privileged and, if I/O bound, will
unjustly steal bandwidth to truly interactive or soft real-time
bfq_queues, harming responsiveness and low latency.

This commit fixes this issue by just setting wr_start_at_switch_to_srt
to minus infinity (farthest past time instant according to jiffies
macros): when the soft-real-time weight-raising period ends, certainly
no interactive weight-raising period will be considered as still in
progress.

[1] Background I/O Type: Random - Background I/O mix: Reads and writes
- Application to start: LibreOffice Writer in
http://www.phoronix.com/scan.php?page=news_item&px=Linux-4.13-IO-Laptop

Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
Signed-off-by: Angelo Ruocco <angeloruocco90@gmail.com>
Tested-by: Oleksandr Natalenko <oleksandr@natalenko.name>
Tested-by: Lee Tibbert <lee.tibbert@gmail.com>
Tested-by: Mirko Montanari <mirkomontanari91@gmail.com>
---
 block/bfq-iosched.c | 50 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index a4783da..c25955c 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -1202,6 +1202,24 @@ static unsigned int bfq_wr_duration(struct bfq_data *bfqd)
 	return dur;
 }
 
+/*
+ * Return the farthest future time instant according to jiffies
+ * macros.
+ */
+static unsigned long bfq_greatest_from_now(void)
+{
+	return jiffies + MAX_JIFFY_OFFSET;
+}
+
+/*
+ * Return the farthest past time instant according to jiffies
+ * macros.
+ */
+static unsigned long bfq_smallest_from_now(void)
+{
+	return jiffies - MAX_JIFFY_OFFSET;
+}
+
 static void bfq_update_bfqq_wr_on_rq_arrival(struct bfq_data *bfqd,
 					     struct bfq_queue *bfqq,
 					     unsigned int old_wr_coeff,
@@ -1216,7 +1234,19 @@ static void bfq_update_bfqq_wr_on_rq_arrival(struct bfq_data *bfqd,
 			bfqq->wr_coeff = bfqd->bfq_wr_coeff;
 			bfqq->wr_cur_max_time = bfq_wr_duration(bfqd);
 		} else {
-			bfqq->wr_start_at_switch_to_srt = jiffies;
+			/*
+			 * No interactive weight raising in progress
+			 * here: assign minus infinity to
+			 * wr_start_at_switch_to_srt, to make sure
+			 * that, at the end of the soft-real-time
+			 * weight raising periods that is starting
+			 * now, no interactive weight-raising period
+			 * may be wrongly considered as still in
+			 * progress (and thus actually started by
+			 * mistake).
+			 */
+			bfqq->wr_start_at_switch_to_srt =
+				bfq_smallest_from_now();
 			bfqq->wr_coeff = bfqd->bfq_wr_coeff *
 				BFQ_SOFTRT_WEIGHT_FACTOR;
 			bfqq->wr_cur_max_time =
@@ -2897,24 +2927,6 @@ static unsigned long bfq_bfqq_softrt_next_start(struct bfq_data *bfqd,
 		   jiffies + nsecs_to_jiffies(bfqq->bfqd->bfq_slice_idle) + 4);
 }
 
-/*
- * Return the farthest future time instant according to jiffies
- * macros.
- */
-static unsigned long bfq_greatest_from_now(void)
-{
-	return jiffies + MAX_JIFFY_OFFSET;
-}
-
-/*
- * Return the farthest past time instant according to jiffies
- * macros.
- */
-static unsigned long bfq_smallest_from_now(void)
-{
-	return jiffies - MAX_JIFFY_OFFSET;
-}
-
 /**
  * bfq_bfqq_expire - expire a queue.
  * @bfqd: device owning the queue.
-- 
2.10.0

[PATCH BUGFIX/IMPROVEMENT 2/4] block, bfq: check and switch back to interactive wr also on queue split

[Paolo Valente]

As already explained in the message of commit "block, bfq: fix
wrong init of saved start time for weight raising", if a soft
real-time weight-raising period happens to be nested in a larger
interactive weight-raising period, then BFQ restores the interactive
weight raising at the end of the soft real-time weight raising. In
particular, BFQ checks whether the latter has ended only on request
dispatches.

Unfortunately, the above scheme fails to restore interactive weight
raising in the following corner case: if a bfq_queue, say Q,
1) Is merged with another bfq_queue while it is in a nested soft
real-time weight-raising period. The weight-raising state of Q is
then saved, and not considered any longer until a split occurs.
2) Is split from the other bfq_queue(s) at a time instant when its
soft real-time weight raising is already finished.
On the split, while resuming the previous, soft real-time
weight-raised state of the bfq_queue Q, BFQ checks whether the
current soft real-time weight-raising period is actually over. If so,
BFQ switches weight raising off for Q, *without* checking whether the
soft real-time period was actually nested in a non-yet-finished
interactive weight-raising period.

This commit addresses this issue by adding the above missing check in
bfq_queue splits, and restoring interactive weight raising if needed.

Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
Tested-by: Angelo Ruocco <angeloruocco90@gmail.com>
Tested-by: Mirko Montanari <mirkomontanari91@gmail.com>
Tested-by: Oleksandr Natalenko <oleksandr@natalenko.name>
---
 block/bfq-iosched.c | 87 ++++++++++++++++++++++++++++++-----------------------
 1 file changed, 49 insertions(+), 38 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index c25955c..33b63bc 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -724,6 +724,44 @@ static void bfq_updated_next_req(struct bfq_data *bfqd,
 	}
 }
 
+static unsigned int bfq_wr_duration(struct bfq_data *bfqd)
+{
+	u64 dur;
+
+	if (bfqd->bfq_wr_max_time > 0)
+		return bfqd->bfq_wr_max_time;
+
+	dur = bfqd->RT_prod;
+	do_div(dur, bfqd->peak_rate);
+
+	/*
+	 * Limit duration between 3 and 13 seconds. Tests show that
+	 * higher values than 13 seconds often yield the opposite of
+	 * the desired result, i.e., worsen responsiveness by letting
+	 * non-interactive and non-soft-real-time applications
+	 * preserve weight raising for a too long time interval.
+	 *
+	 * On the other end, lower values than 3 seconds make it
+	 * difficult for most interactive tasks to complete their jobs
+	 * before weight-raising finishes.
+	 */
+	if (dur > msecs_to_jiffies(13000))
+		dur = msecs_to_jiffies(13000);
+	else if (dur < msecs_to_jiffies(3000))
+		dur = msecs_to_jiffies(3000);
+
+	return dur;
+}
+
+/* switch back from soft real-time to interactive weight raising */
+static void switch_back_to_interactive_wr(struct bfq_queue *bfqq,
+					  struct bfq_data *bfqd)
+{
+	bfqq->wr_coeff = bfqd->bfq_wr_coeff;
+	bfqq->wr_cur_max_time = bfq_wr_duration(bfqd);
+	bfqq->last_wr_start_finish = bfqq->wr_start_at_switch_to_srt;
+}
+
 static void
 bfq_bfqq_resume_state(struct bfq_queue *bfqq, struct bfq_data *bfqd,
 		      struct bfq_io_cq *bic, bool bfq_already_existing)
@@ -750,10 +788,16 @@ bfq_bfqq_resume_state(struct bfq_queue *bfqq, struct bfq_data *bfqd,
 	if (bfqq->wr_coeff > 1 && (bfq_bfqq_in_large_burst(bfqq) ||
 	    time_is_before_jiffies(bfqq->last_wr_start_finish +
 				   bfqq->wr_cur_max_time))) {
-		bfq_log_bfqq(bfqq->bfqd, bfqq,
-		    "resume state: switching off wr");
-
-		bfqq->wr_coeff = 1;
+		if (bfqq->wr_cur_max_time == bfqd->bfq_wr_rt_max_time &&
+		    !bfq_bfqq_in_large_burst(bfqq) &&
+		    time_is_after_eq_jiffies(bfqq->wr_start_at_switch_to_srt +
+					     bfq_wr_duration(bfqd))) {
+			switch_back_to_interactive_wr(bfqq, bfqd);
+		} else {
+			bfqq->wr_coeff = 1;
+			bfq_log_bfqq(bfqq->bfqd, bfqq,
+				     "resume state: switching off wr");
+		}
 	}
 
 	/* make sure weight will be updated, however we got here */
@@ -1173,35 +1217,6 @@ static bool bfq_bfqq_update_budg_for_activation(struct bfq_data *bfqd,
 	return wr_or_deserves_wr;
 }
 
-static unsigned int bfq_wr_duration(struct bfq_data *bfqd)
-{
-	u64 dur;
-
-	if (bfqd->bfq_wr_max_time > 0)
-		return bfqd->bfq_wr_max_time;
-
-	dur = bfqd->RT_prod;
-	do_div(dur, bfqd->peak_rate);
-
-	/*
-	 * Limit duration between 3 and 13 seconds. Tests show that
-	 * higher values than 13 seconds often yield the opposite of
-	 * the desired result, i.e., worsen responsiveness by letting
-	 * non-interactive and non-soft-real-time applications
-	 * preserve weight raising for a too long time interval.
-	 *
-	 * On the other end, lower values than 3 seconds make it
-	 * difficult for most interactive tasks to complete their jobs
-	 * before weight-raising finishes.
-	 */
-	if (dur > msecs_to_jiffies(13000))
-		dur = msecs_to_jiffies(13000);
-	else if (dur < msecs_to_jiffies(3000))
-		dur = msecs_to_jiffies(3000);
-
-	return dur;
-}
-
 /*
  * Return the farthest future time instant according to jiffies
  * macros.
@@ -3501,11 +3516,7 @@ static void bfq_update_wr_data(struct bfq_data *bfqd, struct bfq_queue *bfqq)
 					       bfq_wr_duration(bfqd)))
 				bfq_bfqq_end_wr(bfqq);
 			else {
-				/* switch back to interactive wr */
-				bfqq->wr_coeff = bfqd->bfq_wr_coeff;
-				bfqq->wr_cur_max_time = bfq_wr_duration(bfqd);
-				bfqq->last_wr_start_finish =
-					bfqq->wr_start_at_switch_to_srt;
+				switch_back_to_interactive_wr(bfqq, bfqd);
 				bfqq->entity.prio_changed = 1;
 			}
 		}
-- 
2.10.0

[PATCH BUGFIX/IMPROVEMENT 3/4] block, bfq: let early-merged queues be weight-raised on split too

[Paolo Valente]

A just-created bfq_queue, say Q, may happen to be merged with another
bfq_queue on the very first invocation of the function
__bfq_insert_request. In such a case, even if Q would clearly deserve
interactive weight raising (as it has just been created), the function
bfq_add_request does not make it to be invoked for Q, and thus to
activate weight raising for Q. As a consequence, when the state of Q
is saved for a possible future restore, after a split of Q from the
other bfq_queue(s), such a state happens to be (unjustly)
non-weight-raised. Then the bfq_queue will not enjoy any weight
raising on the split, even if should still be in an interactive
weight-raising period when the split occurs.

This commit solves this problem as follows, for a just-created
bfq_queue that is being early-merged: it stores directly, in the saved
state of the bfq_queue, the weight-raising state that would have been
assigned to the bfq_queue if not early-merged.

Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
Tested-by: Angelo Ruocco <angeloruocco90@gmail.com>
Tested-by: Mirko Montanari <mirkomontanari91@gmail.com>
Tested-by: Oleksandr Natalenko <oleksandr@natalenko.name>
---
 block/bfq-iosched.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 33b63bc..115747f 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -2061,10 +2061,27 @@ static void bfq_bfqq_save_state(struct bfq_queue *bfqq)
 	bic->saved_IO_bound = bfq_bfqq_IO_bound(bfqq);
 	bic->saved_in_large_burst = bfq_bfqq_in_large_burst(bfqq);
 	bic->was_in_burst_list = !hlist_unhashed(&bfqq->burst_list_node);
-	bic->saved_wr_coeff = bfqq->wr_coeff;
-	bic->saved_wr_start_at_switch_to_srt = bfqq->wr_start_at_switch_to_srt;
-	bic->saved_last_wr_start_finish = bfqq->last_wr_start_finish;
-	bic->saved_wr_cur_max_time = bfqq->wr_cur_max_time;
+	if (unlikely(bfq_bfqq_just_created(bfqq) &&
+		     !bfq_bfqq_in_large_burst(bfqq))) {
+		/*
+		 * bfqq being merged right after being created: bfqq
+		 * would have deserved interactive weight raising, but
+		 * did not make it to be set in a weight-raised state,
+		 * because of this early merge.	Store directly the
+		 * weight-raising state that would have been assigned
+		 * to bfqq, so that to avoid that bfqq unjustly fails
+		 * to enjoy weight raising if split soon.
+		 */
+		bic->saved_wr_coeff = bfqq->bfqd->bfq_wr_coeff;
+		bic->saved_wr_cur_max_time = bfq_wr_duration(bfqq->bfqd);
+		bic->saved_last_wr_start_finish = jiffies;
+	} else {
+		bic->saved_wr_coeff = bfqq->wr_coeff;
+		bic->saved_wr_start_at_switch_to_srt =
+			bfqq->wr_start_at_switch_to_srt;
+		bic->saved_last_wr_start_finish = bfqq->last_wr_start_finish;
+		bic->saved_wr_cur_max_time = bfqq->wr_cur_max_time;
+	}
 }
 
 static void
@@ -4150,7 +4167,6 @@ static void __bfq_insert_request(struct bfq_data *bfqd, struct request *rq)
 		new_bfqq->allocated++;
 		bfqq->allocated--;
 		new_bfqq->ref++;
-		bfq_clear_bfqq_just_created(bfqq);
 		/*
 		 * If the bic associated with the process
 		 * issuing this request still points to bfqq
@@ -4162,6 +4178,8 @@ static void __bfq_insert_request(struct bfq_data *bfqd, struct request *rq)
 		if (bic_to_bfqq(RQ_BIC(rq), 1) == bfqq)
 			bfq_merge_bfqqs(bfqd, RQ_BIC(rq),
 					bfqq, new_bfqq);
+
+		bfq_clear_bfqq_just_created(bfqq);
 		/*
 		 * rq is about to be enqueued into new_bfqq,
 		 * release rq reference on bfqq
-- 
2.10.0

[PATCH BUGFIX/IMPROVEMENT 4/4] block, bfq: decrease burst size when queues in burst exit

[Paolo Valente]

If many queues belonging to the same group happen to be created
shortly after each other, then the concurrent processes associated
with these queues have typically a common goal, and they get it done
as soon as possible if not hampered by device idling.  Examples are
processes spawned by git grep, or by systemd during boot. As for
device idling, this mechanism is currently necessary for weight
raising to succeed in its goal: privileging I/O.  In view of these
facts, BFQ does not provide the above queues with either weight
raising or device idling.

On the other hand, a burst of queue creations may be caused also by
the start-up of a complex application. In this case, these queues need
usually to be served one after the other, and as quickly as possible,
to maximise responsiveness. Therefore, in this case the best strategy
is to weight-raise all the queues created during the burst, i.e., the
exact opposite of the strategy for the above case.

To distinguish between the two cases, BFQ uses an empirical burst-size
threshold, found through extensive tests and monitoring of daily
usage. Only large bursts, i.e., burst with a size above this
threshold, are considered as generated by a high number of parallel
processes. In this respect, upstart-based boot proved to be rather
hard to detect as generating a large burst of queue creations, because
with upstart most of the queues created in a burst exit *before* the
next queues in the same burst are created. To address this issue, I
changed the burst-detection mechanism so as to not decrease the size
of the current burst even if one of the queues in the burst is
eliminated.

Unfortunately, this missing decrease causes false positives on very
fast systems: on the start-up of a complex application, such as
libreoffice writer, so many queues are created, served and exited
shortly after each other, that a large burst of queue creations is
wrongly detected as occurring. These false positives just disappear if
the size of a burst is decreased when one of the queues in the burst
exits. This commit restores the missing burst-size decrease, relying
of the fact that upstart is apparently unlikely to be used on systems
running this and future versions of the kernel.

Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
Signed-off-by: Mauro Andreolini <mauro.andreolini@unimore.it>
Signed-off-by: Angelo Ruocco <angeloruocco90@gmail.com>
Tested-by: Mirko Montanari <mirkomontanari91@gmail.com>
Tested-by: Oleksandr Natalenko <oleksandr@natalenko.name>
---
 block/bfq-iosched.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 115747f..70f9177 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -3725,16 +3725,10 @@ void bfq_put_queue(struct bfq_queue *bfqq)
 	if (bfqq->ref)
 		return;
 
-	if (bfq_bfqq_sync(bfqq))
-		/*
-		 * The fact that this queue is being destroyed does not
-		 * invalidate the fact that this queue may have been
-		 * activated during the current burst. As a consequence,
-		 * although the queue does not exist anymore, and hence
-		 * needs to be removed from the burst list if there,
-		 * the burst size has not to be decremented.
-		 */
+	if (bfq_bfqq_sync(bfqq) && !hlist_unhashed(&bfqq->burst_list_node)) {
 		hlist_del_init(&bfqq->burst_list_node);
+		bfqq->bfqd->burst_size--;
+	}
 
 	kmem_cache_free(bfq_pool, bfqq);
 #ifdef CONFIG_BFQ_GROUP_IOSCHED
-- 
2.10.0

Re: [PATCH BUGFIX/IMPROVEMENT 0/4] block, bfq: series of fixes of bugs

[Lee Tibbert]

On Thu, Sep 21, 2017 at 5:03 AM, Paolo Valente <paolo.valente@linaro.org>
wrote:

> Hi,
> the first patch in this series fixes a bug that causes bfq to fail to
> guarantee a high responsiveness on some drives, if there is heavy
> random read+write I/O in the background. More precisely, such a
> failure allowed this bug to be found [1], but the bug may well cause
> other yet unreported anomalies.
>
> This fix uncovered other bugs that were concealed by the fixed bug,
> for rather subtle reasons. These further bugs caused similar
> responsiveness failures, but with sequential reaad+write workloads in
> the background. The remaining three patches fix these further bugs.
>
> The sum of these fixes makes responsiveness much stabler with BFQ. In
> the presence of write hogs, it is however still impossible for an I/O
> scheduler to guarantee perfect responsiveness in any circustance,
> because of throttling issues in the virtual-memory management
> subsystem, and in other higher-level components.
>
> Thanks,
> Paolo
>
> [1] Background I/O Type: Random - Background I/O mix: Reads and writes
> - Application to start: LibreOffice Writer in
> http://www.phoronix.com/scan.php?page=news_item&px=Linux-4.13-IO-Laptop
>
>
> Paolo Valente (4):
>   block, bfq: fix wrong init of saved start time for weight raising
>   block, bfq: check and switch back to interactive wr also on queue
>     split
>   block, bfq: let early-merged queues be weight-raised on split too
>   block, bfq: decrease burst size when queues in burst exit
>
>  block/bfq-iosched.c | 169 ++++++++++++++++++++++++++++++
> +---------------------
>  1 file changed, 102 insertions(+), 67 deletions(-)
>
> --
> 2.10.0
>

Paolo,

Please feel free to add:

  Tested-by: Lee Tibbert <lee.tibbert@gmail.com>

I thought that you and linux-block readers might be interested
in a data point. I have been running the algodev variant of these
patches for about two weeks. Now I am running these exact patches.

At one point, I had firefox running in my main windows, and, in other
windows, a full linux build, a full scala-native build, and a git gc. I
barely noticed the builds going on.  The variance of latency in
firefox was well within my normal network variance. Usually any
one of the build jobs would have made firefox useless.

Pretty amazing!  In fact, it was so amazing that I did not trust my
memory several days later, so I repeated the full test.
Same result:  great reduced latency.

There being no such thing as a free lunch, the build jobs did
take longer than if executed standalone. However, I did not mind
because I was able to continue getting work done on the tasks
I needed in interactive time.

I hope these patches are accepted into linux mainline & will
I will continue to exercise them.

Lee

Re: [PATCH BUGFIX/IMPROVEMENT 0/4] block, bfq: series of fixes of bugs affecting service guarantees

[Lee Tibbert]

On Thu, Sep 21, 2017 at 5:03 AM, Paolo Valente <paolo.valente@linaro.org>
wrote:

> Hi,
> the first patch in this series fixes a bug that causes bfq to fail to
> guarantee a high responsiveness on some drives, if there is heavy
> random read+write I/O in the background. More precisely, such a
> failure allowed this bug to be found [1], but the bug may well cause
> other yet unreported anomalies.
>
> This fix uncovered other bugs that were concealed by the fixed bug,
> for rather subtle reasons. These further bugs caused similar
> responsiveness failures, but with sequential reaad+write workloads in
> the background. The remaining three patches fix these further bugs.
>
> The sum of these fixes makes responsiveness much stabler with BFQ. In
> the presence of write hogs, it is however still impossible for an I/O
> scheduler to guarantee perfect responsiveness in any circustance,
> because of throttling issues in the virtual-memory management
> subsystem, and in other higher-level components.
>
> Thanks,
> Paolo
>
> [1] Background I/O Type: Random - Background I/O mix: Reads and writes
> - Application to start: LibreOffice Writer in
> http://www.phoronix.com/scan.php?page=news_item&px=Linux-4.13-IO-Laptop
>
>
> Paolo Valente (4):
>   block, bfq: fix wrong init of saved start time for weight raising
>   block, bfq: check and switch back to interactive wr also on queue
>     split
>   block, bfq: let early-merged queues be weight-raised on split too
>   block, bfq: decrease burst size when queues in burst exit
>
>  block/bfq-iosched.c | 169 ++++++++++++++++++++++++++++++
> +---------------------
>  1 file changed, 102 insertions(+), 67 deletions(-)
>
> --
> 2.10.0
>

Paolo,

Please feel free to add:

  Tested-by: Lee Tibbert <lee.tibbert@gmail.com>

I thought that you and linux-block readers might be interested
in a data point. I have been running the algodev variant of these
patches for about two weeks. Now I am running these exact patches.

At one point, I had firefox running in my main windows, and, in other
windows, a full linux build, a full scala-native build, and a git gc. I
barely noticed the builds going on.  The variance of latency in
firefox was well within my normal network variance. Usually any
one of the build jobs would have made firefox useless.

Pretty amazing!  In fact, it was so amazing that I did not trust my
memory several days later, so I repeated the full test.
Same result:  great reduced latency.

There being no such thing as a free lunch, the build jobs did
take longer than if executed standalone. However, I did not mind
because I was able to continue getting work done on the tasks
I needed in interactive time.

I hope these patches are accepted into linux mainline & will
I will continue to exercise them.

Lee

Re: [PATCH BUGFIX/IMPROVEMENT 0/4] block, bfq: series of fixes of bugs affecting service guarantees

[lee.tibbert]

On Thu, Sep 21, 2017 at 5:03 AM, Paolo Valente <paolo.valente@linaro.org>
wrote:

> Hi,
> the first patch in this series fixes a bug that causes bfq to fail to
> guarantee a high responsiveness on some drives, if there is heavy
> random read+write I/O in the background. More precisely, such a
> failure allowed this bug to be found [1], but the bug may well cause
> other yet unreported anomalies.
>
> This fix uncovered other bugs that were concealed by the fixed bug,
> for rather subtle reasons. These further bugs caused similar
> responsiveness failures, but with sequential reaad+write workloads in
> the background. The remaining three patches fix these further bugs.
>
> The sum of these fixes makes responsiveness much stabler with BFQ. In
> the presence of write hogs, it is however still impossible for an I/O
> scheduler to guarantee perfect responsiveness in any circustance,
> because of throttling issues in the virtual-memory management
> subsystem, and in other higher-level components.
>
> Thanks,
> Paolo
>
> [1] Background I/O Type: Random - Background I/O mix: Reads and writes
> - Application to start: LibreOffice Writer in
> http://www.phoronix.com/scan.php?page=news_item&px=Linux-4.13-IO-Laptop
>
>
> Paolo Valente (4):
>   block, bfq: fix wrong init of saved start time for weight raising
>   block, bfq: check and switch back to interactive wr also on queue
>     split
>   block, bfq: let early-merged queues be weight-raised on split too
>   block, bfq: decrease burst size when queues in burst exit
>
>  block/bfq-iosched.c | 169 ++++++++++++++++++++++++++++++
> +---------------------
>  1 file changed, 102 insertions(+), 67 deletions(-)
>
> --
> 2.10.0
>

Paolo,

Please feel free to add:

  Tested-by: Lee Tibbert <lee.tibbert@gmail.com>

I thought that you and linux-block readers might be interested
in a data point. I have been running the algodev variant of these
patches for about two weeks. Now I am running these exact patches.

At one point, I had firefox running in my main windows, and, in other
windows, a full linux build, a full scala-native build, and a git gc. I
barely noticed the builds going on.  The variance of latency in
firefox was well within my normal network variance. Usually any
one of the build jobs would have made firefox useless.

Pretty amazing!  In fact, it was so amazing that I did not trust my
memory several days later, so I repeated the full test.
Same result:  great reduced latency.

There being no such thing as a free lunch, the build jobs did
take longer than if executed standalone. However, I did not mind
because I was able to continue getting work done on the tasks
I needed in interactive time.

I hope these patches are accepted into linux mainline & will
I will continue to exercise them.

Lee

Re: [PATCH BUGFIX/IMPROVEMENT 0/4] block, bfq: series of fixes of bugs affecting service guarantees

[Jens Axboe]

On 09/21/2017 03:03 AM, Paolo Valente wrote:
> Hi,
> the first patch in this series fixes a bug that causes bfq to fail to
> guarantee a high responsiveness on some drives, if there is heavy
> random read+write I/O in the background. More precisely, such a
> failure allowed this bug to be found [1], but the bug may well cause
> other yet unreported anomalies.
> 
> This fix uncovered other bugs that were concealed by the fixed bug,
> for rather subtle reasons. These further bugs caused similar
> responsiveness failures, but with sequential reaad+write workloads in
> the background. The remaining three patches fix these further bugs.
> 
> The sum of these fixes makes responsiveness much stabler with BFQ. In
> the presence of write hogs, it is however still impossible for an I/O
> scheduler to guarantee perfect responsiveness in any circustance,
> because of throttling issues in the virtual-memory management
> subsystem, and in other higher-level components.

Added for 4.15, thanks.

-- 
Jens Axboe