Re: [RFC PATCH v2 2/4] blk-cgroup: use a request_queue rhashtable for blkg lookup

Hi，

在 2026/8/14 15:12, Christoph Hellwig 写道:
> On Tue, Aug 11, 2026 at 02:47:42PM +0800, Yu Kuai wrote:
>> Keep q->blkg_list for ordered policy and scheduler walks. Initialize and
>> destroy the hash with request_queue, and remove the radix-tree preload
>> paths which are no longer needed.
> Are these fast path operations?  Otherwise we can walk all rhashtable
> entries without an extra list, but it might be slower.

All users are from sysfs/cgroupfs API, I think they can be considered slow path,
however currently spinlock is held in these procedures, I think it's better to
convert them to blkg_lookup based iterate after spinlock is converted to the
blkcg_mutex.

>
>> @@ -191,10 +198,15 @@ static void blkg_release(struct percpu_ref *ref)
>>   {
>>   	struct blkcg_gq *blkg = container_of(ref, struct blkcg_gq, refcnt);
>>   	struct blkcg *blkcg = blkg->blkcg;
>>   	int cpu;
>>   
>> +	if (!list_empty(&blkg->q_node))
>> +		WARN_ON_ONCE(rhashtable_remove_fast(&blkg->q->blkg_hash,
>> +						    &blkg->q_hash_node,
>> +						    blkg_hash_params));
>> +
> The list_empty case is for initialization failure?  Or can we end up
> with that by other means?

Yes, this is for initialization failure, blkg_alloc() failure after percpu_ref_init(),
or blkg_create() failure before rhashtable_insert succeed.

>
>> + * Lookup a blkg for the @blkcg - @q pair, whether it is online or dying.
>> + *
>> + * Must be called in a RCU critical section.
>> + */
> Please add must_hold and/or lockdep annotations for this instead of just
> a comment.
>
> Also maybe mention that this does not acquire a reference and the caller
> must already hold one?

Perhaps it's more accurate that the blkg is pinned by IO or caller already
hold one?

>
>
-- 
Thanks,
Kuai