public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/1] block/blk-mq: use atomic_t for quiesce_depth to avoid lock contention on RT
@ 2026-05-06  6:56 Ionut Nechita (Wind River)
  2026-05-06  6:56 ` [PATCH v6 1/1] " Ionut Nechita (Wind River)
  0 siblings, 1 reply; 5+ messages in thread
From: Ionut Nechita (Wind River) @ 2026-05-06  6:56 UTC (permalink / raw)
  To: axboe, linux-block
  Cc: bigeasy, bvanassche, clrkwllms, rostedt, ming.lei, muchun.song,
	mkhalfella, chris.friesen, linux-kernel, linux-rt-devel,
	linux-rt-users, stable, ionut_n2001, sunlightlinux

Hi Jens,

This is v6 of the fix for the RT kernel performance regression caused by
commit 6bda857bcbb86 ("block: fix ordering between checking
QUEUE_FLAG_QUIESCED request adding").

Changes since v5 (Mar 3):
- Rewrote the memory-ordering comments per Bart Van Assche's review.
  The previous wording incorrectly described smp_mb__after_atomic() as
  ordering against "subsequent loads in blk_mq_run_hw_queue()". The
  comments now describe the actual reader/writer pairing: writer-side
  smp_mb__after_atomic() in blk_mq_quiesce_queue_nowait() and
  blk_mq_unquiesce_queue() pairs with reader-side smp_rmb() in
  blk_mq_run_hw_queue() so the re-check observes the latest
  quiesce_depth value.
- Rebased on top of linux-next (next-20260505).
- No functional / code-generation changes.

Changes since v4 (Feb 13):
- Rebased on top of linux-next (20260302)
- No code changes

Changes since v3 (Feb 11):
- Rebased on top of axboe/for-7.0/block
- Fixed Fixes tag commit hash to match upstream (6bda857bcbb86)
- Added Reviewed-by from Sebastian Andrzej Siewior
- No code changes

Changes since v2 (Feb 10):
- Replaced raw_spinlock_t quiesce_sync_lock with atomic_t for
  quiesce_depth, as suggested by Sebastian Andrzej Siewior
- Eliminated QUEUE_FLAG_QUIESCED entirely; blk_queue_quiesced() now
  checks atomic_read(&q->quiesce_depth) > 0
- Use atomic_dec_if_positive() in blk_mq_unquiesce_queue() to avoid
  race between WARN check and decrement
- Removed the unrelated blk_mq_run_hw_queues() async=true change
- Removed blk-mq-debugfs.c QUIESCED flag entry
- Uses smp_mb__after_atomic() / smp_rmb() for memory ordering instead
  of any spinlock in the hot path

Changes since v1 (RESEND, Jan 9):
- Rebased on top of axboe/for-7.0/block
- No code changes

The problem: on PREEMPT_RT kernels, the spinlock_t queue_lock added in
blk_mq_run_hw_queue() converts to a sleeping rt_mutex, causing all IRQ
threads (one per MSI-X vector) to serialize. On megaraid_sas with 128
MSI-X vectors and 120 hw queues, throughput drops from 640 MB/s to
153 MB/s.

The fix converts quiesce_depth to atomic_t, which serves as both the
depth tracker and the quiesce indicator (depth > 0 means quiesced).
This eliminates QUEUE_FLAG_QUIESCED and removes the need for any lock
in the hot path. Memory ordering is ensured by smp_mb__after_atomic()
on the writer side (after modifying quiesce_depth) paired with
smp_rmb() on the reader side (before re-checking quiesce state in
blk_mq_run_hw_queue()).

Link: https://lore.kernel.org/linux-block/20260303073744.20585-1-ionut.nechita@windriver.com/

Ionut Nechita (1):
  block/blk-mq: use atomic_t for quiesce_depth to avoid lock contention
    on RT

 block/blk-core.c       |  1 +
 block/blk-mq-debugfs.c |  1 -
 block/blk-mq.c         | 53 +++++++++++++++++++++---------------------
 include/linux/blkdev.h |  9 ++++---
 4 files changed, 34 insertions(+), 30 deletions(-)

--
2.54.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v6 1/1] block/blk-mq: use atomic_t for quiesce_depth to avoid lock contention on RT
  2026-05-06  6:56 [PATCH v6 0/1] block/blk-mq: use atomic_t for quiesce_depth to avoid lock contention on RT Ionut Nechita (Wind River)
@ 2026-05-06  6:56 ` Ionut Nechita (Wind River)
  2026-05-06  7:14   ` Bart Van Assche
  0 siblings, 1 reply; 5+ messages in thread
From: Ionut Nechita (Wind River) @ 2026-05-06  6:56 UTC (permalink / raw)
  To: axboe, linux-block
  Cc: bigeasy, bvanassche, clrkwllms, rostedt, ming.lei, muchun.song,
	mkhalfella, chris.friesen, linux-kernel, linux-rt-devel,
	linux-rt-users, stable, ionut_n2001, sunlightlinux

In RT kernel (PREEMPT_RT), commit 6bda857bcbb86 ("block: fix ordering
between checking QUEUE_FLAG_QUIESCED request adding") causes severe
performance regression on systems with multiple MSI-X interrupt
vectors.

The above change introduced spinlock_t queue_lock usage in
blk_mq_run_hw_queue() to synchronize QUEUE_FLAG_QUIESCED checks
with blk_mq_unquiesce_queue(). While this works correctly in
standard kernel, it causes catastrophic serialization in RT kernel
where spinlock_t converts to sleeping rt_mutex.

Problem in RT kernel:
- blk_mq_run_hw_queue() is called from IRQ thread context
- With multiple MSI-X vectors, all IRQ threads contend on
  the same queue_lock
- queue_lock becomes rt_mutex (sleeping) in RT kernel
- IRQ threads serialize and enter D-state waiting for lock
- Throughput drops from 640 MB/s to 153 MB/s

Solution:
Convert quiesce_depth to atomic_t and use it directly for quiesce
state checking, eliminating QUEUE_FLAG_QUIESCED entirely. This
removes the need for any locking in the hot path.

The atomic counter serves as both the depth tracker and the quiesce
indicator (depth > 0 means quiesced). This eliminates the race
window that existed between updating the depth and the flag.

Memory ordering is ensured by:
- smp_mb__after_atomic() after modifying quiesce_depth in
  blk_mq_quiesce_queue_nowait() and blk_mq_unquiesce_queue()
- smp_rmb() in blk_mq_run_hw_queue() before re-checking the
  quiesce state, paired with the writer-side barriers above

Performance impact:
- RT kernel: eliminates lock contention, restores full throughput
- Non-RT kernel: atomic ops are similar cost to the previous
  spinlock acquire/release, no regression expected

Test results on RT kernel:
Hardware: Broadcom/LSI MegaRAID 12GSAS/PCIe Secure SAS39xx
  (megaraid_sas driver, 128 MSI-X vectors, 120 hw queues)
- Before: 153 MB/s, IRQ threads in D-state
- After:  640 MB/s, no IRQ threads blocked

Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Fixes: 6bda857bcbb86 ("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-core.c       |  1 +
 block/blk-mq-debugfs.c |  1 -
 block/blk-mq.c         | 53 +++++++++++++++++++++---------------------
 include/linux/blkdev.h |  9 ++++---
 4 files changed, 34 insertions(+), 30 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 17450058ea6d..1cafcca0975a 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -434,6 +434,7 @@ struct request_queue *blk_alloc_queue(struct queue_limits *lim, int node_id)
 	mutex_init(&q->limits_lock);
 	mutex_init(&q->rq_qos_mutex);
 	spin_lock_init(&q->queue_lock);
+	atomic_set(&q->quiesce_depth, 0);
 
 	init_waitqueue_head(&q->mq_freeze_wq);
 	mutex_init(&q->mq_freeze_lock);
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 047ec887456b..1b0aec3036e6 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -89,7 +89,6 @@ static const char *const blk_queue_flag_name[] = {
 	QUEUE_FLAG_NAME(INIT_DONE),
 	QUEUE_FLAG_NAME(STATS),
 	QUEUE_FLAG_NAME(REGISTERED),
-	QUEUE_FLAG_NAME(QUIESCED),
 	QUEUE_FLAG_NAME(RQ_ALLOC_TIME),
 	QUEUE_FLAG_NAME(HCTX_ACTIVE),
 	QUEUE_FLAG_NAME(SQ_SCHED),
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 4c5c16cce4f8..c1281e4d4d83 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -260,12 +260,13 @@ EXPORT_SYMBOL_GPL(blk_mq_unfreeze_queue_non_owner);
  */
 void blk_mq_quiesce_queue_nowait(struct request_queue *q)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&q->queue_lock, flags);
-	if (!q->quiesce_depth++)
-		blk_queue_flag_set(QUEUE_FLAG_QUIESCED, q);
-	spin_unlock_irqrestore(&q->queue_lock, flags);
+	atomic_inc(&q->quiesce_depth);
+	/*
+	 * Pairs with smp_rmb() in blk_mq_run_hw_queue(): make the
+	 * incremented quiesce_depth observable to readers re-checking
+	 * the quiesce state, so they don't dispatch on a quiesced queue.
+	 */
+	smp_mb__after_atomic();
 }
 EXPORT_SYMBOL_GPL(blk_mq_quiesce_queue_nowait);
 
@@ -314,21 +315,23 @@ EXPORT_SYMBOL_GPL(blk_mq_quiesce_queue);
  */
 void blk_mq_unquiesce_queue(struct request_queue *q)
 {
-	unsigned long flags;
-	bool run_queue = false;
+	int depth;
 
-	spin_lock_irqsave(&q->queue_lock, flags);
-	if (WARN_ON_ONCE(q->quiesce_depth <= 0)) {
-		;
-	} else if (!--q->quiesce_depth) {
-		blk_queue_flag_clear(QUEUE_FLAG_QUIESCED, q);
-		run_queue = true;
-	}
-	spin_unlock_irqrestore(&q->queue_lock, flags);
+	depth = atomic_dec_if_positive(&q->quiesce_depth);
+	if (WARN_ON_ONCE(depth < 0))
+		return;
 
-	/* dispatch requests which are inserted during quiescing */
-	if (run_queue)
+	if (depth == 0) {
+		/*
+		 * Pairs with smp_rmb() in blk_mq_run_hw_queue(): make the
+		 * decrement of quiesce_depth observable before we kick the
+		 * hw queues, so a concurrent blk_mq_run_hw_queue() that
+		 * re-checks the state sees the queue as no longer quiesced.
+		 */
+		smp_mb__after_atomic();
+		/* dispatch requests which are inserted during quiescing */
 		blk_mq_run_hw_queues(q, true);
+	}
 }
 EXPORT_SYMBOL_GPL(blk_mq_unquiesce_queue);
 
@@ -2362,17 +2365,15 @@ void blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx, bool async)
 
 	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.
+		 * Re-check the quiesce state after a read barrier. Pairs with
+		 * smp_mb__after_atomic() in blk_mq_quiesce_queue_nowait() and
+		 * blk_mq_unquiesce_queue() so we don't miss rerunning the hw
+		 * queue when a concurrent unquiesce has just dropped the
+		 * quiesce_depth to zero.
 		 */
-		spin_lock_irqsave(&hctx->queue->queue_lock, flags);
+		smp_rmb();
 		need_run = blk_mq_hw_queue_need_run(hctx);
-		spin_unlock_irqrestore(&hctx->queue->queue_lock, flags);
 
 		if (!need_run)
 			return;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 890128cdea1c..5d582c70fb8a 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -521,7 +521,8 @@ struct request_queue {
 
 	spinlock_t		queue_lock;
 
-	int			quiesce_depth;
+	/* Atomic quiesce depth - also serves as quiesced indicator (depth > 0) */
+	atomic_t		quiesce_depth;
 
 	struct gendisk		*disk;
 
@@ -666,7 +667,6 @@ enum {
 	QUEUE_FLAG_INIT_DONE,		/* queue is initialized */
 	QUEUE_FLAG_STATS,		/* track IO start and completion times */
 	QUEUE_FLAG_REGISTERED,		/* queue has been registered to a disk */
-	QUEUE_FLAG_QUIESCED,		/* queue has been quiesced */
 	QUEUE_FLAG_RQ_ALLOC_TIME,	/* record rq->alloc_time_ns */
 	QUEUE_FLAG_HCTX_ACTIVE,		/* at least one blk-mq hctx is active */
 	QUEUE_FLAG_SQ_SCHED,		/* single queue style io dispatch */
@@ -704,7 +704,10 @@ void blk_queue_flag_clear(unsigned int flag, struct request_queue *q);
 #define blk_noretry_request(rq) \
 	((rq)->cmd_flags & (REQ_FAILFAST_DEV|REQ_FAILFAST_TRANSPORT| \
 			     REQ_FAILFAST_DRIVER))
-#define blk_queue_quiesced(q)	test_bit(QUEUE_FLAG_QUIESCED, &(q)->queue_flags)
+static inline bool blk_queue_quiesced(struct request_queue *q)
+{
+	return atomic_read(&q->quiesce_depth) > 0;
+}
 #define blk_queue_pm_only(q)	atomic_read(&(q)->pm_only)
 #define blk_queue_registered(q)	test_bit(QUEUE_FLAG_REGISTERED, &(q)->queue_flags)
 #define blk_queue_sq_sched(q)	test_bit(QUEUE_FLAG_SQ_SCHED, &(q)->queue_flags)
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v6 1/1] block/blk-mq: use atomic_t for quiesce_depth to avoid lock contention on RT
  2026-05-06  6:56 ` [PATCH v6 1/1] " Ionut Nechita (Wind River)
@ 2026-05-06  7:14   ` Bart Van Assche
  2026-05-06  7:47     ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 5+ messages in thread
From: Bart Van Assche @ 2026-05-06  7:14 UTC (permalink / raw)
  To: Ionut Nechita (Wind River), axboe, linux-block
  Cc: bigeasy, clrkwllms, rostedt, ming.lei, muchun.song, mkhalfella,
	chris.friesen, linux-kernel, linux-rt-devel, linux-rt-users,
	stable, ionut_n2001, sunlightlinux

On 5/6/26 8:56 AM, Ionut Nechita (Wind River) wrote:
>   void blk_mq_quiesce_queue_nowait(struct request_queue *q)
>   {
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&q->queue_lock, flags);
> -	if (!q->quiesce_depth++)
> -		blk_queue_flag_set(QUEUE_FLAG_QUIESCED, q);
> -	spin_unlock_irqrestore(&q->queue_lock, flags);
> +	atomic_inc(&q->quiesce_depth);
> +	/*
> +	 * Pairs with smp_rmb() in blk_mq_run_hw_queue(): make the
> +	 * incremented quiesce_depth observable to readers re-checking
> +	 * the quiesce state, so they don't dispatch on a quiesced queue.
> +	 */
> +	smp_mb__after_atomic();
>   }

No, this is not sufficient to guarantee that blk_mq_run_hw_queue() sees
the latest value of q->quiesce_depth. If you want to achieve that I
think the only option is to protect the atomic_inc() above with
hctx->queue->queue_lock.

> @@ -2362,17 +2365,15 @@ void blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx, bool async)
>   
>   	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.
> +		 * Re-check the quiesce state after a read barrier. Pairs with
> +		 * smp_mb__after_atomic() in blk_mq_quiesce_queue_nowait() and
> +		 * blk_mq_unquiesce_queue() so we don't miss rerunning the hw
> +		 * queue when a concurrent unquiesce has just dropped the
> +		 * quiesce_depth to zero.
>   		 */
> -		spin_lock_irqsave(&hctx->queue->queue_lock, flags);
> +		smp_rmb();
>   		need_run = blk_mq_hw_queue_need_run(hctx);
> -		spin_unlock_irqrestore(&hctx->queue->queue_lock, flags);

If the atomic_inc() in blk_mq_quiesce_queue_nowait() is protected by
hctx->queue->queue_lock then the above code doesn't have to be modified.

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v6 1/1] block/blk-mq: use atomic_t for quiesce_depth to avoid lock contention on RT
  2026-05-06  7:14   ` Bart Van Assche
@ 2026-05-06  7:47     ` Sebastian Andrzej Siewior
  2026-05-06  9:43       ` Bart Van Assche
  0 siblings, 1 reply; 5+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-05-06  7:47 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Ionut Nechita (Wind River), axboe, linux-block, clrkwllms,
	rostedt, ming.lei, muchun.song, mkhalfella, chris.friesen,
	linux-kernel, linux-rt-devel, linux-rt-users, stable, ionut_n2001,
	sunlightlinux

On 2026-05-06 09:14:33 [+0200], Bart Van Assche wrote:
> On 5/6/26 8:56 AM, Ionut Nechita (Wind River) wrote:
> >   void blk_mq_quiesce_queue_nowait(struct request_queue *q)
> >   {
> > -	unsigned long flags;
> > -
> > -	spin_lock_irqsave(&q->queue_lock, flags);
> > -	if (!q->quiesce_depth++)
> > -		blk_queue_flag_set(QUEUE_FLAG_QUIESCED, q);
> > -	spin_unlock_irqrestore(&q->queue_lock, flags);
> > +	atomic_inc(&q->quiesce_depth);
> > +	/*
> > +	 * Pairs with smp_rmb() in blk_mq_run_hw_queue(): make the
> > +	 * incremented quiesce_depth observable to readers re-checking
> > +	 * the quiesce state, so they don't dispatch on a quiesced queue.
> > +	 */
> > +	smp_mb__after_atomic();
> >   }
> 
> No, this is not sufficient to guarantee that blk_mq_run_hw_queue() sees
> the latest value of q->quiesce_depth. If you want to achieve that I
> think the only option is to protect the atomic_inc() above with
> hctx->queue->queue_lock.
> 
> > @@ -2362,17 +2365,15 @@ void blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx, bool async)
> >   	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.
> > +		 * Re-check the quiesce state after a read barrier. Pairs with
> > +		 * smp_mb__after_atomic() in blk_mq_quiesce_queue_nowait() and
> > +		 * blk_mq_unquiesce_queue() so we don't miss rerunning the hw
> > +		 * queue when a concurrent unquiesce has just dropped the
> > +		 * quiesce_depth to zero.
> >   		 */
> > -		spin_lock_irqsave(&hctx->queue->queue_lock, flags);
> > +		smp_rmb();
> >   		need_run = blk_mq_hw_queue_need_run(hctx);
> > -		spin_unlock_irqrestore(&hctx->queue->queue_lock, flags);
> 
> If the atomic_inc() in blk_mq_quiesce_queue_nowait() is protected by
> hctx->queue->queue_lock then the above code doesn't have to be modified.

But wouldn't the atomic_inc + barrier avoid the need to have the lock?
Isn't this a normal pattern? If the lock is kept, we could use
non-atomic ops here then. But this avoids having the lock.

> Thanks,
> 
> Bart.

Sebastian

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v6 1/1] block/blk-mq: use atomic_t for quiesce_depth to avoid lock contention on RT
  2026-05-06  7:47     ` Sebastian Andrzej Siewior
@ 2026-05-06  9:43       ` Bart Van Assche
  0 siblings, 0 replies; 5+ messages in thread
From: Bart Van Assche @ 2026-05-06  9:43 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Ionut Nechita (Wind River), axboe, linux-block, clrkwllms,
	rostedt, ming.lei, muchun.song, mkhalfella, chris.friesen,
	linux-kernel, linux-rt-devel, linux-rt-users, stable, ionut_n2001,
	sunlightlinux

On 5/6/26 9:47 AM, Sebastian Andrzej Siewior wrote:
> On 2026-05-06 09:14:33 [+0200], Bart Van Assche wrote:
>> If the atomic_inc() in blk_mq_quiesce_queue_nowait() is protected by
>> hctx->queue->queue_lock then the above code doesn't have to be modified.
> 
> But wouldn't the atomic_inc + barrier avoid the need to have the lock?
> Isn't this a normal pattern? If the lock is kept, we could use
> non-atomic ops here then. But this avoids having the lock.

I strongly prefer a spinlock + non-atomic variables rather than using an
atomic variable and barriers because algorithms that use a spinlock are
easier to verify.

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-05-06  9:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-06  6:56 [PATCH v6 0/1] block/blk-mq: use atomic_t for quiesce_depth to avoid lock contention on RT Ionut Nechita (Wind River)
2026-05-06  6:56 ` [PATCH v6 1/1] " Ionut Nechita (Wind River)
2026-05-06  7:14   ` Bart Van Assche
2026-05-06  7:47     ` Sebastian Andrzej Siewior
2026-05-06  9:43       ` Bart Van Assche

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox