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

[Bart Van Assche]

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@xxxxxxxxxxxxxxxx>
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.