diff -urN -X ../dontdiff ../base/linux-2.5.31/include/linux/ipc.h 2.5.31-ipc/include/linux/ipc.h
--- ../base/linux-2.5.31/include/linux/ipc.h Sat Aug 10 18:41:16 2002
+++ 2.5.31-ipc/include/linux/ipc.h Tue Aug 13 10:23:59 2002
@@ -56,6 +56,7 @@
/* used by in-kernel data structures */
struct kern_ipc_perm
{
+ spinlock_t lock;
key_t key;
uid_t uid;
gid_t gid;
diff -urN -X ../dontdiff ../base/linux-2.5.31/ipc/util.c 2.5.31-ipc/ipc/util.c
--- ../base/linux-2.5.31/ipc/util.c Sat Aug 10 18:41:27 2002
+++ 2.5.31-ipc/ipc/util.c Wed Aug 14 15:59:19 2002
@@ -74,7 +74,7 @@
printk(KERN_ERR "ipc_init_ids() failed, ipc service disabled.\n");
ids->size = 0;
}
- ids->ary = SPIN_LOCK_UNLOCKED;
+ ids->ary =RW_LOCK_UNLOCKED;
for(i=0;i<ids->size;i++)
ids->entries[i].p = NULL;
}
@@ -120,13 +120,13 @@
for(i=ids->size;i<newsize;i++) {
new[i].p = NULL;
}
- spin_lock(&ids->ary);
+ write_lock(&ids->ary);
old = ids->entries;
ids->entries = new;
i = ids->size;
ids->size = newsize;
- spin_unlock(&ids->ary);
+ write_unlock(&ids->ary);
ipc_free(old, sizeof(struct ipc_id)*i);
return ids->size;
}
@@ -165,7 +165,9 @@
if(ids->seq > ids->seq_max)
ids->seq = 0;
- spin_lock(&ids->ary);
+ new->lock = SPIN_LOCK_UNLOCKED;
+ read_lock(&ids->ary);
+ spin_lock(&new->lock);
ids->entries[id].p = new;
return id;
}
diff -urN -X ../dontdiff ../base/linux-2.5.31/ipc/util.h 2.5.31-ipc/ipc/util.h
--- ../base/linux-2.5.31/ipc/util.h Sat Aug 10 18:41:40 2002
+++ 2.5.31-ipc/ipc/util.h Wed Aug 14 17:05:22 2002
@@ -19,7 +19,7 @@
unsigned short seq;
unsigned short seq_max;
struct semaphore sem;
- spinlock_t ary;
+ rwlock_t ary;
struct ipc_id* entries;
};
@@ -47,7 +47,7 @@
extern inline void ipc_lockall(struct ipc_ids* ids)
{
- spin_lock(&ids->ary);
+ write_lock(&ids->ary);
}
extern inline struct kern_ipc_perm* ipc_get(struct ipc_ids* ids, int id)
@@ -63,7 +63,7 @@
extern inline void ipc_unlockall(struct ipc_ids* ids)
{
- spin_unlock(&ids->ary);
+ write_unlock(&ids->ary);
}
extern inline struct kern_ipc_perm* ipc_lock(struct ipc_ids* ids, int id)
{
@@ -72,16 +72,29 @@
if(lid >= ids->size)
return NULL;
- spin_lock(&ids->ary);
+ read_lock(&ids->ary);
out = ids->entries[lid].p;
- if(out==NULL)
- spin_unlock(&ids->ary);
+ if(out==NULL) {
+ read_unlock(&ids->ary);
+ return NULL;
+ }
+ spin_lock(&out->lock);
return out;
}
extern inline void ipc_unlock(struct ipc_ids* ids, int id)
{
- spin_unlock(&ids->ary);
+ int lid = id % SEQ_MULTIPLIER;
+ struct kern_ipc_perm* out;
+
+ if(lid >= ids->size)
+ return;
+ out = ids->entries[lid].p;
+ if (out == NULL)
+ return;
+
+ spin_unlock(&out->lock);
+ read_unlock(&ids->ary);
}
extern inline int ipc_buildid(struct ipc_ids* ids, int id, int seq)
On Tue, 20 Aug 2002, mingming cao wrote:
> >
> > This patch breaks the three global IPC locks into one lock per IPC ID.
> > By doing so it could reduce possible lock contention in workloads which
> > make heavy use of IPC semaphores, message queues and Shared
> > memories...etc.
>
> Here is the patch again. Fixed a typo. *_^
Looks good to me...
Except last time around I persuaded you that ipc_lockall, ipc_unlockall
(shm_lockall, shm_unlockall) needed updating; and now I think that they
were redundant all along and can just be removed completely. Only used
by SHM_INFO, I'd expected you to make them read_locks: surprised to find
write_locks, thought about it some more, realized you would need to use
write_locks - except that the down(&shm_ids.sem) is already protecting
against everything that the write_lock would protect against (the values
reported, concurrent freeing of an entry, concurrent reallocation of the
entries array). If you agree, please just delete all ipc_lockall
ipc_unlockall shm_lockall shm_unlockall lines. Sorry for not
noticing that earlier.
You convinced me that it's not worth trying to remove the ipc_ids.sems,
but I'm still slightly worried that you add another layer of locking.
There's going to be no contention over those read_locks (the write_lock
only taken when reallocating entries array), but their cachelines will
still bounce around. I don't know if contention or bouncing was the
more important effect before, but if bouncing then these changes may
be disappointing in practice. Performance results (or an experienced
voice, I've little experience of such tradeoffs) would help dispel doubt.
Perhaps, if ReadCopyUpdate support is added into the kernel in future,
RCU could be used here instead of rwlocking?
Nit: I'd prefer "= RW_LOCK_UNLOCKED" instead of "=RW_LOCK_UNLOCKED".
Hugh