PERFORCE change 93510 for review

[Robert Watson]

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

Change 93510 by rwatson at rwatson_peppercorn on 2006/03/18 16:57:39

	Move wait loop up to top of audit_worker work loop, and add
	replacement as a condition for breaking from the wait loop.
	
	Move drain logic into its own function, audit_worker_drain(), and
	invoke that.  Follow advice of comment and don't wake up when
	going below the watermark if we are draining, since no one is
	interested.

Affected files ...

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

Differences ...

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

@@ -524,6 +524,22 @@
 }
 
 /*
+ * Drain the audit commit queue and free the records.  Used if there are
+ * records present, but no audit log target.
+ */
+static void
+audit_worker_drain(void)
+{
+	struct kaudit_record *ar;
+
+	while ((ar = TAILQ_FIRST(&audit_q))) {
+		TAILQ_REMOVE(&audit_q, ar, k_q);
+		uma_zfree(audit_record_zone, ar);
+		audit_q_len--;
+	}
+}
+
+/*
  * The audit_worker thread is responsible for watching the event queue,
  * dequeueing records, converting them to BSM format, and committing them to
  * disk.  In order to minimize lock thrashing, records are dequeued in sets
@@ -556,10 +572,16 @@
 		mtx_assert(&audit_mtx, MA_OWNED);
 
 		/*
-		 * XXXRW: Logic here should really be: while (!events and
-		 * !records) cv_wait(), then process events, and then
-		 * records.
+		 * Wait for record or rotation events.
 		 */
+		while (!audit_replacement_flag && TAILQ_EMPTY(&audit_q)) {
+			AUDIT_PRINTF(("audit_worker waiting\n"));
+			cv_wait(&audit_cv, &audit_mtx);
+			AUDIT_PRINTF(("audit_worker woken up\n"));
+			AUDIT_PRINTF(("audit_worker: new vp = %p; value of "
+			    "flag %d\n", audit_replacement_vp,
+			    audit_replacement_flag));
+		}
 
 		/*
 		 * First priority: replace the audit log target if requested.
@@ -567,19 +589,6 @@
 		audit_worker_rotate(&audit_cred, &audit_vp, audit_td);
 
 		/*
-		 * Next, check to see if we have any records to drain into
-		 * the vnode.  If not, go back to waiting for an event.
-		 */
-		if (TAILQ_EMPTY(&audit_q)) {
-			AUDIT_PRINTF(("audit_worker waiting\n"));
-			cv_wait(&audit_cv, &audit_mtx);
-			AUDIT_PRINTF(("audit_worker woken up\n"));
-	AUDIT_PRINTF(("audit_worker: new vp = %p; value of flag %d\n",
-	    audit_replacement_vp, audit_replacement_flag));
-			continue;
-		}
-
-		/*
 		 * If we have records, but there's no active vnode to write
 		 * to, drain the record queue.  Generally, we prevent the
 		 * unnecessary allocation of records elsewhere, but we need
@@ -587,17 +596,7 @@
 		 * queueing.  Go back to waiting when we're done.
 		 */
 		if (audit_vp == NULL) {
-			while ((ar = TAILQ_FIRST(&audit_q))) {
-				TAILQ_REMOVE(&audit_q, ar, k_q);
-				uma_zfree(audit_record_zone, ar);
-				audit_q_len--;
-				/*
-				 * XXXRW: Why broadcast if we hold the
-				 * mutex and know that audit_vp is NULL?
-				 */
-				if (audit_q_len <= audit_qctrl.aq_lowater)
-					cv_broadcast(&audit_commit_cv);
-			}
+			audit_worker_drain();
 			continue;
 		}