Re: [PATCH] blk-throttle: fix repeat limit on bio with BIO_BPS_THROTTLED

From: Yu Kuai
Date: Mon Apr 22 2024 - 21:51:17 EST


Hi, Tejun!

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