On 4/18/25 10:07 PM, Ming Lei wrote:
> scheduler's ->exit() is called with queue frozen and elevator lock is held, and
> wbt_enable_default() can't be called with queue frozen, otherwise the
> following lockdep warning is triggered:
>
> #6 (&q->rq_qos_mutex){+.+.}-{4:4}:
> #5 (&eq->sysfs_lock){+.+.}-{4:4}:
> #4 (&q->elevator_lock){+.+.}-{4:4}:
> #3 (&q->q_usage_counter(io)#3){++++}-{0:0}:
> #2 (fs_reclaim){+.+.}-{0:0}:
> #1 (&sb->s_type->i_mutex_key#3){+.+.}-{4:4}:
> #0 (&q->debugfs_mutex){+.+.}-{4:4}:
>
> Fix the issue by moving wbt_enable_default() out of bfq's exit(), and
> call it from elevator_change_done().
>
> Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
> ---
> block/bfq-iosched.c | 2 +-
> block/elevator.c | 5 +++++
> block/elevator.h | 1 +
> 3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index 40e4106a71e7..310ce1d8c41e 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -7211,7 +7211,7 @@ static void bfq_exit_queue(struct elevator_queue *e)
>
> blk_stat_disable_accounting(bfqd->queue);
> blk_queue_flag_clear(QUEUE_FLAG_DISABLE_WBT, bfqd->queue);
> - wbt_enable_default(bfqd->queue->disk);
> + set_bit(ELEVATOR_FLAG_ENABLE_WBT_ON_EXIT, &e->flags);
>
> kfree(bfqd);
> }
> diff --git a/block/elevator.c b/block/elevator.c
> index 8652fe45a2db..378553fce5d8 100644
> --- a/block/elevator.c
> +++ b/block/elevator.c
> @@ -687,8 +687,13 @@ int elevator_change_done(struct request_queue *q, struct elv_change_ctx *ctx)
> int ret = 0;
>
> if (ctx->old) {
> + bool enable_wbt = test_bit(ELEVATOR_FLAG_ENABLE_WBT_ON_EXIT,
> + &ctx->old->flags);
> +
> elv_unregister_queue(q, ctx->old);
> kobject_put(&ctx->old->kobj);
> + if (enable_wbt)
> + wbt_enable_default(q->disk);
> }
> if (ctx->new) {
> ret = elv_register_queue(q, ctx->new, ctx->uevent);
> diff --git a/block/elevator.h b/block/elevator.h
> index 486be0690499..b14c611c74b6 100644
> --- a/block/elevator.h
> +++ b/block/elevator.h
> @@ -122,6 +122,7 @@ struct elevator_queue
>
> #define ELEVATOR_FLAG_REGISTERED 0
> #define ELEVATOR_FLAG_DYING 1
> +#define ELEVATOR_FLAG_ENABLE_WBT_ON_EXIT 2
>
> /* Holding context data for changing elevator */
> struct elv_change_ctx {
It seems invoking wbt_enable_default from elevator_change_done could probably
still race with ioc_qos_write or queue_wb_lat_store. Both ioc_qos_write and
queue_wb_lat_store run with ->freeze_lock and ->elevator_lock protection.
Thanks,
--Nilay
