Re: [PATCH RFC v2 5/6] blk-throttle: support to destroy throtl_data when blk-throttle is disabled

Yu Kuai
2024-04-17T01:39:49Z

Hi,

在 2024/04/17 9:22, Tejun Heo 写道:
> Hello,
>
> On Wed, Apr 17, 2024 at 09:13:34AM +0800, Yu Kuai wrote:
>>> Probably a better interface is for unloading to force blk-throtl to be
>>> deactivated rather than asking the user to nuke all configs.
>>
>> I was thinking that rmmod in this case should return busy, for example,
>> if bfq is currently used for some disk, rmmod bfq will return busy.
>>
>> Is there any example that unloading will deactivate resources that users
>> are still using?
>
> Hmm... yeah, I'm not sure. Pinning the module while in use is definitely
> more conventional, so let's stick with that. It's usually achieved by
> inc'ing the module's ref on each usage, so here, the module refs would be
> counting the number of active rules, I guess.

Yes, aggred.
>
> I'm not sure about modularization tho mostly because we've historically had
> a lot of lifetime issues around block and blkcg data structures and the
> supposed gain here is rather minimal. We only have a handful of these
> policies and they aren't that big.
>
> If hot path overhead when not being used is concern, lazy init solves most
> of it, no?

For performance, yes. Another point is that we can reduce kernel size
this way, without losing support for blk-cgroup policies.

Yes, it's just blk-throttle is the most pain in the ass becasue it
exposed lots of implementations to block layer. Other rq_qos based
policy should be much easier.

I guess I'll do lazy init first, and then modularization for rq_qos,
and leave blk-throtl there for now. Perhaps add a new throtl model in
iocost can replace blk-throtl in the future.

BTW, currently during test of iocost, I found that iocost can already
achieve that, for example, by following configure:

echo "$dev enable=1 min=100 max=100" > qos
echo "$dev wbps=4096 wseqiops=1 wrandiops=1" > model

In the test, I found that wbps and iops is actually limited to the
set value.

Thanks,
Kuai

>
> Thanks.
>