block, cfq: move ioc ioprio/cgroup changed handling to cic

ioprio/cgroup change was handled by marking the changed state in ioc and, on the following access to the ioc, performing RCU-protected iteration through all cic's grabbing the matching queue_lock. This patch moves the changed state to each cic. When ioprio or cgroup changes, the respective bit is set on all cic's of the ioc and when each of those cic (not ioc) is accessed, change is applied for that specific ioc-queue pair. This also fixes the following two race conditions between setting and clearing of changed states. * Missing barrier between assign/load of ioprio and ioprio_changed allowed applying old ioprio. * Change requests could happen between application of change and clearing of changed variables. Signed-off-by: Tejun Heo <tj@kernel.org> Signed-off-by: Jens Axboe <axboe@kernel.dk>

block, cfq: move ioc ioprio/cgroup changed handling to cic
ioprio/cgroup change was handled by marking the changed state in ioc and, on the following access to the ioc, performing RCU-protected iteration through all cic's grabbing the matching queue_lock. This patch moves the changed state to each cic. When ioprio or cgroup changes, the respective bit is set on all cic's of the ioc and when each of those cic (not ioc) is accessed, change is applied for that specific ioc-queue pair. This also fixes the following two race conditions between setting and clearing of changed states. * Missing barrier between assign/load of ioprio and ioprio_changed allowed applying old ioprio. * Change requests could happen between application of change and clearing of changed variables. Signed-off-by: Tejun Heo <tj@kernel.org> Signed-off-by: Jens Axboe <axboe@kernel.dk>
Tejun Heo · Jens Axboe
1 parent 283287a52e
Showing 5 changed files with 65 additions and 27 deletions Side-by-side Diff
block/blk-cgroup.c
block/blk-ioc.c
block/cfq-iosched.c
fs/ioprio.c
include/linux/iocontext.h
@@ -1648,7 +1648,7 @@
 	/* we don't lose anything even if ioc allocation fails */
 	ioc = get_task_io_context(tsk, GFP_ATOMIC, NUMA_NO_NODE);
 	if (ioc) {
-		ioc->cgroup_changed = 1;
+		ioc_cgroup_changed(ioc);
 		put_io_context(ioc);
 	}
 }
@@ -188,6 +188,51 @@
 }
 EXPORT_SYMBOL(get_task_io_context);
  
+void ioc_set_changed(struct io_context *ioc, int which)
+{
+	struct cfq_io_context *cic;
+	struct hlist_node *n;
+
+	hlist_for_each_entry(cic, n, &ioc->cic_list, cic_list)
+		set_bit(which, &cic->changed);
+}
+
+/**
+ * ioc_ioprio_changed - notify ioprio change
+ * @ioc: io_context of interest
+ * @ioprio: new ioprio
+ *
+ * @ioc's ioprio has changed to @ioprio.  Set %CIC_IOPRIO_CHANGED for all
+ * cic's.  iosched is responsible for checking the bit and applying it on
+ * request issue path.
+ */
+void ioc_ioprio_changed(struct io_context *ioc, int ioprio)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ioc->lock, flags);
+	ioc->ioprio = ioprio;
+	ioc_set_changed(ioc, CIC_IOPRIO_CHANGED);
+	spin_unlock_irqrestore(&ioc->lock, flags);
+}
+
+/**
+ * ioc_cgroup_changed - notify cgroup change
+ * @ioc: io_context of interest
+ *
+ * @ioc's cgroup has changed.  Set %CIC_CGROUP_CHANGED for all cic's.
+ * iosched is responsible for checking the bit and applying it on request
+ * issue path.
+ */
+void ioc_cgroup_changed(struct io_context *ioc)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ioc->lock, flags);
+	ioc_set_changed(ioc, CIC_CGROUP_CHANGED);
+	spin_unlock_irqrestore(&ioc->lock, flags);
+}
+
 static int __init blk_ioc_init(void)
 {
 	iocontext_cachep = kmem_cache_create("blkdev_ioc",
@@ -2904,7 +2904,7 @@
 	cfq_clear_cfqq_prio_changed(cfqq);
 }
  
-static void changed_ioprio(struct io_context *ioc, struct cfq_io_context *cic)
+static void changed_ioprio(struct cfq_io_context *cic)
 {
 	struct cfq_data *cfqd = cic_to_cfqd(cic);
 	struct cfq_queue *cfqq;
@@ -2933,12 +2933,6 @@
 	spin_unlock_irqrestore(cfqd->queue->queue_lock, flags);
 }
  
-static void cfq_ioc_set_ioprio(struct io_context *ioc)
-{
-	call_for_each_cic(ioc, changed_ioprio);
-	ioc->ioprio_changed = 0;
-}
-
 static void cfq_init_cfqq(struct cfq_data *cfqd, struct cfq_queue *cfqq,
 			  pid_t pid, bool is_sync)
 {
@@ -2960,7 +2954,7 @@
 }
  
 #ifdef CONFIG_CFQ_GROUP_IOSCHED
-static void changed_cgroup(struct io_context *ioc, struct cfq_io_context *cic)
+static void changed_cgroup(struct cfq_io_context *cic)
 {
 	struct cfq_queue *sync_cfqq = cic_to_cfqq(cic, 1);
 	struct cfq_data *cfqd = cic_to_cfqd(cic);
@@ -2986,12 +2980,6 @@
  
 	spin_unlock_irqrestore(q->queue_lock, flags);
 }
-
-static void cfq_ioc_set_cgroup(struct io_context *ioc)
-{
-	call_for_each_cic(ioc, changed_cgroup);
-	ioc->cgroup_changed = 0;
-}
 #endif  /* CONFIG_CFQ_GROUP_IOSCHED */
  
 static struct cfq_queue *
  
  
@@ -3222,13 +3210,15 @@
 out:
 	get_io_context(ioc);
  
-	if (unlikely(ioc->ioprio_changed))
-		cfq_ioc_set_ioprio(ioc);
-
+	if (unlikely(cic->changed)) {
+		if (test_and_clear_bit(CIC_IOPRIO_CHANGED, &cic->changed))
+			changed_ioprio(cic);
 #ifdef CONFIG_CFQ_GROUP_IOSCHED
-	if (unlikely(ioc->cgroup_changed))
-		cfq_ioc_set_cgroup(ioc);
+		if (test_and_clear_bit(CIC_CGROUP_CHANGED, &cic->changed))
+			changed_cgroup(cic);
 #endif
+	}
+
 	return cic;
 err:
 	if (cic)
@@ -50,8 +50,7 @@
  
 	ioc = get_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
 	if (ioc) {
-		ioc->ioprio = ioprio;
-		ioc->ioprio_changed = 1;
+		ioc_ioprio_changed(ioc, ioprio);
 		put_io_context(ioc);
 	}
  
@@ -13,6 +13,11 @@
 	unsigned long ttime_mean;
 };
  
+enum {
+	CIC_IOPRIO_CHANGED,
+	CIC_CGROUP_CHANGED,
+};
+
 struct cfq_io_context {
 	void *key;
 	struct request_queue *q;
@@ -26,6 +31,8 @@
 	struct list_head queue_list;
 	struct hlist_node cic_list;
  
+	unsigned long changed;
+
 	void (*dtor)(struct io_context *); /* destructor */
 	void (*exit)(struct io_context *); /* called on task exit */
  
  
@@ -44,12 +51,7 @@
 	spinlock_t lock;
  
 	unsigned short ioprio;
-	unsigned short ioprio_changed;
  
-#if defined(CONFIG_BLK_CGROUP) || defined(CONFIG_BLK_CGROUP_MODULE)
-	unsigned short cgroup_changed;
-#endif
-
 	/*
 	 * For request batching
 	 */
@@ -81,6 +83,8 @@
 void exit_io_context(struct task_struct *task);
 struct io_context *get_task_io_context(struct task_struct *task,
 				       gfp_t gfp_flags, int node);
+void ioc_ioprio_changed(struct io_context *ioc, int ioprio);
+void ioc_cgroup_changed(struct io_context *ioc);
 #else
 struct io_context;
 static inline void put_io_context(struct io_context *ioc) { }
...	...	@@ -1648,7 +1648,7 @@
1648	1648	/* we don't lose anything even if ioc allocation fails */
1649	1649	ioc = get_task_io_context(tsk, GFP_ATOMIC, NUMA_NO_NODE);
1650	1650	if (ioc) {
1651		- ioc->cgroup_changed = 1;
	1651	+ ioc_cgroup_changed(ioc);
1652	1652	put_io_context(ioc);
1653	1653	}
1654	1654	}
...	...	@@ -188,6 +188,51 @@
188	188	}
189	189	EXPORT_SYMBOL(get_task_io_context);
190	190
	191	+void ioc_set_changed(struct io_context *ioc, int which)
	192	+{
	193	+ struct cfq_io_context *cic;
	194	+ struct hlist_node *n;
	195	+
	196	+ hlist_for_each_entry(cic, n, &ioc->cic_list, cic_list)
	197	+ set_bit(which, &cic->changed);
	198	+}
	199	+
	200	+/**
	201	+ * ioc_ioprio_changed - notify ioprio change
	202	+ * @ioc: io_context of interest
	203	+ * @ioprio: new ioprio
	204	+ *
	205	+ * @ioc's ioprio has changed to @ioprio. Set %CIC_IOPRIO_CHANGED for all
	206	+ * cic's. iosched is responsible for checking the bit and applying it on
	207	+ * request issue path.
	208	+ */
	209	+void ioc_ioprio_changed(struct io_context *ioc, int ioprio)
	210	+{
	211	+ unsigned long flags;
	212	+
	213	+ spin_lock_irqsave(&ioc->lock, flags);
	214	+ ioc->ioprio = ioprio;
	215	+ ioc_set_changed(ioc, CIC_IOPRIO_CHANGED);
	216	+ spin_unlock_irqrestore(&ioc->lock, flags);
	217	+}
	218	+
	219	+/**
	220	+ * ioc_cgroup_changed - notify cgroup change
	221	+ * @ioc: io_context of interest
	222	+ *
	223	+ * @ioc's cgroup has changed. Set %CIC_CGROUP_CHANGED for all cic's.
	224	+ * iosched is responsible for checking the bit and applying it on request
	225	+ * issue path.
	226	+ */
	227	+void ioc_cgroup_changed(struct io_context *ioc)
	228	+{
	229	+ unsigned long flags;
	230	+
	231	+ spin_lock_irqsave(&ioc->lock, flags);
	232	+ ioc_set_changed(ioc, CIC_CGROUP_CHANGED);
	233	+ spin_unlock_irqrestore(&ioc->lock, flags);
	234	+}
	235	+
191	236	static int __init blk_ioc_init(void)
192	237	{
193	238	iocontext_cachep = kmem_cache_create("blkdev_ioc",
...	...	@@ -2904,7 +2904,7 @@
2904	2904	cfq_clear_cfqq_prio_changed(cfqq);
2905	2905	}
2906	2906
2907		-static void changed_ioprio(struct io_context ioc, struct cfq_io_context cic)
	2907	+static void changed_ioprio(struct cfq_io_context *cic)
2908	2908	{
2909	2909	struct cfq_data *cfqd = cic_to_cfqd(cic);
2910	2910	struct cfq_queue *cfqq;
...	...	@@ -2933,12 +2933,6 @@
2933	2933	spin_unlock_irqrestore(cfqd->queue->queue_lock, flags);
2934	2934	}
2935	2935
2936		-static void cfq_ioc_set_ioprio(struct io_context *ioc)
2937		-{
2938		- call_for_each_cic(ioc, changed_ioprio);
2939		- ioc->ioprio_changed = 0;
2940		-}
2941		-
2942	2936	static void cfq_init_cfqq(struct cfq_data cfqd, struct cfq_queue cfqq,
2943	2937	pid_t pid, bool is_sync)
2944	2938	{
...	...	@@ -2960,7 +2954,7 @@
2960	2954	}
2961	2955
2962	2956	#ifdef CONFIG_CFQ_GROUP_IOSCHED
2963		-static void changed_cgroup(struct io_context ioc, struct cfq_io_context cic)
	2957	+static void changed_cgroup(struct cfq_io_context *cic)
2964	2958	{
2965	2959	struct cfq_queue *sync_cfqq = cic_to_cfqq(cic, 1);
2966	2960	struct cfq_data *cfqd = cic_to_cfqd(cic);
...	...	@@ -2986,12 +2980,6 @@
2986	2980
2987	2981	spin_unlock_irqrestore(q->queue_lock, flags);
2988	2982	}
2989		-
2990		-static void cfq_ioc_set_cgroup(struct io_context *ioc)
2991		-{
2992		- call_for_each_cic(ioc, changed_cgroup);
2993		- ioc->cgroup_changed = 0;
2994		-}
2995	2983	#endif /* CONFIG_CFQ_GROUP_IOSCHED */
2996	2984
2997	2985	static struct cfq_queue *
2998	2986
2999	2987
...	...	@@ -3222,13 +3210,15 @@
3222	3210	out:
3223	3211	get_io_context(ioc);
3224	3212
3225		- if (unlikely(ioc->ioprio_changed))
3226		- cfq_ioc_set_ioprio(ioc);
3227		-
	3213	+ if (unlikely(cic->changed)) {
	3214	+ if (test_and_clear_bit(CIC_IOPRIO_CHANGED, &cic->changed))
	3215	+ changed_ioprio(cic);
3228	3216	#ifdef CONFIG_CFQ_GROUP_IOSCHED
3229		- if (unlikely(ioc->cgroup_changed))
3230		- cfq_ioc_set_cgroup(ioc);
	3217	+ if (test_and_clear_bit(CIC_CGROUP_CHANGED, &cic->changed))
	3218	+ changed_cgroup(cic);
3231	3219	#endif
	3220	+ }
	3221	+
3232	3222	return cic;
3233	3223	err:
3234	3224	if (cic)
...	...	@@ -50,8 +50,7 @@
50	50
51	51	ioc = get_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
52	52	if (ioc) {
53		- ioc->ioprio = ioprio;
54		- ioc->ioprio_changed = 1;
	53	+ ioc_ioprio_changed(ioc, ioprio);
55	54	put_io_context(ioc);
56	55	}
57	56
...	...	@@ -13,6 +13,11 @@
13	13	unsigned long ttime_mean;
14	14	};
15	15
	16	+enum {
	17	+ CIC_IOPRIO_CHANGED,
	18	+ CIC_CGROUP_CHANGED,
	19	+};
	20	+
16	21	struct cfq_io_context {
17	22	void *key;
18	23	struct request_queue *q;
...	...	@@ -26,6 +31,8 @@
26	31	struct list_head queue_list;
27	32	struct hlist_node cic_list;
28	33
	34	+ unsigned long changed;
	35	+
29	36	void (dtor)(struct io_context ); /* destructor */
30	37	void (exit)(struct io_context ); /* called on task exit */
31	38
32	39
...	...	@@ -44,12 +51,7 @@
44	51	spinlock_t lock;
45	52
46	53	unsigned short ioprio;
47		- unsigned short ioprio_changed;
48	54
49		-#if defined(CONFIG_BLK_CGROUP) \|\| defined(CONFIG_BLK_CGROUP_MODULE)
50		- unsigned short cgroup_changed;
51		-#endif
52		-
53	55	/*
54	56	* For request batching
55	57	*/
...	...	@@ -81,6 +83,8 @@
81	83	void exit_io_context(struct task_struct *task);
82	84	struct io_context get_task_io_context(struct task_struct task,
83	85	gfp_t gfp_flags, int node);
	86	+void ioc_ioprio_changed(struct io_context *ioc, int ioprio);
	87	+void ioc_cgroup_changed(struct io_context *ioc);
84	88	#else
85	89	struct io_context;
86	90	static inline void put_io_context(struct io_context *ioc) { }