Re: [PATCH 2/3] blk-cgroup: store blkcg in bio instead of blkg
From: Nilay Shroff <hidden>
Date: 2026-09-06 10:57:57
Also in:
cgroups, dm-devel, gfs2, linux-bcache, linux-block, linux-doc, linux-fsdevel, linux-mm, linux-raid, lkml, nvdimm
On 8/23/26 7:00 PM, Yu Kuai wrote:
quoted hunk ↗ jump to hunk
@@ -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 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