[PATCH] cfq-iosched: kill cfq_exit_lock

cfq_exit_lock is protecting two things now: - The per-ioc rbtree of cfq_io_contexts - The per-cfqd linked list of cfq_io_contexts The per-cfqd linked list can be protected by the queue lock, as it is (by definition) per cfqd as the queue lock is. The per-ioc rbtree is mainly used and updated by the process itself only. The only outside use is the io priority changing. If we move the priority changing to not browsing the rbtree, we can remove any locking from the rbtree updates and lookup completely. Let the sys_ioprio syscall just mark processes as having the iopriority changed and lazily update the private cfq io contexts the next time io is queued, and we can remove this locking as well. Signed-off-by: Jens Axboe <axboe@suse.de>

[PATCH] cfq-iosched: kill cfq_exit_lock
cfq_exit_lock is protecting two things now: - The per-ioc rbtree of cfq_io_contexts - The per-cfqd linked list of cfq_io_contexts The per-cfqd linked list can be protected by the queue lock, as it is (by definition) per cfqd as the queue lock is. The per-ioc rbtree is mainly used and updated by the process itself only. The only outside use is the io priority changing. If we move the priority changing to not browsing the rbtree, we can remove any locking from the rbtree updates and lookup completely. Let the sys_ioprio syscall just mark processes as having the iopriority changed and lazily update the private cfq io contexts the next time io is queued, and we can remove this locking as well. Signed-off-by: Jens Axboe <axboe@suse.de>
Jens Axboe · Jens Axboe
1 parent 89850f7ee9
Showing 4 changed files with 21 additions and 41 deletions Side-by-side Diff
block/cfq-iosched.c
block/ll_rw_blk.c
fs/ioprio.c
include/linux/blkdev.h
@@ -31,8 +31,6 @@
  
 #define CFQ_KEY_ASYNC		(0)
  
-static DEFINE_SPINLOCK(cfq_exit_lock);
-
 /*
  * for the hash of cfqq inside the cfqd
  */
@@ -1084,12 +1082,6 @@
 		complete(ioc_gone);
 }
  
-static void cfq_trim(struct io_context *ioc)
-{
-	ioc->set_ioprio = NULL;
-	cfq_free_io_context(ioc);
-}
-
 static void cfq_exit_cfqq(struct cfq_data *cfqd, struct cfq_queue *cfqq)
 {
 	if (unlikely(cfqq == cfqd->active_queue))
@@ -1101,6 +1093,10 @@
 static void __cfq_exit_single_io_context(struct cfq_data *cfqd,
 					 struct cfq_io_context *cic)
 {
+	list_del_init(&cic->queue_list);
+	smp_wmb();
+	cic->key = NULL;
+
 	if (cic->cfqq[ASYNC]) {
 		cfq_exit_cfqq(cfqd, cic->cfqq[ASYNC]);
 		cic->cfqq[ASYNC] = NULL;
@@ -1110,9 +1106,6 @@
 		cfq_exit_cfqq(cfqd, cic->cfqq[SYNC]);
 		cic->cfqq[SYNC] = NULL;
 	}
-
-	cic->key = NULL;
-	list_del_init(&cic->queue_list);
 }
  
  
  
  
  
  
@@ -1123,27 +1116,23 @@
 {
 	struct cfq_data *cfqd = cic->key;
  
-	WARN_ON(!irqs_disabled());
-
 	if (cfqd) {
 		request_queue_t *q = cfqd->queue;
  
-		spin_lock(q->queue_lock);
+		spin_lock_irq(q->queue_lock);
 		__cfq_exit_single_io_context(cfqd, cic);
-		spin_unlock(q->queue_lock);
+		spin_unlock_irq(q->queue_lock);
 	}
 }
  
 static void cfq_exit_io_context(struct io_context *ioc)
 {
 	struct cfq_io_context *__cic;
-	unsigned long flags;
 	struct rb_node *n;
  
 	/*
 	 * put the reference this task is holding to the various queues
 	 */
-	spin_lock_irqsave(&cfq_exit_lock, flags);
  
 	n = rb_first(&ioc->cic_root);
 	while (n != NULL) {
@@ -1152,8 +1141,6 @@
 		cfq_exit_single_io_context(__cic);
 		n = rb_next(n);
 	}
-
-	spin_unlock_irqrestore(&cfq_exit_lock, flags);
 }
  
 static struct cfq_io_context *
  
@@ -1248,15 +1235,12 @@
 	spin_unlock(cfqd->queue->queue_lock);
 }
  
-/*
- * callback from sys_ioprio_set, irqs are disabled
- */
-static int cfq_ioc_set_ioprio(struct io_context *ioc, unsigned int ioprio)
+static void cfq_ioc_set_ioprio(struct io_context *ioc)
 {
 	struct cfq_io_context *cic;
 	struct rb_node *n;
  
-	spin_lock(&cfq_exit_lock);
+	ioc->ioprio_changed = 0;
  
 	n = rb_first(&ioc->cic_root);
 	while (n != NULL) {
@@ -1265,10 +1249,6 @@
 		changed_ioprio(cic);
 		n = rb_next(n);
 	}
-
-	spin_unlock(&cfq_exit_lock);
-
-	return 0;
 }
  
 static struct cfq_queue *
  
@@ -1336,10 +1316,8 @@
 static void
 cfq_drop_dead_cic(struct io_context *ioc, struct cfq_io_context *cic)
 {
-	spin_lock(&cfq_exit_lock);
+	WARN_ON(!list_empty(&cic->queue_list));
 	rb_erase(&cic->rb_node, &ioc->cic_root);
-	list_del_init(&cic->queue_list);
-	spin_unlock(&cfq_exit_lock);
 	kmem_cache_free(cfq_ioc_pool, cic);
 	atomic_dec(&ioc_count);
 }
@@ -1385,7 +1363,6 @@
 	cic->ioc = ioc;
 	cic->key = cfqd;
  
-	ioc->set_ioprio = cfq_ioc_set_ioprio;
 restart:
 	parent = NULL;
 	p = &ioc->cic_root.rb_node;
  
  
@@ -1407,11 +1384,12 @@
 			BUG();
 	}
  
-	spin_lock(&cfq_exit_lock);
 	rb_link_node(&cic->rb_node, parent, p);
 	rb_insert_color(&cic->rb_node, &ioc->cic_root);
+
+	spin_lock_irq(cfqd->queue->queue_lock);
 	list_add(&cic->queue_list, &cfqd->cic_list);
-	spin_unlock(&cfq_exit_lock);
+	spin_unlock_irq(cfqd->queue->queue_lock);
 }
  
 /*
@@ -1441,6 +1419,10 @@
  
 	cfq_cic_link(cfqd, ioc, cic);
 out:
+	smp_read_barrier_depends();
+	if (unlikely(ioc->ioprio_changed))
+		cfq_ioc_set_ioprio(ioc);
+
 	return cic;
 err:
 	put_io_context(ioc);
@@ -1945,7 +1927,6 @@
  
 	cfq_shutdown_timer_wq(cfqd);
  
-	spin_lock(&cfq_exit_lock);
 	spin_lock_irq(q->queue_lock);
  
 	if (cfqd->active_queue)
@@ -1960,7 +1941,6 @@
 	}
  
 	spin_unlock_irq(q->queue_lock);
-	spin_unlock(&cfq_exit_lock);
  
 	cfq_shutdown_timer_wq(cfqd);
  
@@ -2149,7 +2129,7 @@
 		.elevator_may_queue_fn =	cfq_may_queue,
 		.elevator_init_fn =		cfq_init_queue,
 		.elevator_exit_fn =		cfq_exit_queue,
-		.trim =				cfq_trim,
+		.trim =				cfq_free_io_context,
 	},
 	.elevator_attrs =	cfq_attrs,
 	.elevator_name =	"cfq",
@@ -3654,7 +3654,7 @@
 	if (ret) {
 		atomic_set(&ret->refcount, 1);
 		ret->task = current;
-		ret->set_ioprio = NULL;
+		ret->ioprio_changed = 0;
 		ret->last_waited = jiffies; /* doesn't matter... */
 		ret->nr_batch_requests = 0; /* because this is 0 */
 		ret->aic = NULL;
@@ -47,8 +47,8 @@
 	/* see wmb() in current_io_context() */
 	smp_read_barrier_depends();
  
-	if (ioc && ioc->set_ioprio)
-		ioc->set_ioprio(ioc, ioprio);
+	if (ioc)
+		ioc->ioprio_changed = 1;
  
 	task_unlock(task);
 	return 0;
@@ -90,7 +90,7 @@
 	atomic_t refcount;
 	struct task_struct *task;
  
-	int (*set_ioprio)(struct io_context *, unsigned int);
+	unsigned int ioprio_changed;
  
 	/*
 	 * For request batching
...	...	@@ -31,8 +31,6 @@
31	31
32	32	#define CFQ_KEY_ASYNC (0)
33	33
34		-static DEFINE_SPINLOCK(cfq_exit_lock);
35		-
36	34	/*
37	35	* for the hash of cfqq inside the cfqd
38	36	*/
...	...	@@ -1084,12 +1082,6 @@
1084	1082	complete(ioc_gone);
1085	1083	}
1086	1084
1087		-static void cfq_trim(struct io_context *ioc)
1088		-{
1089		- ioc->set_ioprio = NULL;
1090		- cfq_free_io_context(ioc);
1091		-}
1092		-
1093	1085	static void cfq_exit_cfqq(struct cfq_data cfqd, struct cfq_queue cfqq)
1094	1086	{
1095	1087	if (unlikely(cfqq == cfqd->active_queue))
...	...	@@ -1101,6 +1093,10 @@
1101	1093	static void __cfq_exit_single_io_context(struct cfq_data *cfqd,
1102	1094	struct cfq_io_context *cic)
1103	1095	{
	1096	+ list_del_init(&cic->queue_list);
	1097	+ smp_wmb();
	1098	+ cic->key = NULL;
	1099	+
1104	1100	if (cic->cfqq[ASYNC]) {
1105	1101	cfq_exit_cfqq(cfqd, cic->cfqq[ASYNC]);
1106	1102	cic->cfqq[ASYNC] = NULL;
...	...	@@ -1110,9 +1106,6 @@
1110	1106	cfq_exit_cfqq(cfqd, cic->cfqq[SYNC]);
1111	1107	cic->cfqq[SYNC] = NULL;
1112	1108	}
1113		-
1114		- cic->key = NULL;
1115		- list_del_init(&cic->queue_list);
1116	1109	}
1117	1110
1118	1111
1119	1112
1120	1113
1121	1114
1122	1115
...	...	@@ -1123,27 +1116,23 @@
1123	1116	{
1124	1117	struct cfq_data *cfqd = cic->key;
1125	1118
1126		- WARN_ON(!irqs_disabled());
1127		-
1128	1119	if (cfqd) {
1129	1120	request_queue_t *q = cfqd->queue;
1130	1121
1131		- spin_lock(q->queue_lock);
	1122	+ spin_lock_irq(q->queue_lock);
1132	1123	__cfq_exit_single_io_context(cfqd, cic);
1133		- spin_unlock(q->queue_lock);
	1124	+ spin_unlock_irq(q->queue_lock);
1134	1125	}
1135	1126	}
1136	1127
1137	1128	static void cfq_exit_io_context(struct io_context *ioc)
1138	1129	{
1139	1130	struct cfq_io_context *__cic;
1140		- unsigned long flags;
1141	1131	struct rb_node *n;
1142	1132
1143	1133	/*
1144	1134	* put the reference this task is holding to the various queues
1145	1135	*/
1146		- spin_lock_irqsave(&cfq_exit_lock, flags);
1147	1136
1148	1137	n = rb_first(&ioc->cic_root);
1149	1138	while (n != NULL) {
...	...	@@ -1152,8 +1141,6 @@
1152	1141	cfq_exit_single_io_context(__cic);
1153	1142	n = rb_next(n);
1154	1143	}
1155		-
1156		- spin_unlock_irqrestore(&cfq_exit_lock, flags);
1157	1144	}
1158	1145
1159	1146	static struct cfq_io_context *
1160	1147
...	...	@@ -1248,15 +1235,12 @@
1248	1235	spin_unlock(cfqd->queue->queue_lock);
1249	1236	}
1250	1237
1251		-/*
1252		- * callback from sys_ioprio_set, irqs are disabled
1253		- */
1254		-static int cfq_ioc_set_ioprio(struct io_context *ioc, unsigned int ioprio)
	1238	+static void cfq_ioc_set_ioprio(struct io_context *ioc)
1255	1239	{
1256	1240	struct cfq_io_context *cic;
1257	1241	struct rb_node *n;
1258	1242
1259		- spin_lock(&cfq_exit_lock);
	1243	+ ioc->ioprio_changed = 0;
1260	1244
1261	1245	n = rb_first(&ioc->cic_root);
1262	1246	while (n != NULL) {
...	...	@@ -1265,10 +1249,6 @@
1265	1249	changed_ioprio(cic);
1266	1250	n = rb_next(n);
1267	1251	}
1268		-
1269		- spin_unlock(&cfq_exit_lock);
1270		-
1271		- return 0;
1272	1252	}
1273	1253
1274	1254	static struct cfq_queue *
1275	1255
...	...	@@ -1336,10 +1316,8 @@
1336	1316	static void
1337	1317	cfq_drop_dead_cic(struct io_context ioc, struct cfq_io_context cic)
1338	1318	{
1339		- spin_lock(&cfq_exit_lock);
	1319	+ WARN_ON(!list_empty(&cic->queue_list));
1340	1320	rb_erase(&cic->rb_node, &ioc->cic_root);
1341		- list_del_init(&cic->queue_list);
1342		- spin_unlock(&cfq_exit_lock);
1343	1321	kmem_cache_free(cfq_ioc_pool, cic);
1344	1322	atomic_dec(&ioc_count);
1345	1323	}
...	...	@@ -1385,7 +1363,6 @@
1385	1363	cic->ioc = ioc;
1386	1364	cic->key = cfqd;
1387	1365
1388		- ioc->set_ioprio = cfq_ioc_set_ioprio;
1389	1366	restart:
1390	1367	parent = NULL;
1391	1368	p = &ioc->cic_root.rb_node;
1392	1369
1393	1370
...	...	@@ -1407,11 +1384,12 @@
1407	1384	BUG();
1408	1385	}
1409	1386
1410		- spin_lock(&cfq_exit_lock);
1411	1387	rb_link_node(&cic->rb_node, parent, p);
1412	1388	rb_insert_color(&cic->rb_node, &ioc->cic_root);
	1389	+
	1390	+ spin_lock_irq(cfqd->queue->queue_lock);
1413	1391	list_add(&cic->queue_list, &cfqd->cic_list);
1414		- spin_unlock(&cfq_exit_lock);
	1392	+ spin_unlock_irq(cfqd->queue->queue_lock);
1415	1393	}
1416	1394
1417	1395	/*
...	...	@@ -1441,6 +1419,10 @@
1441	1419
1442	1420	cfq_cic_link(cfqd, ioc, cic);
1443	1421	out:
	1422	+ smp_read_barrier_depends();
	1423	+ if (unlikely(ioc->ioprio_changed))
	1424	+ cfq_ioc_set_ioprio(ioc);
	1425	+
1444	1426	return cic;
1445	1427	err:
1446	1428	put_io_context(ioc);
...	...	@@ -1945,7 +1927,6 @@
1945	1927
1946	1928	cfq_shutdown_timer_wq(cfqd);
1947	1929
1948		- spin_lock(&cfq_exit_lock);
1949	1930	spin_lock_irq(q->queue_lock);
1950	1931
1951	1932	if (cfqd->active_queue)
...	...	@@ -1960,7 +1941,6 @@
1960	1941	}
1961	1942
1962	1943	spin_unlock_irq(q->queue_lock);
1963		- spin_unlock(&cfq_exit_lock);
1964	1944
1965	1945	cfq_shutdown_timer_wq(cfqd);
1966	1946
...	...	@@ -2149,7 +2129,7 @@
2149	2129	.elevator_may_queue_fn = cfq_may_queue,
2150	2130	.elevator_init_fn = cfq_init_queue,
2151	2131	.elevator_exit_fn = cfq_exit_queue,
2152		- .trim = cfq_trim,
	2132	+ .trim = cfq_free_io_context,
2153	2133	},
2154	2134	.elevator_attrs = cfq_attrs,
2155	2135	.elevator_name = "cfq",
...	...	@@ -3654,7 +3654,7 @@
3654	3654	if (ret) {
3655	3655	atomic_set(&ret->refcount, 1);
3656	3656	ret->task = current;
3657		- ret->set_ioprio = NULL;
	3657	+ ret->ioprio_changed = 0;
3658	3658	ret->last_waited = jiffies; /* doesn't matter... */
3659	3659	ret->nr_batch_requests = 0; /* because this is 0 */
3660	3660	ret->aic = NULL;
...	...	@@ -47,8 +47,8 @@
47	47	/* see wmb() in current_io_context() */
48	48	smp_read_barrier_depends();
49	49
50		- if (ioc && ioc->set_ioprio)
51		- ioc->set_ioprio(ioc, ioprio);
	50	+ if (ioc)
	51	+ ioc->ioprio_changed = 1;
52	52
53	53	task_unlock(task);
54	54	return 0;
...	...	@@ -90,7 +90,7 @@
90	90	atomic_t refcount;
91	91	struct task_struct *task;
92	92
93		- int (set_ioprio)(struct io_context , unsigned int);
	93	+ unsigned int ioprio_changed;
94	94
95	95	/*
96	96	* For request batching