[PATCHv2 0/2] block: blk-rq-qos: replace static key with atomic bitop

[PATCHv2 0/2] block: blk-rq-qos: replace static key with atomic bitop

[Nilay Shroff]

Hi,

This patchset replaces the use of a static key in the I/O path (rq_qos_
xxx()) with an atomic queue flag (QUEUE_FLAG_QOS_ENABLED). This change
is made to eliminate a potential deadlock introduced by the use of static
keys in the blk-rq-qos infrastructure, as reported by lockdep during
blktests block/005[1].

The original static key approach was introduced to avoid unnecessary
dereferencing of q->rq_qos when no blk-rq-qos module (e.g., blk-wbt or
blk-iolatency) is configured. While efficient, enabling a static key at
runtime requires taking cpu_hotplug_lock and jump_label_mutex, which
becomes problematic if the queue is already frozen — causing a reverse
dependency on ->freeze_lock. This results in a lockdep splat indicating
a potential deadlock.

To resolve this, we now gate q->rq_qos access with a q->queue_flags
bitop (QUEUE_FLAG_QOS_ENABLED), avoiding the static key and the associated
locking altogether.

I compared both static key and atomic bitop implementations using ftrace
function graph tracer over ~50 invocations of rq_qos_issue() while ensuring
blk-wbt/blk-iolatency were disabled (i.e., no QoS functionality). For
easy comparision, I made rq_qos_issue() noinline. The comparision was
made on PowerPC machine.

Static Key disabled (QoS is not configured):
5d0: 00 00 00 60     nop    # patched in by static key framework
5d4: 20 00 80 4e     blr    # return (branch to link register)

Only a nop and blr (branch to link register) are executed — very lightweight.

atomic bitop (QoS is not configured):
5d0: 20 00 23 e9     ld      r9,32(r3)     # load q->queue_flags
5d4: 00 80 29 71     andi.   r9,r9,32768   # check QUEUE_FLAG_QOS_ENABLED (bit 15)
5d8: 20 00 82 4d     beqlr                 # return if bit not set

This performs an ld and andi. before returning. Slightly more work,
but q->queue_flags is typically hot in cache during I/O submission.

With Static Key (disabled):
Duration (us): min=0.668 max=0.816 avg≈0.750

With atomic bitop QUEUE_FLAG_QOS_ENABLED (bit not set):
Duration (us): min=0.684 max=0.834 avg≈0.759

As expected, both versions are almost similar in cost. The added latency
from an extra ld and andi. is in the range of ~9ns.

There're two patches in the series. The first patch replaces static key
with QUEUE_FLAG_QOS_ENABLED. The second patch ensures that we disable
the QUEUE_FLAG_QOS_ENABLED when the queue no longer has any associated
rq_qos policies.

As usual, feedback and review comments are welcome!

[1] https://lore.kernel.org/linux-block/4fdm37so3o4xricdgfosgmohn63aa7wj3ua4e5vpihoamwg3ui@fq42f5q5t5ic/


Changes from v1:
  - For debugging I made rq_qos_issue() noinline in my local workspace,
    but then inadvertently it slipped through the patchset upstream. So
    reverted it and made rq_qos_issue() inline again as earlier.
  - Added Reported-by and Closes tags in the first patch, which I
    obviously missed to add in the first version.

Nilay Shroff (2):
  block: avoid cpu_hotplug_lock depedency on freeze_lock
  block: clear QUEUE_FLAG_QOS_ENABLED in rq_qos_del()

 block/blk-mq-debugfs.c |  1 +
 block/blk-rq-qos.c     |  8 ++++----
 block/blk-rq-qos.h     | 43 +++++++++++++++++++++++++-----------------
 include/linux/blkdev.h |  1 +
 4 files changed, 32 insertions(+), 21 deletions(-)

-- 
2.50.1

[PATCHv2 1/2] block: avoid cpu_hotplug_lock depedency on freeze_lock

[Nilay Shroff]

A recent lockdep[1] splat observed while running blktest block/005
reveals a potential deadlock caused by the cpu_hotplug_lock dependency
on ->freeze_lock. This dependency was introduced by commit 033b667a823e
("block: blk-rq-qos: guard rq-qos helpers by static key").

That change added a static key to avoid fetching q->rq_qos when neither
blk-wbt nor blk-iolatency is configured. The static key dynamically
patches kernel text to a NOP when disabled, eliminating overhead of
fetching q->rq_qos in the I/O hot path. However, enabling a static key
at runtime requires acquiring both cpu_hotplug_lock and jump_label_mutex.
When this happens after the queue has already been frozen (i.e., while
holding ->freeze_lock), it creates a locking dependency from cpu_hotplug_
lock to ->freeze_lock, which leads to a potential deadlock reported by
lockdep [1].

To resolve this, replace the static key mechanism with q->queue_flags:
QUEUE_FLAG_QOS_ENABLED. This flag is evaluated in the fast path before
accessing q->rq_qos. If the flag is set, we proceed to fetch q->rq_qos;
otherwise, the access is skipped.

Since q->queue_flags is commonly accessed in IO hotpath and resides in
the first cacheline of struct request_queue, checking it imposes minimal
overhead while eliminating the deadlock risk.

This change avoids the lockdep splat without introducing performance
regressions.

[1] https://lore.kernel.org/linux-block/4fdm37so3o4xricdgfosgmohn63aa7wj3ua4e5vpihoamwg3ui@fq42f5q5t5ic/

Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Closes: https://lore.kernel.org/linux-block/4fdm37so3o4xricdgfosgmohn63aa7wj3ua4e5vpihoamwg3ui@fq42f5q5t5ic/
Fixes: 033b667a823e ("block: blk-rq-qos: guard rq-qos helpers by static key")
Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 block/blk-mq-debugfs.c |  1 +
 block/blk-rq-qos.c     |  6 ++----
 block/blk-rq-qos.h     | 43 +++++++++++++++++++++++++-----------------
 include/linux/blkdev.h |  1 +
 4 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 7ed3e71f2fc0..32c65efdda46 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -95,6 +95,7 @@ static const char *const blk_queue_flag_name[] = {
 	QUEUE_FLAG_NAME(SQ_SCHED),
 	QUEUE_FLAG_NAME(DISABLE_WBT_DEF),
 	QUEUE_FLAG_NAME(NO_ELV_SWITCH),
+	QUEUE_FLAG_NAME(QOS_ENABLED),
 };
 #undef QUEUE_FLAG_NAME
 
diff --git a/block/blk-rq-qos.c b/block/blk-rq-qos.c
index 848591fb3c57..460c04715321 100644
--- a/block/blk-rq-qos.c
+++ b/block/blk-rq-qos.c
@@ -2,8 +2,6 @@
 
 #include "blk-rq-qos.h"
 
-__read_mostly DEFINE_STATIC_KEY_FALSE(block_rq_qos);
-
 /*
  * Increment 'v', if 'v' is below 'below'. Returns true if we succeeded,
  * false if 'v' + 1 would be bigger than 'below'.
@@ -319,8 +317,8 @@ void rq_qos_exit(struct request_queue *q)
 		struct rq_qos *rqos = q->rq_qos;
 		q->rq_qos = rqos->next;
 		rqos->ops->exit(rqos);
-		static_branch_dec(&block_rq_qos);
 	}
+	blk_queue_flag_clear(QUEUE_FLAG_QOS_ENABLED, q);
 	mutex_unlock(&q->rq_qos_mutex);
 }
 
@@ -346,7 +344,7 @@ int rq_qos_add(struct rq_qos *rqos, struct gendisk *disk, enum rq_qos_id id,
 		goto ebusy;
 	rqos->next = q->rq_qos;
 	q->rq_qos = rqos;
-	static_branch_inc(&block_rq_qos);
+	blk_queue_flag_set(QUEUE_FLAG_QOS_ENABLED, q);
 
 	blk_mq_unfreeze_queue(q, memflags);
 
diff --git a/block/blk-rq-qos.h b/block/blk-rq-qos.h
index 39749f4066fb..c4242508fa5e 100644
--- a/block/blk-rq-qos.h
+++ b/block/blk-rq-qos.h
@@ -12,7 +12,6 @@
 #include "blk-mq-debugfs.h"
 
 struct blk_mq_debugfs_attr;
-extern struct static_key_false block_rq_qos;
 
 enum rq_qos_id {
 	RQ_QOS_WBT,
@@ -113,43 +112,50 @@ void __rq_qos_queue_depth_changed(struct rq_qos *rqos);
 
 static inline void rq_qos_cleanup(struct request_queue *q, struct bio *bio)
 {
-	if (static_branch_unlikely(&block_rq_qos) && q->rq_qos)
+	if (unlikely(test_bit(QUEUE_FLAG_QOS_ENABLED, &q->queue_flags)) &&
+			q->rq_qos)
 		__rq_qos_cleanup(q->rq_qos, bio);
 }
 
 static inline void rq_qos_done(struct request_queue *q, struct request *rq)
 {
-	if (static_branch_unlikely(&block_rq_qos) && q->rq_qos &&
-	    !blk_rq_is_passthrough(rq))
+	if (unlikely(test_bit(QUEUE_FLAG_QOS_ENABLED, &q->queue_flags)) &&
+			q->rq_qos && !blk_rq_is_passthrough(rq))
 		__rq_qos_done(q->rq_qos, rq);
 }
 
 static inline void rq_qos_issue(struct request_queue *q, struct request *rq)
 {
-	if (static_branch_unlikely(&block_rq_qos) && q->rq_qos)
+	if (unlikely(test_bit(QUEUE_FLAG_QOS_ENABLED, &q->queue_flags)) &&
+			q->rq_qos)
 		__rq_qos_issue(q->rq_qos, rq);
 }
 
 static inline void rq_qos_requeue(struct request_queue *q, struct request *rq)
 {
-	if (static_branch_unlikely(&block_rq_qos) && q->rq_qos)
+	if (unlikely(test_bit(QUEUE_FLAG_QOS_ENABLED, &q->queue_flags)) &&
+			q->rq_qos)
 		__rq_qos_requeue(q->rq_qos, rq);
 }
 
 static inline void rq_qos_done_bio(struct bio *bio)
 {
-	if (static_branch_unlikely(&block_rq_qos) &&
-	    bio->bi_bdev && (bio_flagged(bio, BIO_QOS_THROTTLED) ||
-			     bio_flagged(bio, BIO_QOS_MERGED))) {
-		struct request_queue *q = bdev_get_queue(bio->bi_bdev);
-		if (q->rq_qos)
-			__rq_qos_done_bio(q->rq_qos, bio);
-	}
+	struct request_queue *q;
+
+	if (!bio->bi_bdev || (!bio_flagged(bio, BIO_QOS_THROTTLED) &&
+			     !bio_flagged(bio, BIO_QOS_MERGED)))
+		return;
+
+	q = bdev_get_queue(bio->bi_bdev);
+	if (unlikely(test_bit(QUEUE_FLAG_QOS_ENABLED, &q->queue_flags)) &&
+			q->rq_qos)
+		__rq_qos_done_bio(q->rq_qos, bio);
 }
 
 static inline void rq_qos_throttle(struct request_queue *q, struct bio *bio)
 {
-	if (static_branch_unlikely(&block_rq_qos) && q->rq_qos) {
+	if (unlikely(test_bit(QUEUE_FLAG_QOS_ENABLED, &q->queue_flags)) &&
+			q->rq_qos) {
 		bio_set_flag(bio, BIO_QOS_THROTTLED);
 		__rq_qos_throttle(q->rq_qos, bio);
 	}
@@ -158,14 +164,16 @@ static inline void rq_qos_throttle(struct request_queue *q, struct bio *bio)
 static inline void rq_qos_track(struct request_queue *q, struct request *rq,
 				struct bio *bio)
 {
-	if (static_branch_unlikely(&block_rq_qos) && q->rq_qos)
+	if (unlikely(test_bit(QUEUE_FLAG_QOS_ENABLED, &q->queue_flags)) &&
+			q->rq_qos)
 		__rq_qos_track(q->rq_qos, rq, bio);
 }
 
 static inline void rq_qos_merge(struct request_queue *q, struct request *rq,
 				struct bio *bio)
 {
-	if (static_branch_unlikely(&block_rq_qos) && q->rq_qos) {
+	if (unlikely(test_bit(QUEUE_FLAG_QOS_ENABLED, &q->queue_flags)) &&
+			q->rq_qos) {
 		bio_set_flag(bio, BIO_QOS_MERGED);
 		__rq_qos_merge(q->rq_qos, rq, bio);
 	}
@@ -173,7 +181,8 @@ static inline void rq_qos_merge(struct request_queue *q, struct request *rq,
 
 static inline void rq_qos_queue_depth_changed(struct request_queue *q)
 {
-	if (static_branch_unlikely(&block_rq_qos) && q->rq_qos)
+	if (unlikely(test_bit(QUEUE_FLAG_QOS_ENABLED, &q->queue_flags)) &&
+			q->rq_qos)
 		__rq_qos_queue_depth_changed(q->rq_qos);
 }
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 95886b404b16..fe1797bbec42 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -656,6 +656,7 @@ enum {
 	QUEUE_FLAG_SQ_SCHED,		/* single queue style io dispatch */
 	QUEUE_FLAG_DISABLE_WBT_DEF,	/* for sched to disable/enable wbt */
 	QUEUE_FLAG_NO_ELV_SWITCH,	/* can't switch elevator any more */
+	QUEUE_FLAG_QOS_ENABLED,		/* qos is enabled */
 	QUEUE_FLAG_MAX
 };
 
-- 
2.50.1

[PATCHv2 2/2] block: clear QUEUE_FLAG_QOS_ENABLED in rq_qos_del()

[Nilay Shroff]

When a QoS function is removed via rq_qos_del(), and it happens to be the
last QoS function on the request queue, q->rq_qos becomes NULL. In this
case, the QUEUE_FLAG_QOS_ENABLED bit should also be cleared to reflect
that no QoS hooks remain active.

This patch ensures that the QUEUE_FLAG_QOS_ENABLED flag is cleared if the
queue no longer has any associated rq_qos policies. Failing to do so
could cause unnecessary dereferences of a now-null q->rq_qos pointer in
the I/O path.

Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 block/blk-rq-qos.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/blk-rq-qos.c b/block/blk-rq-qos.c
index 460c04715321..654478dfbc20 100644
--- a/block/blk-rq-qos.c
+++ b/block/blk-rq-qos.c
@@ -375,6 +375,8 @@ void rq_qos_del(struct rq_qos *rqos)
 			break;
 		}
 	}
+	if (!q->rq_qos)
+		blk_queue_flag_clear(QUEUE_FLAG_QOS_ENABLED, q);
 	blk_mq_unfreeze_queue(q, memflags);
 
 	mutex_lock(&q->debugfs_mutex);
-- 
2.50.1

Re: [PATCHv2 1/2] block: avoid cpu_hotplug_lock depedency on freeze_lock

[Yu Kuai]

Hi,

在 2025/08/06 1:17, Nilay Shroff 写道:
> A recent lockdep[1] splat observed while running blktest block/005
> reveals a potential deadlock caused by the cpu_hotplug_lock dependency
> on ->freeze_lock. This dependency was introduced by commit 033b667a823e
> ("block: blk-rq-qos: guard rq-qos helpers by static key").
> 
> That change added a static key to avoid fetching q->rq_qos when neither
> blk-wbt nor blk-iolatency is configured. The static key dynamically
> patches kernel text to a NOP when disabled, eliminating overhead of
> fetching q->rq_qos in the I/O hot path. However, enabling a static key
> at runtime requires acquiring both cpu_hotplug_lock and jump_label_mutex.
> When this happens after the queue has already been frozen (i.e., while
> holding ->freeze_lock), it creates a locking dependency from cpu_hotplug_
> lock to ->freeze_lock, which leads to a potential deadlock reported by
> lockdep [1].
> 
> To resolve this, replace the static key mechanism with q->queue_flags:
> QUEUE_FLAG_QOS_ENABLED. This flag is evaluated in the fast path before
> accessing q->rq_qos. If the flag is set, we proceed to fetch q->rq_qos;
> otherwise, the access is skipped.
> 
> Since q->queue_flags is commonly accessed in IO hotpath and resides in
> the first cacheline of struct request_queue, checking it imposes minimal
> overhead while eliminating the deadlock risk.
> 
> This change avoids the lockdep splat without introducing performance
> regressions.
> 
> [1] https://lore.kernel.org/linux-block/4fdm37so3o4xricdgfosgmohn63aa7wj3ua4e5vpihoamwg3ui@fq42f5q5t5ic/
> 
> Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Closes: https://lore.kernel.org/linux-block/4fdm37so3o4xricdgfosgmohn63aa7wj3ua4e5vpihoamwg3ui@fq42f5q5t5ic/
> Fixes: 033b667a823e ("block: blk-rq-qos: guard rq-qos helpers by static key")
> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
> ---
>   block/blk-mq-debugfs.c |  1 +
>   block/blk-rq-qos.c     |  6 ++----
>   block/blk-rq-qos.h     | 43 +++++++++++++++++++++++++-----------------
>   include/linux/blkdev.h |  1 +
>   4 files changed, 30 insertions(+), 21 deletions(-)
> 
This patch LGTM, just one nit below.

> diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
> index 7ed3e71f2fc0..32c65efdda46 100644
> --- a/block/blk-mq-debugfs.c
> +++ b/block/blk-mq-debugfs.c
> @@ -95,6 +95,7 @@ static const char *const blk_queue_flag_name[] = {
>   	QUEUE_FLAG_NAME(SQ_SCHED),
>   	QUEUE_FLAG_NAME(DISABLE_WBT_DEF),
>   	QUEUE_FLAG_NAME(NO_ELV_SWITCH),
> +	QUEUE_FLAG_NAME(QOS_ENABLED),
>   };
>   #undef QUEUE_FLAG_NAME
>   
> diff --git a/block/blk-rq-qos.c b/block/blk-rq-qos.c
> index 848591fb3c57..460c04715321 100644
> --- a/block/blk-rq-qos.c
> +++ b/block/blk-rq-qos.c
> @@ -2,8 +2,6 @@
>   
>   #include "blk-rq-qos.h"
>   
> -__read_mostly DEFINE_STATIC_KEY_FALSE(block_rq_qos);
> -
>   /*
>    * Increment 'v', if 'v' is below 'below'. Returns true if we succeeded,
>    * false if 'v' + 1 would be bigger than 'below'.
> @@ -319,8 +317,8 @@ void rq_qos_exit(struct request_queue *q)
>   		struct rq_qos *rqos = q->rq_qos;
>   		q->rq_qos = rqos->next;
>   		rqos->ops->exit(rqos);
> -		static_branch_dec(&block_rq_qos);
>   	}
> +	blk_queue_flag_clear(QUEUE_FLAG_QOS_ENABLED, q);
>   	mutex_unlock(&q->rq_qos_mutex);
>   }
>   
> @@ -346,7 +344,7 @@ int rq_qos_add(struct rq_qos *rqos, struct gendisk *disk, enum rq_qos_id id,
>   		goto ebusy;
>   	rqos->next = q->rq_qos;
>   	q->rq_qos = rqos;
> -	static_branch_inc(&block_rq_qos);
> +	blk_queue_flag_set(QUEUE_FLAG_QOS_ENABLED, q);
>   
>   	blk_mq_unfreeze_queue(q, memflags);
>   
> diff --git a/block/blk-rq-qos.h b/block/blk-rq-qos.h
> index 39749f4066fb..c4242508fa5e 100644
> --- a/block/blk-rq-qos.h
> +++ b/block/blk-rq-qos.h
> @@ -12,7 +12,6 @@
>   #include "blk-mq-debugfs.h"
>   
>   struct blk_mq_debugfs_attr;
> -extern struct static_key_false block_rq_qos;
>   
>   enum rq_qos_id {
>   	RQ_QOS_WBT,
> @@ -113,43 +112,50 @@ void __rq_qos_queue_depth_changed(struct rq_qos *rqos);
>   
>   static inline void rq_qos_cleanup(struct request_queue *q, struct bio *bio)
>   {
> -	if (static_branch_unlikely(&block_rq_qos) && q->rq_qos)
> +	if (unlikely(test_bit(QUEUE_FLAG_QOS_ENABLED, &q->queue_flags)) &&
> +			q->rq_qos)
>   		__rq_qos_cleanup(q->rq_qos, bio);
>   }
>   
>   static inline void rq_qos_done(struct request_queue *q, struct request *rq)
>   {
> -	if (static_branch_unlikely(&block_rq_qos) && q->rq_qos &&
> -	    !blk_rq_is_passthrough(rq))
> +	if (unlikely(test_bit(QUEUE_FLAG_QOS_ENABLED, &q->queue_flags)) &&
> +			q->rq_qos && !blk_rq_is_passthrough(rq))
>   		__rq_qos_done(q->rq_qos, rq);
>   }
>   
>   static inline void rq_qos_issue(struct request_queue *q, struct request *rq)
>   {
> -	if (static_branch_unlikely(&block_rq_qos) && q->rq_qos)
> +	if (unlikely(test_bit(QUEUE_FLAG_QOS_ENABLED, &q->queue_flags)) &&
> +			q->rq_qos)
>   		__rq_qos_issue(q->rq_qos, rq);
>   }
>   
>   static inline void rq_qos_requeue(struct request_queue *q, struct request *rq)
>   {
> -	if (static_branch_unlikely(&block_rq_qos) && q->rq_qos)
> +	if (unlikely(test_bit(QUEUE_FLAG_QOS_ENABLED, &q->queue_flags)) &&
> +			q->rq_qos)
>   		__rq_qos_requeue(q->rq_qos, rq);
>   }
>   
>   static inline void rq_qos_done_bio(struct bio *bio)
>   {
> -	if (static_branch_unlikely(&block_rq_qos) &&
> -	    bio->bi_bdev && (bio_flagged(bio, BIO_QOS_THROTTLED) ||
> -			     bio_flagged(bio, BIO_QOS_MERGED))) {
> -		struct request_queue *q = bdev_get_queue(bio->bi_bdev);
> -		if (q->rq_qos)
> -			__rq_qos_done_bio(q->rq_qos, bio);
> -	}
> +	struct request_queue *q;
> +
> +	if (!bio->bi_bdev || (!bio_flagged(bio, BIO_QOS_THROTTLED) &&
> +			     !bio_flagged(bio, BIO_QOS_MERGED)))
> +		return;
> +
> +	q = bdev_get_queue(bio->bi_bdev);
> +	if (unlikely(test_bit(QUEUE_FLAG_QOS_ENABLED, &q->queue_flags)) &&
> +			q->rq_qos)
This unlinkey doesn't make sense, BIO_QOS_THROTTLED or BIO_QOS_MERGED
already indicates rq_qos is enabled while issuing IO, and rq_qos should
still be valid until this IO is done.

Perhaps a prep cleanup patch to remove this checking?

Thanks,
Kuai

> +		__rq_qos_done_bio(q->rq_qos, bio);
>   }
>   
>   static inline void rq_qos_throttle(struct request_queue *q, struct bio *bio)
>   {
> -	if (static_branch_unlikely(&block_rq_qos) && q->rq_qos) {
> +	if (unlikely(test_bit(QUEUE_FLAG_QOS_ENABLED, &q->queue_flags)) &&
> +			q->rq_qos) {
>   		bio_set_flag(bio, BIO_QOS_THROTTLED);
>   		__rq_qos_throttle(q->rq_qos, bio);
>   	}
> @@ -158,14 +164,16 @@ static inline void rq_qos_throttle(struct request_queue *q, struct bio *bio)
>   static inline void rq_qos_track(struct request_queue *q, struct request *rq,
>   				struct bio *bio)
>   {
> -	if (static_branch_unlikely(&block_rq_qos) && q->rq_qos)
> +	if (unlikely(test_bit(QUEUE_FLAG_QOS_ENABLED, &q->queue_flags)) &&
> +			q->rq_qos)
>   		__rq_qos_track(q->rq_qos, rq, bio);
>   }
>   
>   static inline void rq_qos_merge(struct request_queue *q, struct request *rq,
>   				struct bio *bio)
>   {
> -	if (static_branch_unlikely(&block_rq_qos) && q->rq_qos) {
> +	if (unlikely(test_bit(QUEUE_FLAG_QOS_ENABLED, &q->queue_flags)) &&
> +			q->rq_qos) {
>   		bio_set_flag(bio, BIO_QOS_MERGED);
>   		__rq_qos_merge(q->rq_qos, rq, bio);
>   	}
> @@ -173,7 +181,8 @@ static inline void rq_qos_merge(struct request_queue *q, struct request *rq,
>   
>   static inline void rq_qos_queue_depth_changed(struct request_queue *q)
>   {
> -	if (static_branch_unlikely(&block_rq_qos) && q->rq_qos)
> +	if (unlikely(test_bit(QUEUE_FLAG_QOS_ENABLED, &q->queue_flags)) &&
> +			q->rq_qos)
>   		__rq_qos_queue_depth_changed(q->rq_qos);
>   }
>   
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 95886b404b16..fe1797bbec42 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -656,6 +656,7 @@ enum {
>   	QUEUE_FLAG_SQ_SCHED,		/* single queue style io dispatch */
>   	QUEUE_FLAG_DISABLE_WBT_DEF,	/* for sched to disable/enable wbt */
>   	QUEUE_FLAG_NO_ELV_SWITCH,	/* can't switch elevator any more */
> +	QUEUE_FLAG_QOS_ENABLED,		/* qos is enabled */
>   	QUEUE_FLAG_MAX
>   };
>   
> 

Re: [PATCHv2 2/2] block: clear QUEUE_FLAG_QOS_ENABLED in rq_qos_del()

[Yu Kuai]

Hi,

在 2025/08/06 1:17, Nilay Shroff 写道:
> When a QoS function is removed via rq_qos_del(), and it happens to be the
> last QoS function on the request queue, q->rq_qos becomes NULL. In this
> case, the QUEUE_FLAG_QOS_ENABLED bit should also be cleared to reflect
> that no QoS hooks remain active.
> 
> This patch ensures that the QUEUE_FLAG_QOS_ENABLED flag is cleared if the
> queue no longer has any associated rq_qos policies. Failing to do so
> could cause unnecessary dereferences of a now-null q->rq_qos pointer in
> the I/O path.
> 
> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
> ---
>   block/blk-rq-qos.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
There is no fixtag, and can be missing during backport easily.

I feel it's better to fix missing static_branch_dec() in rq_qos_del()
first, and then fix the deadlock problem.

Thanks,
Kuai

> diff --git a/block/blk-rq-qos.c b/block/blk-rq-qos.c
> index 460c04715321..654478dfbc20 100644
> --- a/block/blk-rq-qos.c
> +++ b/block/blk-rq-qos.c
> @@ -375,6 +375,8 @@ void rq_qos_del(struct rq_qos *rqos)
>   			break;
>   		}
>   	}
> +	if (!q->rq_qos)
> +		blk_queue_flag_clear(QUEUE_FLAG_QOS_ENABLED, q);
>   	blk_mq_unfreeze_queue(q, memflags);
>   
>   	mutex_lock(&q->debugfs_mutex);
> 

Re: [PATCHv2 1/2] block: avoid cpu_hotplug_lock depedency on freeze_lock

[Nilay Shroff]

On 8/6/25 6:48 AM, Yu Kuai wrote:
> Hi,
> 
> 在 2025/08/06 1:17, Nilay Shroff 写道:
>>     static inline void rq_qos_done_bio(struct bio *bio)
>>   {
>> -    if (static_branch_unlikely(&block_rq_qos) &&
>> -        bio->bi_bdev && (bio_flagged(bio, BIO_QOS_THROTTLED) ||
>> -                 bio_flagged(bio, BIO_QOS_MERGED))) {
>> -        struct request_queue *q = bdev_get_queue(bio->bi_bdev);
>> -        if (q->rq_qos)
>> -            __rq_qos_done_bio(q->rq_qos, bio);
>> -    }
>> +    struct request_queue *q;
>> +
>> +    if (!bio->bi_bdev || (!bio_flagged(bio, BIO_QOS_THROTTLED) &&
>> +                 !bio_flagged(bio, BIO_QOS_MERGED)))
>> +        return;
>> +
>> +    q = bdev_get_queue(bio->bi_bdev);
>> +    if (unlikely(test_bit(QUEUE_FLAG_QOS_ENABLED, &q->queue_flags)) &&
>> +            q->rq_qos)
> This unlinkey doesn't make sense, BIO_QOS_THROTTLED or BIO_QOS_MERGED
> already indicates rq_qos is enabled while issuing IO, and rq_qos should
> still be valid until this IO is done.
> 
Yes good point! Agreed that having BIO_QOS_THROTTLED or BIO_QOS_MERGED set,
implicitly implies that q->rq_qos must not be NULL.

> Perhaps a prep cleanup patch to remove this checking?
Yes I'd do that but lets wait for Jens or Ming if in case they have any
further comment before I spin v3.

Thanks,
--Nilay

Re: [PATCHv2 2/2] block: clear QUEUE_FLAG_QOS_ENABLED in rq_qos_del()

[Nilay Shroff]

On 8/6/25 6:55 AM, Yu Kuai wrote:
> Hi,
> 
> 在 2025/08/06 1:17, Nilay Shroff 写道:
>> When a QoS function is removed via rq_qos_del(), and it happens to be the
>> last QoS function on the request queue, q->rq_qos becomes NULL. In this
>> case, the QUEUE_FLAG_QOS_ENABLED bit should also be cleared to reflect
>> that no QoS hooks remain active.
>>
>> This patch ensures that the QUEUE_FLAG_QOS_ENABLED flag is cleared if the
>> queue no longer has any associated rq_qos policies. Failing to do so
>> could cause unnecessary dereferences of a now-null q->rq_qos pointer in
>> the I/O path.
>>
>> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
>> ---
>>   block/blk-rq-qos.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
> There is no fixtag, and can be missing during backport easily.
> 
> I feel it's better to fix missing static_branch_dec() in rq_qos_del()
> first, and then fix the deadlock problem.
> 
Yes makes sense, will do it in v3. But again lets wait for Jens and Ming
in case they have any further comment.

Thanks,
--Nilay

Re: [PATCHv2 0/2] block: blk-rq-qos: replace static key with atomic bitop

[Shinichiro Kawasaki]

On Aug 05, 2025 / 22:47, Nilay Shroff wrote:
> Hi,
> 
> This patchset replaces the use of a static key in the I/O path (rq_qos_
> xxx()) with an atomic queue flag (QUEUE_FLAG_QOS_ENABLED). This change
> is made to eliminate a potential deadlock introduced by the use of static
> keys in the blk-rq-qos infrastructure, as reported by lockdep during
> blktests block/005[1].

...

> [1] https://lore.kernel.org/linux-block/4fdm37so3o4xricdgfosgmohn63aa7wj3ua4e5vpihoamwg3ui@fq42f5q5t5ic/

Thanks for this effort. I confirmed this series avoids the block/005 failure.
I'm not sure if this fix approach will be agreed upon, but if that is the case:

Tested-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>