Add a new VNODE_LOCKDEBUG option, which enables checks in the VOP_*()

calls to ensure that the vnode lock state is as expected when the VOP
call is made.  Modify vnode_if.src to set the expected state according
to the documenting lock table for each VOP.  Modify vnode_if.sh to emit
the checks.

Notes:
- The checks are only performed if the vnode has the VLOCKSWORK bit
  set.  Some file systems (e.g. specfs) don't even bother with vnode
  locks, so of course the checks will fail.
- We can't actually run with VNODE_LOCKDEBUG because there are so many
  vnode locking problems, not the least of which is the "use SHARED for
  VOP_READ()" issue, which screws things up for the entire call chain.

Inspired by similar changes in OpenBSD, but implemented differently.
This commit is contained in:
thorpej 2004-09-21 03:10:35 +00:00
parent 14a961d318
commit 11afd11faa
9 changed files with 121 additions and 63 deletions

View File

@ -1,4 +1,4 @@
# $NetBSD: files,v 1.684 2004/09/14 16:57:31 jdolecek Exp $
# $NetBSD: files,v 1.685 2004/09/21 03:10:35 thorpej Exp $
# @(#)files.newconf 7.5 (Berkeley) 5/10/93
@ -153,6 +153,7 @@ defflag opt_uvm.h USE_TOPDOWN_VM UVMMAP_NOCOUNTERS
defflag SOFTDEP # XXX files.ufs?
defflag QUOTA # XXX files.ufs?
defflag VNODE_OP_NOINLINE
defflag VNODE_LOCKDEBUG
# buffer cache size options
#

View File

@ -1,4 +1,4 @@
/* $NetBSD: vfs_subr.c,v 1.233 2004/09/13 19:45:21 jdolecek Exp $ */
/* $NetBSD: vfs_subr.c,v 1.234 2004/09/21 03:10:35 thorpej Exp $ */
/*-
* Copyright (c) 1997, 1998 The NetBSD Foundation, Inc.
@ -78,7 +78,7 @@
*/
#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: vfs_subr.c,v 1.233 2004/09/13 19:45:21 jdolecek Exp $");
__KERNEL_RCSID(0, "$NetBSD: vfs_subr.c,v 1.234 2004/09/21 03:10:35 thorpej Exp $");
#include "opt_inet.h"
#include "opt_ddb.h"
@ -1742,7 +1742,7 @@ vclean(vp, flags, p)
vp->v_tag = VT_NON;
simple_lock(&vp->v_interlock);
VN_KNOTE(vp, NOTE_REVOKE); /* FreeBSD has this in vn_pollgone() */
vp->v_flag &= ~VXLOCK;
vp->v_flag &= ~(VXLOCK|VLOCKSWORK);
if (vp->v_flag & VXWANT) {
vp->v_flag &= ~VXWANT;
simple_unlock(&vp->v_interlock);

View File

@ -29,7 +29,7 @@ copyright="\
* SUCH DAMAGE.
*/
"
SCRIPT_ID='$NetBSD: vnode_if.sh,v 1.34 2004/01/25 18:02:04 hannken Exp $'
SCRIPT_ID='$NetBSD: vnode_if.sh,v 1.35 2004/09/21 03:10:35 thorpej Exp $'
# Script to produce VFS front-end sugar.
#
@ -103,13 +103,26 @@ awk_parser='
# Middle lines of description
{
argdir[argc] = $1; i=2;
if ($2 == "WILLRELE") {
if ($2 == "LOCKED=YES") {
lockstate[argc] = 1;
i++;
} else if ($2 == "LOCKED=NO") {
lockstate[argc] = 0;
i++;
} else
lockstate[argc] = -1;
if ($2 == "WILLRELE" ||
$3 == "WILLRELE") {
willrele[argc] = 1;
i++;
} else if ($2 == "WILLUNLOCK") {
} else if ($2 == "WILLUNLOCK" ||
$3 == "WILLUNLOCK") {
willrele[argc] = 2;
i++;
} else if ($2 == "WILLPUT") {
} else if ($2 == "WILLPUT" ||
$3 == "WILLPUT") {
willrele[argc] = 3;
i++;
} else
@ -163,7 +176,10 @@ echo '/* LKMs always use non-inlined vnode ops. */'
echo '#define VNODE_OP_NOINLINE'
echo '#else'
echo '#include "opt_vnode_op_noinline.h"'
echo '#endif'
echo '#endif /* _LKM || LKM */'
echo '#ifdef _KERNEL_OPT'
echo '#include "opt_vnode_lockdebug.h"'
echo '#endif /* _KERNEL_OPT */'
echo '#endif /* _KERNEL */'
echo '
extern const struct vnodeop_desc vop_default_desc;
@ -217,9 +233,24 @@ function doit() {
printf("\t%s %s;\n", argtype[i], argname[i]);
}
printf("{\n\tstruct %s_args a;\n", name);
printf("#ifdef VNODE_LOCKDEBUG\n");
for (i=0; i<argc; i++) {
if (lockstate[i] != -1)
printf("\tint islocked_%s;\n", argname[i]);
}
printf("#endif\n");
printf("\ta.a_desc = VDESC(%s);\n", name);
for (i=0; i<argc; i++) {
printf("\ta.a_%s = %s;\n", argname[i], argname[i]);
if (lockstate[i] != -1) {
printf("#ifdef VNODE_LOCKDEBUG\n");
printf("\tislocked_%s = (%s->v_flag & VLOCKSWORK) ? (VOP_ISLOCKED(%s) == LK_EXCLUSIVE) : %d;\n",
argname[i], argname[i], argname[i], lockstate[i]);
printf("\tif (islocked_%s != %d)\n", argname[i],
lockstate[i]);
printf("\t\tpanic(\"%s: %s: locked %%d, expected %%d\", islocked_%s, %d);\n", name, argname[i], argname[i], lockstate[i]);
printf("#endif\n");
}
}
printf("\treturn (VCALL(%s%s, VOFFSET(%s), &a));\n}\n",
argname[0], arg0special, name);
@ -235,6 +266,7 @@ END {
argc=1;
argtype[0]="struct buf *";
argname[0]="bp";
lockstate[0] = -1;
arg0special="->b_vp";
name="vop_bwrite";
doit();
@ -273,7 +305,8 @@ echo '
#define VNODE_OP_NOINLINE
#else
#include "opt_vnode_op_noinline.h"
#endif'
#endif
#include "opt_vnode_lockdebug.h"'
echo '
#include <sys/param.h>
#include <sys/mount.h>
@ -370,9 +403,24 @@ function doit() {
printf("\t%s %s;\n", argtype[i], argname[i]);
}
printf("{\n\tstruct %s_args a;\n", name);
printf("#ifdef VNODE_LOCKDEBUG\n");
for (i=0; i<argc; i++) {
if (lockstate[i] != -1)
printf("\tint islocked_%s;\n", argname[i]);
}
printf("#endif\n");
printf("\ta.a_desc = VDESC(%s);\n", name);
for (i=0; i<argc; i++) {
printf("\ta.a_%s = %s;\n", argname[i], argname[i]);
if (lockstate[i] != -1) {
printf("#ifdef VNODE_LOCKDEBUG\n");
printf("\tislocked_%s = (%s->v_flag & VLOCKSWORK) ? (VOP_ISLOCKED(%s) == LK_EXCLUSIVE) : %d;\n",
argname[i], argname[i], argname[i], lockstate[i]);
printf("\tif (islocked_%s != %d)\n", argname[i],
lockstate[i]);
printf("\t\tpanic(\"%s: %s: locked %%d, expected %%d\", islocked_%s, %d);\n", name, argname[i], argname[i], lockstate[i]);
printf("#endif\n");
}
}
printf("\treturn (VCALL(%s%s, VOFFSET(%s), &a));\n}\n",
argname[0], arg0special, name);
@ -386,6 +434,7 @@ BEGIN {
argdir[0]="IN";
argtype[0]="struct buf *";
argname[0]="bp";
lockstate[0] = -1;
arg0special="->b_vp";
willrele[0]=0;
name="vop_bwrite";

View File

@ -1,4 +1,4 @@
# $NetBSD: vnode_if.src,v 1.40 2004/09/10 09:37:41 yamt Exp $
# $NetBSD: vnode_if.src,v 1.41 2004/09/21 03:10:35 thorpej Exp $
#
# Copyright (c) 1992, 1993
# The Regents of the University of California. All rights reserved.
@ -105,7 +105,7 @@ vop_lookup {
#! create cnp CREATE, LOCKPARENT
#
vop_create {
IN WILLPUT struct vnode *dvp;
IN LOCKED=YES WILLPUT struct vnode *dvp;
OUT struct vnode **vpp;
IN struct componentname *cnp;
IN struct vattr *vap;
@ -118,7 +118,7 @@ vop_create {
#! mknod cnp CREATE, LOCKPARENT
#
vop_mknod {
IN WILLPUT struct vnode *dvp;
IN LOCKED=YES WILLPUT struct vnode *dvp;
OUT struct vnode **vpp;
IN struct componentname *cnp;
IN struct vattr *vap;
@ -128,7 +128,7 @@ vop_mknod {
#% open vp L L L
#
vop_open {
IN struct vnode *vp;
IN LOCKED=YES struct vnode *vp;
IN int mode;
IN struct ucred *cred;
IN struct proc *p;
@ -138,7 +138,7 @@ vop_open {
#% close vp L L L
#
vop_close {
IN struct vnode *vp;
IN LOCKED=YES struct vnode *vp;
IN int fflag;
IN struct ucred *cred;
IN struct proc *p;
@ -148,7 +148,7 @@ vop_close {
#% access vp L L L
#
vop_access {
IN struct vnode *vp;
IN LOCKED=YES struct vnode *vp;
IN int mode;
IN struct ucred *cred;
IN struct proc *p;
@ -168,7 +168,7 @@ vop_getattr {
#% setattr vp L L L
#
vop_setattr {
IN struct vnode *vp;
IN LOCKED=YES struct vnode *vp;
IN struct vattr *vap;
IN struct ucred *cred;
IN struct proc *p;
@ -178,7 +178,7 @@ vop_setattr {
#% read vp L L L
#
vop_read {
IN struct vnode *vp;
IN LOCKED=YES struct vnode *vp;
INOUT struct uio *uio;
IN int ioflag;
IN struct ucred *cred;
@ -188,7 +188,7 @@ vop_read {
#% write vp L L L
#
vop_write {
IN struct vnode *vp;
IN LOCKED=YES struct vnode *vp;
INOUT struct uio *uio;
IN int ioflag;
IN struct ucred *cred;
@ -198,7 +198,7 @@ vop_write {
#% ioctl vp U U U
#
vop_ioctl {
IN struct vnode *vp;
IN LOCKED=NO struct vnode *vp;
IN u_long command;
IN void *data;
IN int fflag;
@ -210,7 +210,7 @@ vop_ioctl {
#% fcntl vp L L L
#
vop_fcntl {
IN struct vnode *vp;
IN LOCKED=YES struct vnode *vp;
IN u_int command;
IN void *data;
IN int fflag;
@ -222,7 +222,7 @@ vop_fcntl {
#% poll vp U U U
#
vop_poll {
IN struct vnode *vp;
IN LOCKED=NO struct vnode *vp;
IN int events;
IN struct proc *p;
};
@ -231,7 +231,7 @@ vop_poll {
#% kqfilter vp U U U
#
vop_kqfilter {
IN struct vnode *vp;
IN LOCKED=NO struct vnode *vp;
IN struct knote *kn;
};
@ -239,7 +239,7 @@ vop_kqfilter {
#% revoke vp U U U
#
vop_revoke {
IN struct vnode *vp;
IN LOCKED=NO struct vnode *vp;
IN int flags;
};
@ -257,7 +257,7 @@ vop_mmap {
#% fsync vp L L L
#
vop_fsync {
IN struct vnode *vp;
IN LOCKED=YES struct vnode *vp;
IN struct ucred *cred;
IN int flags;
IN off_t offlo;
@ -267,6 +267,7 @@ vop_fsync {
#
# Needs work: Is newoff right? What's it mean?
# XXX Locking prototocl?
#
vop_seek {
IN struct vnode *vp;
@ -282,20 +283,20 @@ vop_seek {
#! remove cnp DELETE, LOCKPARENT | LOCKLEAF
#
vop_remove {
IN WILLPUT struct vnode *dvp;
IN WILLPUT struct vnode *vp;
IN LOCKED=YES WILLPUT struct vnode *dvp;
IN LOCKED=YES WILLPUT struct vnode *vp;
IN struct componentname *cnp;
};
#
#% link vp U U U
#% link tdvp L U U
#% link dvp L U U
#
#! link cnp CREATE, LOCKPARENT
#
vop_link {
IN WILLPUT struct vnode *dvp;
IN struct vnode *vp;
IN LOCKED=YES WILLPUT struct vnode *dvp;
IN LOCKED=NO struct vnode *vp;
IN struct componentname *cnp;
};
@ -311,10 +312,10 @@ vop_link {
# XXX the vop_rename routines should REALLY NOT be depending on SAVESTART!
#
vop_rename {
IN WILLRELE struct vnode *fdvp;
IN WILLRELE struct vnode *fvp;
IN LOCKED=NO WILLRELE struct vnode *fdvp;
IN LOCKED=NO WILLRELE struct vnode *fvp;
IN struct componentname *fcnp;
IN WILLPUT struct vnode *tdvp;
IN LOCKED=YES WILLPUT struct vnode *tdvp;
IN WILLPUT struct vnode *tvp;
IN struct componentname *tcnp;
};
@ -326,7 +327,7 @@ vop_rename {
#! mkdir cnp CREATE, LOCKPARENT
#
vop_mkdir {
IN WILLPUT struct vnode *dvp;
IN LOCKED=YES WILLPUT struct vnode *dvp;
OUT struct vnode **vpp;
IN struct componentname *cnp;
IN struct vattr *vap;
@ -339,8 +340,8 @@ vop_mkdir {
#! rmdir cnp DELETE, LOCKPARENT | LOCKLEAF
#
vop_rmdir {
IN WILLPUT struct vnode *dvp;
IN WILLPUT struct vnode *vp;
IN LOCKED=YES WILLPUT struct vnode *dvp;
IN LOCKED=YES WILLPUT struct vnode *vp;
IN struct componentname *cnp;
};
@ -351,7 +352,7 @@ vop_rmdir {
#! symlink cnp CREATE, LOCKPARENT
#
vop_symlink {
IN WILLPUT struct vnode *dvp;
IN LOCKED=YES WILLPUT struct vnode *dvp;
OUT struct vnode **vpp;
IN struct componentname *cnp;
IN struct vattr *vap;
@ -362,7 +363,7 @@ vop_symlink {
#% readdir vp L L L
#
vop_readdir {
IN struct vnode *vp;
IN LOCKED=YES struct vnode *vp;
INOUT struct uio *uio;
IN struct ucred *cred;
OUT int *eofflag;
@ -374,7 +375,7 @@ vop_readdir {
#% readlink vp L L L
#
vop_readlink {
IN struct vnode *vp;
IN LOCKED=YES struct vnode *vp;
INOUT struct uio *uio;
IN struct ucred *cred;
};
@ -393,7 +394,7 @@ vop_abortop {
#% inactive vp L U U
#
vop_inactive {
IN WILLUNLOCK struct vnode *vp;
IN LOCKED=YES WILLUNLOCK struct vnode *vp;
IN struct proc *p;
};
@ -401,7 +402,7 @@ vop_inactive {
#% reclaim vp U U U
#
vop_reclaim {
IN struct vnode *vp;
IN LOCKED=NO struct vnode *vp;
IN struct proc *p;
};
@ -409,7 +410,7 @@ vop_reclaim {
#% lock vp U L U
#
vop_lock {
IN struct vnode *vp;
IN LOCKED=NO struct vnode *vp;
IN int flags;
};
@ -417,7 +418,7 @@ vop_lock {
#% unlock vp L U L
#
vop_unlock {
IN struct vnode *vp;
IN LOCKED=YES struct vnode *vp;
IN int flags;
};
@ -426,7 +427,7 @@ vop_unlock {
#% bmap vpp - U -
#
vop_bmap {
IN struct vnode *vp;
IN LOCKED=YES struct vnode *vp;
IN daddr_t bn;
OUT struct vnode **vpp;
IN daddr_t *bnp;
@ -459,7 +460,7 @@ vop_islocked {
#% pathconf vp L L L
#
vop_pathconf {
IN struct vnode *vp;
IN LOCKED=YES struct vnode *vp;
IN int name;
OUT register_t *retval;
};
@ -468,7 +469,7 @@ vop_pathconf {
#% advlock vp U U U
#
vop_advlock {
IN struct vnode *vp;
IN LOCKED=NO struct vnode *vp;
IN void *id;
IN int op;
IN struct flock *fl;
@ -479,7 +480,7 @@ vop_advlock {
#% blkatoff vp L L L
#
vop_blkatoff {
IN struct vnode *vp;
IN LOCKED=YES struct vnode *vp;
IN off_t offset;
OUT char **res;
OUT struct buf **bpp;
@ -489,7 +490,7 @@ vop_blkatoff {
#% valloc pvp L L L
#
vop_valloc {
IN struct vnode *pvp;
IN LOCKED=YES struct vnode *pvp;
IN int mode;
IN struct ucred *cred;
OUT struct vnode **vpp;
@ -499,7 +500,7 @@ vop_valloc {
#% balloc vp L L L
#
vop_balloc {
IN struct vnode *vp;
IN LOCKED=YES struct vnode *vp;
IN off_t startoffset;
IN int size;
IN struct ucred *cred;
@ -511,7 +512,7 @@ vop_balloc {
#% reallocblks vp L L L
#
vop_reallocblks {
IN struct vnode *vp;
IN LOCKED=YES struct vnode *vp;
IN struct cluster_save *buflist;
};
@ -519,7 +520,7 @@ vop_reallocblks {
#% vfree pvp L L L
#
vop_vfree {
IN struct vnode *pvp;
IN LOCKED=YES struct vnode *pvp;
IN ino_t ino;
IN int mode;
};
@ -528,7 +529,7 @@ vop_vfree {
#% truncate vp L L L
#
vop_truncate {
IN struct vnode *vp;
IN LOCKED=YES struct vnode *vp;
IN off_t length;
IN int flags;
IN struct ucred *cred;
@ -539,7 +540,7 @@ vop_truncate {
#% update vp L L L
#
vop_update {
IN struct vnode *vp;
IN LOCKED=YES struct vnode *vp;
IN struct timespec *access;
IN struct timespec *modify;
IN int flags;
@ -563,7 +564,7 @@ vop_lease {
#! whiteout cnp CREATE, LOCKPARENT
#
vop_whiteout {
IN struct vnode *dvp;
IN LOCKED=YES struct vnode *dvp;
IN struct componentname *cnp;
IN int flags;
};

View File

@ -1,4 +1,4 @@
/* $NetBSD: vnode.h,v 1.125 2004/08/13 23:15:39 mycroft Exp $ */
/* $NetBSD: vnode.h,v 1.126 2004/09/21 03:10:35 thorpej Exp $ */
/*
* Copyright (c) 1989, 1993
@ -144,6 +144,7 @@ struct vnode {
/* VISTTY used when reading dead vnodes */
#define VISTTY 0x0008 /* vnode represents a tty */
#define VEXECMAP 0x0010 /* vnode has PROT_EXEC mappings */
#define VLOCKSWORK 0x0080 /* FS supports locking discipline */
#define VXLOCK 0x0100 /* vnode is locked to change underlying type */
#define VXWANT 0x0200 /* process is waiting for vnode */
#define VBWAIT 0x0400 /* waiting for output to complete */

View File

@ -1,4 +1,4 @@
/* $NetBSD: ext2fs_vfsops.c,v 1.75 2004/08/15 07:19:56 mycroft Exp $ */
/* $NetBSD: ext2fs_vfsops.c,v 1.76 2004/09/21 03:10:35 thorpej Exp $ */
/*
* Copyright (c) 1989, 1991, 1993, 1994
@ -65,7 +65,7 @@
*/
#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: ext2fs_vfsops.c,v 1.75 2004/08/15 07:19:56 mycroft Exp $");
__KERNEL_RCSID(0, "$NetBSD: ext2fs_vfsops.c,v 1.76 2004/09/21 03:10:35 thorpej Exp $");
#if defined(_KERNEL_OPT)
#include "opt_compat_netbsd.h"
@ -930,6 +930,8 @@ ext2fs_vget(mp, ino, vpp)
}
} while (lockmgr(&ufs_hashlock, LK_EXCLUSIVE|LK_SLEEPFAIL, 0));
vp->v_flag |= VLOCKSWORK;
ip = pool_get(&ext2fs_inode_pool, PR_WAITOK);
memset(ip, 0, sizeof(struct inode));
vp->v_data = ip;

View File

@ -1,4 +1,4 @@
/* $NetBSD: ext2fs_vnops.c,v 1.56 2004/09/17 14:11:27 skrll Exp $ */
/* $NetBSD: ext2fs_vnops.c,v 1.57 2004/09/21 03:10:35 thorpej Exp $ */
/*
* Copyright (c) 1982, 1986, 1989, 1993
@ -70,7 +70,7 @@
*/
#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: ext2fs_vnops.c,v 1.56 2004/09/17 14:11:27 skrll Exp $");
__KERNEL_RCSID(0, "$NetBSD: ext2fs_vnops.c,v 1.57 2004/09/21 03:10:35 thorpej Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@ -1343,6 +1343,7 @@ ext2fs_vinit(mntp, specops, fifoops, vpp)
vp->v_data = NULL;
VOP_UNLOCK(vp, 0);
vp->v_op = spec_vnodeop_p;
vp->v_flag &= ~VLOCKSWORK;
vrele(vp);
vgone(vp);
lockmgr(&nvp->v_lock, LK_EXCLUSIVE, &nvp->v_interlock);

View File

@ -1,4 +1,4 @@
/* $NetBSD: ffs_vfsops.c,v 1.154 2004/09/19 11:58:29 yamt Exp $ */
/* $NetBSD: ffs_vfsops.c,v 1.155 2004/09/21 03:10:36 thorpej Exp $ */
/*
* Copyright (c) 1989, 1991, 1993, 1994
@ -32,7 +32,7 @@
*/
#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: ffs_vfsops.c,v 1.154 2004/09/19 11:58:29 yamt Exp $");
__KERNEL_RCSID(0, "$NetBSD: ffs_vfsops.c,v 1.155 2004/09/21 03:10:36 thorpej Exp $");
#if defined(_KERNEL_OPT)
#include "opt_ffs.h"
@ -1414,6 +1414,8 @@ ffs_vget(mp, ino, vpp)
}
} while (lockmgr(&ufs_hashlock, LK_EXCLUSIVE|LK_SLEEPFAIL, 0));
vp->v_flag |= VLOCKSWORK;
/*
* XXX MFS ends up here, too, to allocate an inode. Should we
* XXX create another pool for MFS inodes?

View File

@ -1,4 +1,4 @@
/* $NetBSD: ufs_vnops.c,v 1.122 2004/09/17 14:11:27 skrll Exp $ */
/* $NetBSD: ufs_vnops.c,v 1.123 2004/09/21 03:10:36 thorpej Exp $ */
/*
* Copyright (c) 1982, 1986, 1989, 1993, 1995
@ -37,7 +37,7 @@
*/
#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: ufs_vnops.c,v 1.122 2004/09/17 14:11:27 skrll Exp $");
__KERNEL_RCSID(0, "$NetBSD: ufs_vnops.c,v 1.123 2004/09/21 03:10:36 thorpej Exp $");
#ifndef _LKM
#include "opt_quota.h"
@ -2009,6 +2009,7 @@ ufs_vinit(struct mount *mntp, int (**specops)(void *), int (**fifoops)(void *),
/* XXX spec_vnodeops has no locking, do it explicitly */
VOP_UNLOCK(vp, 0);
vp->v_op = spec_vnodeop_p;
vp->v_flag &= ~VLOCKSWORK;
vrele(vp);
vgone(vp);
lockmgr(&nvp->v_lock, LK_EXCLUSIVE, &nvp->v_interlock);