Commit 559dc111 authored by Ming Lei's avatar Ming Lei Committed by Jens Axboe
Browse files

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: default avatarHannes Reinecke <hare@suse.de>
Signed-off-by: default avatarMing Lei <ming.lei@redhat.com>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Link: https://lore.kernel.org/r/20250505141805.2751237-22-ming.lei@redhat.com


Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent 21eed794

block/blk-mq.c

+1 −2
Original line number Diff line number Diff line
@@ -5041,11 +5041,10 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, 
		blk_mq_debugfs_register_hctxs(q);

	}



	/* elv_update_nr_hw_queues() unfreeze queue for us */

	list_for_each_entry(q, &set->tag_list, tag_set_list)

		elv_update_nr_hw_queues(q);



	list_for_each_entry(q, &set->tag_list, tag_set_list)

		blk_mq_unfreeze_queue_nomemrestore(q);

	memalloc_noio_restore(memflags);



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

block/elevator.c

+52 −16
Original line number Diff line number Diff line
@@ -49,6 +49,11 @@ 
struct elv_change_ctx {

	const char *name;

	bool no_uevent;



	/* for unregistering old elevator */

	struct elevator_queue *old;

	/* for registering new elevator */

	struct elevator_queue *new;

};



static DEFINE_SPINLOCK(elv_list_lock);
@@ -158,14 +163,14 @@ static void elevator_exit(struct request_queue *q) 
{

	struct elevator_queue *e = q->elevator;



	lockdep_assert_held(&q->elevator_lock);



	ioc_clear_queue(q);

	blk_mq_sched_free_rqs(q);



	mutex_lock(&e->sysfs_lock);

	blk_mq_exit_sched(q, e);

	mutex_unlock(&e->sysfs_lock);



	kobject_put(&e->kobj);

}



static inline void __elv_rqhash_del(struct request *rq)
@@ -466,8 +471,6 @@ static int elv_register_queue(struct request_queue *q, 
{

	int error;



	lockdep_assert_held(&q->elevator_lock);



	error = kobject_add(&e->kobj, &q->disk->queue_kobj, "iosched");

	if (!error) {

		const struct elv_fs_entry *attr = e->type->elevator_attrs;
@@ -494,8 +497,6 @@ static int elv_register_queue(struct request_queue *q, 
static void elv_unregister_queue(struct request_queue *q,

				 struct elevator_queue *e)

{

	lockdep_assert_held(&q->elevator_lock);



	if (e && test_and_clear_bit(ELEVATOR_FLAG_REGISTERED, &e->flags)) {

		kobject_uevent(&e->kobj, KOBJ_REMOVE);

		kobject_del(&e->kobj);
@@ -586,7 +587,7 @@ static int elevator_switch(struct request_queue *q, struct elv_change_ctx *ctx) 
	blk_mq_quiesce_queue(q);



	if (q->elevator) {

		elv_unregister_queue(q, q->elevator);

		ctx->old = q->elevator;

		elevator_exit(q);

	}
@@ -594,11 +595,7 @@ static int elevator_switch(struct request_queue *q, struct elv_change_ctx *ctx) 
		ret = blk_mq_init_sched(q, new_e);

		if (ret)

			goto out_unfreeze;

		ret = elv_register_queue(q, q->elevator, !ctx->no_uevent);

		if (ret) {

			elevator_exit(q);

			goto out_unfreeze;

		}

		ctx->new = q->elevator;

	} else {

		blk_queue_flag_clear(QUEUE_FLAG_SQ_SCHED, q);

		q->elevator = NULL;
@@ -619,6 +616,38 @@ static int elevator_switch(struct request_queue *q, struct elv_change_ctx *ctx) 
	return ret;

}



static void elv_exit_and_release(struct request_queue *q)

{

	struct elevator_queue *e;

	unsigned memflags;



	memflags = blk_mq_freeze_queue(q);

	mutex_lock(&q->elevator_lock);

	e = q->elevator;

	elevator_exit(q);

	mutex_unlock(&q->elevator_lock);

	blk_mq_unfreeze_queue(q, memflags);

	if (e)

		kobject_put(&e->kobj);

}



static int elevator_change_done(struct request_queue *q,

				struct elv_change_ctx *ctx)

{

	int ret = 0;



	if (ctx->old) {

		elv_unregister_queue(q, ctx->old);

		kobject_put(&ctx->old->kobj);

	}

	if (ctx->new) {

		ret = elv_register_queue(q, ctx->new, !ctx->no_uevent);

		if (ret)

			elv_exit_and_release(q);

	}

	return ret;

}



/*

 * Switch this queue to the given IO scheduler.

 */
@@ -645,6 +674,9 @@ static int elevator_change(struct request_queue *q, struct elv_change_ctx *ctx) 
		ret = elevator_switch(q, ctx);

	mutex_unlock(&q->elevator_lock);

	blk_mq_unfreeze_queue(q, memflags);

	if (!ret)

		ret = elevator_change_done(q, ctx);



	return ret;

}
@@ -654,18 +686,22 @@ static int elevator_change(struct request_queue *q, struct elv_change_ctx *ctx) 
 */

void elv_update_nr_hw_queues(struct request_queue *q)

{

	struct elv_change_ctx ctx = {};

	int ret = -ENODEV;



	WARN_ON_ONCE(q->mq_freeze_depth == 0);



	mutex_lock(&q->elevator_lock);

	if (q->elevator && !blk_queue_dying(q) && !blk_queue_registered(q)) {

		struct elv_change_ctx ctx = {

			.name = q->elevator->type->elevator_name,

		};

		ctx.name = q->elevator->type->elevator_name;



		/* force to reattach elevator after nr_hw_queue is updated */

		elevator_switch(q, &ctx);

		ret = elevator_switch(q, &ctx);

	}

	mutex_unlock(&q->elevator_lock);

	blk_mq_unfreeze_queue_nomemrestore(q);

	if (!ret)

		WARN_ON_ONCE(elevator_change_done(q, &ctx));

}



/*