Re: [PATCH v4 1/1] block: Use RCU in blk_mq_[un]quiesce_tagset() instead of set->tag_list_lock

Bart Van Assche
2025-12-09T17:01:12Z

On 12/8/25 11:30 PM, Hannes Reinecke wrote:
>> @@ -4294,7 +4294,7 @@ static void blk_mq_del_queue_tag_set(struct
>> request_queue *q)
>>       struct blk_mq_tag_set *set = q->tag_set;
>>       mutex_lock(&set->tag_list_lock);
>> -    list_del(&q->tag_set_list);
>> +    list_del_rcu(&q->tag_set_list);
>>       if (list_is_singular(&set->tag_list)) {
>>           /* just transitioned to unshared */
>>           set->flags &= ~BLK_MQ_F_TAG_QUEUE_SHARED;
>> @@ -4302,7 +4302,6 @@ static void blk_mq_del_queue_tag_set(struct
>> request_queue *q)
>>           blk_mq_update_tag_set_shared(set, false);
>>       }
>>       mutex_unlock(&set->tag_list_lock);
>> -    INIT_LIST_HEAD(&q->tag_set_list);
>>   }
> I'm ever so sceptical whether we can remove the INIT_LIST_HEAD() here.
> If we can it was pointless to begin with, but I somehow doubt that.
> Do you have a rationale for that (except from the fact that you
> are moving to RCU, and hence the 'q' pointer might not be valid then).

My understanding is that calling INIT_LIST_HEAD() after list_del_rcu()
without letting a grace period expire first is not allowed because it
introduces a race condition. From the block layer git history:

commit a347c7ad8edf4c5685154f3fdc3c12fc1db800ba
Author: Roman Pen <roman.penyaev@profitbricks.com>
Date: Sun Jun 10 22:38:24 2018 +0200

blk-mq: reinit q->tag_set_list entry only after grace period

It is not allowed to reinit q->tag_set_list list entry while RCU grace
period has not completed yet, otherwise the following soft lockup in
blk_mq_sched_restart() happens: [ ... ]

diff --git a/block/blk-mq.c b/block/blk-mq.c
index d2de0a719ab8..2be78cc30ec5 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2349,7 +2349,6 @@ static void blk_mq_del_queue_tag_set(struct
request_queue *q)

mutex_lock(&set->tag_list_lock);
list_del_rcu(&q->tag_set_list);
- INIT_LIST_HEAD(&q->tag_set_list);
if (list_is_singular(&set->tag_list)) {
/* just transitioned to unshared */
set->flags &= ~BLK_MQ_F_TAG_SHARED;
@@ -2357,8 +2356,8 @@ static void blk_mq_del_queue_tag_set(struct
request_queue *q)
blk_mq_update_tag_set_depth(set, false);
}
mutex_unlock(&set->tag_list_lock);
-
synchronize_rcu();
+ INIT_LIST_HEAD(&q->tag_set_list);
}

Thanks,

Bart.