[PATCH RESEND 0/5] block/blk-rq-qos: fix incorrect lock order for rq_qos_mutex and freeze queue

[PATCH RESEND 0/5] block/blk-rq-qos: fix incorrect lock order for rq_qos_mutex and freeze queue

[Yu Kuai]

 - Resend with kernel.org email server.

Currently rq_qos_add() will be called with rq_qos_mutex held, and freeze
queue, this is not expected. This set make sure queu is always freezed
before holding rq_qos_mutex.

Yu Kuai (5):
  block/blk-rq-qos: add a new helper rq_qos_add_freezed()
  blk-wbt: fix incorrect lock order for rq_qos_mutex and freeze queue
  blk-iocost: fix incorrect lock order for rq_qos_mutex and freeze queue
  blk-iolatency: fix incorrect lock order for rq_qos_mutex and freeze
    queue
  block/blk-rq-qos: cleanup rq_qos_add()

 block/blk-iocost.c    | 15 +++++++--------
 block/blk-iolatency.c | 11 +++++++----
 block/blk-rq-qos.c    | 23 ++++++-----------------
 block/blk-rq-qos.h    |  4 ++--
 block/blk-wbt.c       |  6 +++++-
 5 files changed, 27 insertions(+), 32 deletions(-)

-- 
2.51.0

[PATCH RESEND 1/5] block/blk-rq-qos: add a new helper rq_qos_add_freezed()

[Yu Kuai]

queue should not be freezed under rq_qos_mutex, see example index
commit 9730763f4756 ("block: correct locking order for protecting blk-wbt
parameters"), which means current implementation of rq_qos_add() is
problematic. Add a new helper and prepare to fix this problem in
following patches.

Signed-off-by: Yu Kuai <yukuai@fnnas.com>
---
 block/blk-rq-qos.c | 27 +++++++++++++++++++++++++++
 block/blk-rq-qos.h |  2 ++
 2 files changed, 29 insertions(+)

diff --git a/block/blk-rq-qos.c b/block/blk-rq-qos.c
index 654478dfbc20..353397d7e126 100644
--- a/block/blk-rq-qos.c
+++ b/block/blk-rq-qos.c
@@ -322,6 +322,33 @@ void rq_qos_exit(struct request_queue *q)
 	mutex_unlock(&q->rq_qos_mutex);
 }
 
+int rq_qos_add_freezed(struct rq_qos *rqos, struct gendisk *disk,
+		       enum rq_qos_id id, const struct rq_qos_ops *ops)
+{
+	struct request_queue *q = disk->queue;
+
+	WARN_ON_ONCE(q->mq_freeze_depth == 0);
+	lockdep_assert_held(&q->rq_qos_mutex);
+
+	if (rq_qos_id(q, id))
+		return -EBUSY;
+
+	rqos->disk = disk;
+	rqos->id = id;
+	rqos->ops = ops;
+	rqos->next = q->rq_qos;
+	q->rq_qos = rqos;
+	blk_queue_flag_set(QUEUE_FLAG_QOS_ENABLED, q);
+
+	if (rqos->ops->debugfs_attrs) {
+		mutex_lock(&q->debugfs_mutex);
+		blk_mq_debugfs_register_rqos(rqos);
+		mutex_unlock(&q->debugfs_mutex);
+	}
+
+	return 0;
+}
+
 int rq_qos_add(struct rq_qos *rqos, struct gendisk *disk, enum rq_qos_id id,
 		const struct rq_qos_ops *ops)
 {
diff --git a/block/blk-rq-qos.h b/block/blk-rq-qos.h
index b538f2c0febc..4a7fec01600b 100644
--- a/block/blk-rq-qos.h
+++ b/block/blk-rq-qos.h
@@ -87,6 +87,8 @@ static inline void rq_wait_init(struct rq_wait *rq_wait)
 
 int rq_qos_add(struct rq_qos *rqos, struct gendisk *disk, enum rq_qos_id id,
 		const struct rq_qos_ops *ops);
+int rq_qos_add_freezed(struct rq_qos *rqos, struct gendisk *disk,
+		       enum rq_qos_id id, const struct rq_qos_ops *ops);
 void rq_qos_del(struct rq_qos *rqos);
 
 typedef bool (acquire_inflight_cb_t)(struct rq_wait *rqw, void *private_data);
-- 
2.51.0

Re: [PATCH RESEND 1/5] block/blk-rq-qos: add a new helper rq_qos_add_freezed()

[Nilay Shroff]

On 11/16/25 9:40 AM, Yu Kuai wrote:
> queue should not be freezed under rq_qos_mutex, see example index
> commit 9730763f4756 ("block: correct locking order for protecting blk-wbt
> parameters"), which means current implementation of rq_qos_add() is
> problematic. Add a new helper and prepare to fix this problem in
> following patches.
> 
> Signed-off-by: Yu Kuai <yukuai@fnnas.com>

Looks good to me:
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>

Re: [PATCH RESEND 1/5] block/blk-rq-qos: add a new helper rq_qos_add_freezed()

[Ming Lei]

On Sun, Nov 16, 2025 at 12:10:20PM +0800, Yu Kuai wrote:
> queue should not be freezed under rq_qos_mutex, see example index
> commit 9730763f4756 ("block: correct locking order for protecting blk-wbt
> parameters"), which means current implementation of rq_qos_add() is
> problematic. Add a new helper and prepare to fix this problem in
> following patches.
> 
> Signed-off-by: Yu Kuai <yukuai@fnnas.com>
> ---
>  block/blk-rq-qos.c | 27 +++++++++++++++++++++++++++
>  block/blk-rq-qos.h |  2 ++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/block/blk-rq-qos.c b/block/blk-rq-qos.c
> index 654478dfbc20..353397d7e126 100644
> --- a/block/blk-rq-qos.c
> +++ b/block/blk-rq-qos.c
> @@ -322,6 +322,33 @@ void rq_qos_exit(struct request_queue *q)
>  	mutex_unlock(&q->rq_qos_mutex);
>  }
>  
> +int rq_qos_add_freezed(struct rq_qos *rqos, struct gendisk *disk,
> +		       enum rq_qos_id id, const struct rq_qos_ops *ops)
> +{
> +	struct request_queue *q = disk->queue;
> +
> +	WARN_ON_ONCE(q->mq_freeze_depth == 0);
> +	lockdep_assert_held(&q->rq_qos_mutex);
> +
> +	if (rq_qos_id(q, id))
> +		return -EBUSY;
> +
> +	rqos->disk = disk;
> +	rqos->id = id;
> +	rqos->ops = ops;
> +	rqos->next = q->rq_qos;
> +	q->rq_qos = rqos;
> +	blk_queue_flag_set(QUEUE_FLAG_QOS_ENABLED, q);
> +
> +	if (rqos->ops->debugfs_attrs) {
> +		mutex_lock(&q->debugfs_mutex);
> +		blk_mq_debugfs_register_rqos(rqos);
> +		mutex_unlock(&q->debugfs_mutex);
> +	}

It will cause more lockdep splat to let q->debugfs_mutex depend on queue freeze,

Also blk_mq_debugfs_register_rqos() does _not_ require queue to be frozen,
and it should be fine to move blk_mq_debugfs_register_rqos() out of queue
freeze.


Thanks,
Ming

Re: [PATCH RESEND 1/5] block/blk-rq-qos: add a new helper rq_qos_add_freezed()

[Nilay Shroff]

On 11/17/25 4:31 PM, Ming Lei wrote:
> On Sun, Nov 16, 2025 at 12:10:20PM +0800, Yu Kuai wrote:
>> queue should not be freezed under rq_qos_mutex, see example index
>> commit 9730763f4756 ("block: correct locking order for protecting blk-wbt
>> parameters"), which means current implementation of rq_qos_add() is
>> problematic. Add a new helper and prepare to fix this problem in
>> following patches.
>>
>> Signed-off-by: Yu Kuai <yukuai@fnnas.com>
>> ---
>>  block/blk-rq-qos.c | 27 +++++++++++++++++++++++++++
>>  block/blk-rq-qos.h |  2 ++
>>  2 files changed, 29 insertions(+)
>>
>> diff --git a/block/blk-rq-qos.c b/block/blk-rq-qos.c
>> index 654478dfbc20..353397d7e126 100644
>> --- a/block/blk-rq-qos.c
>> +++ b/block/blk-rq-qos.c
>> @@ -322,6 +322,33 @@ void rq_qos_exit(struct request_queue *q)
>>  	mutex_unlock(&q->rq_qos_mutex);
>>  }
>>  
>> +int rq_qos_add_freezed(struct rq_qos *rqos, struct gendisk *disk,
>> +		       enum rq_qos_id id, const struct rq_qos_ops *ops)
>> +{
>> +	struct request_queue *q = disk->queue;
>> +
>> +	WARN_ON_ONCE(q->mq_freeze_depth == 0);
>> +	lockdep_assert_held(&q->rq_qos_mutex);
>> +
>> +	if (rq_qos_id(q, id))
>> +		return -EBUSY;
>> +
>> +	rqos->disk = disk;
>> +	rqos->id = id;
>> +	rqos->ops = ops;
>> +	rqos->next = q->rq_qos;
>> +	q->rq_qos = rqos;
>> +	blk_queue_flag_set(QUEUE_FLAG_QOS_ENABLED, q);
>> +
>> +	if (rqos->ops->debugfs_attrs) {
>> +		mutex_lock(&q->debugfs_mutex);
>> +		blk_mq_debugfs_register_rqos(rqos);
>> +		mutex_unlock(&q->debugfs_mutex);
>> +	}
> 
> It will cause more lockdep splat to let q->debugfs_mutex depend on queue freeze,
> 
I think we already have that ->debugfs_mutex dependency on ->freeze_lock. 
for instance, 
  ioc_qos_write  => freeze-queue
   blk_iocost_init
     rq_qos_add 

and also, 
  queue_wb_lat_store  => freeze-queue
    wbt_init
      rq_qos_add

> Also blk_mq_debugfs_register_rqos() does _not_ require queue to be frozen,
> and it should be fine to move blk_mq_debugfs_register_rqos() out of queue
> freeze.
> 
Yes correct, but I thought this pacthset is meant only to address incorrect 
locking order between ->rq_qos_mutex and ->freeze_lock. So do you suggest
also refactoring code to avoid ->debugfs_mutex dependency on ->freeze_lock?
If yes then shouldn't that be handled in a separate patchset?

Thanks,
--Nilay

Re: [PATCH RESEND 1/5] block/blk-rq-qos: add a new helper rq_qos_add_freezed()

[Ming Lei]

On Mon, Nov 17, 2025 at 04:43:11PM +0530, Nilay Shroff wrote:
> 
> 
> On 11/17/25 4:31 PM, Ming Lei wrote:
> > On Sun, Nov 16, 2025 at 12:10:20PM +0800, Yu Kuai wrote:
> >> queue should not be freezed under rq_qos_mutex, see example index
> >> commit 9730763f4756 ("block: correct locking order for protecting blk-wbt
> >> parameters"), which means current implementation of rq_qos_add() is
> >> problematic. Add a new helper and prepare to fix this problem in
> >> following patches.
> >>
> >> Signed-off-by: Yu Kuai <yukuai@fnnas.com>
> >> ---
> >>  block/blk-rq-qos.c | 27 +++++++++++++++++++++++++++
> >>  block/blk-rq-qos.h |  2 ++
> >>  2 files changed, 29 insertions(+)
> >>
> >> diff --git a/block/blk-rq-qos.c b/block/blk-rq-qos.c
> >> index 654478dfbc20..353397d7e126 100644
> >> --- a/block/blk-rq-qos.c
> >> +++ b/block/blk-rq-qos.c
> >> @@ -322,6 +322,33 @@ void rq_qos_exit(struct request_queue *q)
> >>  	mutex_unlock(&q->rq_qos_mutex);
> >>  }
> >>  
> >> +int rq_qos_add_freezed(struct rq_qos *rqos, struct gendisk *disk,
> >> +		       enum rq_qos_id id, const struct rq_qos_ops *ops)
> >> +{
> >> +	struct request_queue *q = disk->queue;
> >> +
> >> +	WARN_ON_ONCE(q->mq_freeze_depth == 0);
> >> +	lockdep_assert_held(&q->rq_qos_mutex);
> >> +
> >> +	if (rq_qos_id(q, id))
> >> +		return -EBUSY;
> >> +
> >> +	rqos->disk = disk;
> >> +	rqos->id = id;
> >> +	rqos->ops = ops;
> >> +	rqos->next = q->rq_qos;
> >> +	q->rq_qos = rqos;
> >> +	blk_queue_flag_set(QUEUE_FLAG_QOS_ENABLED, q);
> >> +
> >> +	if (rqos->ops->debugfs_attrs) {
> >> +		mutex_lock(&q->debugfs_mutex);
> >> +		blk_mq_debugfs_register_rqos(rqos);
> >> +		mutex_unlock(&q->debugfs_mutex);
> >> +	}
> > 
> > It will cause more lockdep splat to let q->debugfs_mutex depend on queue freeze,
> > 
> I think we already have that ->debugfs_mutex dependency on ->freeze_lock. 
> for instance, 
>   ioc_qos_write  => freeze-queue
>    blk_iocost_init
>      rq_qos_add 

Why is queue freeze needed in above code path?

Also blk_iolatency_init()/rq_qos_add() doesn't freeze queue.

> 
> and also, 
>   queue_wb_lat_store  => freeze-queue
>     wbt_init
>       rq_qos_add

Not all wbt_enable_default()/wbt_init() is called with queue frozen, but Kuai's
patchset changes all to freeze queue before registering debugfs entry, people will
complain new warning.

> 
> > Also blk_mq_debugfs_register_rqos() does _not_ require queue to be frozen,
> > and it should be fine to move blk_mq_debugfs_register_rqos() out of queue
> > freeze.
> > 
> Yes correct, but I thought this pacthset is meant only to address incorrect 
> locking order between ->rq_qos_mutex and ->freeze_lock. So do you suggest
> also refactoring code to avoid ->debugfs_mutex dependency on ->freeze_lock?
> If yes then shouldn't that be handled in a separate patchset?

It is fine to fix in that way, but at least regression shouldn't be caused.

More importantly we shouldn't add new unnecessary dependency on queue freeze.

Thanks,
Ming

Re: [PATCH RESEND 1/5] block/blk-rq-qos: add a new helper rq_qos_add_freezed()

[Yu Kuai]

Hi,

在 2025/11/17 19:30, Ming Lei 写道:
> On Mon, Nov 17, 2025 at 04:43:11PM +0530, Nilay Shroff wrote:
>>
>> On 11/17/25 4:31 PM, Ming Lei wrote:
>>> On Sun, Nov 16, 2025 at 12:10:20PM +0800, Yu Kuai wrote:
>>>> queue should not be freezed under rq_qos_mutex, see example index
>>>> commit 9730763f4756 ("block: correct locking order for protecting blk-wbt
>>>> parameters"), which means current implementation of rq_qos_add() is
>>>> problematic. Add a new helper and prepare to fix this problem in
>>>> following patches.
>>>>
>>>> Signed-off-by: Yu Kuai <yukuai@fnnas.com>
>>>> ---
>>>>   block/blk-rq-qos.c | 27 +++++++++++++++++++++++++++
>>>>   block/blk-rq-qos.h |  2 ++
>>>>   2 files changed, 29 insertions(+)
>>>>
>>>> diff --git a/block/blk-rq-qos.c b/block/blk-rq-qos.c
>>>> index 654478dfbc20..353397d7e126 100644
>>>> --- a/block/blk-rq-qos.c
>>>> +++ b/block/blk-rq-qos.c
>>>> @@ -322,6 +322,33 @@ void rq_qos_exit(struct request_queue *q)
>>>>   	mutex_unlock(&q->rq_qos_mutex);
>>>>   }
>>>>   
>>>> +int rq_qos_add_freezed(struct rq_qos *rqos, struct gendisk *disk,
>>>> +		       enum rq_qos_id id, const struct rq_qos_ops *ops)
>>>> +{
>>>> +	struct request_queue *q = disk->queue;
>>>> +
>>>> +	WARN_ON_ONCE(q->mq_freeze_depth == 0);
>>>> +	lockdep_assert_held(&q->rq_qos_mutex);
>>>> +
>>>> +	if (rq_qos_id(q, id))
>>>> +		return -EBUSY;
>>>> +
>>>> +	rqos->disk = disk;
>>>> +	rqos->id = id;
>>>> +	rqos->ops = ops;
>>>> +	rqos->next = q->rq_qos;
>>>> +	q->rq_qos = rqos;
>>>> +	blk_queue_flag_set(QUEUE_FLAG_QOS_ENABLED, q);
>>>> +
>>>> +	if (rqos->ops->debugfs_attrs) {
>>>> +		mutex_lock(&q->debugfs_mutex);
>>>> +		blk_mq_debugfs_register_rqos(rqos);
>>>> +		mutex_unlock(&q->debugfs_mutex);
>>>> +	}
>>> It will cause more lockdep splat to let q->debugfs_mutex depend on queue freeze,
>>>
>> I think we already have that ->debugfs_mutex dependency on ->freeze_lock.
>> for instance,
>>    ioc_qos_write  => freeze-queue
>>     blk_iocost_init
>>       rq_qos_add
> Why is queue freeze needed in above code path?
>
> Also blk_iolatency_init()/rq_qos_add() doesn't freeze queue.

I don't quite understand, rq_qos_add() always require queue freeze, prevent
deference q->rq_qos from IO path concurrently.

>
>> and also,
>>    queue_wb_lat_store  => freeze-queue
>>      wbt_init
>>        rq_qos_add
> Not all wbt_enable_default()/wbt_init() is called with queue frozen, but Kuai's
> patchset changes all to freeze queue before registering debugfs entry, people will
> complain new warning.

Yes, but the same as above, rq_qos_add() from wbt_init() will always freeze queue
before this set, so I don't understand why is there new warning?

>
>>> Also blk_mq_debugfs_register_rqos() does _not_ require queue to be frozen,
>>> and it should be fine to move blk_mq_debugfs_register_rqos() out of queue
>>> freeze.
>>>
>> Yes correct, but I thought this pacthset is meant only to address incorrect
>> locking order between ->rq_qos_mutex and ->freeze_lock. So do you suggest
>> also refactoring code to avoid ->debugfs_mutex dependency on ->freeze_lock?
>> If yes then shouldn't that be handled in a separate patchset?
> It is fine to fix in that way, but at least regression shouldn't be caused.
>
> More importantly we shouldn't add new unnecessary dependency on queue freeze.

This is correct, I'll work on the v2 set to move debugfs_mutex outside of freeze
queue, however, as you suggested before we should we should fix this incorrect
lock order first. How about I make them in a single set?

>
> Thanks,
> Ming
>
>

Re: [PATCH RESEND 1/5] block/blk-rq-qos: add a new helper rq_qos_add_freezed()

[Ming Lei]

On Mon, Nov 17, 2025 at 07:39:57PM +0800, Yu Kuai wrote:
> Hi,
> 
> 在 2025/11/17 19:30, Ming Lei 写道:
> > On Mon, Nov 17, 2025 at 04:43:11PM +0530, Nilay Shroff wrote:
> >>
> >> On 11/17/25 4:31 PM, Ming Lei wrote:
> >>> On Sun, Nov 16, 2025 at 12:10:20PM +0800, Yu Kuai wrote:
> >>>> queue should not be freezed under rq_qos_mutex, see example index
> >>>> commit 9730763f4756 ("block: correct locking order for protecting blk-wbt
> >>>> parameters"), which means current implementation of rq_qos_add() is
> >>>> problematic. Add a new helper and prepare to fix this problem in
> >>>> following patches.
> >>>>
> >>>> Signed-off-by: Yu Kuai <yukuai@fnnas.com>
> >>>> ---
> >>>>   block/blk-rq-qos.c | 27 +++++++++++++++++++++++++++
> >>>>   block/blk-rq-qos.h |  2 ++
> >>>>   2 files changed, 29 insertions(+)
> >>>>
> >>>> diff --git a/block/blk-rq-qos.c b/block/blk-rq-qos.c
> >>>> index 654478dfbc20..353397d7e126 100644
> >>>> --- a/block/blk-rq-qos.c
> >>>> +++ b/block/blk-rq-qos.c
> >>>> @@ -322,6 +322,33 @@ void rq_qos_exit(struct request_queue *q)
> >>>>   	mutex_unlock(&q->rq_qos_mutex);
> >>>>   }
> >>>>   
> >>>> +int rq_qos_add_freezed(struct rq_qos *rqos, struct gendisk *disk,
> >>>> +		       enum rq_qos_id id, const struct rq_qos_ops *ops)
> >>>> +{
> >>>> +	struct request_queue *q = disk->queue;
> >>>> +
> >>>> +	WARN_ON_ONCE(q->mq_freeze_depth == 0);
> >>>> +	lockdep_assert_held(&q->rq_qos_mutex);
> >>>> +
> >>>> +	if (rq_qos_id(q, id))
> >>>> +		return -EBUSY;
> >>>> +
> >>>> +	rqos->disk = disk;
> >>>> +	rqos->id = id;
> >>>> +	rqos->ops = ops;
> >>>> +	rqos->next = q->rq_qos;
> >>>> +	q->rq_qos = rqos;
> >>>> +	blk_queue_flag_set(QUEUE_FLAG_QOS_ENABLED, q);
> >>>> +
> >>>> +	if (rqos->ops->debugfs_attrs) {
> >>>> +		mutex_lock(&q->debugfs_mutex);
> >>>> +		blk_mq_debugfs_register_rqos(rqos);
> >>>> +		mutex_unlock(&q->debugfs_mutex);
> >>>> +	}
> >>> It will cause more lockdep splat to let q->debugfs_mutex depend on queue freeze,
> >>>
> >> I think we already have that ->debugfs_mutex dependency on ->freeze_lock.
> >> for instance,
> >>    ioc_qos_write  => freeze-queue
> >>     blk_iocost_init
> >>       rq_qos_add
> > Why is queue freeze needed in above code path?
> >
> > Also blk_iolatency_init()/rq_qos_add() doesn't freeze queue.
> 
> I don't quite understand, rq_qos_add() always require queue freeze, prevent
> deference q->rq_qos from IO path concurrently.
> 
> >
> >> and also,
> >>    queue_wb_lat_store  => freeze-queue
> >>      wbt_init
> >>        rq_qos_add
> > Not all wbt_enable_default()/wbt_init() is called with queue frozen, but Kuai's
> > patchset changes all to freeze queue before registering debugfs entry, people will
> > complain new warning.
> 
> Yes, but the same as above, rq_qos_add() from wbt_init() will always freeze queue
> before this set, so I don't understand why is there new warning?

The in-tree rq_qos_add() registers debugfs after queue is unfreeze, but
your patchset basically moves queue freeze/unfreeze to callsite of rq_qos_add(),
then debugfs register is always done with queue frozen.

Dependency between queue freeze and q->debugfs_mutex is introduced in some
code paths, such as, elevator switch, blk_iolatency_init, ..., this way
will trigger warning because it isn't strange to run into memory
allocation in debugfs_create_*().

> 
> >
> >>> Also blk_mq_debugfs_register_rqos() does _not_ require queue to be frozen,
> >>> and it should be fine to move blk_mq_debugfs_register_rqos() out of queue
> >>> freeze.
> >>>
> >> Yes correct, but I thought this pacthset is meant only to address incorrect
> >> locking order between ->rq_qos_mutex and ->freeze_lock. So do you suggest
> >> also refactoring code to avoid ->debugfs_mutex dependency on ->freeze_lock?
> >> If yes then shouldn't that be handled in a separate patchset?
> > It is fine to fix in that way, but at least regression shouldn't be caused.
> >
> > More importantly we shouldn't add new unnecessary dependency on queue freeze.
> 
> This is correct, I'll work on the v2 set to move debugfs_mutex outside of freeze
> queue, however, as you suggested before we should we should fix this incorrect
> lock order first. How about I make them in a single set?

That is fine, but patches for moving debugfs_mutex should be put before
this patchset, which is always friendly for 'git bisect'.


Thanks,
Ming

Re: [PATCH RESEND 1/5] block/blk-rq-qos: add a new helper rq_qos_add_freezed()

[Yu Kuai]

Hi,

在 2025/11/17 19:54, Ming Lei 写道:
> On Mon, Nov 17, 2025 at 07:39:57PM +0800, Yu Kuai wrote:
>> Hi,
>>
>> 在 2025/11/17 19:30, Ming Lei 写道:
>>> On Mon, Nov 17, 2025 at 04:43:11PM +0530, Nilay Shroff wrote:
>>>> On 11/17/25 4:31 PM, Ming Lei wrote:
>>>>> On Sun, Nov 16, 2025 at 12:10:20PM +0800, Yu Kuai wrote:
>>>>>> queue should not be freezed under rq_qos_mutex, see example index
>>>>>> commit 9730763f4756 ("block: correct locking order for protecting blk-wbt
>>>>>> parameters"), which means current implementation of rq_qos_add() is
>>>>>> problematic. Add a new helper and prepare to fix this problem in
>>>>>> following patches.
>>>>>>
>>>>>> Signed-off-by: Yu Kuai <yukuai@fnnas.com>
>>>>>> ---
>>>>>>    block/blk-rq-qos.c | 27 +++++++++++++++++++++++++++
>>>>>>    block/blk-rq-qos.h |  2 ++
>>>>>>    2 files changed, 29 insertions(+)
>>>>>>
>>>>>> diff --git a/block/blk-rq-qos.c b/block/blk-rq-qos.c
>>>>>> index 654478dfbc20..353397d7e126 100644
>>>>>> --- a/block/blk-rq-qos.c
>>>>>> +++ b/block/blk-rq-qos.c
>>>>>> @@ -322,6 +322,33 @@ void rq_qos_exit(struct request_queue *q)
>>>>>>    	mutex_unlock(&q->rq_qos_mutex);
>>>>>>    }
>>>>>>    
>>>>>> +int rq_qos_add_freezed(struct rq_qos *rqos, struct gendisk *disk,
>>>>>> +		       enum rq_qos_id id, const struct rq_qos_ops *ops)
>>>>>> +{
>>>>>> +	struct request_queue *q = disk->queue;
>>>>>> +
>>>>>> +	WARN_ON_ONCE(q->mq_freeze_depth == 0);
>>>>>> +	lockdep_assert_held(&q->rq_qos_mutex);
>>>>>> +
>>>>>> +	if (rq_qos_id(q, id))
>>>>>> +		return -EBUSY;
>>>>>> +
>>>>>> +	rqos->disk = disk;
>>>>>> +	rqos->id = id;
>>>>>> +	rqos->ops = ops;
>>>>>> +	rqos->next = q->rq_qos;
>>>>>> +	q->rq_qos = rqos;
>>>>>> +	blk_queue_flag_set(QUEUE_FLAG_QOS_ENABLED, q);
>>>>>> +
>>>>>> +	if (rqos->ops->debugfs_attrs) {
>>>>>> +		mutex_lock(&q->debugfs_mutex);
>>>>>> +		blk_mq_debugfs_register_rqos(rqos);
>>>>>> +		mutex_unlock(&q->debugfs_mutex);
>>>>>> +	}
>>>>> It will cause more lockdep splat to let q->debugfs_mutex depend on queue freeze,
>>>>>
>>>> I think we already have that ->debugfs_mutex dependency on ->freeze_lock.
>>>> for instance,
>>>>     ioc_qos_write  => freeze-queue
>>>>      blk_iocost_init
>>>>        rq_qos_add
>>> Why is queue freeze needed in above code path?
>>>
>>> Also blk_iolatency_init()/rq_qos_add() doesn't freeze queue.
>> I don't quite understand, rq_qos_add() always require queue freeze, prevent
>> deference q->rq_qos from IO path concurrently.
>>
>>>> and also,
>>>>     queue_wb_lat_store  => freeze-queue
>>>>       wbt_init
>>>>         rq_qos_add
>>> Not all wbt_enable_default()/wbt_init() is called with queue frozen, but Kuai's
>>> patchset changes all to freeze queue before registering debugfs entry, people will
>>> complain new warning.
>> Yes, but the same as above, rq_qos_add() from wbt_init() will always freeze queue
>> before this set, so I don't understand why is there new warning?
> The in-tree rq_qos_add() registers debugfs after queue is unfreeze, but
> your patchset basically moves queue freeze/unfreeze to callsite of rq_qos_add(),
> then debugfs register is always done with queue frozen.
>
> Dependency between queue freeze and q->debugfs_mutex is introduced in some
> code paths, such as, elevator switch, blk_iolatency_init, ..., this way
> will trigger warning because it isn't strange to run into memory
> allocation in debugfs_create_*().

Yes, I realized I do misunderstand in previous email.

>
>>>>> Also blk_mq_debugfs_register_rqos() does _not_ require queue to be frozen,
>>>>> and it should be fine to move blk_mq_debugfs_register_rqos() out of queue
>>>>> freeze.
>>>>>
>>>> Yes correct, but I thought this pacthset is meant only to address incorrect
>>>> locking order between ->rq_qos_mutex and ->freeze_lock. So do you suggest
>>>> also refactoring code to avoid ->debugfs_mutex dependency on ->freeze_lock?
>>>> If yes then shouldn't that be handled in a separate patchset?
>>> It is fine to fix in that way, but at least regression shouldn't be caused.
>>>
>>> More importantly we shouldn't add new unnecessary dependency on queue freeze.
>> This is correct, I'll work on the v2 set to move debugfs_mutex outside of freeze
>> queue, however, as you suggested before we should we should fix this incorrect
>> lock order first. How about I make them in a single set?
> That is fine, but patches for moving debugfs_mutex should be put before
> this patchset, which is always friendly for 'git bisect'.

Sounds good :)

Thanks,
Kuai

>
>
> Thanks,
> Ming
>
>

Re: [PATCH RESEND 1/5] block/blk-rq-qos: add a new helper rq_qos_add_freezed()

[Bart Van Assche]

On 11/15/25 8:10 PM, Yu Kuai wrote:
> queue should not be freezed under rq_qos_mutex, see example index
   ^^^^^               ^^^^^^^                         ^^^^^^^^^^^^^
A queue               frozen                               also

> +int rq_qos_add_freezed(struct rq_qos *rqos, struct gendisk *disk,
> +		       enum rq_qos_id id, const struct rq_qos_ops *ops)
> +{
In this patch and also in the following patches, please fix the name of
this function and change it into "rq_qos_add_frozen()".

Thanks,

Bart.

[PATCH RESEND 2/5] blk-wbt: fix incorrect lock order for rq_qos_mutex and freeze queue

[Yu Kuai]

wbt_init() can be called from sysfs attribute and wbt_enable_default(),
however the lock order are inversely.

- queue_wb_lat_store() freeze queue first, and then wbt_init() hold
  rq_qos_mutex. In this case queue will be freezed again inside
  rq_qos_add(), however, in this case freeze queue recursivly is
  inoperative;
- wbt_enable_default() from elevator switch will hold rq_qos_mutex
  first, and then rq_qos_add() will freeze queue;

Fix this problem by converting to use new helper rq_qos_add_freezed() in
wbt_init(), and for wbt_enable_default(), freeze queue before calling
wbt_init().

Fixes: a13bd91be223 ("block/rq_qos: protect rq_qos apis with a new lock")
Signed-off-by: Yu Kuai <yukuai@fnnas.com>
---
 block/blk-wbt.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/block/blk-wbt.c b/block/blk-wbt.c
index eb8037bae0bd..a784f6d338b4 100644
--- a/block/blk-wbt.c
+++ b/block/blk-wbt.c
@@ -724,8 +724,12 @@ void wbt_enable_default(struct gendisk *disk)
 	if (!blk_queue_registered(q))
 		return;
 
-	if (queue_is_mq(q) && enable)
+	if (queue_is_mq(q) && enable) {
+		unsigned int memflags = blk_mq_freeze_queue(q);
+
 		wbt_init(disk);
+		blk_mq_unfreeze_queue(q, memflags);
+	}
 }
 EXPORT_SYMBOL_GPL(wbt_enable_default);
 
@@ -922,7 +926,7 @@ int wbt_init(struct gendisk *disk)
 	 * Assign rwb and add the stats callback.
 	 */
 	mutex_lock(&q->rq_qos_mutex);
-	ret = rq_qos_add(&rwb->rqos, disk, RQ_QOS_WBT, &wbt_rqos_ops);
+	ret = rq_qos_add_freezed(&rwb->rqos, disk, RQ_QOS_WBT, &wbt_rqos_ops);
 	mutex_unlock(&q->rq_qos_mutex);
 	if (ret)
 		goto err_free;
-- 
2.51.0

Re: [PATCH RESEND 2/5] blk-wbt: fix incorrect lock order for rq_qos_mutex and freeze queue

[Nilay Shroff]

On 11/16/25 9:40 AM, Yu Kuai wrote:
> wbt_init() can be called from sysfs attribute and wbt_enable_default(),
> however the lock order are inversely.
> 
> - queue_wb_lat_store() freeze queue first, and then wbt_init() hold
>   rq_qos_mutex. In this case queue will be freezed again inside
>   rq_qos_add(), however, in this case freeze queue recursivly is
>   inoperative;
> - wbt_enable_default() from elevator switch will hold rq_qos_mutex
>   first, and then rq_qos_add() will freeze queue;
> 
> Fix this problem by converting to use new helper rq_qos_add_freezed() in
> wbt_init(), and for wbt_enable_default(), freeze queue before calling
> wbt_init().
> 
> Fixes: a13bd91be223 ("block/rq_qos: protect rq_qos apis with a new lock")
> Signed-off-by: Yu Kuai <yukuai@fnnas.com>

Looks good to me:
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>

Re: [PATCH RESEND 2/5] blk-wbt: fix incorrect lock order for rq_qos_mutex and freeze queue

[kernel test robot]

Hello,

kernel test robot noticed "WARNING:possible_circular_locking_dependency_detected" on:

commit: 9b76049c7ab17a3352a58ee216f444769e216c5c ("[PATCH RESEND 2/5] blk-wbt: fix incorrect lock order for rq_qos_mutex and freeze queue")
url: https://github.com/intel-lab-lkp/linux/commits/Yu-Kuai/block-blk-rq-qos-add-a-new-helper-rq_qos_add_freezed/20251116-121353
base: https://git.kernel.org/cgit/linux/kernel/git/axboe/linux.git for-next
patch link: https://lore.kernel.org/all/20251116041024.120500-3-yukuai@fnnas.com/
patch subject: [PATCH RESEND 2/5] blk-wbt: fix incorrect lock order for rq_qos_mutex and freeze queue

in testcase: boot

config: x86_64-rhel-9.4-kselftests
compiler: gcc-14
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 32G

(please refer to attached dmesg/kmsg for entire log/backtrace)



If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202511191340.643afc3a-lkp@intel.com


[   36.217408][  T108] WARNING: possible circular locking dependency detected
[   36.218277][  T108] 6.18.0-rc5-00238-g9b76049c7ab1 #1 Not tainted
[   36.219067][  T108] ------------------------------------------------------
[   36.219956][  T108] udevd/108 is trying to acquire lock:
[   36.220622][  T108] ffff88813b4b6a40 (&q->debugfs_mutex){+.+.}-{4:4}, at: rq_qos_add_freezed (block/blk-rq-qos.c:345)
[   36.221938][  T108]
[   36.221938][  T108] but task is already holding lock:
[   36.222851][  T108] ffff88813b4b63e0 (&q->rq_qos_mutex){+.+.}-{4:4}, at: wbt_init (block/blk-wbt.c:929)
[   36.223964][  T108]
[   36.223964][  T108] which lock already depends on the new lock.
[   36.223964][  T108]
[   36.225282][  T108]
[   36.225282][  T108] the existing dependency chain (in reverse order) is:
[   36.226380][  T108]
[   36.226380][  T108] -> #4 (&q->rq_qos_mutex){+.+.}-{4:4}:
[   36.228833][  T108]        __lock_acquire (kernel/locking/lockdep.c:5237 (discriminator 1))
[   36.230959][  T108]        lock_acquire (include/linux/preempt.h:471 (discriminator 2) include/trace/events/lock.h:24 (discriminator 2) include/trace/events/lock.h:24 (discriminator 2) kernel/locking/lockdep.c:5831 (discriminator 2))
[   36.233086][  T108]        __mutex_lock (arch/x86/include/asm/jump_label.h:36 include/trace/events/lock.h:95 kernel/locking/mutex.c:600 kernel/locking/mutex.c:760)
[   36.234852][  T108]        wbt_init (block/blk-wbt.c:929)
[   36.236509][  T108]        wbt_enable_default (include/linux/blk-mq.h:960 block/blk-wbt.c:731)
[   36.238663][  T108]        blk_register_queue (block/blk-sysfs.c:948)
[   36.240650][  T108]        __add_disk (block/genhd.c:528)
[   36.242626][  T108]        add_disk_fwnode (block/genhd.c:598)
[   36.244660][  T108] sr_probe (drivers/scsi/sr.c:703) sr_mod
[   36.246799][  T108]        really_probe (drivers/base/dd.c:581 drivers/base/dd.c:659)
[   36.248845][  T108]        __driver_probe_device (drivers/base/dd.c:801)
[   36.250799][  T108]        driver_probe_device (drivers/base/dd.c:831)
[   36.252778][  T108]        __driver_attach (drivers/base/dd.c:1218)
[   36.254752][  T108]        bus_for_each_dev (drivers/base/bus.c:369)
[   36.256776][  T108]        bus_add_driver (drivers/base/bus.c:678)
[   36.258650][  T108]        driver_register (drivers/base/driver.c:249)
[   36.261124][  T108] init_sr (drivers/scsi/sr.c:152) sr_mod
[   36.262938][  T108]        do_one_initcall (init/main.c:1283)
[   36.264802][  T108]        do_init_module (kernel/module/main.c:3039)
[   36.266560][  T108]        load_module (kernel/module/main.c:3509)
[   36.268367][  T108]        init_module_from_file (kernel/module/main.c:3701)
[   36.270150][  T108]        idempotent_init_module (kernel/module/main.c:3713)
[   36.272050][  T108]        __x64_sys_finit_module (include/linux/file.h:62 (discriminator 1) include/linux/file.h:83 (discriminator 1) kernel/module/main.c:3736 (discriminator 1) kernel/module/main.c:3723 (discriminator 1) kernel/module/main.c:3723 (discriminator 1))
[   36.273921][  T108]        do_syscall_64 (arch/x86/entry/syscall_64.c:63 (discriminator 1) arch/x86/entry/syscall_64.c:94 (discriminator 1))
[   36.275672][  T108]        entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
[   36.277584][  T108]
[   36.277584][  T108] -> #3 (&q->q_usage_counter(io)){++++}-{0:0}:
[   36.280765][  T108]        __lock_acquire (kernel/locking/lockdep.c:5237 (discriminator 1))
[   36.282560][  T108]        lock_acquire (include/linux/preempt.h:471 (discriminator 2) include/trace/events/lock.h:24 (discriminator 2) include/trace/events/lock.h:24 (discriminator 2) kernel/locking/lockdep.c:5831 (discriminator 2))
[   36.284435][  T108]        blk_alloc_queue (block/blk-core.c:462 (discriminator 1))
[   36.286153][  T108]        blk_mq_alloc_queue (block/blk-mq.c:4405)
[   36.287937][  T108]        scsi_alloc_sdev (drivers/scsi/scsi_scan.c:339)
[   36.289536][  T108]        scsi_probe_and_add_lun (drivers/scsi/scsi_scan.c:1211)
[   36.291280][  T108]        __scsi_add_device (drivers/scsi/scsi_scan.c:1625)
[   36.292952][  T108] ata_scsi_scan_host (drivers/ata/libata-scsi.c:4577 (discriminator 1)) libata
[   36.295269][  T108]        async_run_entry_fn (arch/x86/include/asm/jump_label.h:36 kernel/async.c:131)
[   36.297162][  T108]        process_one_work (arch/x86/include/asm/jump_label.h:36 include/trace/events/workqueue.h:110 kernel/workqueue.c:3268)
[   36.299000][  T108]        worker_thread (kernel/workqueue.c:3340 (discriminator 2) kernel/workqueue.c:3427 (discriminator 2))
[   36.300695][  T108]        kthread (kernel/kthread.c:463)
[   36.302155][  T108]        ret_from_fork (arch/x86/kernel/process.c:164)
[   36.303743][  T108]        ret_from_fork_asm (arch/x86/entry/entry_64.S:255)
[   36.305346][  T108]
[   36.305346][  T108] -> #2 (fs_reclaim){+.+.}-{0:0}:
[   36.307985][  T108]        __lock_acquire (kernel/locking/lockdep.c:5237 (discriminator 1))
[   36.309621][  T108]        lock_acquire (include/linux/preempt.h:471 (discriminator 2) include/trace/events/lock.h:24 (discriminator 2) include/trace/events/lock.h:24 (discriminator 2) kernel/locking/lockdep.c:5831 (discriminator 2))
[   36.311256][  T108]        fs_reclaim_acquire (mm/page_alloc.c:4270 mm/page_alloc.c:4283)
[   36.312913][  T108]        kmem_cache_alloc_lru_noprof (include/linux/sched/mm.h:319 mm/slub.c:4925 mm/slub.c:5260 mm/slub.c:5303)
[   36.314550][  T108]        __d_alloc (fs/dcache.c:1692)
[   36.316022][  T108]        d_alloc_parallel (fs/dcache.c:2549)
[   36.317503][  T108]        __lookup_slow (fs/namei.c:1801)
[   36.319003][  T108]        simple_start_creating (fs/libfs.c:2304 (discriminator 1))
[   36.320529][  T108]        debugfs_start_creating+0x4f/0xe0
[   36.322219][  T108]        debugfs_create_dir (fs/debugfs/inode.c:374 (discriminator 1) fs/debugfs/inode.c:581 (discriminator 1))
[   36.323776][  T108]        pinctrl_init (drivers/pinctrl/core.c:2028 (discriminator 1) drivers/pinctrl/core.c:2420 (discriminator 1))
[   36.325323][  T108]        do_one_initcall (init/main.c:1283)
[   36.326866][  T108]        do_initcalls (init/main.c:1344 (discriminator 3) init/main.c:1361 (discriminator 3))
[   36.328322][  T108]        kernel_init_freeable (init/main.c:1597)
[   36.329944][  T108]        kernel_init (init/main.c:1485)
[   36.331399][  T108]        ret_from_fork (arch/x86/kernel/process.c:164)
[   36.332850][  T108]        ret_from_fork_asm (arch/x86/entry/entry_64.S:255)
[   36.334399][  T108]
[   36.334399][  T108] -> #1 (&sb->s_type->i_mutex_key#3){+.+.}-{4:4}:
[   36.337887][  T108]        __lock_acquire (kernel/locking/lockdep.c:5237 (discriminator 1))
[   36.339409][  T108]        lock_acquire (include/linux/preempt.h:471 (discriminator 2) include/trace/events/lock.h:24 (discriminator 2) include/trace/events/lock.h:24 (discriminator 2) kernel/locking/lockdep.c:5831 (discriminator 2))
[   36.341061][  T108]        down_write (arch/x86/include/asm/preempt.h:80 (discriminator 10) kernel/locking/rwsem.c:1315 (discriminator 10) kernel/locking/rwsem.c:1326 (discriminator 10) kernel/locking/rwsem.c:1591 (discriminator 10))
[   36.342598][  T108]        simple_start_creating (fs/libfs.c:2299)
[   36.344192][  T108]        debugfs_start_creating+0x4f/0xe0
[   36.345959][  T108]        debugfs_create_dir (fs/debugfs/inode.c:374 (discriminator 1) fs/debugfs/inode.c:581 (discriminator 1))
[   36.347535][  T108]        blk_register_queue (block/blk-sysfs.c:928 (discriminator 1))
[   36.349145][  T108]        __add_disk (block/genhd.c:528)
[   36.350605][  T108]        add_disk_fwnode (block/genhd.c:598)
[   36.352129][  T108] sr_probe (drivers/scsi/sr.c:703) sr_mod
[   36.353727][  T108]        really_probe (drivers/base/dd.c:581 drivers/base/dd.c:659)
[   36.355225][  T108]        __driver_probe_device (drivers/base/dd.c:801)
[   36.356818][  T108]        driver_probe_device (drivers/base/dd.c:831)
[   36.358406][  T108]        __driver_attach (drivers/base/dd.c:1218)
[   36.359984][  T108]        bus_for_each_dev (drivers/base/bus.c:369)
[   36.361462][  T108]        bus_add_driver (drivers/base/bus.c:678)
[   36.362950][  T108]        driver_register (drivers/base/driver.c:249)
[   36.364384][  T108] init_sr (drivers/scsi/sr.c:152) sr_mod
[   36.365900][  T108]        do_one_initcall (init/main.c:1283)
[   36.367417][  T108]        do_init_module (kernel/module/main.c:3039)
[   36.368926][  T108]        load_module (kernel/module/main.c:3509)
[   36.370434][  T108]        init_module_from_file (kernel/module/main.c:3701)
[   36.372090][  T108]        idempotent_init_module (kernel/module/main.c:3713)
[   36.373729][  T108]        __x64_sys_finit_module (include/linux/file.h:62 (discriminator 1) include/linux/file.h:83 (discriminator 1) kernel/module/main.c:3736 (discriminator 1) kernel/module/main.c:3723 (discriminator 1) kernel/module/main.c:3723 (discriminator 1))
[   36.375279][  T108]        do_syscall_64 (arch/x86/entry/syscall_64.c:63 (discriminator 1) arch/x86/entry/syscall_64.c:94 (discriminator 1))
[   36.376796][  T108]        entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
[   36.378301][  T108]
[   36.378301][  T108] -> #0 (&q->debugfs_mutex){+.+.}-{4:4}:
[   36.381043][  T108]        check_prev_add (kernel/locking/lockdep.c:3166 (discriminator 2))
[   36.382551][  T108]        validate_chain (kernel/locking/lockdep.c:3285 kernel/locking/lockdep.c:3908)
[   36.384101][  T108]        __lock_acquire (kernel/locking/lockdep.c:5237 (discriminator 1))
[   36.385608][  T108]        lock_acquire (include/linux/preempt.h:471 (discriminator 2) include/trace/events/lock.h:24 (discriminator 2) include/trace/events/lock.h:24 (discriminator 2) kernel/locking/lockdep.c:5831 (discriminator 2))
[   36.387119][  T108]        __mutex_lock (arch/x86/include/asm/jump_label.h:36 include/trace/events/lock.h:95 kernel/locking/mutex.c:600 kernel/locking/mutex.c:760)
[   36.388585][  T108]        rq_qos_add_freezed (block/blk-rq-qos.c:345)
[   36.390240][  T108]        wbt_init (block/blk-wbt.c:930)
[   36.391782][  T108]        wbt_enable_default (include/linux/blk-mq.h:960 block/blk-wbt.c:731)
[   36.393333][  T108]        blk_register_queue (block/blk-sysfs.c:948)
[   36.394867][  T108]        __add_disk (block/genhd.c:528)
[   36.396410][  T108]        add_disk_fwnode (block/genhd.c:598)
[   36.398009][  T108] sr_probe (drivers/scsi/sr.c:703) sr_mod
[   36.399514][  T108]        really_probe (drivers/base/dd.c:581 drivers/base/dd.c:659)
[   36.401035][  T108]        __driver_probe_device (drivers/base/dd.c:801)
[   36.402511][  T108]        driver_probe_device (drivers/base/dd.c:831)
[   36.403990][  T108]        __driver_attach (drivers/base/dd.c:1218)
[   36.405471][  T108]        bus_for_each_dev (drivers/base/bus.c:369)
[   36.407062][  T108]        bus_add_driver (drivers/base/bus.c:678)
[   36.408549][  T108]        driver_register (drivers/base/driver.c:249)
[   36.410081][  T108] init_sr (drivers/scsi/sr.c:152) sr_mod
[   36.411625][  T108]        do_one_initcall (init/main.c:1283)
[   36.413166][  T108]        do_init_module (kernel/module/main.c:3039)
[   36.414682][  T108]        load_module (kernel/module/main.c:3509)
[   36.416196][  T108]        init_module_from_file (kernel/module/main.c:3701)
[   36.417822][  T108]        idempotent_init_module (kernel/module/main.c:3713)
[   36.419453][  T108]        __x64_sys_finit_module (include/linux/file.h:62 (discriminator 1) include/linux/file.h:83 (discriminator 1) kernel/module/main.c:3736 (discriminator 1) kernel/module/main.c:3723 (discriminator 1) kernel/module/main.c:3723 (discriminator 1))
[   36.421144][  T108]        do_syscall_64 (arch/x86/entry/syscall_64.c:63 (discriminator 1) arch/x86/entry/syscall_64.c:94 (discriminator 1))
[   36.422741][  T108]        entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
[   36.424468][  T108]
[   36.424468][  T108] other info that might help us debug this:
[   36.424468][  T108]
[   36.428405][  T108] Chain exists of:
[   36.428405][  T108]   &q->debugfs_mutex --> &q->q_usage_counter(io) --> &q->rq_qos_mutex
[   36.428405][  T108]
[   36.432967][  T108]  Possible unsafe locking scenario:
[   36.432967][  T108]
[   36.435485][  T108]        CPU0                    CPU1
[   36.437074][  T108]        ----                    ----
[   36.438592][  T108]   lock(&q->rq_qos_mutex);
[   36.440094][  T108]                                lock(&q->q_usage_counter(io));
[   36.441920][  T108]                                lock(&q->rq_qos_mutex);
[   36.443715][  T108]   lock(&q->debugfs_mutex);
[   36.445174][  T108]
[   36.445174][  T108]  *** DEADLOCK ***
[   36.445174][  T108]
[   36.448803][  T108] 6 locks held by udevd/108:


The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20251119/202511191340.643afc3a-lkp@intel.com



-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH RESEND 3/5] blk-iocost: fix incorrect lock order for rq_qos_mutex and freeze queue

[Yu Kuai]

Like wbt, rq_qos_add() can be called from two path and the lock order
are inversely:

- From ioc_qos_write(), queue is already freezed before rq_qos_add();
- From ioc_cost_model_write(), rq_qos_add() is called directly;

Fix this problem by converting to use blkg_conf_open_bdev_frozen()
from ioc_cost_model_write(), then since all rq_qos_add() callers
already freeze queue, convert to use rq_qos_add_freezed.

Signed-off-by: Yu Kuai <yukuai@fnnas.com>
---
 block/blk-iocost.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 5bfd70311359..233c9749bfc9 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -2927,7 +2927,7 @@ static int blk_iocost_init(struct gendisk *disk)
 	 * called before policy activation completion, can't assume that the
 	 * target bio has an iocg associated and need to test for NULL iocg.
 	 */
-	ret = rq_qos_add(&ioc->rqos, disk, RQ_QOS_COST, &ioc_rqos_ops);
+	ret = rq_qos_add_freezed(&ioc->rqos, disk, RQ_QOS_COST, &ioc_rqos_ops);
 	if (ret)
 		goto err_free_ioc;
 
@@ -3410,7 +3410,7 @@ static ssize_t ioc_cost_model_write(struct kernfs_open_file *of, char *input,
 {
 	struct blkg_conf_ctx ctx;
 	struct request_queue *q;
-	unsigned int memflags;
+	unsigned long memflags;
 	struct ioc *ioc;
 	u64 u[NR_I_LCOEFS];
 	bool user;
@@ -3419,9 +3419,11 @@ static ssize_t ioc_cost_model_write(struct kernfs_open_file *of, char *input,
 
 	blkg_conf_init(&ctx, input);
 
-	ret = blkg_conf_open_bdev(&ctx);
-	if (ret)
+	memflags = blkg_conf_open_bdev_frozen(&ctx);
+	if (IS_ERR_VALUE(memflags)) {
+		ret = memflags;
 		goto err;
+	}
 
 	body = ctx.body;
 	q = bdev_get_queue(ctx.bdev);
@@ -3438,7 +3440,6 @@ static ssize_t ioc_cost_model_write(struct kernfs_open_file *of, char *input,
 		ioc = q_to_ioc(q);
 	}
 
-	memflags = blk_mq_freeze_queue(q);
 	blk_mq_quiesce_queue(q);
 
 	spin_lock_irq(&ioc->lock);
@@ -3490,20 +3491,18 @@ static ssize_t ioc_cost_model_write(struct kernfs_open_file *of, char *input,
 	spin_unlock_irq(&ioc->lock);
 
 	blk_mq_unquiesce_queue(q);
-	blk_mq_unfreeze_queue(q, memflags);
 
-	blkg_conf_exit(&ctx);
+	blkg_conf_exit_frozen(&ctx, memflags);
 	return nbytes;
 
 einval:
 	spin_unlock_irq(&ioc->lock);
 
 	blk_mq_unquiesce_queue(q);
-	blk_mq_unfreeze_queue(q, memflags);
 
 	ret = -EINVAL;
 err:
-	blkg_conf_exit(&ctx);
+	blkg_conf_exit_frozen(&ctx, memflags);
 	return ret;
 }
 
-- 
2.51.0

Re: [PATCH RESEND 3/5] blk-iocost: fix incorrect lock order for rq_qos_mutex and freeze queue

[Nilay Shroff]

On 11/16/25 9:40 AM, Yu Kuai wrote:
> Like wbt, rq_qos_add() can be called from two path and the lock order
> are inversely:
> 
> - From ioc_qos_write(), queue is already freezed before rq_qos_add();
> - From ioc_cost_model_write(), rq_qos_add() is called directly;
> 
> Fix this problem by converting to use blkg_conf_open_bdev_frozen()
> from ioc_cost_model_write(), then since all rq_qos_add() callers
> already freeze queue, convert to use rq_qos_add_freezed.
> 
> Signed-off-by: Yu Kuai <yukuai@fnnas.com>

Looks good to me:
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>

[PATCH RESEND 4/5] blk-iolatency: fix incorrect lock order for rq_qos_mutex and freeze queue

[Yu Kuai]

Currently blk-iolatency will hold rq_qos_mutex first and then call
rq_qos_add() to freeze queue.

Fix this problem by converting to use blkg_conf_open_bdev_frozen()
from iolatency_set_limit(), and convert to use rq_qos_add_freezed().

Signed-off-by: Yu Kuai <yukuai@fnnas.com>
---
 block/blk-iolatency.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c
index 45bd18f68541..1565352b176d 100644
--- a/block/blk-iolatency.c
+++ b/block/blk-iolatency.c
@@ -764,8 +764,8 @@ static int blk_iolatency_init(struct gendisk *disk)
 	if (!blkiolat)
 		return -ENOMEM;
 
-	ret = rq_qos_add(&blkiolat->rqos, disk, RQ_QOS_LATENCY,
-			 &blkcg_iolatency_ops);
+	ret = rq_qos_add_freezed(&blkiolat->rqos, disk, RQ_QOS_LATENCY,
+				 &blkcg_iolatency_ops);
 	if (ret)
 		goto err_free;
 	ret = blkcg_activate_policy(disk, &blkcg_policy_iolatency);
@@ -831,16 +831,19 @@ static ssize_t iolatency_set_limit(struct kernfs_open_file *of, char *buf,
 	struct blkcg_gq *blkg;
 	struct blkg_conf_ctx ctx;
 	struct iolatency_grp *iolat;
+	unsigned long memflags;
 	char *p, *tok;
 	u64 lat_val = 0;
 	u64 oldval;
-	int ret;
+	int ret = 0;
 
 	blkg_conf_init(&ctx, buf);
 
-	ret = blkg_conf_open_bdev(&ctx);
-	if (ret)
+	memflags = blkg_conf_open_bdev_frozen(&ctx);
+	if (IS_ERR_VALUE(memflags)) {
+		ret = memflags;
 		goto out;
+	}
 
 	/*
 	 * blk_iolatency_init() may fail after rq_qos_add() succeeds which can
@@ -890,7 +893,7 @@ static ssize_t iolatency_set_limit(struct kernfs_open_file *of, char *buf,
 		iolatency_clear_scaling(blkg);
 	ret = 0;
 out:
-	blkg_conf_exit(&ctx);
+	blkg_conf_exit_frozen(&ctx, memflags);
 	return ret ?: nbytes;
 }
 
-- 
2.51.0

Re: [PATCH RESEND 4/5] blk-iolatency: fix incorrect lock order for rq_qos_mutex and freeze queue

[Nilay Shroff]

On 11/16/25 9:40 AM, Yu Kuai wrote:
> Currently blk-iolatency will hold rq_qos_mutex first and then call
> rq_qos_add() to freeze queue.
> 
> Fix this problem by converting to use blkg_conf_open_bdev_frozen()
> from iolatency_set_limit(), and convert to use rq_qos_add_freezed().
> 
> Signed-off-by: Yu Kuai <yukuai@fnnas.com>

Looks good to me:
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>

[PATCH RESEND 5/5] block/blk-rq-qos: cleanup rq_qos_add()

[Yu Kuai]

Now that there is no caller of rq_qos_add(), remove it, and also rename
rq_qos_add_freezed() back to rq_qos_add().

Signed-off-by: Yu Kuai <yukuai@fnnas.com>
---
 block/blk-iocost.c    |  2 +-
 block/blk-iolatency.c |  4 ++--
 block/blk-rq-qos.c    | 42 ++----------------------------------------
 block/blk-rq-qos.h    |  6 ++----
 block/blk-wbt.c       |  2 +-
 5 files changed, 8 insertions(+), 48 deletions(-)

diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 233c9749bfc9..0948f628386f 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -2927,7 +2927,7 @@ static int blk_iocost_init(struct gendisk *disk)
 	 * called before policy activation completion, can't assume that the
 	 * target bio has an iocg associated and need to test for NULL iocg.
 	 */
-	ret = rq_qos_add_freezed(&ioc->rqos, disk, RQ_QOS_COST, &ioc_rqos_ops);
+	ret = rq_qos_add(&ioc->rqos, disk, RQ_QOS_COST, &ioc_rqos_ops);
 	if (ret)
 		goto err_free_ioc;
 
diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c
index 1565352b176d..5b18125e21c9 100644
--- a/block/blk-iolatency.c
+++ b/block/blk-iolatency.c
@@ -764,8 +764,8 @@ static int blk_iolatency_init(struct gendisk *disk)
 	if (!blkiolat)
 		return -ENOMEM;
 
-	ret = rq_qos_add_freezed(&blkiolat->rqos, disk, RQ_QOS_LATENCY,
-				 &blkcg_iolatency_ops);
+	ret = rq_qos_add(&blkiolat->rqos, disk, RQ_QOS_LATENCY,
+			 &blkcg_iolatency_ops);
 	if (ret)
 		goto err_free;
 	ret = blkcg_activate_policy(disk, &blkcg_policy_iolatency);
diff --git a/block/blk-rq-qos.c b/block/blk-rq-qos.c
index 353397d7e126..3a49af00b738 100644
--- a/block/blk-rq-qos.c
+++ b/block/blk-rq-qos.c
@@ -322,8 +322,8 @@ void rq_qos_exit(struct request_queue *q)
 	mutex_unlock(&q->rq_qos_mutex);
 }
 
-int rq_qos_add_freezed(struct rq_qos *rqos, struct gendisk *disk,
-		       enum rq_qos_id id, const struct rq_qos_ops *ops)
+int rq_qos_add(struct rq_qos *rqos, struct gendisk *disk,
+	       enum rq_qos_id id, const struct rq_qos_ops *ops)
 {
 	struct request_queue *q = disk->queue;
 
@@ -349,44 +349,6 @@ int rq_qos_add_freezed(struct rq_qos *rqos, struct gendisk *disk,
 	return 0;
 }
 
-int rq_qos_add(struct rq_qos *rqos, struct gendisk *disk, enum rq_qos_id id,
-		const struct rq_qos_ops *ops)
-{
-	struct request_queue *q = disk->queue;
-	unsigned int memflags;
-
-	lockdep_assert_held(&q->rq_qos_mutex);
-
-	rqos->disk = disk;
-	rqos->id = id;
-	rqos->ops = ops;
-
-	/*
-	 * No IO can be in-flight when adding rqos, so freeze queue, which
-	 * is fine since we only support rq_qos for blk-mq queue.
-	 */
-	memflags = blk_mq_freeze_queue(q);
-
-	if (rq_qos_id(q, rqos->id))
-		goto ebusy;
-	rqos->next = q->rq_qos;
-	q->rq_qos = rqos;
-	blk_queue_flag_set(QUEUE_FLAG_QOS_ENABLED, q);
-
-	blk_mq_unfreeze_queue(q, memflags);
-
-	if (rqos->ops->debugfs_attrs) {
-		mutex_lock(&q->debugfs_mutex);
-		blk_mq_debugfs_register_rqos(rqos);
-		mutex_unlock(&q->debugfs_mutex);
-	}
-
-	return 0;
-ebusy:
-	blk_mq_unfreeze_queue(q, memflags);
-	return -EBUSY;
-}
-
 void rq_qos_del(struct rq_qos *rqos)
 {
 	struct request_queue *q = rqos->disk->queue;
diff --git a/block/blk-rq-qos.h b/block/blk-rq-qos.h
index 4a7fec01600b..8bbf178c16b0 100644
--- a/block/blk-rq-qos.h
+++ b/block/blk-rq-qos.h
@@ -85,10 +85,8 @@ static inline void rq_wait_init(struct rq_wait *rq_wait)
 	init_waitqueue_head(&rq_wait->wait);
 }
 
-int rq_qos_add(struct rq_qos *rqos, struct gendisk *disk, enum rq_qos_id id,
-		const struct rq_qos_ops *ops);
-int rq_qos_add_freezed(struct rq_qos *rqos, struct gendisk *disk,
-		       enum rq_qos_id id, const struct rq_qos_ops *ops);
+int rq_qos_add(struct rq_qos *rqos, struct gendisk *disk,
+	       enum rq_qos_id id, const struct rq_qos_ops *ops);
 void rq_qos_del(struct rq_qos *rqos);
 
 typedef bool (acquire_inflight_cb_t)(struct rq_wait *rqw, void *private_data);
diff --git a/block/blk-wbt.c b/block/blk-wbt.c
index a784f6d338b4..d7f1e6ba1790 100644
--- a/block/blk-wbt.c
+++ b/block/blk-wbt.c
@@ -926,7 +926,7 @@ int wbt_init(struct gendisk *disk)
 	 * Assign rwb and add the stats callback.
 	 */
 	mutex_lock(&q->rq_qos_mutex);
-	ret = rq_qos_add_freezed(&rwb->rqos, disk, RQ_QOS_WBT, &wbt_rqos_ops);
+	ret = rq_qos_add(&rwb->rqos, disk, RQ_QOS_WBT, &wbt_rqos_ops);
 	mutex_unlock(&q->rq_qos_mutex);
 	if (ret)
 		goto err_free;
-- 
2.51.0

Re: [PATCH RESEND 5/5] block/blk-rq-qos: cleanup rq_qos_add()

[Nilay Shroff]

On 11/16/25 9:40 AM, Yu Kuai wrote:
> Now that there is no caller of rq_qos_add(), remove it, and also rename
> rq_qos_add_freezed() back to rq_qos_add().
> 
> Signed-off-by: Yu Kuai <yukuai@fnnas.com>

Looks good to me:
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>