Re: [PATCH 1/2] blk-throtl: make latency= absolute

From: Shaohua Li
Date: Thu Nov 09 2017 - 18:12:18 EST


On Thu, Nov 09, 2017 at 02:19:24PM -0800, Tejun Heo wrote:
> Currently, the latency= parameter specifies the extra latency on top
> of the estimated baseline latency. This doesn't seem ideal for the
> following two reasons.
>
> 1. Sometimes we want to use absolute target latencies, especially as
> the baseline latency estimation isn't always reliable.
>
> 2. If we want to set a latency target relative to the estimated
> baseline latency, it makes a lot more sense to express the target
> as a percentage of the baseline (say, 120% of the expected latency)
> rather than the baseline latency + an absolute offset.
>
> This patch makes the existing latency= parameter to be interpreted as
> an absolute latency target instead of an offset to the baseline. The
> next patch will add support for relative latency target expressed in
> terms of percentage.
>
> While this is a userland visible change, io.low support is still
> evoling and highly experimental. This isn't expected to break any
> actual usages.

The percentage latency makes sense, but the absolute latency doesn't to me. A
4k IO latency could be much smaller than 1M IO latency. If we don't add
baseline latency, we can't specify a latency target which works for both 4k and
1M IO.

Thanks,
Shaohua
> Signed-off-by: Tejun Heo <tj@xxxxxxxxxx>
> Cc: Shaohua Li <shli@xxxxxxxxxx>
> ---
> block/blk-throttle.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> --- a/block/blk-throttle.c
> +++ b/block/blk-throttle.c
> @@ -2299,8 +2299,7 @@ void blk_throtl_bio_endio(struct bio *bi
>
> bucket = request_bucket_index(
> blk_stat_size(&bio->bi_issue_stat));
> - threshold = tg->td->avg_buckets[bucket].latency +
> - tg->latency_target;
> + threshold = tg->latency_target;
> if (lat > threshold)
> tg->bad_bio_cnt++;
> /*