| Message ID | 20250418163708.442085-21-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | block: unify elevator changing and fix lockdep warning | expand |
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@redhat.com> > --- > 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
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 {
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@redhat.com> --- block/bfq-iosched.c | 2 +- block/elevator.c | 5 +++++ block/elevator.h | 1 + 3 files changed, 7 insertions(+), 1 deletion(-)