lkml.org 
[lkml]   [2002]   [Nov]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Subject[PATCH] Memory barriers in IPC RCU
From
Date
Hi Andrew,

Attached patch includes the following changes:

- ipc_lock() need a read_barrier_depends() to prevent indexing
uninitialized new array on the read side. This is corresponding to the
write memory barrier added in grow_ary() from Dipankar's patch to
prevent indexing uninitialized array.

- Replaced "wmb()" in IPC code with "smp_wmb()"."wmb()" produces a
full write memory barrier in both UP and SMP kernels, while "smp_wmb()"
provides a full write memory barrier in an SMP kernel, but only a
compiler directive in a UP kernel. The same change are made for "rmb()".

- Removed rmb() in ipc_get(). We do not need a read memory barrier there
since ipc_get() is protected by ipc_ids.sem semaphore.

- Added more comments about why write barriers and read barriers are
needed (or not needed) here or there.

Patch against 2.5.49 kernel.


Thanks,
Mingming
diff -urN /home/ming/views/base/linux-2.5.49/ipc/util.c 2549-ipc/ipc/util.c
--- /home/ming/views/base/linux-2.5.49/ipc/util.c 2002-11-22 13:40:39.000000000 -0800
+++ 2549-ipc/ipc/util.c 2002-11-26 10:37:35.000000000 -0800
@@ -98,6 +98,10 @@
struct kern_ipc_perm* p;
int max_id = ids->max_id;

+ /*
+ * read_barrier_depends is not needed here
+ * since ipc_ids.sem is held
+ */
for (id = 0; id <= max_id; id++) {
p = ids->entries[id].p;
if(p==NULL)
@@ -134,12 +138,12 @@

/*
* before setting the ids->entries to the new array, there must be a
- * wmb() to make sure that the memcpyed contents of the new array are
+ * smp_wmb() to make sure the memcpyed contents of the new array are
* visible before the new array becomes visible.
*/
- wmb();
+ smp_wmb(); /* prevent seeing new array uninitialized. */
ids->entries = new;
- wmb();
+ smp_wmb(); /* prevent indexing into old array based on new size. */
ids->size = newsize;

ipc_rcu_free(old, sizeof(struct ipc_id)*i);
@@ -156,6 +160,8 @@
* initialised and the first free entry is set up and the id assigned
* is returned. The list is returned in a locked state on success.
* On failure the list is not locked and -1 is returned.
+ *
+ * Called with ipc_ids.sem held.
*/

int ipc_addid(struct ipc_ids* ids, struct kern_ipc_perm* new, int size)
@@ -163,6 +169,11 @@
int id;

size = grow_ary(ids,size);
+
+ /*
+ * read_barrier_depends() is not needed here since
+ * ipc_ids.sem is held
+ */
for (id = 0; id < size; id++) {
if(ids->entries[id].p == NULL)
goto found;
@@ -207,7 +218,11 @@
int lid = id % SEQ_MULTIPLIER;
if(lid >= ids->size)
BUG();
-
+
+ /*
+ * do not need a read_barrier_depends() here to force ordering
+ * on Alpha, since the ipc_ids.sem is held.
+ */
p = ids->entries[lid].p;
ids->entries[lid].p = NULL;
if(p==NULL)
@@ -414,15 +429,15 @@
}

/*
- * ipc_get() requires ipc_ids.sem down, otherwise we need a rmb() here
- * to sync with grow_ary();
- *
- * So far only shm_get_stat() uses ipc_get() via shm_get(). So ipc_get()
- * is called with shm_ids.sem locked. Thus a rmb() is not needed here,
- * as grow_ary() also requires shm_ids.sem down(for shm).
- *
- * But if ipc_get() is used in the future without ipc_ids.sem down,
- * we need to add a rmb() before accessing the entries array
+ * So far only shm_get_stat() calls ipc_get() via shm_get(), so ipc_get()
+ * is called with shm_ids.sem locked. Since grow_ary() is also called with
+ * shm_ids.sem down(for Shared Memory), there is no need to add read
+ * barriers here to gurantee the writes in grow_ary() are seen in order
+ * here (for Alpha).
+ *
+ * However ipc_get() itself does not necessary require ipc_ids.sem down. So
+ * if in the future ipc_get() is used by other places without ipc_ids.sem
+ * down, then ipc_get() needs read memery barriers as ipc_lock() does.
*/
struct kern_ipc_perm* ipc_get(struct ipc_ids* ids, int id)
{
@@ -430,7 +445,6 @@
int lid = id % SEQ_MULTIPLIER;
if(lid >= ids->size)
return NULL;
- rmb();
out = ids->entries[lid].p;
return out;
}
@@ -439,6 +453,7 @@
{
struct kern_ipc_perm* out;
int lid = id % SEQ_MULTIPLIER;
+ struct ipc_id* entries;

rcu_read_lock();
if(lid >= ids->size) {
@@ -446,9 +461,18 @@
return NULL;
}

- /* we need a barrier here to sync with grow_ary() */
- rmb();
- out = ids->entries[lid].p;
+ /*
+ * Note: The following two read barriers are corresponding
+ * to the two write barriers in grow_ary(). They gurantee
+ * the writes are seen in the same order on the read side.
+ * smp_rmb() has effect on all CPUs. read_barrier_depends()
+ * is used if there are data dependency between two reads, and
+ * has effect only on Alpha.
+ */
+ smp_rmb(); /* prevent indexing old array with new size */
+ entries = ids->entries;
+ read_barrier_depends(); /*prevent seeing new array unitialized */
+ out = entries[lid].p;
if(out == NULL) {
rcu_read_unlock();
return NULL;
\
 
 \ /
  Last update: 2005-03-22 13:31    [W:0.230 / U:0.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site