PERFORCE change 45500 for review

[Robert Watson]

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

Change 45500 by rwatson at rwatson_tislabs on 2004/01/17 11:51:56

	s/funnel/Giant/ and fix up some comments about locking.  There's
	some locking cleanup we can do on FreeBSD that wasn't possible
	on Darwin.

Affected files ...

.. //depot/projects/trustedbsd/audit2/sys/security/audit/audit.c#15 edit

Differences ...

==== //depot/projects/trustedbsd/audit2/sys/security/audit/audit.c#15 (text+ko) ====

@@ -196,7 +196,9 @@
 		return (-1);
 	}
 	
-	/* XXX This function can be called with the kernel funnel held,
+	/*
+	 * XXX
+	 * This function must be called with Giant held,
 	 * which is not optimal. We should break the write functionality
 	 * away from the BSM record generation and have the BSM generation
 	 * done before this function is called. This function will then
@@ -213,7 +215,7 @@
 static void
 audit_worker(void *arg)
 {
-	int do_replacement_signal, error, release_funnel;
+	int do_replacement_signal, error, release_giant;
 	TAILQ_HEAD(, kaudit_record) ar_worklist;
 	struct kaudit_record *ar;
 	struct vnode *audit_vp, *old_vp;
@@ -222,25 +224,20 @@
 
 	AUDIT_PRINTF(("audit_worker starting\n"));
 
+	/*
+	 * These are thread-local variables requiring no synchronization.
+	 */
 	TAILQ_INIT(&ar_worklist);
 	audit_cred = NULL;
 	audit_td = curthread;
 	audit_vp = NULL;
 
-	/*
-	 * XXX: Presumably we can assume Mach threads are started without
-	 * holding the BSD kernel funnel?
-	 */
-#ifdef DARWIN_FOO
-	thread_funnel_set(kernel_flock, FALSE);
-#endif
-
 	mtx_lock(&audit_mtx);
 	while (1) {
 		/*
 		 * First priority: replace the audit log target if requested.
 		 * As we actually close the vnode in the worker thread, we
-		 * need to grab the funnel, which means releasing audit_mtx.
+		 * need to grab Giant, which means releasing audit_mtx.
 		 * In case another replacement was scheduled while the mutex
 		 * we released, we loop.
 		 *
@@ -263,9 +260,9 @@
 			if (old_vp != NULL || audit_vp != NULL) {
 				mtx_unlock(&audit_mtx);
 				mtx_lock(&Giant);
-				release_funnel = 1;
+				release_giant = 1;
 			} else
-				release_funnel = 0;
+				release_giant = 0;
 			/*
 			 * XXX: What to do about write failures here?
 			 */
@@ -281,7 +278,7 @@
 			if (audit_vp != NULL) {
 				AUDIT_PRINTF(("Opening new audit file\n"));
 			}
-			if (release_funnel) {
+			if (release_giant) {
 				mtx_unlock(&Giant);
 				mtx_lock(&audit_mtx);
 			}
@@ -321,7 +318,7 @@
 		 *
 		 * XXX: We go out of our way to avoid calling audit_free()
 		 * with the audit_mtx held, to avoid a lock order reversal
-		 * as free() may grab the funnel.  This will be fixed at
+		 * as free() may grab Giant.  This will be fixed at
 		 * some point.
 		 */
 		if (audit_vp == NULL) {
@@ -347,15 +344,15 @@
 		 *
 		 * XXX: We go out of our way to avoid calling audit_free()
 		 * with the audit_mtx held, to avoid a lock order reversal
-		 * as free() may grab the funnel.  This will be fixed at
-		 * some point.
+		 * as free() may grab Giant.
+		 * XXX: Note that this is not true in FreeBSD.
 		 */
 		while ((ar = TAILQ_FIRST(&audit_q))) {
 			TAILQ_REMOVE(&audit_q, ar, k_q);
 			TAILQ_INSERT_TAIL(&ar_worklist, ar, k_q);
 		}
 		mtx_unlock(&audit_mtx);
-		release_funnel = 0;
+		release_giant = 0;
 		while ((ar = TAILQ_FIRST(&ar_worklist))) {
 			TAILQ_REMOVE(&ar_worklist, ar, k_q);
 			if (audit_vp != NULL) {
@@ -363,13 +360,9 @@
 				 * XXX: What should happen if there's a write
 				 * error here?
 				 */
-				if (!release_funnel) {
-#ifdef DARWIN_FOO
-					thread_funnel_set(kernel_flock, TRUE);
-#else
+				if (!release_giant) {
 					mtx_lock(&Giant);
-#endif
-					release_funnel = 1;
+					release_giant = 1;
 				}
 				VOP_LEASE(audit_vp, audit_td, audit_cred,
 				    LEASE_WRITE);
@@ -381,7 +374,7 @@
 			}
 			audit_free(ar);
 		}
-		if (release_funnel)
+		if (release_giant)
 			mtx_unlock(&Giant);
 		mtx_lock(&audit_mtx);
 	}
To Unsubscribe: send mail to majordomo at trustedbsd.org
with "unsubscribe trustedbsd-cvs" in the body of the message