PERFORCE change 56803 for review

[Robert Watson]

http://perforce.freebsd.org/chv.cgi?CH=56803

Change 56803 by rwatson at 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);
 }
 
To Unsubscribe: send mail to majordomo at trustedbsd.org
with "unsubscribe trustedbsd-cvs" in the body of the message