[PATCH] memory ordering in __kfifo primitives

Both __kfifo_put() and __kfifo_get() have header comments stating that if there is but one concurrent reader and one concurrent writer, locking is not necessary. This is almost the case, but a couple of memory barriers are needed. Another option would be to change the header comments to remove the bit about locking not being needed, and to change the those callers who currently don't use locking to add the required locking. The attachment analyzes this approach, but the patch below seems simpler. Signed-off-by: Paul E. McKenney <paulmck@us.ibm.com> Cc: Stelian Pop <stelian@popies.net> Signed-off-by: Andrew Morton <akpm@osdl.org> Signed-off-by: Linus Torvalds <torvalds@osdl.org>

[PATCH] memory ordering in __kfifo primitives
Both __kfifo_put() and __kfifo_get() have header comments stating that if there is but one concurrent reader and one concurrent writer, locking is not necessary. This is almost the case, but a couple of memory barriers are needed. Another option would be to change the header comments to remove the bit about locking not being needed, and to change the those callers who currently don't use locking to add the required locking. The attachment analyzes this approach, but the patch below seems simpler. Signed-off-by: Paul E. McKenney <paulmck@us.ibm.com> Cc: Stelian Pop <stelian@popies.net> Signed-off-by: Andrew Morton <akpm@osdl.org> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Paul E. McKenney · Linus Torvalds
1 parent 99de055ac0
Showing 1 changed file with 28 additions and 0 deletions Side-by-side Diff
kernel/kfifo.c
@@ -122,6 +122,13 @@
  
 	len = min(len, fifo->size - fifo->in + fifo->out);
  
+	/*
+	 * Ensure that we sample the fifo->out index -before- we
+	 * start putting bytes into the kfifo.
+	 */
+
+	smp_mb();
+
 	/* first put the data starting from fifo->in to buffer end */
 	l = min(len, fifo->size - (fifo->in & (fifo->size - 1)));
 	memcpy(fifo->buffer + (fifo->in & (fifo->size - 1)), buffer, l);
@@ -129,6 +136,13 @@
 	/* then put the rest (if any) at the beginning of the buffer */
 	memcpy(fifo->buffer, buffer + l, len - l);
  
+	/*
+	 * Ensure that we add the bytes to the kfifo -before-
+	 * we update the fifo->in index.
+	 */
+
+	smp_wmb();
+
 	fifo->in += len;
  
 	return len;
  
@@ -154,12 +168,26 @@
  
 	len = min(len, fifo->in - fifo->out);
  
+	/*
+	 * Ensure that we sample the fifo->in index -before- we
+	 * start removing bytes from the kfifo.
+	 */
+
+	smp_rmb();
+
 	/* first get the data from fifo->out until the end of the buffer */
 	l = min(len, fifo->size - (fifo->out & (fifo->size - 1)));
 	memcpy(buffer, fifo->buffer + (fifo->out & (fifo->size - 1)), l);
  
 	/* then get the rest (if any) from the beginning of the buffer */
 	memcpy(buffer + l, fifo->buffer, len - l);
+
+	/*
+	 * Ensure that we remove the bytes from the kfifo -before-
+	 * we update the fifo->out index.
+	 */
+
+	smp_mb();
  
 	fifo->out += len;
...	...	@@ -122,6 +122,13 @@
122	122
123	123	len = min(len, fifo->size - fifo->in + fifo->out);
124	124
	125	+ /*
	126	+ * Ensure that we sample the fifo->out index -before- we
	127	+ * start putting bytes into the kfifo.
	128	+ */
	129	+
	130	+ smp_mb();
	131	+
125	132	/* first put the data starting from fifo->in to buffer end */
126	133	l = min(len, fifo->size - (fifo->in & (fifo->size - 1)));
127	134	memcpy(fifo->buffer + (fifo->in & (fifo->size - 1)), buffer, l);
...	...	@@ -129,6 +136,13 @@
129	136	/* then put the rest (if any) at the beginning of the buffer */
130	137	memcpy(fifo->buffer, buffer + l, len - l);
131	138
	139	+ /*
	140	+ * Ensure that we add the bytes to the kfifo -before-
	141	+ * we update the fifo->in index.
	142	+ */
	143	+
	144	+ smp_wmb();
	145	+
132	146	fifo->in += len;
133	147
134	148	return len;
135	149
...	...	@@ -154,12 +168,26 @@
154	168
155	169	len = min(len, fifo->in - fifo->out);
156	170
	171	+ /*
	172	+ * Ensure that we sample the fifo->in index -before- we
	173	+ * start removing bytes from the kfifo.
	174	+ */
	175	+
	176	+ smp_rmb();
	177	+
157	178	/* first get the data from fifo->out until the end of the buffer */
158	179	l = min(len, fifo->size - (fifo->out & (fifo->size - 1)));
159	180	memcpy(buffer, fifo->buffer + (fifo->out & (fifo->size - 1)), l);
160	181
161	182	/* then get the rest (if any) from the beginning of the buffer */
162	183	memcpy(buffer + l, fifo->buffer, len - l);
	184	+
	185	+ /*
	186	+ * Ensure that we remove the bytes from the kfifo -before-
	187	+ * we update the fifo->out index.
	188	+ */
	189	+
	190	+ smp_mb();
163	191
164	192	fifo->out += len;
165	193