PERFORCE change 94001 for review
John Baldwin
jhb at FreeBSD.org
Sat Mar 25 22:23:05 UTC 2006
http://perforce.freebsd.org/chv.cgi?CH=94001
Change 94001 by jhb at jhb_zion on 2006/03/25 22:22:54
Various VFS Giant locking for ndis, acct, alq, and ktrace.
Affected files ...
.. //depot/projects/smpng/sys/compat/ndis/subr_ndis.c#36 edit
.. //depot/projects/smpng/sys/kern/kern_acct.c#42 edit
.. //depot/projects/smpng/sys/kern/kern_alq.c#10 edit
.. //depot/projects/smpng/sys/kern/kern_ktrace.c#50 edit
Differences ...
==== //depot/projects/smpng/sys/compat/ndis/subr_ndis.c#36 (text+ko) ====
@@ -2899,7 +2899,7 @@
char *afilename = NULL;
struct thread *td = curthread;
struct nameidata nd;
- int flags, error;
+ int flags, error, vfslocked;
struct vattr vat;
struct vattr *vap = &vat;
ndis_fh *fh;
@@ -2986,21 +2986,19 @@
snprintf(path, MAXPATHLEN, "%s/%s", ndis_filepath, afilename);
- mtx_lock(&Giant);
-
/* Some threads don't have a current working directory. */
+ /* XXX: locking? Which threads don't have these, kthreads maybe? */
if (td->td_proc->p_fd->fd_rdir == NULL)
td->td_proc->p_fd->fd_rdir = rootvnode;
if (td->td_proc->p_fd->fd_cdir == NULL)
td->td_proc->p_fd->fd_cdir = rootvnode;
- NDINIT(&nd, LOOKUP, FOLLOW, UIO_SYSSPACE, path, td);
+ NDINIT(&nd, LOOKUP, FOLLOW | MPSAFE, UIO_SYSSPACE, path, td);
flags = FREAD;
error = vn_open(&nd, &flags, 0, -1);
if (error) {
- mtx_unlock(&Giant);
*status = NDIS_STATUS_FILE_NOT_FOUND;
ExFreePool(fh);
printf("NDIS: open file %s failed: %d\n", path, error);
@@ -3008,6 +3006,7 @@
free(afilename, M_DEVBUF);
return;
}
+ vfslocked = NDHASGIANT(&nd);
ExFreePool(path);
@@ -3016,7 +3015,7 @@
/* Get the file size. */
VOP_GETATTR(nd.ni_vp, vap, td->td_ucred, td);
VOP_UNLOCK(nd.ni_vp, 0, td);
- mtx_unlock(&Giant);
+ VFS_UNLOCK_GIANT(vfslocked);
fh->nf_vp = nd.ni_vp;
fh->nf_map = NULL;
@@ -3038,7 +3037,7 @@
struct thread *td = curthread;
linker_file_t lf;
caddr_t kldstart;
- int error, resid;
+ int error, resid, vfslocked;
if (filehandle == NULL) {
*status = NDIS_STATUS_FAILURE;
@@ -3076,10 +3075,10 @@
return;
}
- mtx_lock(&Giant);
+ vfslocked = VFS_LOCK_GIANT(vp->v_mount);
error = vn_rdwr(UIO_READ, fh->nf_vp, fh->nf_map, fh->nf_maplen, 0,
UIO_SYSSPACE, 0, td->td_ucred, NOCRED, &resid, td);
- mtx_unlock(&Giant);
+ VFS_UNLOCK_GIANT(vfslocked);
if (error)
*status = NDIS_STATUS_FAILURE;
@@ -3114,6 +3113,7 @@
{
struct thread *td = curthread;
ndis_fh *fh;
+ int vfslocked;
if (filehandle == NULL)
return;
@@ -3129,9 +3129,9 @@
return;
if (fh->nf_type == NDIS_FH_TYPE_VFS) {
- mtx_lock(&Giant);
+ vfslocked = VFS_LOCK_GIANT(vp->v_mount);
vn_close(fh->nf_vp, FREAD, td->td_ucred, td);
- mtx_unlock(&Giant);
+ VFS_UNLOCK_GIANT(vfslocked);
}
fh->nf_vp = NULL;
==== //depot/projects/smpng/sys/kern/kern_acct.c#42 (text+ko) ====
@@ -159,7 +159,7 @@
acct(struct thread *td, struct acct_args *uap)
{
struct nameidata nd;
- int error, flags;
+ int error, flags, vfslocked;
/* Make sure that the caller is root. */
error = suser(td);
@@ -168,38 +168,38 @@
/*
* If accounting is to be started to a file, open that file for
- * appending and make sure it's a 'normal'. While we could
- * conditionally acquire Giant here, we're actually interacting with
- * vnodes from possibly two file systems, making the logic a bit
- * complicated. For now, use Giant unconditionally.
+ * appending and make sure it's a 'normal'.
*/
- mtx_lock(&Giant);
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);
if (error)
- goto done;
+ return (error);
+ vfslocked = NDHASGIANT(&nd);
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 done;
+ VFS_UNLOCK_GIANT(vfslocked);
+ return (error);
}
#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 done;
+ VFS_UNLOCK_GIANT(vfslocked);
+ return (EACCES);
}
+ VFS_UNLOCK_GIANT(vfslocked);
#ifdef MAC
} else {
error = mac_check_system_acct(td->td_ucred, NULL);
if (error)
- goto done;
+ return (error);
#endif
}
@@ -216,15 +216,18 @@
* enabled.
*/
acct_suspended = 0;
- if (acct_vp != NULL)
+ if (acct_vp != NULL) {
+ vfslocked = VFS_LOCK_GIANT(acct_vp->v_mount);
error = acct_disable(td);
+ VFS_UNLOCK_GIANT(vfslocked);
+ }
if (uap->path == NULL) {
if (acct_state & ACCT_RUNNING) {
acct_state |= ACCT_EXITREQ;
wakeup(&acct_state);
}
sx_xunlock(&acct_sx);
- goto done;
+ return (error);
}
/*
@@ -245,20 +248,20 @@
error = kthread_create(acct_thread, NULL, NULL, 0, 0,
"accounting");
if (error) {
+ vfslocked = VFS_LOCK_GIANT(acct_vp->v_mount);
(void) vn_close(acct_vp, acct_flags, acct_cred, td);
+ VFS_UNLOCK_GIANT(vfslocked);
crfree(acct_cred);
acct_vp = NULL;
acct_cred = NULL;
acct_flags = 0;
sx_xunlock(&acct_sx);
log(LOG_NOTICE, "Unable to start accounting thread\n");
- goto done;
+ return (error);
}
}
sx_xunlock(&acct_sx);
log(LOG_NOTICE, "Accounting enabled\n");
-done:
- mtx_unlock(&Giant);
return (error);
}
==== //depot/projects/smpng/sys/kern/kern_alq.c#10 (text+ko) ====
@@ -172,8 +172,6 @@
int needwakeup;
struct alq *alq;
- mtx_lock(&Giant);
-
ald_thread = FIRST_THREAD_IN_PROC(ald_proc);
EVENTHANDLER_REGISTER(shutdown_pre_sync, ald_shutdown, NULL,
@@ -250,6 +248,7 @@
struct ale *alstart;
int totlen;
int iov;
+ int vfslocked;
vp = alq->aq_vp;
td = curthread;
@@ -291,6 +290,7 @@
/*
* Do all of the junk required to write now.
*/
+ vfslocked = VFS_LOCK_GIANT(vp->v_mount);
vn_start_write(vp, &mp, V_WAIT);
vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, td);
VOP_LEASE(vp, td, alq->aq_cred, LEASE_WRITE);
@@ -303,6 +303,7 @@
VOP_WRITE(vp, &auio, IO_UNIT | IO_APPEND, alq->aq_cred);
VOP_UNLOCK(vp, 0, td);
vn_finished_write(mp);
+ VFS_UNLOCK_GIANT(vfslocked);
ALQ_LOCK(alq);
alq->aq_flags &= ~AQ_FLUSHING;
@@ -345,21 +346,23 @@
char *bufp;
int flags;
int error;
- int i;
+ int i, vfslocked;
*alqp = NULL;
td = curthread;
- NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE, file, td);
+ NDINIT(&nd, LOOKUP, NOFOLLOW | MPSAFE, UIO_SYSSPACE, file, td);
flags = FWRITE | O_NOFOLLOW | O_CREAT;
error = vn_open_cred(&nd, &flags, cmode, cred, -1);
if (error)
return (error);
+ vfslocked = NDHASGIANT(&nd);
NDFREE(&nd, NDF_ONLY_PNBUF);
/* We just unlock so we hold a reference */
VOP_UNLOCK(nd.ni_vp, 0, td);
+ VFS_UNLOCK_GIANT(vfslocked);
alq = malloc(sizeof(*alq), M_ALD, M_WAITOK|M_ZERO);
alq->aq_entbuf = malloc(count * size, M_ALD, M_WAITOK|M_ZERO);
==== //depot/projects/smpng/sys/kern/kern_ktrace.c#50 (text+ko) ====
@@ -598,7 +598,7 @@
int ops = KTROP(uap->ops);
int descend = uap->ops & KTRFLAG_DESCEND;
int nfound, ret = 0;
- int flags, error = 0;
+ int flags, error = 0, vfslocked;
struct nameidata nd;
struct ucred *cred;
@@ -613,25 +613,25 @@
/*
* an operation which requires a file argument.
*/
- NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_USERSPACE, uap->fname, td);
+ NDINIT(&nd, LOOKUP, NOFOLLOW | MPSAFE, UIO_USERSPACE,
+ uap->fname, td);
flags = FREAD | FWRITE | O_NOFOLLOW;
- mtx_lock(&Giant);
error = vn_open(&nd, &flags, 0, -1);
if (error) {
- mtx_unlock(&Giant);
ktrace_exit(td);
return (error);
}
+ vfslocked = NDHASGIANT(&nd);
NDFREE(&nd, NDF_ONLY_PNBUF);
vp = nd.ni_vp;
VOP_UNLOCK(vp, 0, td);
if (vp->v_type != VREG) {
(void) vn_close(vp, FREAD|FWRITE, td->td_ucred, td);
- mtx_unlock(&Giant);
+ VFS_UNLOCK_GIANT(vfslocked);
ktrace_exit(td);
return (EACCES);
}
- mtx_unlock(&Giant);
+ VFS_UNLOCK_GIANT(vfslocked);
}
/*
* Clear all uses of the tracefile.
@@ -649,10 +649,10 @@
p->p_traceflag = 0;
mtx_unlock(&ktrace_mtx);
PROC_UNLOCK(p);
- mtx_lock(&Giant);
+ vfslocked = VFS_LOCK_GIANT(vp->v_mount);
(void) vn_close(vp, FREAD|FWRITE,
cred, td);
- mtx_unlock(&Giant);
+ VFS_UNLOCK_GIANT(vfslocked);
crfree(cred);
} else {
PROC_UNLOCK(p);
@@ -732,9 +732,9 @@
error = EPERM;
done:
if (vp != NULL) {
- mtx_lock(&Giant);
+ vfslocked = VFS_LOCK_GIANT(vp->v_mount);
(void) vn_close(vp, FWRITE, td->td_ucred, td);
- mtx_unlock(&Giant);
+ VFS_UNLOCK_GIANT(vfslocked);
}
ktrace_exit(td);
return (error);
@@ -888,7 +888,7 @@
struct iovec aiov[3];
struct mount *mp;
int datalen, buflen, vrele_count;
- int error;
+ int error, vfslocked;
/*
* We hold the vnode and credential for use in I/O in case ktrace is
@@ -944,7 +944,7 @@
auio.uio_iovcnt++;
}
- mtx_lock(&Giant);
+ vfslocked = VFS_LOCK_GIANT(vp->v_mount);
vn_start_write(vp, &mp, V_WAIT);
vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, td);
(void)VOP_LEASE(vp, td, cred, LEASE_WRITE);
@@ -956,7 +956,7 @@
VOP_UNLOCK(vp, 0, td);
vn_finished_write(mp);
vrele(vp);
- mtx_unlock(&Giant);
+ VFS_UNLOCK_GIANT(vfslocked);
if (!error)
return;
/*
@@ -1000,10 +1000,10 @@
* them but not yet committed them, as those are per-thread. The
* thread will have to clear it itself on system call return.
*/
- mtx_lock(&Giant);
+ vfslocked = VFS_LOCK_GIANT(vp->v_mount);
while (vrele_count-- > 0)
vrele(vp);
- mtx_unlock(&Giant);
+ VFS_UNLOCK_GIANT(vfslocked);
}
/*
More information about the p4-projects
mailing list