PERFORCE change 84050 for review

[Robert Watson]

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

Change 84050 by rwatson at rwatson_zoo on 2005/09/21 11:38:53

	In audit_record_write(), discussion of Giant isn't required
	anymore.
	
	Remove Giant frobbing from audit_worker(), where it is also no
	longer required.  This simplifies things some.

Affected files ...

.. //depot/projects/trustedbsd/audit3/sys/security/audit/kern_audit.c#35 edit

Differences ...

==== //depot/projects/trustedbsd/audit3/sys/security/audit/kern_audit.c#35 (text+ko) ====

@@ -516,13 +516,10 @@
 	
 	/*
 	 * 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
-	 * take the BSM record as a parameter.
-	 *
-	 * XXXRW: In the new world order, this is no longer true.
+	 * 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 take the BSM record as a
+	 * parameter.
 	 */
 	ret = (vn_rdwr(UIO_WRITE, vp, (void *)bsm->data, bsm->len,
 	    (off_t)0, UIO_SYSSPACE, IO_APPEND|IO_UNIT, cred, NULL, NULL, td));
@@ -556,13 +553,11 @@
  * to a thread-local work queue.  In addition, the audit_work performs the
  * actual exchange of audit log vnode pointer, as audit_vp is a thread-local
  * variable.
- *
- * XXXAUDIT: Giant is now less required here.
  */
 static void
 audit_worker(void *arg)
 {
-	int do_replacement_signal, error, release_giant;
+	int do_replacement_signal, error;
 	TAILQ_HEAD(, kaudit_record) ar_worklist;
 	struct kaudit_record *ar;
 	struct vnode *audit_vp, *old_vp;
@@ -584,10 +579,9 @@
 	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 Giant, which means releasing audit_mtx.
-		 * In case another replacement was scheduled while the mutex
-		 * we released, we loop.
+		 * Accessing the vnode here requires dropping the audit_mtx;
+		 * in case another replacement was scheduled while the mutex
+		 * was released, we loop.
 		 *
 		 * XXX It could well be we should drain existing records
 		 * first to ensure that the timestamps and ordering
@@ -605,20 +599,16 @@
 
 			audit_enabled = (audit_vp != NULL);
 
-			if (old_vp != NULL || audit_vp != NULL) {
-				mtx_unlock(&audit_mtx);
-				mtx_lock(&Giant);
-				release_giant = 1;
-			} else
-				release_giant = 0;
 			/*
 			 * XXX: What to do about write failures here?
 			 */
 			if (old_vp != NULL) {
 				AUDIT_PRINTF(("Closing old audit file\n"));
+				mtx_unlock(&audit_mtx);
 				vn_close(old_vp, audit_close_flags, old_cred,
 				    audit_td);
 				crfree(old_cred);
+				mtx_lock(&audit_mtx);
 				old_cred = NULL;
 				old_vp = NULL;
 				AUDIT_PRINTF(("Audit file closed\n"));
@@ -626,10 +616,6 @@
 			if (audit_vp != NULL) {
 				AUDIT_PRINTF(("Opening new audit file\n"));
 			}
-			if (release_giant) {
-				mtx_unlock(&Giant);
-				mtx_lock(&audit_mtx);
-			}
 			do_replacement_signal = 1;
 		}
 		/*
@@ -711,20 +697,9 @@
 		}
 
 		mtx_unlock(&audit_mtx);
-		release_giant = 0;
 		while ((ar = TAILQ_FIRST(&ar_worklist))) {
 			TAILQ_REMOVE(&ar_worklist, ar, k_q);
 			if (audit_vp != NULL) {
-				/*
-				 * XXX: What should happen if there's a write
-				 * error here?
-				 */
-				if (!release_giant) {
-					mtx_lock(&Giant);
-					release_giant = 1;
-				}
-				VOP_LEASE(audit_vp, audit_td, audit_cred,
-				    LEASE_WRITE);
 				error = audit_record_write(audit_vp, ar, 
 				    audit_cred, audit_td);
 				if (error && audit_panic_on_write_fail)
@@ -736,8 +711,6 @@
 			}
 			audit_free(ar);
 		}
-		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