From owner-p4-projects@FreeBSD.ORG Wed Sep 21 11:39:01 2005 Return-Path: X-Original-To: p4-projects@freebsd.org Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id C272E16A421; Wed, 21 Sep 2005 11:39:00 +0000 (GMT) X-Original-To: perforce@freebsd.org 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 6D55916A41F for ; Wed, 21 Sep 2005 11:39:00 +0000 (GMT) (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id 22E9E43D45 for ; Wed, 21 Sep 2005 11:39:00 +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.13.1/8.13.1) with ESMTP id j8LBd05D043304 for ; Wed, 21 Sep 2005 11:39:00 GMT (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.13.1/8.13.1/Submit) id j8LBcxij043298 for perforce@freebsd.org; Wed, 21 Sep 2005 11:38:59 GMT (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Date: Wed, 21 Sep 2005 11:38:59 GMT Message-Id: <200509211138.j8LBcxij043298@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 Cc: Subject: PERFORCE change 84050 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Sep 2005 11:39:01 -0000 http://perforce.freebsd.org/chv.cgi?CH=84050 Change 84050 by rwatson@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); } }