Commit 16df3674 authored by Davidlohr Bueso's avatar Davidlohr Bueso Committed by Linus Torvalds

ipc,sem: do not hold ipc lock more than necessary

Instead of holding the ipc lock for permissions and security checks, among
others, only acquire it when necessary.

Some numbers....

1) With Rik's semop-multi.c microbenchmark we can see the following
   results:

Baseline (3.9-rc1):
cpus 4, threads: 256, semaphores: 128, test duration: 30 secs
total operations: 151452270, ops/sec 5048409

+  59.40%            a.out  [kernel.kallsyms]  [k] _raw_spin_lock
+   6.14%            a.out  [kernel.kallsyms]  [k] sys_semtimedop
+   3.84%            a.out  [kernel.kallsyms]  [k] avc_has_perm_flags
+   3.64%            a.out  [kernel.kallsyms]  [k] __audit_syscall_exit
+   2.06%            a.out  [kernel.kallsyms]  [k] copy_user_enhanced_fast_string
+   1.86%            a.out  [kernel.kallsyms]  [k] ipc_lock

With this patchset:
cpus 4, threads: 256, semaphores: 128, test duration: 30 secs
total operations: 273156400, ops/sec 9105213

+  18.54%            a.out  [kernel.kallsyms]  [k] _raw_spin_lock
+  11.72%            a.out  [kernel.kallsyms]  [k] sys_semtimedop
+   7.70%            a.out  [kernel.kallsyms]  [k] ipc_has_perm.isra.21
+   6.58%            a.out  [kernel.kallsyms]  [k] avc_has_perm_flags
+   6.54%            a.out  [kernel.kallsyms]  [k] __audit_syscall_exit
+   4.71%            a.out  [kernel.kallsyms]  [k] ipc_obtain_object_check

2) While on an Oracle swingbench DSS (data mining) workload the
   improvements are not as exciting as with Rik's benchmark, we can see
   some positive numbers.  For an 8 socket machine the following are the
   percentages of %sys time incurred in the ipc lock:

Baseline (3.9-rc1):
100 swingbench users: 8,74%
400 swingbench users: 21,86%
800 swingbench users: 84,35%

With this patchset:
100 swingbench users: 8,11%
400 swingbench users: 19,93%
800 swingbench users: 77,69%

[riel@redhat.com: fix two locking bugs]
[sasha.levin@oracle.com: prevent releasing RCU read lock twice in semctl_main]
[akpm@linux-foundation.org: coding-style fixes]
Signed-off-by: default avatarDavidlohr Bueso <davidlohr.bueso@hp.com>
Signed-off-by: default avatarRik van Riel <riel@redhat.com>
Reviewed-by: default avatarChegu Vinod <chegu_vinod@hp.com>
Acked-by: default avatarMichel Lespinasse <walken@google.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Jason Low <jason.low2@hp.com>
Cc: Emmanuel Benisty <benisty.e@gmail.com>
Cc: Peter Hurley <peter@hurleysoftware.com>
Cc: Stanislav Kinsbursky <skinsbursky@parallels.com>
Tested-by: default avatarSedat Dilek <sedat.dilek@gmail.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 444d0f62
...@@ -204,13 +204,34 @@ static inline struct sem_array *sem_lock(struct ipc_namespace *ns, int id) ...@@ -204,13 +204,34 @@ static inline struct sem_array *sem_lock(struct ipc_namespace *ns, int id)
return container_of(ipcp, struct sem_array, sem_perm); return container_of(ipcp, struct sem_array, sem_perm);
} }
static inline struct sem_array *sem_obtain_object(struct ipc_namespace *ns, int id)
{
struct kern_ipc_perm *ipcp = ipc_obtain_object(&sem_ids(ns), id);
if (IS_ERR(ipcp))
return ERR_CAST(ipcp);
return container_of(ipcp, struct sem_array, sem_perm);
}
static inline struct sem_array *sem_lock_check(struct ipc_namespace *ns, static inline struct sem_array *sem_lock_check(struct ipc_namespace *ns,
int id) int id)
{ {
struct kern_ipc_perm *ipcp = ipc_lock_check(&sem_ids(ns), id); struct kern_ipc_perm *ipcp = ipc_lock_check(&sem_ids(ns), id);
if (IS_ERR(ipcp)) if (IS_ERR(ipcp))
return (struct sem_array *)ipcp; return ERR_CAST(ipcp);
return container_of(ipcp, struct sem_array, sem_perm);
}
static inline struct sem_array *sem_obtain_object_check(struct ipc_namespace *ns,
int id)
{
struct kern_ipc_perm *ipcp = ipc_obtain_object_check(&sem_ids(ns), id);
if (IS_ERR(ipcp))
return ERR_CAST(ipcp);
return container_of(ipcp, struct sem_array, sem_perm); return container_of(ipcp, struct sem_array, sem_perm);
} }
...@@ -234,6 +255,16 @@ static inline void sem_putref(struct sem_array *sma) ...@@ -234,6 +255,16 @@ static inline void sem_putref(struct sem_array *sma)
ipc_unlock(&(sma)->sem_perm); ipc_unlock(&(sma)->sem_perm);
} }
/*
* Call inside the rcu read section.
*/
static inline void sem_getref(struct sem_array *sma)
{
spin_lock(&(sma)->sem_perm.lock);
ipc_rcu_getref(sma);
ipc_unlock(&(sma)->sem_perm);
}
static inline void sem_rmid(struct ipc_namespace *ns, struct sem_array *s) static inline void sem_rmid(struct ipc_namespace *ns, struct sem_array *s)
{ {
ipc_rmid(&sem_ids(ns), &s->sem_perm); ipc_rmid(&sem_ids(ns), &s->sem_perm);
...@@ -842,18 +873,25 @@ static int semctl_nolock(struct ipc_namespace *ns, int semid, ...@@ -842,18 +873,25 @@ static int semctl_nolock(struct ipc_namespace *ns, int semid,
case SEM_STAT: case SEM_STAT:
{ {
struct semid64_ds tbuf; struct semid64_ds tbuf;
int id; int id = 0;
memset(&tbuf, 0, sizeof(tbuf));
if (cmd == SEM_STAT) { if (cmd == SEM_STAT) {
sma = sem_lock(ns, semid); rcu_read_lock();
if (IS_ERR(sma)) sma = sem_obtain_object(ns, semid);
return PTR_ERR(sma); if (IS_ERR(sma)) {
err = PTR_ERR(sma);
goto out_unlock;
}
id = sma->sem_perm.id; id = sma->sem_perm.id;
} else { } else {
sma = sem_lock_check(ns, semid); rcu_read_lock();
if (IS_ERR(sma)) sma = sem_obtain_object_check(ns, semid);
return PTR_ERR(sma); if (IS_ERR(sma)) {
id = 0; err = PTR_ERR(sma);
goto out_unlock;
}
} }
err = -EACCES; err = -EACCES;
...@@ -864,13 +902,11 @@ static int semctl_nolock(struct ipc_namespace *ns, int semid, ...@@ -864,13 +902,11 @@ static int semctl_nolock(struct ipc_namespace *ns, int semid,
if (err) if (err)
goto out_unlock; goto out_unlock;
memset(&tbuf, 0, sizeof(tbuf));
kernel_to_ipc64_perm(&sma->sem_perm, &tbuf.sem_perm); kernel_to_ipc64_perm(&sma->sem_perm, &tbuf.sem_perm);
tbuf.sem_otime = sma->sem_otime; tbuf.sem_otime = sma->sem_otime;
tbuf.sem_ctime = sma->sem_ctime; tbuf.sem_ctime = sma->sem_ctime;
tbuf.sem_nsems = sma->sem_nsems; tbuf.sem_nsems = sma->sem_nsems;
sem_unlock(sma); rcu_read_unlock();
if (copy_semid_to_user(p, &tbuf, version)) if (copy_semid_to_user(p, &tbuf, version))
return -EFAULT; return -EFAULT;
return id; return id;
...@@ -879,7 +915,7 @@ static int semctl_nolock(struct ipc_namespace *ns, int semid, ...@@ -879,7 +915,7 @@ static int semctl_nolock(struct ipc_namespace *ns, int semid,
return -EINVAL; return -EINVAL;
} }
out_unlock: out_unlock:
sem_unlock(sma); rcu_read_unlock();
return err; return err;
} }
...@@ -947,27 +983,34 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum, ...@@ -947,27 +983,34 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum,
{ {
struct sem_array *sma; struct sem_array *sma;
struct sem* curr; struct sem* curr;
int err; int err, nsems;
ushort fast_sem_io[SEMMSL_FAST]; ushort fast_sem_io[SEMMSL_FAST];
ushort* sem_io = fast_sem_io; ushort* sem_io = fast_sem_io;
int nsems;
struct list_head tasks; struct list_head tasks;
sma = sem_lock_check(ns, semid); INIT_LIST_HEAD(&tasks);
if (IS_ERR(sma))
rcu_read_lock();
sma = sem_obtain_object_check(ns, semid);
if (IS_ERR(sma)) {
rcu_read_unlock();
return PTR_ERR(sma); return PTR_ERR(sma);
}
INIT_LIST_HEAD(&tasks);
nsems = sma->sem_nsems; nsems = sma->sem_nsems;
err = -EACCES; err = -EACCES;
if (ipcperms(ns, &sma->sem_perm, if (ipcperms(ns, &sma->sem_perm,
cmd == SETALL ? S_IWUGO : S_IRUGO)) cmd == SETALL ? S_IWUGO : S_IRUGO)) {
goto out_unlock; rcu_read_unlock();
goto out_wakeup;
}
err = security_sem_semctl(sma, cmd); err = security_sem_semctl(sma, cmd);
if (err) if (err) {
goto out_unlock; rcu_read_unlock();
goto out_wakeup;
}
err = -EACCES; err = -EACCES;
switch (cmd) { switch (cmd) {
...@@ -977,7 +1020,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum, ...@@ -977,7 +1020,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum,
int i; int i;
if(nsems > SEMMSL_FAST) { if(nsems > SEMMSL_FAST) {
sem_getref_and_unlock(sma); sem_getref(sma);
sem_io = ipc_alloc(sizeof(ushort)*nsems); sem_io = ipc_alloc(sizeof(ushort)*nsems);
if(sem_io == NULL) { if(sem_io == NULL) {
...@@ -993,6 +1036,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum, ...@@ -993,6 +1036,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum,
} }
} }
spin_lock(&sma->sem_perm.lock);
for (i = 0; i < sma->sem_nsems; i++) for (i = 0; i < sma->sem_nsems; i++)
sem_io[i] = sma->sem_base[i].semval; sem_io[i] = sma->sem_base[i].semval;
sem_unlock(sma); sem_unlock(sma);
...@@ -1006,7 +1050,8 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum, ...@@ -1006,7 +1050,8 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum,
int i; int i;
struct sem_undo *un; struct sem_undo *un;
sem_getref_and_unlock(sma); ipc_rcu_getref(sma);
rcu_read_unlock();
if(nsems > SEMMSL_FAST) { if(nsems > SEMMSL_FAST) {
sem_io = ipc_alloc(sizeof(ushort)*nsems); sem_io = ipc_alloc(sizeof(ushort)*nsems);
...@@ -1053,9 +1098,12 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum, ...@@ -1053,9 +1098,12 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum,
/* GETVAL, GETPID, GETNCTN, GETZCNT: fall-through */ /* GETVAL, GETPID, GETNCTN, GETZCNT: fall-through */
} }
err = -EINVAL; err = -EINVAL;
if(semnum < 0 || semnum >= nsems) if (semnum < 0 || semnum >= nsems) {
goto out_unlock; rcu_read_unlock();
goto out_wakeup;
}
spin_lock(&sma->sem_perm.lock);
curr = &sma->sem_base[semnum]; curr = &sma->sem_base[semnum];
switch (cmd) { switch (cmd) {
...@@ -1072,10 +1120,11 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum, ...@@ -1072,10 +1120,11 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum,
err = count_semzcnt(sma,semnum); err = count_semzcnt(sma,semnum);
goto out_unlock; goto out_unlock;
} }
out_unlock: out_unlock:
sem_unlock(sma); sem_unlock(sma);
out_wakeup:
wake_up_sem_queue_do(&tasks); wake_up_sem_queue_do(&tasks);
out_free: out_free:
if(sem_io != fast_sem_io) if(sem_io != fast_sem_io)
ipc_free(sem_io, sizeof(ushort)*nsems); ipc_free(sem_io, sizeof(ushort)*nsems);
...@@ -1126,7 +1175,7 @@ static int semctl_down(struct ipc_namespace *ns, int semid, ...@@ -1126,7 +1175,7 @@ static int semctl_down(struct ipc_namespace *ns, int semid,
return -EFAULT; return -EFAULT;
} }
ipcp = ipcctl_pre_down(ns, &sem_ids(ns), semid, cmd, ipcp = ipcctl_pre_down_nolock(ns, &sem_ids(ns), semid, cmd,
&semid64.sem_perm, 0); &semid64.sem_perm, 0);
if (IS_ERR(ipcp)) if (IS_ERR(ipcp))
return PTR_ERR(ipcp); return PTR_ERR(ipcp);
...@@ -1134,21 +1183,27 @@ static int semctl_down(struct ipc_namespace *ns, int semid, ...@@ -1134,21 +1183,27 @@ static int semctl_down(struct ipc_namespace *ns, int semid,
sma = container_of(ipcp, struct sem_array, sem_perm); sma = container_of(ipcp, struct sem_array, sem_perm);
err = security_sem_semctl(sma, cmd); err = security_sem_semctl(sma, cmd);
if (err) if (err) {
rcu_read_unlock();
goto out_unlock; goto out_unlock;
}
switch(cmd){ switch(cmd){
case IPC_RMID: case IPC_RMID:
ipc_lock_object(&sma->sem_perm);
freeary(ns, ipcp); freeary(ns, ipcp);
goto out_up; goto out_up;
case IPC_SET: case IPC_SET:
ipc_lock_object(&sma->sem_perm);
err = ipc_update_perm(&semid64.sem_perm, ipcp); err = ipc_update_perm(&semid64.sem_perm, ipcp);
if (err) if (err)
goto out_unlock; goto out_unlock;
sma->sem_ctime = get_seconds(); sma->sem_ctime = get_seconds();
break; break;
default: default:
rcu_read_unlock();
err = -EINVAL; err = -EINVAL;
goto out_up;
} }
out_unlock: out_unlock:
...@@ -1277,16 +1332,18 @@ static struct sem_undo *find_alloc_undo(struct ipc_namespace *ns, int semid) ...@@ -1277,16 +1332,18 @@ static struct sem_undo *find_alloc_undo(struct ipc_namespace *ns, int semid)
spin_unlock(&ulp->lock); spin_unlock(&ulp->lock);
if (likely(un!=NULL)) if (likely(un!=NULL))
goto out; goto out;
rcu_read_unlock();
/* no undo structure around - allocate one. */ /* no undo structure around - allocate one. */
/* step 1: figure out the size of the semaphore array */ /* step 1: figure out the size of the semaphore array */
sma = sem_lock_check(ns, semid); sma = sem_obtain_object_check(ns, semid);
if (IS_ERR(sma)) if (IS_ERR(sma)) {
rcu_read_unlock();
return ERR_CAST(sma); return ERR_CAST(sma);
}
nsems = sma->sem_nsems; nsems = sma->sem_nsems;
sem_getref_and_unlock(sma); ipc_rcu_getref(sma);
rcu_read_unlock();
/* step 2: allocate new undo structure */ /* step 2: allocate new undo structure */
new = kzalloc(sizeof(struct sem_undo) + sizeof(short)*nsems, GFP_KERNEL); new = kzalloc(sizeof(struct sem_undo) + sizeof(short)*nsems, GFP_KERNEL);
...@@ -1421,7 +1478,8 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops, ...@@ -1421,7 +1478,8 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops,
INIT_LIST_HEAD(&tasks); INIT_LIST_HEAD(&tasks);
sma = sem_lock_check(ns, semid); rcu_read_lock();
sma = sem_obtain_object_check(ns, semid);
if (IS_ERR(sma)) { if (IS_ERR(sma)) {
if (un) if (un)
rcu_read_unlock(); rcu_read_unlock();
...@@ -1429,6 +1487,24 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops, ...@@ -1429,6 +1487,24 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops,
goto out_free; goto out_free;
} }
error = -EFBIG;
if (max >= sma->sem_nsems) {
rcu_read_unlock();
goto out_wakeup;
}
error = -EACCES;
if (ipcperms(ns, &sma->sem_perm, alter ? S_IWUGO : S_IRUGO)) {
rcu_read_unlock();
goto out_wakeup;
}
error = security_sem_semop(sma, sops, nsops, alter);
if (error) {
rcu_read_unlock();
goto out_wakeup;
}
/* /*
* semid identifiers are not unique - find_alloc_undo may have * semid identifiers are not unique - find_alloc_undo may have
* allocated an undo structure, it was invalidated by an RMID * allocated an undo structure, it was invalidated by an RMID
...@@ -1437,6 +1513,7 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops, ...@@ -1437,6 +1513,7 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops,
* "un" itself is guaranteed by rcu. * "un" itself is guaranteed by rcu.
*/ */
error = -EIDRM; error = -EIDRM;
ipc_lock_object(&sma->sem_perm);
if (un) { if (un) {
if (un->semid == -1) { if (un->semid == -1) {
rcu_read_unlock(); rcu_read_unlock();
...@@ -1454,18 +1531,6 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops, ...@@ -1454,18 +1531,6 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops,
} }
} }
error = -EFBIG;
if (max >= sma->sem_nsems)
goto out_unlock_free;
error = -EACCES;
if (ipcperms(ns, &sma->sem_perm, alter ? S_IWUGO : S_IRUGO))
goto out_unlock_free;
error = security_sem_semop(sma, sops, nsops, alter);
if (error)
goto out_unlock_free;
error = try_atomic_semop (sma, sops, nsops, un, task_tgid_vnr(current)); error = try_atomic_semop (sma, sops, nsops, un, task_tgid_vnr(current));
if (error <= 0) { if (error <= 0) {
if (alter && error == 0) if (alter && error == 0)
...@@ -1568,7 +1633,7 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops, ...@@ -1568,7 +1633,7 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops,
out_unlock_free: out_unlock_free:
sem_unlock(sma); sem_unlock(sma);
out_wakeup:
wake_up_sem_queue_do(&tasks); wake_up_sem_queue_do(&tasks);
out_free: out_free:
if(sops != fast_sops) if(sops != fast_sops)
......
...@@ -171,6 +171,11 @@ static inline void ipc_unlock(struct kern_ipc_perm *perm) ...@@ -171,6 +171,11 @@ static inline void ipc_unlock(struct kern_ipc_perm *perm)
rcu_read_unlock(); rcu_read_unlock();
} }
static inline void ipc_lock_object(struct kern_ipc_perm *perm)
{
spin_lock(&perm->lock);
}
struct kern_ipc_perm *ipc_lock_check(struct ipc_ids *ids, int id); struct kern_ipc_perm *ipc_lock_check(struct ipc_ids *ids, int id);
struct kern_ipc_perm *ipc_obtain_object_check(struct ipc_ids *ids, int id); struct kern_ipc_perm *ipc_obtain_object_check(struct ipc_ids *ids, int id);
int ipcget(struct ipc_namespace *ns, struct ipc_ids *ids, int ipcget(struct ipc_namespace *ns, struct ipc_ids *ids,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment