linux-kernel - Re: [PATCH] blk-throttle: fix repeat limit on bio with BIO_BPS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 23 Apr 2024 09:50:58 +0800
From: Yu Kuai <yukuai1@...weicloud.com>
To: "tj@...nel.org" <tj@...nel.org>, Yu Kuai <yukuai1@...weicloud.com>
Cc: 周泰宇 <zhoutaiyu@...ishou.com>,
 "josef@...icpanda.com" <josef@...icpanda.com>,
 "axboe@...nel.dk" <axboe@...nel.dk>,
 "cgroups@...r.kernel.org" <cgroups@...r.kernel.org>,
 "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH] blk-throttle: fix repeat limit on bio with
 BIO_BPS_THROTTLED

Hi, Tejun!

在 2024/04/23 1:39, tj@...nel.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.
>