Hello,
On Fri, Mar 07, 2025 at 05:01:52PM +0800, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@xxxxxxxxxx>
>
> Commit 9f5ede3c01f9 ("block: throttle split bio in case of iops limit")
> support to account split IO for iops limit, because block layer provides
> io accounting against split bio.
>
> However, io merge is still not handled, while block layer doesn't
> account merged io for iops. Fix this problem by decreasing io_disp
> if bio is merged, and following IO can use the extra budget. If io merge
> concurrent with iops throttling, it's not handled if one more or one
> less bio is dispatched, this is fine because as long as new slice is not
> started, blk-throttle already preserve one extra slice for deviation,
> and it's not worth it to handle the case that iops_limit rate is less than
> one per slice.
>
> A regression test will be added for this case [1], before this patch,
> the test will fail:
>
> +++ /root/blktests-mainline/results/nodev/throtl/007.out.bad
> @@ -1,4 +1,4 @@
> Running throtl/007
> 1
> -1
> +11
> Test complete
>
> [1] https://lore.kernel.org/all/20250307080318.3860858-2-yukuai1@xxxxxxxxxxxxxxx/
For blk-throtl, iops limit has meant the number of bios issued. I'm not
necessarily against this change but this is significantly changing what a
given configuration means. Also, if we're now doing hardware request based
throttling, maybe we should just move this under rq-qos. That has the
problem of not supporting bio-based drivers but maybe we can leave
blk-throtl in deprecation mode and slowly phase it out.
Also, can you please make atomic_t conversion a separate patch and describe
why that's being done?
Thanks.
--
tejun
