Messages in this thread | | | From | harshad shirwadkar <> | Date | Fri, 4 Oct 2019 13:53:08 -0700 | Subject | Re: [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 >
| |