From owner-p4-projects@FreeBSD.ORG Sat Jan 17 11:52:10 2004 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id B054F16A4D0; Sat, 17 Jan 2004 11:52:10 -0800 (PST) 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 7209816A4CE for ; Sat, 17 Jan 2004 11:52:10 -0800 (PST) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id CCD1B43D41 for ; Sat, 17 Jan 2004 11:52:08 -0800 (PST) (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.12.10/8.12.10) with ESMTP id i0HJq80B088438 for ; Sat, 17 Jan 2004 11:52:08 -0800 (PST) (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.12.10/8.12.10/Submit) id i0HJq7f9088423 for perforce@freebsd.org; Sat, 17 Jan 2004 11:52:07 -0800 (PST) (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Date: Sat, 17 Jan 2004 11:52:07 -0800 (PST) Message-Id: <200401171952.i0HJq7f9088423@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 Subject: PERFORCE change 45500 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 17 Jan 2004 19:52:11 -0000 http://perforce.freebsd.org/chv.cgi?CH=45500 Change 45500 by rwatson@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); }