linux-kernel - Re: [PATCH] blk-throttle: check policy bit in blk_throtl

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <5b626062-d2c2-4811-bb0f-2e4276ad28c0@yukuai.org.cn>
Date: Thu, 4 Sep 2025 18:59:20 +0800
From: Yu Kuai <hailan@...uai.org.cn>
To: Han Guangjiang <gj.han@...mail.com>, yukuai1@...weicloud.com
Cc: axboe@...nel.dk, fanggeng@...iang.com, hanguangjiang@...iang.com,
 liangjie@...iang.com, linux-block@...r.kernel.org,
 linux-kernel@...r.kernel.org, yangchen11@...iang.com, yukuai3@...wei.com
Subject: Re: [PATCH] blk-throttle: check policy bit in blk_throtl_activated()

Hi,

在 2025/9/4 18:05, Han Guangjiang 写道:
> Hi,
>
>>>    static inline bool blk_throtl_activated(struct request_queue *q)
>>>    {
>>> -        return q->td != NULL;
>>> +        return q->td != NULL && test_bit(blkcg_policy_throtl.plid, q->blkcg_pols);
>>>    }
>> You can just remove the fist checking, p->td must be set if policy is
>> enabled. And please make blkcg_policy_enabled() inline function in
>> blk-cgroup.h and use it here.
> We intentionally kept the q->td != NULL check because we cannot guarantee
> that the policy module is fully loaded when this function is called.
> If the policy module is not loaded yet, blkcg_policy_throtl.plid might not be
> assigned, which could cause the test_bit() check to be incorrect.
>
> By keeping this check, we ensure that we have at least reached the cgroup
> configuration flow, indicating that the policy loading is complete.
>
> I'm wondering if there are any risks here and whether we should remove
> the q->td != NULL check?

I think there is none. blk-throttle can't be build as module, if config is n,
blk_throtl_bio() is a non-function, if config is y, policy is registered during
init. And throtl_init() failure will panic, noted blkcg_policy_register() will
never fail for blk-throttle. BTW, policy pid is not a dynamic value at runtime.

Perhaps remove the checking and add some comments?

Thanks,
Kuai

>
> Thanks,
> Han Guangjiang
>
>