From 48b38f01c1735eba161fde8a1a591d2b61f67599 Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Sat, 4 Sep 2004 23:12:55 +0000 Subject: [PATCH] Fix a bug in sillyrename handling in nfs_inactive(). The code was improperly ignoring the lock state of the passed vp and recursing nfs_inactive() by calling vrele() from within nfs_inactive(). Since NFS uses real vnode locking now, this resulted in a panic. KDE startup problems reported by: Emiel Kollof --- sys/kern/vfs_subr.c | 33 ++++++++++++++++++++++++++++++--- sys/sys/vnode.h | 3 ++- sys/vfs/nfs/nfs_node.c | 23 +++++++++++------------ 3 files changed, 43 insertions(+), 16 deletions(-) diff --git a/sys/kern/vfs_subr.c b/sys/kern/vfs_subr.c index e363b53d04..9a9272cbfc 100644 --- a/sys/kern/vfs_subr.c +++ b/sys/kern/vfs_subr.c @@ -37,7 +37,7 @@ * * @(#)vfs_subr.c 8.31 (Berkeley) 5/26/95 * $FreeBSD: src/sys/kern/vfs_subr.c,v 1.249.2.30 2003/04/04 20:35:57 tegge Exp $ - * $DragonFly: src/sys/kern/vfs_subr.c,v 1.38 2004/08/28 19:02:05 dillon Exp $ + * $DragonFly: src/sys/kern/vfs_subr.c,v 1.39 2004/09/04 23:12:54 dillon Exp $ */ /* @@ -1681,8 +1681,11 @@ vref(struct vnode *vp) } /* - * Vnode put/release. - * If count drops to zero, call inactive routine and return to freelist. + * Release a usecount on a vnode. This routine does not call unlock on the + * vnode. + * + * If the usecount drops to zero, call the inactive routine and return the + * vnode to the freelist. */ void vrele(struct vnode *vp) @@ -1723,6 +1726,30 @@ vrele(struct vnode *vp) } } +/* + * Release a usecount on a vnode. This routine does not call unlock on the + * vnode. No action is taken if the usecount drops to zero. This routine + * is typically called only from within a *_inactive() procedure to avoid + * recursing the procedure. + */ +void +vrele_noinactive(struct vnode *vp) +{ + struct thread *td = curthread; /* XXX */ + lwkt_tokref vlock; + + KASSERT(vp != NULL && vp->v_usecount >= 0, + ("vrele: null vp or <=0 v_usecount")); + + lwkt_gettoken(&vlock, vp->v_interlock); + vp->v_usecount--; + lwkt_reltoken(&vlock); +} + +/* + * Unlock a vnode and release a usecount on it, inactivating the vnode if + * the count drops to 0. + */ void vput(struct vnode *vp) { diff --git a/sys/sys/vnode.h b/sys/sys/vnode.h index e6b1c54408..af2298518d 100644 --- a/sys/sys/vnode.h +++ b/sys/sys/vnode.h @@ -32,7 +32,7 @@ * * @(#)vnode.h 8.7 (Berkeley) 2/4/94 * $FreeBSD: src/sys/sys/vnode.h,v 1.111.2.19 2002/12/29 18:19:53 dillon Exp $ - * $DragonFly: src/sys/sys/vnode.h,v 1.21 2004/08/28 19:02:07 dillon Exp $ + * $DragonFly: src/sys/sys/vnode.h,v 1.22 2004/09/04 23:12:53 dillon Exp $ */ #ifndef _SYS_VNODE_H_ @@ -579,6 +579,7 @@ int vop_stdgetvobject (struct vop_getvobject_args *ap); void vput (struct vnode *vp); void vrele (struct vnode *vp); +void vrele_noinactive (struct vnode *vp); void vref (struct vnode *vp); extern struct vop_ops *default_vnode_vops; diff --git a/sys/vfs/nfs/nfs_node.c b/sys/vfs/nfs/nfs_node.c index 1496bd8d6b..3682871183 100644 --- a/sys/vfs/nfs/nfs_node.c +++ b/sys/vfs/nfs/nfs_node.c @@ -35,7 +35,7 @@ * * @(#)nfs_node.c 8.6 (Berkeley) 5/22/95 * $FreeBSD: src/sys/nfs/nfs_node.c,v 1.36.2.3 2002/01/05 22:25:04 dillon Exp $ - * $DragonFly: src/sys/vfs/nfs/nfs_node.c,v 1.13 2004/08/28 19:02:20 dillon Exp $ + * $DragonFly: src/sys/vfs/nfs/nfs_node.c,v 1.14 2004/09/04 23:12:55 dillon Exp $ */ @@ -189,6 +189,9 @@ loop: /* * nfs_inactive(struct vnode *a_vp, struct thread *a_td) + * + * NOTE: the passed vnode is locked but not referenced. On return the + * vnode must be unlocked and not referenced. */ int nfs_inactive(struct vop_inactive_args *ap) @@ -208,18 +211,14 @@ nfs_inactive(struct vop_inactive_args *ap) /* * We need a reference to keep the vnode from being * recycled by getnewvnode while we do the I/O - * associated with discarding the buffers unless we - * are being forcibly unmounted in which case we already - * have our own reference. + * associated with discarding the buffers. The vnode + * is already locked. */ - if (ap->a_vp->v_usecount > 0) - (void) nfs_vinvalbuf(ap->a_vp, 0, ap->a_td, 1); - else if (vget(ap->a_vp, NULL, 0, ap->a_td)) - panic("nfs_inactive: lost vnode"); - else { - (void) nfs_vinvalbuf(ap->a_vp, 0, ap->a_td, 1); - vrele(ap->a_vp); - } + vref(ap->a_vp); + nfs_vinvalbuf(ap->a_vp, 0, ap->a_td, 1); + vrele_noinactive(ap->a_vp); + KKASSERT(ap->a_vp->v_usecount == 0); + /* * Remove the silly file that was rename'd earlier */ -- 2.41.0