Re: [PATCH RESEND 1/5] block/blk-rq-qos: add a new helper rq_qos_add_freezed()
From: Nilay Shroff <hidden>
Date: 2025-11-17 11:13:28
Also in:
lkml
On 11/17/25 4:31 PM, Ming Lei wrote:
On Sun, Nov 16, 2025 at 12:10:20PM +0800, Yu Kuai wrote:quoted
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 <redacted> --- 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