Creating new debugfs entries can trigger fs reclaim, hence we can't do this with queue freezed, meanwhile, other locks that can be held while queue is freezed should not be held as well. Signed-off-by: Yu Kuai --- block/blk-mq-debugfs.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c index 4896525b1c05..66864ed0b77f 100644 --- a/block/blk-mq-debugfs.c +++ b/block/blk-mq-debugfs.c @@ -608,9 +608,23 @@ static const struct blk_mq_debugfs_attr blk_mq_debugfs_ctx_attrs[] = { {}, }; -static void debugfs_create_files(struct dentry *parent, void *data, +static void debugfs_create_files(struct request_queue *q, struct dentry *parent, + void *data, const struct blk_mq_debugfs_attr *attr) { + /* + * Creating new debugfs entries with queue freezed has the rist of + * deadlock. + */ + WARN_ON_ONCE(q->mq_freeze_depth != 0); + /* + * debugfs_mutex should not be nested under other locks that can be + * grabbed while queue is freezed. + */ + lockdep_assert_not_held(&q->elevator_lock); + lockdep_assert_not_held(&q->rq_qos_mutex); + lockdep_assert_not_held(&q->blkcg_mutex); + if (IS_ERR_OR_NULL(parent)) return; @@ -624,7 +638,7 @@ void blk_mq_debugfs_register(struct request_queue *q) struct blk_mq_hw_ctx *hctx; unsigned long i; - debugfs_create_files(q->debugfs_dir, q, blk_mq_debugfs_queue_attrs); + debugfs_create_files(q, q->debugfs_dir, q, blk_mq_debugfs_queue_attrs); queue_for_each_hw_ctx(q, hctx, i) { if (!hctx->debugfs_dir) @@ -650,7 +664,8 @@ static void blk_mq_debugfs_register_ctx(struct blk_mq_hw_ctx *hctx, snprintf(name, sizeof(name), "cpu%u", ctx->cpu); ctx_dir = debugfs_create_dir(name, hctx->debugfs_dir); - debugfs_create_files(ctx_dir, ctx, blk_mq_debugfs_ctx_attrs); + debugfs_create_files(hctx->queue, ctx_dir, ctx, + blk_mq_debugfs_ctx_attrs); } void blk_mq_debugfs_register_hctx(struct request_queue *q, @@ -666,7 +681,8 @@ void blk_mq_debugfs_register_hctx(struct request_queue *q, snprintf(name, sizeof(name), "hctx%u", hctx->queue_num); hctx->debugfs_dir = debugfs_create_dir(name, q->debugfs_dir); - debugfs_create_files(hctx->debugfs_dir, hctx, blk_mq_debugfs_hctx_attrs); + debugfs_create_files(q, hctx->debugfs_dir, hctx, + blk_mq_debugfs_hctx_attrs); hctx_for_each_ctx(hctx, ctx, i) blk_mq_debugfs_register_ctx(hctx, ctx); @@ -717,7 +733,7 @@ void blk_mq_debugfs_register_sched(struct request_queue *q) q->sched_debugfs_dir = debugfs_create_dir("sched", q->debugfs_dir); - debugfs_create_files(q->sched_debugfs_dir, q, e->queue_debugfs_attrs); + debugfs_create_files(q, q->sched_debugfs_dir, q, e->queue_debugfs_attrs); } void blk_mq_debugfs_unregister_sched(struct request_queue *q) @@ -766,7 +782,8 @@ void blk_mq_debugfs_register_rqos(struct rq_qos *rqos) q->debugfs_dir); rqos->debugfs_dir = debugfs_create_dir(dir_name, q->rqos_debugfs_dir); - debugfs_create_files(rqos->debugfs_dir, rqos, rqos->ops->debugfs_attrs); + debugfs_create_files(q, rqos->debugfs_dir, rqos, + rqos->ops->debugfs_attrs); } void blk_mq_debugfs_register_sched_hctx(struct request_queue *q, @@ -789,7 +806,7 @@ void blk_mq_debugfs_register_sched_hctx(struct request_queue *q, hctx->sched_debugfs_dir = debugfs_create_dir("sched", hctx->debugfs_dir); - debugfs_create_files(hctx->sched_debugfs_dir, hctx, + debugfs_create_files(q, hctx->sched_debugfs_dir, hctx, e->hctx_debugfs_attrs); } -- 2.39.2 Prepare to fix possible deadlock to create blk-mq debugfs entries while queue is still freezed. Signed-off-by: Yu Kuai --- block/blk-mq-debugfs.c | 23 +++++++++++++++-------- block/blk-mq-debugfs.h | 5 +++++ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c index 66864ed0b77f..1de9bab7ba80 100644 --- a/block/blk-mq-debugfs.c +++ b/block/blk-mq-debugfs.c @@ -633,6 +633,20 @@ static void debugfs_create_files(struct request_queue *q, struct dentry *parent, (void *)attr, data, &blk_mq_debugfs_fops); } +void blk_mq_debugfs_register_rq_qos(struct request_queue *q) +{ + lockdep_assert_held(&q->debugfs_mutex); + + if (q->rq_qos) { + struct rq_qos *rqos = q->rq_qos; + + while (rqos) { + blk_mq_debugfs_register_rqos(rqos); + rqos = rqos->next; + } + } +} + void blk_mq_debugfs_register(struct request_queue *q) { struct blk_mq_hw_ctx *hctx; @@ -645,14 +659,7 @@ void blk_mq_debugfs_register(struct request_queue *q) blk_mq_debugfs_register_hctx(q, hctx); } - if (q->rq_qos) { - struct rq_qos *rqos = q->rq_qos; - - while (rqos) { - blk_mq_debugfs_register_rqos(rqos); - rqos = rqos->next; - } - } + blk_mq_debugfs_register_rq_qos(q); } static void blk_mq_debugfs_register_ctx(struct blk_mq_hw_ctx *hctx, diff --git a/block/blk-mq-debugfs.h b/block/blk-mq-debugfs.h index c80e453e3014..9f76603792fe 100644 --- a/block/blk-mq-debugfs.h +++ b/block/blk-mq-debugfs.h @@ -34,6 +34,7 @@ void blk_mq_debugfs_register_sched_hctx(struct request_queue *q, void blk_mq_debugfs_unregister_sched_hctx(struct blk_mq_hw_ctx *hctx); void blk_mq_debugfs_register_rqos(struct rq_qos *rqos); +void blk_mq_debugfs_register_rq_qos(struct request_queue *q); void blk_mq_debugfs_unregister_rqos(struct rq_qos *rqos); #else static inline void blk_mq_debugfs_register(struct request_queue *q) @@ -78,6 +79,10 @@ static inline void blk_mq_debugfs_register_rqos(struct rq_qos *rqos) { } +static inline void blk_mq_debugfs_register_rq_qos(struct request_queue *q) +{ +} + static inline void blk_mq_debugfs_unregister_rqos(struct rq_qos *rqos) { } -- 2.39.2 Currently rq-qos debugfs entries is created from rq_qos_add(), while rq_qos_add() requires queue to be freezed. This can deadlock because creating new entries can trigger fs reclaim. Fix this problem by delaying creating rq-qos debugfs entries until it's initialization is complete. - For wbt, it can be initialized by default of by blk-sysfs, fix it by calling blk_mq_debugfs_register_rq_qos() after wbt_init; - For other policies, they can only be initialized by blkg configuration, fix it by calling blk_mq_debugfs_register_rq_qos() from blkg_conf_end(); Signed-off-by: Yu Kuai --- block/blk-cgroup.c | 6 ++++++ block/blk-rq-qos.c | 7 ------- block/blk-sysfs.c | 4 ++++ block/blk-wbt.c | 7 ++++++- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index d93654334854..e4ccabf132c0 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -33,6 +33,7 @@ #include "blk-cgroup.h" #include "blk-ioprio.h" #include "blk-throttle.h" +#include "blk-mq-debugfs.h" static void __blkcg_rstat_flush(struct blkcg *blkcg, int cpu); @@ -746,6 +747,11 @@ void blkg_conf_end(struct blkg_conf_ctx *ctx) mutex_unlock(&q->elevator_lock); blk_mq_unfreeze_queue(q, ctx->memflags); blkdev_put_no_open(ctx->bdev); + + mutex_lock(&q->debugfs_mutex); + blk_mq_debugfs_register_rq_qos(q); + mutex_unlock(&q->debugfs_mutex); + } EXPORT_SYMBOL_GPL(blkg_conf_end); 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-sysfs.c b/block/blk-sysfs.c index 76c47fe9b8d6..52bb4db25cf5 100644 --- a/block/blk-sysfs.c +++ b/block/blk-sysfs.c @@ -688,6 +688,10 @@ static ssize_t queue_wb_lat_store(struct gendisk *disk, const char *page, mutex_unlock(&disk->rqos_state_mutex); blk_mq_unquiesce_queue(q); + + mutex_lock(&q->debugfs_mutex); + blk_mq_debugfs_register_rq_qos(q); + mutex_unlock(&q->debugfs_mutex); out: blk_mq_unfreeze_queue(q, memflags); diff --git a/block/blk-wbt.c b/block/blk-wbt.c index eb8037bae0bd..a120b5ba54db 100644 --- a/block/blk-wbt.c +++ b/block/blk-wbt.c @@ -724,8 +724,13 @@ 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) { wbt_init(disk); + + mutex_lock(&q->debugfs_mutex); + blk_mq_debugfs_register_rq_qos(q); + mutex_unlock(&q->debugfs_mutex); + } } EXPORT_SYMBOL_GPL(wbt_enable_default); -- 2.39.2 Because it's only used inside blk-mq-debugfs.c now. Signed-off-by: Yu Kuai --- block/blk-mq-debugfs.c | 4 +++- block/blk-mq-debugfs.h | 5 ----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c index 1de9bab7ba80..919e484aa1b2 100644 --- a/block/blk-mq-debugfs.c +++ b/block/blk-mq-debugfs.c @@ -14,6 +14,8 @@ #include "blk-mq-sched.h" #include "blk-rq-qos.h" +static void blk_mq_debugfs_register_rqos(struct rq_qos *rqos); + static int queue_poll_stat_show(void *data, struct seq_file *m) { return 0; @@ -774,7 +776,7 @@ void blk_mq_debugfs_unregister_rqos(struct rq_qos *rqos) rqos->debugfs_dir = NULL; } -void blk_mq_debugfs_register_rqos(struct rq_qos *rqos) +static void blk_mq_debugfs_register_rqos(struct rq_qos *rqos) { struct request_queue *q = rqos->disk->queue; const char *dir_name = rq_qos_id_to_name(rqos->id); diff --git a/block/blk-mq-debugfs.h b/block/blk-mq-debugfs.h index 9f76603792fe..d94daa66556b 100644 --- a/block/blk-mq-debugfs.h +++ b/block/blk-mq-debugfs.h @@ -33,7 +33,6 @@ void blk_mq_debugfs_register_sched_hctx(struct request_queue *q, struct blk_mq_hw_ctx *hctx); void blk_mq_debugfs_unregister_sched_hctx(struct blk_mq_hw_ctx *hctx); -void blk_mq_debugfs_register_rqos(struct rq_qos *rqos); void blk_mq_debugfs_register_rq_qos(struct request_queue *q); void blk_mq_debugfs_unregister_rqos(struct rq_qos *rqos); #else @@ -75,10 +74,6 @@ static inline void blk_mq_debugfs_unregister_sched_hctx(struct blk_mq_hw_ctx *hc { } -static inline void blk_mq_debugfs_register_rqos(struct rq_qos *rqos) -{ -} - static inline void blk_mq_debugfs_register_rq_qos(struct request_queue *q) { } -- 2.39.2