From owner-p4-projects@FreeBSD.ORG Thu Jul 8 20:23:10 2004 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id D0E0716A4D0; Thu, 8 Jul 2004 20:23:09 +0000 (GMT) Delivered-To: perforce@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id AC2BA16A4CE for ; Thu, 8 Jul 2004 20:23:09 +0000 (GMT) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id 8EBD643D2F for ; Thu, 8 Jul 2004 20:23:09 +0000 (GMT) (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.12.11/8.12.11) with ESMTP id i68KN9VZ026293 for ; Thu, 8 Jul 2004 20:23:09 GMT (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.12.11/8.12.11/Submit) id i68KN9ET026290 for perforce@freebsd.org; Thu, 8 Jul 2004 20:23:09 GMT (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Date: Thu, 8 Jul 2004 20:23:09 GMT Message-Id: <200407082023.i68KN9ET026290@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to bb+lists.freebsd.perforce@cyrus.watson.org using -f From: Robert Watson To: Perforce Change Reviews Subject: PERFORCE change 56803 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Jul 2004 20:23:10 -0000 http://perforce.freebsd.org/chv.cgi?CH=56803 Change 56803 by rwatson@rwatson_tislabs on 2004/07/08 20:22:27 Simplify reference counting out of current sysv_sem implementation: right idea, wrong way. Affected files ... .. //depot/projects/trustedbsd/mac/sys/kern/sysv_sem.c#23 edit Differences ... ==== //depot/projects/trustedbsd/mac/sys/kern/sysv_sem.c#23 (text+ko) ==== @@ -78,8 +78,6 @@ SLIST_HEAD(, sem_undo) semu_list; /* list of active undo structures */ static int *semu; /* undo structure pool */ static eventhandler_tag semexit_tag; -static int refcount; /* to ensure consistency during and after semunload */ -static struct mtx refcnt_mtx; /* global mutex for refcount. */ #define SEMUNDO_MTX sem_mtx #define SEMUNDO_LOCK() mtx_lock(&SEMUNDO_MTX); @@ -222,16 +220,6 @@ } SLIST_INIT(&semu_list); mtx_init(&sem_mtx, "sem", NULL, MTX_DEF); - refcount =0; - /* - * It is not permissible to pass the same mutex to mtx_init() - * multiple times without intervening calls to mtx_destroy(). - * Since we cannot destroy the refcnt_mtx during semunload, we check - * if the mtx_init has ever been called. If so, we dont need to do - * mtx_init as the mutex is already initialized. - */ - if (mtx_initialized(&refcnt_mtx) == 0) - mtx_init(&refcnt_mtx, "semrefcnt", NULL, MTX_DEF); semexit_tag = EVENTHANDLER_REGISTER(process_exit, semexit_myhook, NULL, EVENTHANDLER_PRI_ANY); } @@ -241,21 +229,6 @@ { int i; - /* - * Make sure that the semunload maintains the consistency of the sem - * and sema data structures. This assures that the unload doesn't take - * place if any thread is in any of the code-paths (tinkering with the - * data structures), and also that no thread can enter the code-paths - * once the module is unloaded. - */ - mtx_lock(&refcnt_mtx); - if ((refcount > 0) || (semtot != 0)) { - mtx_unlock(&refcnt_mtx); - return (EBUSY); - } - refcount= -1; /* Mark the module as being unloaded */ - mtx_unlock(&refcnt_mtx); - EVENTHANDLER_DEREGISTER(process_exit, semexit_tag); #ifdef MAC for (i = 0; i < seminfo.semmni; i++) @@ -267,11 +240,6 @@ for (i = 0; i < seminfo.semmni; i++) mtx_destroy(&sema_mtx[i]); mtx_destroy(&sem_mtx); - /* - * NOTE: We cannot destroy the refcnt_mtx as it is possible that some - * thread might (attempt to) hold the mutex. - */ -/* mtx_destroy(&refcnt_mtx); */ return (0); } @@ -563,28 +531,14 @@ if (!jail_sysvipc_allowed && jailed(td->td_ucred)) return (ENOSYS); - /* - * Prevent thread from going any further if module is (being) - * unloaded. - */ - mtx_lock(&refcnt_mtx); - if (refcount < 0 ) { - mtx_unlock(&refcnt_mtx); - return (ENOSYS); - } - refcount++; /* Indicate that thread is active in the code-path */ - mtx_unlock(&refcnt_mtx); - array = NULL; switch(cmd) { case SEM_STAT: - if (semid < 0 || semid >= seminfo.semmni) { - error = EINVAL; - goto done3; - } + if (semid < 0 || semid >= seminfo.semmni) + return (EINVAL); if ((error = copyin(arg, &real_arg, sizeof(real_arg))) != 0) - goto done3; + return (error); semakptr = &sema[semid]; sema_mtxp = &sema_mtx[semid]; mtx_lock(sema_mtxp); @@ -607,14 +561,12 @@ rval = IXSEQ_TO_IPCID(semid, semakptr->u.sem_perm); if (error == 0) td->td_retval[0] = rval; - goto done3; + return (error); } semid = IPCID_TO_IX(semid); - if (semid < 0 || semid >= seminfo.semmni) { - error = EINVAL; - goto done3; - } + if (semid < 0 || semid >= seminfo.semmni) + return (EINVAL); semakptr = &sema[semid]; sema_mtxp = &sema_mtx[semid]; @@ -836,10 +788,6 @@ mtx_unlock(sema_mtxp); if (array != NULL) free(array, M_TEMP); -done3: - mtx_lock(&refcnt_mtx); - refcount--; /* Indicate that thread no longer active in the code-path */ - mtx_unlock(&refcnt_mtx); return(error); } @@ -869,18 +817,6 @@ if (!jail_sysvipc_allowed && jailed(td->td_ucred)) return (ENOSYS); - /* - * Prevent thread from going any further if module is (being) - * unloaded. - */ - mtx_lock(&refcnt_mtx); - if (refcount < 0 ) { - mtx_unlock(&refcnt_mtx); - return (ENOSYS); - } - refcount++; /* Indicate that thread is active in the code-path */ - mtx_unlock(&refcnt_mtx); - mtx_lock(&Giant); if (key != IPC_PRIVATE) { for (semid = 0; semid < seminfo.semmni; semid++) { @@ -971,9 +907,6 @@ td->td_retval[0] = IXSEQ_TO_IPCID(semid, sema[semid].u.sem_perm); done2: mtx_unlock(&Giant); - mtx_lock(&refcnt_mtx); - refcount--; /* Indicate that thread no longer active in the code-path */ - mtx_unlock(&refcnt_mtx); return (error); } @@ -1012,24 +945,10 @@ if (!jail_sysvipc_allowed && jailed(td->td_ucred)) return (ENOSYS); - /* - * Prevent thread from going any further if module is (being) - * unloaded - */ - mtx_lock(&refcnt_mtx); - if (refcount < 0 ) { - mtx_unlock(&refcnt_mtx); - return (ENOSYS); - } - refcount++; /* Indicate that thread is active in the code-path */ - mtx_unlock(&refcnt_mtx); - semid = IPCID_TO_IX(semid); /* Convert back to zero origin */ - if (semid < 0 || semid >= seminfo.semmni) { + if (semid < 0 || semid >= seminfo.semmni) error = EINVAL; - goto done3; - } /* Allocate memory for sem_ops */ if (nsops <= SMALL_SOPS) @@ -1039,15 +958,14 @@ else { DPRINTF(("too many sops (max=%d, nsops=%d)\n", seminfo.semopm, nsops)); - error = E2BIG; - goto done3; + return (E2BIG); } if ((error = copyin(uap->sops, sops, nsops * sizeof(sops[0]))) != 0) { DPRINTF(("error = %d from copyin(%08x, %08x, %d)\n", error, uap->sops, sops, nsops * sizeof(sops[0]))); if (sops != small_sops) free(sops, M_SEM); - goto done3; + return (error); } semakptr = &sema[semid]; @@ -1295,10 +1213,6 @@ if (sops != small_sops) free(sops, M_SEM); free(sops, M_SEM); -done3: - mtx_lock(&refcnt_mtx); - refcount--; /* Indicate that thread no longer active in the code-path */ - mtx_unlock(&refcnt_mtx); return (error); }