Re: [PATCH v2 1/2] block/blk-mq: fix RT kernel regression with queue_lock in hot path

[Muchun Song <muchun.song@linux.dev>]

On 2025/12/23 04:15, Ionut Nechita (WindRiver) wrote:
> From: Ionut Nechita <ionut.nechita@windriver.com>
>
> Commit 679b1874eba7 ("block: fix ordering between checking
> QUEUE_FLAG_QUIESCED request adding") introduced queue_lock acquisition
> in blk_mq_run_hw_queue() to synchronize QUEUE_FLAG_QUIESCED checks.
>
> On RT kernels (CONFIG_PREEMPT_RT), regular spinlocks are converted to
> rt_mutex (sleeping locks). When multiple MSI-X IRQ threads process I/O
> completions concurrently, they contend on queue_lock in the hot path,
> causing all IRQ threads to enter D (uninterruptible sleep) state. This
> serializes interrupt processing completely.
>
> Test case (MegaRAID 12GSAS with 8 MSI-X vectors on RT kernel):
> - Good (v6.6.52-rt):  640 MB/s sequential read
> - Bad  (v6.6.64-rt):  153 MB/s sequential read (-76% regression)
> - 6-8 out of 8 MSI-X IRQ threads stuck in D-state waiting on queue_lock
>
> The original commit message mentioned memory barriers as an alternative
> approach. Use full memory barriers (smp_mb) instead of queue_lock to
> provide the same ordering guarantees without sleeping in RT kernel.
>
> Memory barriers ensure proper synchronization:
> - CPU0 either sees QUEUE_FLAG_QUIESCED cleared, OR
> - CPU1 sees dispatch list/sw queue bitmap updates
>
> This maintains correctness while avoiding lock contention that causes
> RT kernel IRQ threads to sleep in the I/O completion path.
>
> Fixes: 679b1874eba7 ("block: fix ordering between checking QUEUE_FLAG_QUIESCED request adding")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ionut Nechita <ionut.nechita@windriver.com>
> ---
>   block/blk-mq.c | 19 ++++++++-----------
>   1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 5da948b07058..5fb8da4958d0 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -2292,22 +2292,19 @@ void blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx, bool async)
>   
>   	might_sleep_if(!async && hctx->flags & BLK_MQ_F_BLOCKING);
>   
> +	/*
> +	 * First lockless check to avoid unnecessary overhead.
> +	 * Memory barrier below synchronizes with blk_mq_unquiesce_queue().
> +	 */
>   	need_run = blk_mq_hw_queue_need_run(hctx);
>   	if (!need_run) {
> -		unsigned long flags;
> -
> -		/*
> -		 * Synchronize with blk_mq_unquiesce_queue(), because we check
> -		 * if hw queue is quiesced locklessly above, we need the use
> -		 * ->queue_lock to make sure we see the up-to-date status to
> -		 * not miss rerunning the hw queue.
> -		 */
> -		spin_lock_irqsave(&hctx->queue->queue_lock, flags);
> +		/* Synchronize with blk_mq_unquiesce_queue() */

Memory barriers must be used in pairs. So how to synchronize?

> +		smp_mb();
>   		need_run = blk_mq_hw_queue_need_run(hctx);
> -		spin_unlock_irqrestore(&hctx->queue->queue_lock, flags);
> -
>   		if (!need_run)
>   			return;
> +		/* Ensure dispatch list/sw queue updates visible before execution */
> +		smp_mb();

Why we need another barrier? What order does this barrier guarantee?

Thanks.
>   	}
>   
>   	if (async || !cpumask_test_cpu(raw_smp_processor_id(), hctx->cpumask)) {