Index: kern_acct.c =================================================================== RCS file: /home/ncvs/src/sys/kern/kern_acct.c,v retrieving revision 1.75 diff -u -r1.75 kern_acct.c --- kern_acct.c 21 Sep 2005 15:28:07 -0000 1.75 +++ kern_acct.c 21 Sep 2005 18:03:58 -0000 @@ -1,4 +1,5 @@ /*- + * Copyright (c) 2005 Robert N. M. Watson * Copyright (c) 1994 Christopher G. Demetriou * Copyright (c) 1982, 1986, 1989, 1993 * The Regents of the University of California. All rights reserved. @@ -56,6 +57,7 @@ #include #include #include +#include #include #include #include @@ -90,15 +92,13 @@ /* * Accounting vnode pointer, saved vnode pointer, and flags for each. */ -static struct vnode *acctp; -static struct ucred *acctcred; -static int acctflags; -static struct vnode *savacctp; -static struct ucred *savacctcred; -static int savacctflags; +static int acct_suspended; +static struct vnode *acct_vp; +static struct ucred *acct_cred; +static int acct_flags; +static struct sx acct_sx; -static struct mtx acct_mtx; -MTX_SYSINIT(acct, &acct_mtx, "accounting", MTX_DEF); +SX_SYSINIT(acct, &acct_sx, "acct_sx"); /* * Values associated with enabling and disabling accounting @@ -129,85 +129,86 @@ } */ *uap; { struct nameidata nd; - int error, flags; + int error, flags, vfslocked = 0; /* Make sure that the caller is root. */ error = suser(td); if (error) return (error); - mtx_lock(&Giant); - /* * If accounting is to be started to a file, open that file for * appending and make sure it's a 'normal'. */ if (uap->path != NULL) { - NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_USERSPACE, uap->path, td); + NDINIT(&nd, LOOKUP, NOFOLLOW | MPSAFE, UIO_USERSPACE, + uap->path, td); flags = FWRITE | O_APPEND; error = vn_open(&nd, &flags, 0, -1); + vfslocked = NDHASGIANT(&nd); if (error) - goto done2; + goto done_vfs; NDFREE(&nd, NDF_ONLY_PNBUF); #ifdef MAC error = mac_check_system_acct(td->td_ucred, nd.ni_vp); if (error) { VOP_UNLOCK(nd.ni_vp, 0, td); vn_close(nd.ni_vp, flags, td->td_ucred, td); - goto done2; + goto done_vfs; } #endif VOP_UNLOCK(nd.ni_vp, 0, td); if (nd.ni_vp->v_type != VREG) { vn_close(nd.ni_vp, flags, td->td_ucred, td); error = EACCES; - goto done2; + goto done_vfs; } #ifdef MAC } else { error = mac_check_system_acct(td->td_ucred, NULL); if (error) - goto done2; + goto done_vfs; #endif } - mtx_lock(&acct_mtx); + /* + * Disallow concurrent access to the accounting vnode while we swap + * it out, in order to prevent access after close. + */ + sx_xlock(&acct_sx); /* * If accounting was previously enabled, kill the old space-watcher, * close the file, and (if no new file was specified, leave). - * - * XXX arr: should not hold lock over vnode operation. */ - if (acctp != NULLVP || savacctp != NULLVP) { + if (acct_vp != NULL) { callout_stop(&acctwatch_callout); - error = vn_close((acctp != NULLVP ? acctp : savacctp), - (acctp != NULLVP ? acctflags : savacctflags), - (acctcred != NOCRED ? acctcred : savacctcred), td); - acctp = savacctp = NULLVP; - crfree(acctcred != NOCRED ? acctcred : savacctcred); - acctcred = savacctcred = NOCRED; + error = vn_close(acct_vp, acct_flags, acct_cred, td); + crfree(acct_cred); + acct_vp = NULL; + acct_cred = NULL; + acct_flags = 0; log(LOG_NOTICE, "Accounting disabled\n"); } - if (uap->path == NULL) { - mtx_unlock(&acct_mtx); - goto done2; - } + if (uap->path == NULL) + goto done_sx; /* * Save the new accounting file vnode, and schedule the new * free space watcher. */ - acctp = nd.ni_vp; - acctcred = crhold(td->td_ucred); - acctflags = flags; + acct_suspended = 0; + acct_vp = nd.ni_vp; + acct_cred = crhold(td->td_ucred); + acct_flags = flags; callout_init(&acctwatch_callout, 0); - mtx_unlock(&acct_mtx); log(LOG_NOTICE, "Accounting enabled\n"); acctwatch(NULL); -done2: - mtx_unlock(&Giant); +done_sx: + sx_xunlock(&acct_sx); +done_vfs: + VFS_UNLOCK_GIANT(vfslocked); return (error); } @@ -226,27 +227,24 @@ struct plimit *newlim, *oldlim; struct proc *p; struct rusage *r; - struct ucred *uc; - struct vnode *vp; int t, ret; /* * Lockless check of accounting condition before doing the hard * work. */ - if (acctp == NULLVP) + if (acct_vp == NULL || acct_suspended) return (0); - mtx_lock(&acct_mtx); + sx_slock(&acct_sx); /* * If accounting isn't enabled, don't bother. Have to check again * once we own the lock in case we raced with disabling of accounting * by another thread. */ - vp = acctp; - if (vp == NULLVP) { - mtx_unlock(&acct_mtx); + if (acct_vp == NULL || acct_suspended) { + sx_sunlock(&acct_sx); return (0); } @@ -303,13 +301,6 @@ PROC_UNLOCK(p); /* - * Finish doing things that require acct_mtx, and release acct_mtx. - */ - uc = crhold(acctcred); - vref(vp); - mtx_unlock(&acct_mtx); - - /* * Eliminate any file size rlimit. */ newlim = lim_alloc(); @@ -324,12 +315,12 @@ /* * Write the accounting information to the file. */ - VOP_LEASE(vp, td, uc, LEASE_WRITE); - ret = vn_rdwr(UIO_WRITE, vp, (caddr_t)&acct, sizeof (acct), - (off_t)0, UIO_SYSSPACE, IO_APPEND|IO_UNIT, uc, NOCRED, + VOP_LEASE(acct_vp, td, acct_cred, LEASE_WRITE); + ret = vn_rdwr(UIO_WRITE, acct_vp, (caddr_t)&acct, sizeof (acct), + (off_t)0, UIO_SYSSPACE, IO_APPEND|IO_UNIT, acct_cred, NOCRED, (int *)0, td); - vrele(vp); - crfree(uc); + + sx_sunlock(&acct_sx); return (ret); } @@ -385,53 +376,39 @@ { struct statfs sb; - mtx_lock(&acct_mtx); + sx_xlock(&acct_sx); /* May change accounting configuration. */ + + if (acct_vp->v_type == VBAD) { + (void) vn_close(acct_vp, acct_flags, acct_cred, NULL); + crfree(acct_cred); + acct_vp = NULL; + acct_cred = NULL; + acct_flags = 0; + sx_xunlock(&acct_sx); + return; + } /* - * XXX arr: need to fix the issue of holding acct_mtx over - * the below vnode operations. + * Stopping here is better than continuing, maybe it will be VBAD + * next time around. */ - if (savacctp != NULLVP) { - if (savacctp->v_type == VBAD) { - (void) vn_close(savacctp, savacctflags, savacctcred, - NULL); - savacctp = NULLVP; - savacctcred = NOCRED; - mtx_unlock(&acct_mtx); - return; - } - (void)VFS_STATFS(savacctp->v_mount, &sb, curthread); + if (VFS_STATFS(acct_vp->v_mount, &sb, curthread) < 0) { + sx_xunlock(&acct_sx); + return; + } + + if (acct_suspended) { if (sb.f_bavail > acctresume * sb.f_blocks / 100) { - acctp = savacctp; - acctcred = savacctcred; - acctflags = savacctflags; - savacctp = NULLVP; - savacctcred = NOCRED; + acct_suspended = 0; log(LOG_NOTICE, "Accounting resumed\n"); } } else { - if (acctp == NULLVP) { - mtx_unlock(&acct_mtx); - return; - } - if (acctp->v_type == VBAD) { - (void) vn_close(acctp, acctflags, acctcred, NULL); - acctp = NULLVP; - crfree(acctcred); - acctcred = NOCRED; - mtx_unlock(&acct_mtx); - return; - } - (void)VFS_STATFS(acctp->v_mount, &sb, curthread); if (sb.f_bavail <= acctsuspend * sb.f_blocks / 100) { - savacctp = acctp; - savacctflags = acctflags; - savacctcred = acctcred; - acctp = NULLVP; - acctcred = NOCRED; + acct_suspended = 1; log(LOG_NOTICE, "Accounting suspended\n"); } } + callout_reset(&acctwatch_callout, acctchkfreq * hz, acctwatch, NULL); - mtx_unlock(&acct_mtx); + sx_xunlock(&acct_sx); }