ipc: move rcu lock out of ipc_addid

This patchset continues the work that began in the sysv ipc semaphore
scaling series: https://lkml.org/lkml/2013/3/20/546

Just like semaphores used to be, sysv shared memory and msg queues also
abuse the ipc lock, unnecessarily holding it for operations such as
permission and security checks.  This patchset mostly deals with mqueues,
and while shared mem can be done in a very similar way, I want to get
these patches out in the open first.  It also does some pending cleanups,
mostly focused on the two level locking we have in ipc code, taking care
of ipc_addid() and ipcctl_pre_down_nolock() - yes there are still
functions that need to be updated as well.

This patch:

Make all callers explicitly take and release the RCU read lock.

This addresses the two level locking seen in newary(), newseg() and
newqueue().  For the last two, explicitly unlock the ipc object and the
rcu lock, instead of calling the custom shm_unlock and msg_unlock
functions.  The next patch will deal with the open coded locking for
->perm.lock

Signed-off-by: Davidlohr Bueso <davidlohr.bueso@hp.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Rik van Riel <riel@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

diff --git a/ipc/msg.c b/ipc/msg.c
index d0c6d967b390a888b913812d772db2c9bf366810..827051112e763b0f6a2bdd8ac58ba0b4da8b4c34 100644 (file)
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -199,11 +199,11 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params)
                return retval;
        }
 
-       /*
-        * ipc_addid() locks msq
-        */
+       /* ipc_addid() locks msq upon success. */
+       rcu_read_lock();
        id = ipc_addid(&msg_ids(ns), &msq->q_perm, ns->msg_ctlmni);
        if (id < 0) {
+               rcu_read_unlock();
                security_msg_queue_free(msq);
                ipc_rcu_putref(msq);
                return id;
@@ -218,7 +218,8 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params)
        INIT_LIST_HEAD(&msq->q_receivers);
        INIT_LIST_HEAD(&msq->q_senders);
 
-       msg_unlock(msq);
+       spin_unlock(&msq->q_perm.lock);
+       rcu_read_unlock();
 
        return msq->q_perm.id;
 }

diff --git a/ipc/sem.c b/ipc/sem.c
index 70480a3aa69891b6ebc6c998e2202c3197a443ec..fefae73a12f3d00e7e4928af6da76fdb12296124 100644 (file)
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -407,8 +407,10 @@ static int newary(struct ipc_namespace *ns, struct ipc_params *params)
                return retval;
        }
 
+       rcu_read_lock();
        id = ipc_addid(&sem_ids(ns), &sma->sem_perm, ns->sc_semmni);
        if (id < 0) {
+               rcu_read_unlock();
                security_sem_free(sma);
                ipc_rcu_putref(sma);
                return id;

diff --git a/ipc/shm.c b/ipc/shm.c
index 85dc001634b1bb6cb63604f79178205504f73aca..c0e1088f5c51340ccdaefaaf227b0f461857dff5 100644 (file)
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -521,9 +521,11 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
        if (IS_ERR(file))
                goto no_file;
 
+       rcu_read_lock();
        id = ipc_addid(&shm_ids(ns), &shp->shm_perm, ns->shm_ctlmni);
        if (id < 0) {
                error = id;
+               rcu_read_unlock();
                goto no_id;
        }
 
@@ -535,6 +537,7 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
        shp->shm_nattch = 0;
        shp->shm_file = file;
        shp->shm_creator = current;
+
        /*
         * shmid gets reported as "inode#" in /proc/pid/maps.
         * proc-ps tools use this. Changing this will break them.
@@ -543,7 +546,9 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
 
        ns->shm_tot += numpages;
        error = shp->shm_perm.id;
-       shm_unlock(shp);
+
+       spin_unlock(&shp->shm_perm.lock);
+       rcu_read_unlock();
        return error;
 
 no_id:

diff --git a/ipc/util.c b/ipc/util.c
index 809ec5ec812295f7ef652b365ff0b35c4a6060d1..92f02422a395ca8bd88f17311197a59df30c8961 100644 (file)
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -246,9 +246,8 @@ int ipc_get_maxid(struct ipc_ids *ids)
  *     is returned. The 'new' entry is returned in a locked state on success.
  *     On failure the entry is not locked and a negative err-code is returned.
  *
- *     Called with ipc_ids.rw_mutex held as a writer.
+ *     Called with RCU read lock and writer ipc_ids.rw_mutex held.
  */
- 
 int ipc_addid(struct ipc_ids* ids, struct kern_ipc_perm* new, int size)
 {
        kuid_t euid;
@@ -266,7 +265,6 @@ int ipc_addid(struct ipc_ids* ids, struct kern_ipc_perm* new, int size)
 
        spin_lock_init(&new->lock);
        new->deleted = 0;
-       rcu_read_lock();
        spin_lock(&new->lock);
 
        id = idr_alloc(&ids->ipcs_idr, new,