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

[Shaohua Li <shli@kernel.org>]

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@kernel.org>
> Cc: Shaohua Li <shli@kernel.org>
> ---
>  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++;
>  		/*