Skip to content

Commit 3d924fc

Browse files
author
Ming Lei
committed
block: move elv_register[unregister]_queue out of elevator_lock
JIRA: https://issues.redhat.com/browse/RHEL-106845 commit 559dc11 Author: Ming Lei <ming.lei@redhat.com> Date: Mon May 5 22:17:59 2025 +0800 block: move elv_register[unregister]_queue out of elevator_lock Move elv_register[unregister]_queue out of ->elevator_lock & queue freezing, so we can kill many lockdep warnings. elv_register[unregister]_queue() is serialized, and just dealing with sysfs/ debugfs things, no need to be done with queue frozen: - when it is called from adding disk, elevator switch isn't possible because ->queue_kobj isn't added yet - when it is called from deleting disk, disable_elv_switch() is responsible for preventing new elevator switch and draining old elevator switch. - when it is called from blk_mq_update_nr_hw_queues(), adding/removing disk and elevator switch can't be allowed or in-progress With this change, elevator's ->exit() is called before calling elv_unregister_queue, then user may call into ->show()/store() of elevator's sysfs attributes, and we have covered this issue by adding `ELEVATOR_FLAG_DYNG`. For blk-mq debugfs, hctx->sched_tags is always checked with ->elevator_lock by debugfs code, meantime hctx->sched_tags is updated with ->elevator_lock, so there isn't such issue. Reviewed-by: Hannes Reinecke <hare@suse.de> Signed-off-by: Ming Lei <ming.lei@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Link: https://lore.kernel.org/r/20250505141805.2751237-22-ming.lei@redhat.com Signed-off-by: Jens Axboe <axboe@kernel.dk> Signed-off-by: Ming Lei <ming.lei@redhat.com>
1 parent 0a954cf commit 3d924fc

File tree

2 files changed

+53
-18
lines changed

2 files changed

+53
-18
lines changed

‎block/blk-mq.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5041,11 +5041,10 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
50415041
blk_mq_debugfs_register_hctxs(q);
50425042
}
50435043

5044+
/* elv_update_nr_hw_queues() unfreeze queue for us */
50445045
list_for_each_entry(q, &set->tag_list, tag_set_list)
50455046
elv_update_nr_hw_queues(q);
50465047

5047-
list_for_each_entry(q, &set->tag_list, tag_set_list)
5048-
blk_mq_unfreeze_queue_nomemrestore(q);
50495048
memalloc_noio_restore(memflags);
50505049

50515050
/* Free the excess tags when nr_hw_queues shrink. */

‎block/elevator.c‎

Lines changed: 52 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@
4949
struct elv_change_ctx {
5050
const char *name;
5151
bool no_uevent;
52+
53+
/* for unregistering old elevator */
54+
struct elevator_queue *old;
55+
/* for registering new elevator */
56+
struct elevator_queue *new;
5257
};
5358

5459
static DEFINE_SPINLOCK(elv_list_lock);
@@ -158,14 +163,14 @@ static void elevator_exit(struct request_queue *q)
158163
{
159164
struct elevator_queue *e = q->elevator;
160165

166+
lockdep_assert_held(&q->elevator_lock);
167+
161168
ioc_clear_queue(q);
162169
blk_mq_sched_free_rqs(q);
163170

164171
mutex_lock(&e->sysfs_lock);
165172
blk_mq_exit_sched(q, e);
166173
mutex_unlock(&e->sysfs_lock);
167-
168-
kobject_put(&e->kobj);
169174
}
170175

171176
static inline void __elv_rqhash_del(struct request *rq)
@@ -466,8 +471,6 @@ static int elv_register_queue(struct request_queue *q,
466471
{
467472
int error;
468473

469-
lockdep_assert_held(&q->elevator_lock);
470-
471474
error = kobject_add(&e->kobj, &q->disk->queue_kobj, "iosched");
472475
if (!error) {
473476
const struct elv_fs_entry *attr = e->type->elevator_attrs;
@@ -494,8 +497,6 @@ static int elv_register_queue(struct request_queue *q,
494497
static void elv_unregister_queue(struct request_queue *q,
495498
struct elevator_queue *e)
496499
{
497-
lockdep_assert_held(&q->elevator_lock);
498-
499500
if (e && test_and_clear_bit(ELEVATOR_FLAG_REGISTERED, &e->flags)) {
500501
kobject_uevent(&e->kobj, KOBJ_REMOVE);
501502
kobject_del(&e->kobj);
@@ -586,19 +587,15 @@ static int elevator_switch(struct request_queue *q, struct elv_change_ctx *ctx)
586587
blk_mq_quiesce_queue(q);
587588

588589
if (q->elevator) {
589-
elv_unregister_queue(q, q->elevator);
590+
ctx->old = q->elevator;
590591
elevator_exit(q);
591592
}
592593

593594
if (new_e) {
594595
ret = blk_mq_init_sched(q, new_e);
595596
if (ret)
596597
goto out_unfreeze;
597-
ret = elv_register_queue(q, q->elevator, !ctx->no_uevent);
598-
if (ret) {
599-
elevator_exit(q);
600-
goto out_unfreeze;
601-
}
598+
ctx->new = q->elevator;
602599
} else {
603600
blk_queue_flag_clear(QUEUE_FLAG_SQ_SCHED, q);
604601
q->elevator = NULL;
@@ -619,6 +616,38 @@ static int elevator_switch(struct request_queue *q, struct elv_change_ctx *ctx)
619616
return ret;
620617
}
621618

619+
static void elv_exit_and_release(struct request_queue *q)
620+
{
621+
struct elevator_queue *e;
622+
unsigned memflags;
623+
624+
memflags = blk_mq_freeze_queue(q);
625+
mutex_lock(&q->elevator_lock);
626+
e = q->elevator;
627+
elevator_exit(q);
628+
mutex_unlock(&q->elevator_lock);
629+
blk_mq_unfreeze_queue(q, memflags);
630+
if (e)
631+
kobject_put(&e->kobj);
632+
}
633+
634+
static int elevator_change_done(struct request_queue *q,
635+
struct elv_change_ctx *ctx)
636+
{
637+
int ret = 0;
638+
639+
if (ctx->old) {
640+
elv_unregister_queue(q, ctx->old);
641+
kobject_put(&ctx->old->kobj);
642+
}
643+
if (ctx->new) {
644+
ret = elv_register_queue(q, ctx->new, !ctx->no_uevent);
645+
if (ret)
646+
elv_exit_and_release(q);
647+
}
648+
return ret;
649+
}
650+
622651
/*
623652
* Switch this queue to the given IO scheduler.
624653
*/
@@ -645,6 +674,9 @@ static int elevator_change(struct request_queue *q, struct elv_change_ctx *ctx)
645674
ret = elevator_switch(q, ctx);
646675
mutex_unlock(&q->elevator_lock);
647676
blk_mq_unfreeze_queue(q, memflags);
677+
if (!ret)
678+
ret = elevator_change_done(q, ctx);
679+
648680
return ret;
649681
}
650682

@@ -654,18 +686,22 @@ static int elevator_change(struct request_queue *q, struct elv_change_ctx *ctx)
654686
*/
655687
void elv_update_nr_hw_queues(struct request_queue *q)
656688
{
689+
struct elv_change_ctx ctx = {};
690+
int ret = -ENODEV;
691+
657692
WARN_ON_ONCE(q->mq_freeze_depth == 0);
658693

659694
mutex_lock(&q->elevator_lock);
660695
if (q->elevator && !blk_queue_dying(q) && !blk_queue_registered(q)) {
661-
struct elv_change_ctx ctx = {
662-
.name = q->elevator->type->elevator_name,
663-
};
696+
ctx.name = q->elevator->type->elevator_name;
664697

665698
/* force to reattach elevator after nr_hw_queue is updated */
666-
elevator_switch(q, &ctx);
699+
ret = elevator_switch(q, &ctx);
667700
}
668701
mutex_unlock(&q->elevator_lock);
702+
blk_mq_unfreeze_queue_nomemrestore(q);
703+
if (!ret)
704+
WARN_ON_ONCE(elevator_change_done(q, &ctx));
669705
}
670706

671707
/*

0 commit comments

Comments
 (0)