Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BF48C21911
Date:   Thu, 9 Nov 2017 15:12:12 -0800
From:   Shaohua Li <shli@kernel.org>
To:     Tejun Heo <tj@kernel.org>
Cc:     Jens Axboe <axboe@kernel.dk>, linux-kernel@vger.kernel.org,
        kernel-team@fb.com
Subject: Re: [PATCH 1/2] blk-throtl: make latency= absolute
Message-ID: <20171109231212.mbqwyzpmciyshxov@kernel.org>
References: <20171109221924.GB983427@devbig577.frc2.facebook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20171109221924.GB983427@devbig577.frc2.facebook.com>
User-Agent: NeoMutt/20170609 (1.8.3)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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++;
>  		/*

From 1583628779065184979@xxx Thu Nov 09 22:21:33 +0000 2017
X-GM-THRID: 1583628779065184979
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread