kernel - Fix MP race in sysv semaphore code
[dragonfly.git] / sys / kern / sysv_sem.c
index 92d4013..6ba0ced 100644 (file)
@@ -1,5 +1,4 @@
 /* $FreeBSD: src/sys/kern/sysv_sem.c,v 1.69 2004/03/17 09:37:13 cperciva Exp $ */
-/* $DragonFly: src/sys/kern/sysv_sem.c,v 1.19 2008/01/06 16:55:51 swildner Exp $ */
 
 /*
  * Implementation of SVID semaphores
@@ -21,8 +20,9 @@
 #include <sys/sysctl.h>
 #include <sys/malloc.h>
 #include <sys/jail.h>
+#include <sys/thread.h>
 
-#include <sys/mplock2.h>
+#include <sys/thread2.h>
 
 static MALLOC_DEFINE(M_SEM, "sem", "SVID compatible semaphores");
 
@@ -39,6 +39,7 @@ static sy_call_t *semcalls[] = {
        (sy_call_t *)sys_semop
 };
 
+static struct lwkt_token semu_token = LWKT_TOKEN_INITIALIZER(semu_token);
 static int     semtot = 0;
 static struct semid_ds *sema;  /* semaphore id pool */
 static struct sem *sem;                /* semaphore pool */
@@ -70,10 +71,10 @@ struct sem_undo {
  * Configuration parameters
  */
 #ifndef SEMMNI
-#define SEMMNI 10              /* # of semaphore identifiers */
+#define SEMMNI 22              /* # of semaphore identifiers */
 #endif
 #ifndef SEMMNS
-#define SEMMNS 60              /* # of semaphores in system */
+#define SEMMNS 341             /* # of semaphores in system */
 #endif
 #ifndef SEMUME
 #define SEMUME 10              /* max # of undo entries per process */
@@ -109,7 +110,8 @@ struct sem_undo {
 /*
  * Macro to find a particular sem_undo vector
  */
-#define SEMU(ix)       ((struct sem_undo *)(((intptr_t)semu)+ix * seminfo.semusz))
+#define SEMU(ix)       ((struct sem_undo *)(((intptr_t)semu) + (ix) * \
+                                            seminfo.semusz))
 
 /*
  * semaphore info struct
@@ -210,21 +212,20 @@ sys_semsys(struct semsys_args *uap)
        if (!jail_sysvipc_allowed && td->td_ucred->cr_prison != NULL)
                return (ENOSYS);
 
-       if (which >= sizeof(semcalls)/sizeof(semcalls[0]))
+       if (which >= NELEM(semcalls))
                return (EINVAL);
        bcopy(&uap->a2, &uap->which,
              sizeof(struct semsys_args) - offsetof(struct semsys_args, a2));
-       get_mplock();
        error = (*semcalls[which])(uap);
-       rel_mplock();
        return (error);
 }
 
 /*
  * Allocate a new sem_undo structure for a process
  * (returns ptr to structure or NULL if no more room)
+ *
+ * semu_token is held by the caller.
  */
-
 static struct sem_undo *
 semu_alloc(struct proc *p)
 {
@@ -238,13 +239,11 @@ semu_alloc(struct proc *p)
         * (we'll purge any empty structures after the first pass so
         * two passes are always enough)
         */
-
        for (attempt = 0; attempt < 2; attempt++) {
                /*
                 * Look for a free structure.
                 * Fill it in and return it if we find one.
                 */
-
                for (i = 0; i < seminfo.semmnu; i++) {
                        suptr = SEMU(i);
                        if (suptr->un_proc == NULL) {
@@ -252,7 +251,7 @@ semu_alloc(struct proc *p)
                                semu_list = suptr;
                                suptr->un_cnt = 0;
                                suptr->un_proc = p;
-                               return(suptr);
+                               goto done;
                        }
                }
 
@@ -271,13 +270,16 @@ semu_alloc(struct proc *p)
                                        suptr->un_proc = NULL;
                                        *supptr = suptr->un_next;
                                        did_something = 1;
-                               } else
+                               } else {
                                        supptr = &(suptr->un_next);
+                               }
                        }
 
                        /* If we didn't free anything then just give-up */
-                       if (!did_something)
-                               return(NULL);
+                       if (!did_something) {
+                               suptr = NULL;
+                               goto done;
+                       }
                } else {
                        /*
                         * The second pass failed even though we freed
@@ -287,7 +289,9 @@ semu_alloc(struct proc *p)
                        panic("semu_alloc - second attempt failed");
                }
        }
-       return (NULL);
+       suptr = NULL;
+done:
+       return (suptr);
 }
 
 /*
@@ -301,10 +305,14 @@ semundo_adjust(struct proc *p, struct sem_undo **supptr, int semid, int semnum,
        struct sem_undo *suptr;
        struct undo *sunptr;
        int i;
+       int error = 0;
 
-       /* Look for and remember the sem_undo if the caller doesn't provide
-          it */
-
+       /*
+        * Look for and remember the sem_undo if the caller doesn't
+        * provide it.
+        */
+       lwkt_gettoken(&semu_token);
+       lwkt_gettoken(&p->p_token);
        suptr = *supptr;
        if (suptr == NULL) {
                for (suptr = semu_list; suptr != NULL;
@@ -316,10 +324,13 @@ semundo_adjust(struct proc *p, struct sem_undo **supptr, int semid, int semnum,
                }
                if (suptr == NULL) {
                        if (adjval == 0)
-                               return(0);
+                               goto done;
+                       p->p_flags |= P_SYSVSEM;
                        suptr = semu_alloc(p);
-                       if (suptr == NULL)
-                               return(ENOSPC);
+                       if (suptr == NULL) {
+                               error = ENOSPC;
+                               goto done;
+                       }
                        *supptr = suptr;
                }
        }
@@ -342,20 +353,24 @@ semundo_adjust(struct proc *p, struct sem_undo **supptr, int semid, int semnum,
                                suptr->un_ent[i] =
                                    suptr->un_ent[suptr->un_cnt];
                }
-               return(0);
+               goto done;
        }
 
        /* Didn't find the right entry - create it */
        if (adjval == 0)
-               return(0);
+               goto done;
        if (suptr->un_cnt != seminfo.semume) {
                sunptr = &suptr->un_ent[suptr->un_cnt];
                suptr->un_cnt++;
                sunptr->un_adjval = adjval;
                sunptr->un_id = semid; sunptr->un_num = semnum;
-       } else
-               return(EINVAL);
-       return(0);
+       } else {
+               error = EINVAL;
+       }
+done:
+       lwkt_reltoken(&p->p_token);
+       lwkt_reltoken(&semu_token);
+       return (error);
 }
 
 static void
@@ -363,6 +378,7 @@ semundo_clear(int semid, int semnum)
 {
        struct sem_undo *suptr;
 
+       lwkt_gettoken(&semu_token);
        for (suptr = semu_list; suptr != NULL; suptr = suptr->un_next) {
                struct undo *sunptr = &suptr->un_ent[0];
                int i = 0;
@@ -383,6 +399,7 @@ semundo_clear(int semid, int semnum)
                        i++, sunptr++;
                }
        }
+       lwkt_reltoken(&semu_token);
 }
 
 /*
@@ -412,7 +429,6 @@ sys___semctl(struct __semctl_args *uap)
        if (!jail_sysvipc_allowed && cred->cr_prison != NULL)
                return (ENOSYS);
 
-       get_mplock();
        switch (cmd) {
        case SEM_STAT:
                /*
@@ -424,28 +440,32 @@ sys___semctl(struct __semctl_args *uap)
                        break;
                }
                semakptr = &sema[semid];
+               lwkt_getpooltoken(semakptr);
                if ((semakptr->sem_perm.mode & SEM_ALLOC) == 0) {
                        eval = EINVAL;
+                       lwkt_relpooltoken(semakptr);
                        break;
                }
-               if ((eval = ipcperm(td->td_proc, &semakptr->sem_perm, IPC_R)))
+               if ((eval = ipcperm(td->td_proc, &semakptr->sem_perm, IPC_R))) {
+                       lwkt_relpooltoken(semakptr);
                        break;
-
+               }
                bcopy(&semakptr, arg->buf, sizeof(struct semid_ds));
                rval = IXSEQ_TO_IPCID(semid, semakptr->sem_perm);
+               lwkt_relpooltoken(semakptr);
                break;
        }
        
        semid = IPCID_TO_IX(semid);
        if (semid < 0 || semid >= seminfo.semmni) {
-               rel_mplock();
                return(EINVAL);
        }
-
        semaptr = &sema[semid];
+       lwkt_getpooltoken(semaptr);
+
        if ((semaptr->sem_perm.mode & SEM_ALLOC) == 0 ||
            semaptr->sem_perm.seq != IPCID_TO_SEQ(uap->semid)) {
-               rel_mplock();
+               lwkt_relpooltoken(semaptr);
                return(EINVAL);
        }
 
@@ -578,8 +598,8 @@ sys___semctl(struct __semctl_args *uap)
                        break;
                for (i = 0; i < semaptr->sem_nsems; i++) {
                        eval = copyin(&real_arg.array[i],
-                           (caddr_t)&semaptr->sem_base[i].semval,
-                           sizeof(real_arg.array[0]));
+                                     (caddr_t)&semaptr->sem_base[i].semval,
+                                     sizeof(real_arg.array[0]));
                        if (eval != 0)
                                break;
                }
@@ -591,7 +611,7 @@ sys___semctl(struct __semctl_args *uap)
                eval = EINVAL;
                break;
        }
-       rel_mplock();
+       lwkt_relpooltoken(semaptr);
 
        if (eval == 0)
                uap->sysmsg_result = rval;
@@ -618,14 +638,21 @@ sys_semget(struct semget_args *uap)
        if (!jail_sysvipc_allowed && cred->cr_prison != NULL)
                return (ENOSYS);
 
-       get_mplock();
        eval = 0;
 
        if (key != IPC_PRIVATE) {
                for (semid = 0; semid < seminfo.semmni; semid++) {
-                       if ((sema[semid].sem_perm.mode & SEM_ALLOC) &&
-                           sema[semid].sem_perm.key == key)
-                               break;
+                       if ((sema[semid].sem_perm.mode & SEM_ALLOC) == 0 ||
+                           sema[semid].sem_perm.key != key) {
+                               continue;
+                       }
+                       lwkt_getpooltoken(&sema[semid]);
+                       if ((sema[semid].sem_perm.mode & SEM_ALLOC) == 0 ||
+                           sema[semid].sem_perm.key != key) {
+                               lwkt_relpooltoken(&sema[semid]);
+                               continue;
+                       }
+                       break;
                }
                if (semid < seminfo.semmni) {
 #ifdef SEM_DEBUG
@@ -634,6 +661,7 @@ sys_semget(struct semget_args *uap)
                        if ((eval = ipcperm(td->td_proc,
                                            &sema[semid].sem_perm,
                                            semflg & 0700))) {
+                               lwkt_relpooltoken(&sema[semid]);
                                goto done;
                        }
                        if (nsems > 0 && sema[semid].sem_nsems < nsems) {
@@ -641,6 +669,7 @@ sys_semget(struct semget_args *uap)
                                kprintf("too small\n");
 #endif
                                eval = EINVAL;
+                               lwkt_relpooltoken(&sema[semid]);
                                goto done;
                        }
                        if ((semflg & IPC_CREAT) && (semflg & IPC_EXCL)) {
@@ -648,6 +677,7 @@ sys_semget(struct semget_args *uap)
                                kprintf("not exclusive\n");
 #endif
                                eval = EEXIST;
+                               lwkt_relpooltoken(&sema[semid]);
                                goto done;
                        }
                        goto done;
@@ -660,23 +690,30 @@ sys_semget(struct semget_args *uap)
        if (key == IPC_PRIVATE || (semflg & IPC_CREAT)) {
                if (nsems <= 0 || nsems > seminfo.semmsl) {
 #ifdef SEM_DEBUG
-                       kprintf("nsems out of range (0<%d<=%d)\n", nsems,
-                           seminfo.semmsl);
+                       kprintf("nsems out of range (0<%d<=%d)\n",
+                               nsems, seminfo.semmsl);
 #endif
                        eval = EINVAL;
                        goto done;
                }
                if (nsems > seminfo.semmns - semtot) {
 #ifdef SEM_DEBUG
-                       kprintf("not enough semaphores left (need %d, got %d)\n",
-                           nsems, seminfo.semmns - semtot);
+                       kprintf("not enough semaphores left "
+                               "(need %d, got %d)\n",
+                               nsems, seminfo.semmns - semtot);
 #endif
                        eval = ENOSPC;
                        goto done;
                }
                for (semid = 0; semid < seminfo.semmni; semid++) {
-                       if ((sema[semid].sem_perm.mode & SEM_ALLOC) == 0)
-                               break;
+                       if (sema[semid].sem_perm.mode & SEM_ALLOC)
+                               continue;
+                       lwkt_getpooltoken(&sema[semid]);
+                       if (sema[semid].sem_perm.mode & SEM_ALLOC) {
+                               lwkt_relpooltoken(&sema[semid]);
+                               continue;
+                       }
+                       break;
                }
                if (semid == seminfo.semmni) {
 #ifdef SEM_DEBUG
@@ -704,9 +741,10 @@ sys_semget(struct semget_args *uap)
                bzero(sema[semid].sem_base,
                    sizeof(sema[semid].sem_base[0])*nsems);
 #ifdef SEM_DEBUG
-               kprintf("sembase = 0x%x, next = 0x%x\n", sema[semid].sem_base,
-                   &sem[semtot]);
+               kprintf("sembase = 0x%x, next = 0x%x\n",
+                       sema[semid].sem_base, &sem[semtot]);
 #endif
+               /* eval == 0 */
        } else {
 #ifdef SEM_DEBUG
                kprintf("didn't find it and wasn't asked to create it\n");
@@ -716,10 +754,10 @@ sys_semget(struct semget_args *uap)
 
 done:
        if (eval == 0) {
-               uap->sysmsg_result = IXSEQ_TO_IPCID(semid,
-                                                   sema[semid].sem_perm);
+               uap->sysmsg_result =
+                       IXSEQ_TO_IPCID(semid, sema[semid].sem_perm);
+               lwkt_relpooltoken(&sema[semid]);
        }
-       rel_mplock();
        return(eval);
 }
 
@@ -747,15 +785,14 @@ sys_semop(struct semop_args *uap)
        if (!jail_sysvipc_allowed && td->td_ucred->cr_prison != NULL)
                return (ENOSYS);
 
-       get_mplock();
        semid = IPCID_TO_IX(semid);     /* Convert back to zero origin */
 
        if (semid < 0 || semid >= seminfo.semmni) {
                eval = EINVAL;
-               goto done;
+               goto done2;
        }
-
        semaptr = &sema[semid];
+       lwkt_getpooltoken(semaptr);
        if ((semaptr->sem_perm.mode & SEM_ALLOC) == 0) {
                eval = EINVAL;
                goto done;
@@ -999,7 +1036,8 @@ donex:
        uap->sysmsg_result = 0;
        eval = 0;
 done:
-       rel_mplock();
+       lwkt_relpooltoken(semaptr);
+done2:
        return(eval);
 }
 
@@ -1017,22 +1055,35 @@ semexit(struct proc *p)
        did_something = 0;
 
        /*
-        * Go through the chain of undo vectors looking for one
-        * associated with this process.
+        * We're getting a global token, don't do it if we couldn't
+        * possibly have any semaphores.
         */
+       if ((p->p_flags & P_SYSVSEM) == 0)
+               return;
 
+       /*
+        * Go through the chain of undo vectors looking for one
+        * associated with this process.  De-link it from the
+        * list right now, while we have the token, but do not
+        * clear un_proc until we finish cleaning up the relationship.
+        */
+       lwkt_gettoken(&semu_token);
        for (supptr = &semu_list; (suptr = *supptr) != NULL;
-           supptr = &suptr->un_next) {
-               if (suptr->un_proc == p)
+            supptr = &suptr->un_next) {
+               if (suptr->un_proc == p) {
+                       *supptr = suptr->un_next;
                        break;
+               }
        }
+       p->p_flags &= ~P_SYSVSEM;
+       lwkt_reltoken(&semu_token);
 
        if (suptr == NULL)
                return;
 
 #ifdef SEM_DEBUG
        kprintf("proc @%08x has undo structure with %d entries\n", p,
-           suptr->un_cnt);
+               suptr->un_cnt);
 #endif
 
        /*
@@ -1080,9 +1131,9 @@ semexit(struct proc *p)
        /*
         * Deallocate the undo vector.
         */
+       cpu_sfence();
+       suptr->un_proc = NULL;
 #ifdef SEM_DEBUG
        kprintf("removing vector\n");
 #endif
-       suptr->un_proc = NULL;
-       *supptr = suptr->un_next;
 }