Re: [PATCH 2/3] blk-cgroup: store blkcg in bio instead of blkg

["yu kuai" <yukuai@fygo.io>]

Hi,

在 2026/9/6 18:56, Nilay Shroff 写道:
> On 8/23/26 7:00 PM, Yu Kuai wrote:
>> @@ -257,20 +262,21 @@ static inline bool 
>> bio_issue_as_root_blkg(struct bio *bio)
>>    * @blkcg: blkcg of interest
>>    * @q: request_queue of interest
>>    *
>>    * Lookup a blkg for the @blkcg - @q pair, whether it is online or 
>> dying.
>>    *
>> - * Must be called in a RCU critical section.
>> + * Must be called with either RCU read lock or queue_lock held.
>>    *
>>    * This does not acquire a reference.  The caller must already hold 
>> one, or
>>    * have the blkg pinned by I/O.
>>    */
>>   static inline struct blkcg_gq *blkg_lookup_any(struct blkcg *blkcg,
>>                              struct request_queue *q)
>>   {
>> -    RCU_LOCKDEP_WARN(!rcu_read_lock_held(),
>> -             "blkg_lookup_any() requires an RCU read lock");
>> +    RCU_LOCKDEP_WARN(!rcu_read_lock_held() &&
>> +             !lockdep_is_held(&q->queue_lock),
>> +             "blkg_lookup_any() requires an RCU read lock or 
>> queue_lock");
>>         return rhashtable_lookup(&q->blkg_hash, &blkcg->css.id,
>>                    blkg_hash_params);
>>   }
>
> The changes look good to me. However, rhashtable_lookup() is annotated
> with __must_hold_shared(RCU). Since we now officially support Clang
> context annotations in the block layer, compiling with the latest
> Clang/LLVM generates a number of context warnings, for example:
>
> block/blk-cgroup.h:277:9: warning: calling function 'rhashtable_lookup'
> requires holding __ctx_lock_RCU 'RCU' [-Wthread-safety-analysis]
>   277 |         return rhashtable_lookup(&q->blkg_hash, &blkcg->css.id,
>       |                ^
>
> The same warning is reported from several other block layer source
> files that include blk-cgroup.h.
>
> This indicates that rhashtable_lookup() requires the RCU read lock
> from Clang's context-analysis perspective. Looking at the call paths of
> blkg_lookup_any(), it appears that not all callers hold
> rcu_read_lock() when invoking it.

I was not aware of the above clang warning, this should not be a real problem
because queue_lock should have the same protection as rcu, and I was trying
not to make this patch too complex.

I add the rcu_read_lock() in the following patch to covert queue_lock to blkcg_mutex:

[RFC PATCH v3 3/6] blk-cgroup: protect blkgs with blkcg_mutex - Yu Kuai <https://lore.kernel.org/all/20260823152926.1043863-4-yukuai@kernel.org/>

Since this is related to compile warning, I'll move the rcu protection to this
patch.

>
> I think we should fix those call paths to acquire rcu reader lock
> before calling blkg_lookup_any(), rather than weakening or bypassing
> the context annotation in blkg_lookup_any().
>
> Thanks,
> --Nilay

-- 
Thanks,
Kuai