diff mbox series

[08/15] block: add `struct elev_change_ctx` for unifying elevator change

Message ID 20250410133029.2487054-9-ming.lei@redhat.com (mailing list archive)
State New
Headers show
Series block: unify elevator changing and fix lockdep warning | expand

Commit Message

Ming Lei April 10, 2025, 1:30 p.m. UTC 
Add `struct elev_change_ctx` and prepare for unifying elevator change,
with this way, any input & output parameter can be provided & observed
in top caller.

This way also helps to move kobject & debugfs things out of
->elevator_lock.

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq.c   | 10 +++++++---
 block/blk.h      |  4 ++--
 block/elevator.c | 33 +++++++++++++++++++--------------
 block/elevator.h |  7 +++++++
 4 files changed, 35 insertions(+), 19 deletions(-)

Comments

Christoph Hellwig April 14, 2025, 6:21 a.m. UTC | #1 
On Thu, Apr 10, 2025 at 09:30:20PM +0800, Ming Lei wrote:
> +		struct elev_change_ctx ctx = {
> +			.name	= "none",
> +			.force	= 1,
> +			.uevent	= 1,
> +		};
>  
>  		mutex_lock(&q->elevator_lock);
>  		if (q->elevator && !blk_queue_dying(q))
> +			ctx.name = q->elevator->type->elevator_name;
> +		__elevator_change(q, &ctx);
>  		mutex_unlock(&q->elevator_lock);

Can we have this whole code section in a helper in elevator.c and
keep the low-level __elevator_change function private there?

> +/* Holding data for changing elevator */
> +struct elev_change_ctx {
> +	const char *name;
> +	unsigned int force:1;
> +	unsigned int uevent:1;

Using an integer type for flags is odd.  This should a boolean, or even
better aflags field with flag names.

And I think with the flag names we can just pass the name and the flags
and do away with the separate structure entirely.
diff mbox series

Patch

diff --git a/block/blk-mq.c b/block/blk-mq.c
index b7e3cd355e66..608a74e3a87c 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4985,12 +4985,16 @@  static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
 	}
 
 	list_for_each_entry(q, &set->tag_list, tag_set_list) {
-		const char *name = "none";
+		struct elev_change_ctx ctx = {
+			.name	= "none",
+			.force	= 1,
+			.uevent	= 1,
+		};
 
 		mutex_lock(&q->elevator_lock);
 		if (q->elevator && !blk_queue_dying(q))
-			name = q->elevator->type->elevator_name;
-		__elevator_change(q, name, true);
+			ctx.name = q->elevator->type->elevator_name;
+		__elevator_change(q, &ctx);
 		mutex_unlock(&q->elevator_lock);
 	}
 
diff --git a/block/blk.h b/block/blk.h
index 0c3cc1af2525..922a429b5363 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -12,6 +12,7 @@ 
 #include "blk-crypto-internal.h"
 
 struct elevator_type;
+struct elev_change_ctx;
 
 #define	BLK_DEV_MAX_SECTORS	(LLONG_MAX >> 9)
 #define	BLK_MIN_SEGMENT_SIZE	4096
@@ -319,8 +320,7 @@  bool blk_bio_list_merge(struct request_queue *q, struct list_head *list,
 
 bool blk_insert_flush(struct request *rq);
 
-int __elevator_change(struct request_queue *q, const char *elevator_name,
-		      bool force);
+int __elevator_change(struct request_queue *q, struct elev_change_ctx *ctx);
 void elevator_exit(struct request_queue *q);
 int elv_register_queue(struct request_queue *q, bool uevent);
 void elv_unregister_queue(struct request_queue *q);
diff --git a/block/elevator.c b/block/elevator.c
index e028d2ff9624..2bc1679dcd1f 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -53,7 +53,8 @@  static LIST_HEAD(elv_list);
  */
 #define rq_hash_key(rq)		(blk_rq_pos(rq) + blk_rq_sectors(rq))
 
-static int elevator_change(struct request_queue *q, const char *name);
+static int elevator_change(struct request_queue *q,
+			   struct elev_change_ctx *ctx);
 
 /*
  * Query io scheduler to see if the current process issuing bio may be
@@ -621,7 +622,8 @@  void elevator_init_mq(struct request_queue *q)
  * If switching fails, we are most likely running out of memory and not able
  * to restore the old io scheduler, so leaving the io scheduler being none.
  */
-static int elevator_switch(struct request_queue *q, struct elevator_type *new_e)
+static int elevator_switch(struct request_queue *q, struct elevator_type *new_e,
+			   struct elev_change_ctx *ctx)
 {
 	int ret;
 
@@ -639,7 +641,7 @@  static int elevator_switch(struct request_queue *q, struct elevator_type *new_e)
 	if (ret)
 		goto out_unfreeze;
 
-	ret = elv_register_queue(q, true);
+	ret = elv_register_queue(q, ctx->uevent);
 	if (ret) {
 		elevator_exit(q);
 		goto out_unfreeze;
@@ -677,9 +679,9 @@  static void elevator_disable(struct request_queue *q)
 /*
  * Switch this queue to the given IO scheduler.
  */
-int __elevator_change(struct request_queue *q, const char *elevator_name,
-		      bool force)
+int __elevator_change(struct request_queue *q, struct elev_change_ctx *ctx)
 {
+	const char *elevator_name = ctx->name;
 	struct elevator_type *e;
 	int ret;
 
@@ -689,19 +691,20 @@  int __elevator_change(struct request_queue *q, const char *elevator_name,
 		return 0;
 	}
 
-	if (!force && q->elevator &&
+	if (!ctx->force && q->elevator &&
 	    elevator_match(q->elevator->type, elevator_name))
 		return 0;
 
 	e = elevator_find_get(elevator_name);
 	if (!e)
 		return -EINVAL;
-	ret = elevator_switch(q, e);
+	ret = elevator_switch(q, e, ctx);
 	elevator_put(e);
 	return ret;
 }
 
-static int elevator_change(struct request_queue *q, const char *name)
+static int elevator_change(struct request_queue *q,
+			   struct elev_change_ctx *ctx)
 {
 	int ret, idx;
 	unsigned int memflags;
@@ -716,7 +719,7 @@  static int elevator_change(struct request_queue *q, const char *name)
 
 	memflags = blk_mq_freeze_queue(q);
 	mutex_lock(&q->elevator_lock);
-	ret = __elevator_change(q, name, false);
+	ret = __elevator_change(q, ctx);
 	mutex_unlock(&q->elevator_lock);
 	blk_mq_unfreeze_queue(q, memflags);
 exit:
@@ -724,7 +727,7 @@  static int elevator_change(struct request_queue *q, const char *name)
 	return ret;
 }
 
-static void elv_iosched_load_module(char *elevator_name)
+static void elv_iosched_load_module(const char *elevator_name)
 {
 	struct elevator_type *found;
 
@@ -741,7 +744,9 @@  ssize_t elv_iosched_store(struct gendisk *disk, const char *buf,
 {
 	struct request_queue *q = disk->queue;
 	char elevator_name[ELV_NAME_MAX];
-	char *name;
+	struct elev_change_ctx ctx = {
+		.uevent = 1,
+	};
 	int ret;
 
 	/*
@@ -750,15 +755,15 @@  ssize_t elv_iosched_store(struct gendisk *disk, const char *buf,
 	 * queue is the one for the device storing the module file.
 	 */
 	strscpy(elevator_name, buf, sizeof(elevator_name));
-	name = strstrip(elevator_name);
+	ctx.name = strstrip(elevator_name);
 
-	elv_iosched_load_module(name);
+	elv_iosched_load_module(ctx.name);
 
 	/* Make sure queue is not in the middle of being removed */
 	if (!blk_queue_registered(q))
 		return -ENOENT;
 
-	ret = elevator_change(q, name);
+	ret = elevator_change(q, &ctx);
 	if (!ret)
 		ret = count;
 	return ret;
diff --git a/block/elevator.h b/block/elevator.h
index 80ff9b28a66f..86b4977cf772 100644
--- a/block/elevator.h
+++ b/block/elevator.h
@@ -123,6 +123,13 @@  struct elevator_queue
 #define ELEVATOR_FLAG_REGISTERED	0
 #define ELEVATOR_FLAG_DISABLE_WBT	1
 
+/* Holding data for changing elevator */
+struct elev_change_ctx {
+	const char *name;
+	unsigned int force:1;
+	unsigned int uevent:1;
+};
+
 /*
  * block elevator interface
  */