Received-SPF: pass (google.com: domain of linux-kernel+bounces-154405-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Subject: Re: [PATCH] blk-throttle: fix repeat limit on bio with
 BIO_BPS_THROTTLED
To: "tj@kernel.org" <tj@kernel.org>, Yu Kuai <yukuai1@huaweicloud.com>
Cc: =?UTF-8?B?5ZGo5rOw5a6H?= <zhoutaiyu@kuaishou.com>,
 "josef@toxicpanda.com" <josef@toxicpanda.com>,
 "axboe@kernel.dk" <axboe@kernel.dk>,
 "cgroups@vger.kernel.org" <cgroups@vger.kernel.org>,
 "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
 "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
 "yukuai (C)" <yukuai3@huawei.com>
References: <20240419120747.38031-1-zhoutaiyu@kuaishou.com>
 <ea781ccc-c29e-894e-c54a-f44ea349edca@huaweicloud.com>
 <e2d291e6b6ed43d89930eb2a7d459ff8@kuaishou.com>
 <fbf135e8-de16-8eb4-9ade-1b979a335e33@huaweicloud.com>
 <Ziag2TL_BqmTRK5D@slm.duckdns.org>
From: Yu Kuai <yukuai1@huaweicloud.com>
Message-ID: <283462e4-c2d7-8d70-bb0f-61db62a86e02@huaweicloud.com>
Date: Tue, 23 Apr 2024 09:50:58 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
Precedence: bulk
MIME-Version: 1.0
In-Reply-To: <Ziag2TL_BqmTRK5D@slm.duckdns.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit

Hi, Tejun!

在 2024/04/23 1:39, tj@kernel.org 写道:
> Hello, Yu Kuai.
> 
> On Mon, Apr 22, 2024 at 11:47:41AM +0800, Yu Kuai wrote:
>> Hi!
>>
>> 在 2024/04/22 11:33, 周泰宇 写道:
>>> What I want to do here was to set an easy to reach value to BPS_LIMIT (10M/s in this example) and an unable to reach value to IOPS_LIMIT (100000 in this example).
>>>
>>>
>>> Under this setting, the iostat shows that the bps is far less than 10M/s and sometimes is far larger than 10M/s.
>>
>> Yes, I know this behaviour, and this is because blk-throttle works
>> before IO split, and io stats is accounting bps for rq-based disk after
>> IO split, if you using Q2C for bps you'll see that bps is stable as
>> limit.
>>
>> Hi, Tejun！
>>
>> Do you think this *phenomenon* need to be fixed? If so, I don't see a
>> easy way other than throttle bio after *IO split*. Perhaps ohter than
>> bio merge case, this can be another motivation to move blk-throttle to
>> rq_qos_throttle().
> 
> Yeah, blk-throtl is sitting too early in the pipeline to easily track how
> the bios actually get issued. However, given that it's been available for
> bio-based drivers for a really long time, I don't think it'd be a good idea
> to move it, so iops limit is always going to be a bit unreliable w.r.t. what
> actually get issued to the device. So, IMHO, if the oddity is just about how
> IOs are counted, I don't think it's a critical problem on its own.

Got it, and agreed. Consider that bps limit for Q stage is stable,
although iostat can observe bps higher or lower sometimes, overall it
should be accurate.

Hi, Zhoutaiyu,

If you really want to fix this, you must come up with a solution with
the respect of FIFO rules, breaking it like this patch is not something
we'll accept, breaking fairness and some other flaws.

Thanks,
Kuai

> 
> Thanks.
>