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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1e4c4cb6-e787-4078-b7b0-787bd45ebd78@acm.org>
Date: Tue, 9 Dec 2025 09:00:53 -0800
From: Bart Van Assche <bvanassche@....org>
To: Hannes Reinecke <hare@...e.de>,
 Mohamed Khalfella <mkhalfella@...estorage.com>,
 Chaitanya Kulkarni <kch@...dia.com>, Christoph Hellwig <hch@....de>,
 Jens Axboe <axboe@...nel.dk>, Keith Busch <kbusch@...nel.org>,
 Sagi Grimberg <sagi@...mberg.me>
Cc: Casey Chen <cachen@...estorage.com>,
 Yuanyuan Zhong <yzhong@...estorage.com>, Ming Lei <ming.lei@...hat.com>,
 Waiman Long <llong@...hat.com>, Hillf Danton <hdanton@...a.com>,
 linux-nvme@...ts.infradead.org, linux-block@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 1/1] block: Use RCU in blk_mq_[un]quiesce_tagset()
 instead of set->tag_list_lock

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