Skip to content

Commit

Permalink
blk-wbt: fix performance regression in wbt scale_up/scale_down
Browse files Browse the repository at this point in the history
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: a790504 ("blk-rq-qos: refactor out common elements of blk-wbt")
Cc: [email protected]
Cc: Josef Bacik <[email protected]>
Signed-off-by: Harshad Shirwadkar <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
  • Loading branch information
harshadjs authored and axboe committed Oct 6, 2019
1 parent 0e48f51 commit b84477d
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 9 deletions.
14 changes: 9 additions & 5 deletions block/blk-rq-qos.c
Original file line number Diff line number Diff line change
Expand Up @@ -160,32 +160,35 @@ 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
* ->scale_step from going to crazy values, if the device can't
* keep up.
*/
if (rqd->max_depth == 1)
return;
return false;

if (rqd->scale_step < 0 && hard_throttle)
rqd->scale_step = 0;
Expand All @@ -194,6 +197,7 @@ void rq_depth_scale_down(struct rq_depth *rqd, bool hard_throttle)

rqd->scaled_max = false;
rq_depth_calc_max_depth(rqd);
return true;
}

struct rq_qos_wait_data {
Expand Down
4 changes: 2 additions & 2 deletions block/blk-rq-qos.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,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);
Expand Down
6 changes: 4 additions & 2 deletions block/blk-wbt.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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");
Expand Down

0 comments on commit b84477d

Please sign in to comment.