Re: [PATCH v7 04/16] blk-rq-qos: fix possible debugfs_mutex deadlock

[Ming Lei <ming.lei@redhat.com>]

On Wed, Dec 31, 2025 at 04:51:14PM +0800, Yu Kuai wrote:
> Currently rq-qos debugfs entries are created from rq_qos_add(), while
> rq_qos_add() can be called while queue is still frozen. This can
> deadlock because creating new entries can trigger fs reclaim.
> 
> Fix this problem by delaying creating rq-qos debugfs entries after queue
> is unfrozen.
> 
> - For wbt, 1) it can be initialized by default, fix it by calling new
>   helper after wbt_init() from wbt_init_enable_default(); 2) it can be
>   initialized by sysfs, fix it by calling new helper after queue is
>   unfrozen from wbt_set_lat().
> - For iocost and iolatency, they can only be initialized by blkcg
>   configuration, however, they don't have debugfs entries for now, hence
>   they are not handled yet.
> 
> Signed-off-by: Yu Kuai <yukuai@fnnas.com>
> Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
> ---
>  block/blk-rq-qos.c |  7 -------
>  block/blk-wbt.c    | 13 ++++++++++++-
>  2 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/block/blk-rq-qos.c b/block/blk-rq-qos.c
> index 654478dfbc20..d7ce99ce2e80 100644
> --- a/block/blk-rq-qos.c
> +++ b/block/blk-rq-qos.c
> @@ -347,13 +347,6 @@ int rq_qos_add(struct rq_qos *rqos, struct gendisk *disk, enum rq_qos_id id,
>  	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);
> diff --git a/block/blk-wbt.c b/block/blk-wbt.c
> index 9bef71ec645d..de3528236545 100644
> --- a/block/blk-wbt.c
> +++ b/block/blk-wbt.c
> @@ -774,6 +774,7 @@ EXPORT_SYMBOL_GPL(wbt_enable_default);
>  
>  void wbt_init_enable_default(struct gendisk *disk)
>  {
> +	struct request_queue *q = disk->queue;
>  	struct rq_wb *rwb;
>  
>  	if (!__wbt_enable_default(disk))
> @@ -783,8 +784,14 @@ void wbt_init_enable_default(struct gendisk *disk)
>  	if (WARN_ON_ONCE(!rwb))
>  		return;
>  
> -	if (WARN_ON_ONCE(wbt_init(disk, rwb)))
> +	if (WARN_ON_ONCE(wbt_init(disk, rwb))) {
>  		wbt_free(rwb);
> +		return;
> +	}
> +
> +	mutex_lock(&q->debugfs_mutex);
> +	blk_mq_debugfs_register_rq_qos(q);
> +	mutex_unlock(&q->debugfs_mutex);
>  }
>  
>  static u64 wbt_default_latency_nsec(struct request_queue *q)
> @@ -1009,5 +1016,9 @@ int wbt_set_lat(struct gendisk *disk, s64 val)
>  	blk_mq_unquiesce_queue(q);
>  out:
>  	blk_mq_unfreeze_queue(q, memflags);
> +	mutex_lock(&q->debugfs_mutex);
> +	blk_mq_debugfs_register_rq_qos(q);
> +	mutex_unlock(&q->debugfs_mutex);
> +

You may add one helper for avoiding the code duplication, but not a big
deal:

Reviewed-by: Ming Lei <ming.lei@redhat.com>


Thanks,
Ming