lkml.org 
[lkml]   [2019]   [Oct]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] blk-wbt: fix performance regression in wbt scale_up/scale_down
On Fri, Oct 4, 2019 at 11:09 AM Harshad Shirwadkar
<harshadshirwadkar@gmail.com> wrote:
>
> scale_up wakes up waiters after scaling up. But after scaling max, it
> should not wake up more waiters as waiters will not have anything to
> do. This patch fixes this by making scale_up (and also scale_down)
> return when threshold is reached.
>
> This bug causes increased fdatasync latency when fdatasync and dd
> conv=sync are performed in parallel on 4.19 compared to 4.14. This
> bug was introduced during refactoring of blk-wbt code.
>
> Fixes: a79050434b45 ("blk-rq-qos: refactor out common elements of blk-wbt")
> Cc: Josef Bacik <jbacik@fb.com>
> Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
> ---
> block/blk-rq-qos.c | 14 +++++++++-----
> block/blk-rq-qos.h | 4 ++--
> block/blk-wbt.c | 6 ++++--
> 3 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/block/blk-rq-qos.c b/block/blk-rq-qos.c
> index 3954c0dc1443..d92abb43000c 100644
> --- a/block/blk-rq-qos.c
> +++ b/block/blk-rq-qos.c
> @@ -142,24 +142,27 @@ bool rq_depth_calc_max_depth(struct rq_depth *rqd)
> return ret;
> }
>
> -void rq_depth_scale_up(struct rq_depth *rqd)
> +/* Returns true on success and false if scaling up wasn't possible */
> +bool rq_depth_scale_up(struct rq_depth *rqd)
> {
> /*
> * Hit max in previous round, stop here
> */
> if (rqd->scaled_max)
> - return;
> + return false;
>
> rqd->scale_step--;
>
> rqd->scaled_max = rq_depth_calc_max_depth(rqd);
> + return true;
> }
>
> /*
> * Scale rwb down. If 'hard_throttle' is set, do it quicker, since we
> - * had a latency violation.
> + * had a latency violation. Returns true on success and returns false if
> + * scaling down wasn't possible.
> */
> -void rq_depth_scale_down(struct rq_depth *rqd, bool hard_throttle)
> +bool rq_depth_scale_down(struct rq_depth *rqd, bool hard_throttle)
> {
> /*
> * Stop scaling down when we've hit the limit. This also prevents
> @@ -167,7 +170,7 @@ void rq_depth_scale_down(struct rq_depth *rqd, bool hard_throttle)
> * keep up.
> */
> if (rqd->max_depth == 1)
> - return;
> + return false;
>
> if (rqd->scale_step < 0 && hard_throttle)
> rqd->scale_step = 0;
> @@ -176,6 +179,7 @@ void rq_depth_scale_down(struct rq_depth *rqd, bool hard_throttle)
>
> rqd->scaled_max = false;
> rq_depth_calc_max_depth(rqd);
> + return 0;
Oops, I meant return true here, thanks Vaibhav (+cc) for pointing this
out. I'll fix this in V2.

> }
>
> struct rq_qos_wait_data {
> diff --git a/block/blk-rq-qos.h b/block/blk-rq-qos.h
> index 2300e038b9fa..c0f0778d5396 100644
> --- a/block/blk-rq-qos.h
> +++ b/block/blk-rq-qos.h
> @@ -125,8 +125,8 @@ void rq_qos_wait(struct rq_wait *rqw, void *private_data,
> acquire_inflight_cb_t *acquire_inflight_cb,
> cleanup_cb_t *cleanup_cb);
> bool rq_wait_inc_below(struct rq_wait *rq_wait, unsigned int limit);
> -void rq_depth_scale_up(struct rq_depth *rqd);
> -void rq_depth_scale_down(struct rq_depth *rqd, bool hard_throttle);
> +bool rq_depth_scale_up(struct rq_depth *rqd);
> +bool rq_depth_scale_down(struct rq_depth *rqd, bool hard_throttle);
> bool rq_depth_calc_max_depth(struct rq_depth *rqd);
>
> void __rq_qos_cleanup(struct rq_qos *rqos, struct bio *bio);
> diff --git a/block/blk-wbt.c b/block/blk-wbt.c
> index 313f45a37e9d..5a96881e7a52 100644
> --- a/block/blk-wbt.c
> +++ b/block/blk-wbt.c
> @@ -308,7 +308,8 @@ static void calc_wb_limits(struct rq_wb *rwb)
>
> static void scale_up(struct rq_wb *rwb)
> {
> - rq_depth_scale_up(&rwb->rq_depth);
> + if (!rq_depth_scale_up(&rwb->rq_depth))
> + return;
> calc_wb_limits(rwb);
> rwb->unknown_cnt = 0;
> rwb_wake_all(rwb);
> @@ -317,7 +318,8 @@ static void scale_up(struct rq_wb *rwb)
>
> static void scale_down(struct rq_wb *rwb, bool hard_throttle)
> {
> - rq_depth_scale_down(&rwb->rq_depth, hard_throttle);
> + if (!rq_depth_scale_down(&rwb->rq_depth, hard_throttle))
> + return;
> calc_wb_limits(rwb);
> rwb->unknown_cnt = 0;
> rwb_trace_step(rwb, "scale down");
> --
> 2.23.0.581.g78d2f28ef7-goog
>

\
 
 \ /
  Last update: 2019-10-04 22:53    [W:0.033 / U:4.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site