block: reorder elevator switch sequence

Elevator switch sequence first attached the new elevator, then tried registering it (sysfs) and if that failed attached back the old elevator. However, sysfs registration doesn't require the elevator to be attached, so there is no reason to do the "detach, attach new, register, maybe re-attach old" sequence. It can just do "register, detach, attach". * elevator_init_queue() is updated to set ->elevator_data directly and return 0 / -errno. This allows elevator_exit() on an unattached elevator. * __elv_unregister_queue() which was necessary to unregister unattached q is removed in favor of __elv_register_queue() which can register unattached q. * elevator_attach() becomes a single assignment and obscures more then it helps. Dropped. This will help cleaning up io_context handling across elevator switch. This patch doesn't introduce visible behavior change. Signed-off-by: Tejun Heo <tj@kernel.org> Signed-off-by: Jens Axboe <axboe@kernel.dk>

block: reorder elevator switch sequence
Elevator switch sequence first attached the new elevator, then tried registering it (sysfs) and if that failed attached back the old elevator. However, sysfs registration doesn't require the elevator to be attached, so there is no reason to do the "detach, attach new, register, maybe re-attach old" sequence. It can just do "register, detach, attach". * elevator_init_queue() is updated to set ->elevator_data directly and return 0 / -errno. This allows elevator_exit() on an unattached elevator. * __elv_unregister_queue() which was necessary to unregister unattached q is removed in favor of __elv_register_queue() which can register unattached q. * elevator_attach() becomes a single assignment and obscures more then it helps. Dropped. This will help cleaning up io_context handling across elevator switch. This patch doesn't introduce visible behavior change. Signed-off-by: Tejun Heo <tj@kernel.org> Signed-off-by: Jens Axboe <axboe@kernel.dk>
Tejun Heo · Jens Axboe
1 parent f2dbd76a0a
Showing 1 changed file with 37 additions and 54 deletions Side-by-side Diff
block/elevator.c
@@ -168,19 +168,15 @@
 	return e;
 }
  
-static void *elevator_init_queue(struct request_queue *q,
-				 struct elevator_queue *eq)
+static int elevator_init_queue(struct request_queue *q,
+			       struct elevator_queue *eq)
 {
-	return eq->ops->elevator_init_fn(q);
+	eq->elevator_data = eq->ops->elevator_init_fn(q);
+	if (eq->elevator_data)
+		return 0;
+	return -ENOMEM;
 }
  
-static void elevator_attach(struct request_queue *q, struct elevator_queue *eq,
-			   void *data)
-{
-	q->elevator = eq;
-	eq->elevator_data = data;
-}
-
 static char chosen_elevator[ELV_NAME_MAX];
  
 static int __init elevator_setup(char *str)
@@ -241,7 +237,7 @@
 {
 	struct elevator_type *e = NULL;
 	struct elevator_queue *eq;
-	void *data;
+	int err;
  
 	if (unlikely(q->elevator))
 		return 0;
  
  
@@ -278,13 +274,13 @@
 	if (!eq)
 		return -ENOMEM;
  
-	data = elevator_init_queue(q, eq);
-	if (!data) {
+	err = elevator_init_queue(q, eq);
+	if (err) {
 		kobject_put(&eq->kobj);
-		return -ENOMEM;
+		return err;
 	}
  
-	elevator_attach(q, eq, data);
+	q->elevator = eq;
 	return 0;
 }
 EXPORT_SYMBOL(elevator_init);
  
@@ -856,9 +852,8 @@
 	.release	= elevator_release,
 };
  
-int elv_register_queue(struct request_queue *q)
+int __elv_register_queue(struct request_queue *q, struct elevator_queue *e)
 {
-	struct elevator_queue *e = q->elevator;
 	int error;
  
 	error = kobject_add(&e->kobj, &q->kobj, "%s", "iosched");
  
  
  
  
@@ -876,19 +871,22 @@
 	}
 	return error;
 }
-EXPORT_SYMBOL(elv_register_queue);
  
-static void __elv_unregister_queue(struct elevator_queue *e)
+int elv_register_queue(struct request_queue *q)
 {
-	kobject_uevent(&e->kobj, KOBJ_REMOVE);
-	kobject_del(&e->kobj);
-	e->registered = 0;
+	return __elv_register_queue(q, q->elevator);
 }
+EXPORT_SYMBOL(elv_register_queue);
  
 void elv_unregister_queue(struct request_queue *q)
 {
-	if (q)
-		__elv_unregister_queue(q->elevator);
+	if (q) {
+		struct elevator_queue *e = q->elevator;
+
+		kobject_uevent(&e->kobj, KOBJ_REMOVE);
+		kobject_del(&e->kobj);
+		e->registered = 0;
+	}
 }
 EXPORT_SYMBOL(elv_unregister_queue);
  
  
  
  
  
  
  
@@ -928,50 +926,36 @@
 static int elevator_switch(struct request_queue *q, struct elevator_type *new_e)
 {
 	struct elevator_queue *old_elevator, *e;
-	void *data;
 	int err;
  
-	/*
-	 * Allocate new elevator
-	 */
+	/* allocate new elevator */
 	e = elevator_alloc(q, new_e);
 	if (!e)
 		return -ENOMEM;
  
-	data = elevator_init_queue(q, e);
-	if (!data) {
+	err = elevator_init_queue(q, e);
+	if (err) {
 		kobject_put(&e->kobj);
-		return -ENOMEM;
+		return err;
 	}
  
-	/*
-	 * Turn on BYPASS and drain all requests w/ elevator private data
-	 */
+	/* turn on BYPASS and drain all requests w/ elevator private data */
 	elv_quiesce_start(q);
  
-	/*
-	 * Remember old elevator.
-	 */
-	old_elevator = q->elevator;
-
-	/*
-	 * attach and start new elevator
-	 */
-	spin_lock_irq(q->queue_lock);
-	elevator_attach(q, e, data);
-	spin_unlock_irq(q->queue_lock);
-
-	if (old_elevator->registered) {
-		__elv_unregister_queue(old_elevator);
-
-		err = elv_register_queue(q);
+	/* unregister old queue, register new one and kill old elevator */
+	if (q->elevator->registered) {
+		elv_unregister_queue(q);
+		err = __elv_register_queue(q, e);
 		if (err)
 			goto fail_register;
 	}
  
-	/*
-	 * finally exit old elevator and turn off BYPASS.
-	 */
+	/* done, replace the old one with new one and turn off BYPASS */
+	spin_lock_irq(q->queue_lock);
+	old_elevator = q->elevator;
+	q->elevator = e;
+	spin_unlock_irq(q->queue_lock);
+
 	elevator_exit(old_elevator);
 	elv_quiesce_end(q);
  
@@ -985,7 +969,6 @@
 	 * one again (along with re-adding the sysfs dir)
 	 */
 	elevator_exit(e);
-	q->elevator = old_elevator;
 	elv_register_queue(q);
 	elv_quiesce_end(q);
...	...	@@ -168,19 +168,15 @@
168	168	return e;
169	169	}
170	170
171		-static void elevator_init_queue(struct request_queue q,
172		- struct elevator_queue *eq)
	171	+static int elevator_init_queue(struct request_queue *q,
	172	+ struct elevator_queue *eq)
173	173	{
174		- return eq->ops->elevator_init_fn(q);
	174	+ eq->elevator_data = eq->ops->elevator_init_fn(q);
	175	+ if (eq->elevator_data)
	176	+ return 0;
	177	+ return -ENOMEM;
175	178	}
176	179
177		-static void elevator_attach(struct request_queue q, struct elevator_queue eq,
178		- void *data)
179		-{
180		- q->elevator = eq;
181		- eq->elevator_data = data;
182		-}
183		-
184	180	static char chosen_elevator[ELV_NAME_MAX];
185	181
186	182	static int __init elevator_setup(char *str)
...	...	@@ -241,7 +237,7 @@
241	237	{
242	238	struct elevator_type *e = NULL;
243	239	struct elevator_queue *eq;
244		- void *data;
	240	+ int err;
245	241
246	242	if (unlikely(q->elevator))
247	243	return 0;
248	244
249	245
...	...	@@ -278,13 +274,13 @@
278	274	if (!eq)
279	275	return -ENOMEM;
280	276
281		- data = elevator_init_queue(q, eq);
282		- if (!data) {
	277	+ err = elevator_init_queue(q, eq);
	278	+ if (err) {
283	279	kobject_put(&eq->kobj);
284		- return -ENOMEM;
	280	+ return err;
285	281	}
286	282
287		- elevator_attach(q, eq, data);
	283	+ q->elevator = eq;
288	284	return 0;
289	285	}
290	286	EXPORT_SYMBOL(elevator_init);
291	287
...	...	@@ -856,9 +852,8 @@
856	852	.release = elevator_release,
857	853	};
858	854
859		-int elv_register_queue(struct request_queue *q)
	855	+int __elv_register_queue(struct request_queue q, struct elevator_queue e)
860	856	{
861		- struct elevator_queue *e = q->elevator;
862	857	int error;
863	858
864	859	error = kobject_add(&e->kobj, &q->kobj, "%s", "iosched");
865	860
866	861
867	862
868	863
...	...	@@ -876,19 +871,22 @@
876	871	}
877	872	return error;
878	873	}
879		-EXPORT_SYMBOL(elv_register_queue);
880	874
881		-static void __elv_unregister_queue(struct elevator_queue *e)
	875	+int elv_register_queue(struct request_queue *q)
882	876	{
883		- kobject_uevent(&e->kobj, KOBJ_REMOVE);
884		- kobject_del(&e->kobj);
885		- e->registered = 0;
	877	+ return __elv_register_queue(q, q->elevator);
886	878	}
	879	+EXPORT_SYMBOL(elv_register_queue);
887	880
888	881	void elv_unregister_queue(struct request_queue *q)
889	882	{
890		- if (q)
891		- __elv_unregister_queue(q->elevator);
	883	+ if (q) {
	884	+ struct elevator_queue *e = q->elevator;
	885	+
	886	+ kobject_uevent(&e->kobj, KOBJ_REMOVE);
	887	+ kobject_del(&e->kobj);
	888	+ e->registered = 0;
	889	+ }
892	890	}
893	891	EXPORT_SYMBOL(elv_unregister_queue);
894	892
895	893
896	894
897	895
898	896
899	897
900	898
...	...	@@ -928,50 +926,36 @@
928	926	static int elevator_switch(struct request_queue q, struct elevator_type new_e)
929	927	{
930	928	struct elevator_queue old_elevator, e;
931		- void *data;
932	929	int err;
933	930
934		- /*
935		- * Allocate new elevator
936		- */
	931	+ /* allocate new elevator */
937	932	e = elevator_alloc(q, new_e);
938	933	if (!e)
939	934	return -ENOMEM;
940	935
941		- data = elevator_init_queue(q, e);
942		- if (!data) {
	936	+ err = elevator_init_queue(q, e);
	937	+ if (err) {
943	938	kobject_put(&e->kobj);
944		- return -ENOMEM;
	939	+ return err;
945	940	}
946	941
947		- /*
948		- * Turn on BYPASS and drain all requests w/ elevator private data
949		- */
	942	+ /* turn on BYPASS and drain all requests w/ elevator private data */
950	943	elv_quiesce_start(q);
951	944
952		- /*
953		- * Remember old elevator.
954		- */
955		- old_elevator = q->elevator;
956		-
957		- /*
958		- * attach and start new elevator
959		- */
960		- spin_lock_irq(q->queue_lock);
961		- elevator_attach(q, e, data);
962		- spin_unlock_irq(q->queue_lock);
963		-
964		- if (old_elevator->registered) {
965		- __elv_unregister_queue(old_elevator);
966		-
967		- err = elv_register_queue(q);
	945	+ /* unregister old queue, register new one and kill old elevator */
	946	+ if (q->elevator->registered) {
	947	+ elv_unregister_queue(q);
	948	+ err = __elv_register_queue(q, e);
968	949	if (err)
969	950	goto fail_register;
970	951	}
971	952
972		- /*
973		- * finally exit old elevator and turn off BYPASS.
974		- */
	953	+ /* done, replace the old one with new one and turn off BYPASS */
	954	+ spin_lock_irq(q->queue_lock);
	955	+ old_elevator = q->elevator;
	956	+ q->elevator = e;
	957	+ spin_unlock_irq(q->queue_lock);
	958	+
975	959	elevator_exit(old_elevator);
976	960	elv_quiesce_end(q);
977	961
...	...	@@ -985,7 +969,6 @@
985	969	* one again (along with re-adding the sysfs dir)
986	970	*/
987	971	elevator_exit(e);
988		- q->elevator = old_elevator;
989	972	elv_register_queue(q);
990	973	elv_quiesce_end(q);
991	974