From owner-freebsd-current Mon Sep 9 21:19:55 2002 Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.FreeBSD.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id D863D37B400; Mon, 9 Sep 2002 21:19:43 -0700 (PDT) Received: from gw.catspoiler.org (217-ip-163.nccn.net [209.79.217.163]) by mx1.FreeBSD.org (Postfix) with ESMTP id 2B93E43E42; Mon, 9 Sep 2002 21:19:43 -0700 (PDT) (envelope-from dl-freebsd@catspoiler.org) Received: from mousie.catspoiler.org (mousie.catspoiler.org [192.168.101.2]) by gw.catspoiler.org (8.12.5/8.12.5) with ESMTP id g8A4JTwr093971; Mon, 9 Sep 2002 21:19:33 -0700 (PDT) (envelope-from dl-freebsd@catspoiler.org) Message-Id: <200209100419.g8A4JTwr093971@gw.catspoiler.org> Date: Mon, 9 Sep 2002 21:19:29 -0700 (PDT) From: Don Lewis Subject: Re: vnode lock assertion problem in nfs_link() To: rwatson@FreeBSD.org Cc: current@FreeBSD.org, jeff@FreeBSD.org In-Reply-To: MIME-Version: 1.0 Content-Type: TEXT/plain; charset=US-ASCII Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On 9 Sep, Robert Watson wrote: > What I'd actually like to do is lock vp on going in to the VOP. I need to > grab the lock in the link() code anyway to do the MAC check. UFS and > others all immediately lock the vnode on entry anyway... Here's a patch to implement this. It compiles and seems to work OK, but needs review. There are a couple of issues that definitely need a look: I believe the new vn_lock() call in kern_link() should use the LK_RETRY flag. The locking changes in union_link() need a thorough review, though the light testing of that I performed didn't turn up any glaring problems. The VOP_LINK(9) man page needs would also need to be updated. I also missed a change to the block comment before union_link() :-( Index: fs/unionfs/union_vnops.c =================================================================== RCS file: /home/ncvs/src/sys/fs/unionfs/union_vnops.c,v retrieving revision 1.89 diff -u -r1.89 union_vnops.c --- fs/unionfs/union_vnops.c 4 Aug 2002 10:29:31 -0000 1.89 +++ fs/unionfs/union_vnops.c 10 Sep 2002 02:45:34 -0000 @@ -1250,7 +1250,6 @@ struct union_node *tun = VTOUNION(ap->a_vp); if (tun->un_uppervp == NULLVP) { - vn_lock(ap->a_vp, LK_EXCLUSIVE | LK_RETRY, td); #if 0 if (dun->un_uppervp == tun->un_dirvp) { if (dun->un_flags & UN_ULOCK) { @@ -1267,9 +1266,9 @@ dun->un_flags |= UN_ULOCK; } #endif - VOP_UNLOCK(ap->a_vp, 0, td); } vp = tun->un_uppervp; + vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, td); } if (error) @@ -1288,6 +1287,8 @@ VOP_UNLOCK(ap->a_tdvp, 0, td); /* unlock calling node */ error = VOP_LINK(tdvp, vp, cnp); /* call link on upper */ + if (ap->a_tdvp->v_op == ap->a_vp->v_op) + VOP_UNLOCK(vp, 0, td); /* * We have to unlock tdvp prior to relocking our calling node in * order to avoid a deadlock. Index: gnu/ext2fs/ext2_vnops.c =================================================================== RCS file: /home/ncvs/src/sys/gnu/ext2fs/ext2_vnops.c,v retrieving revision 1.68 diff -u -r1.68 ext2_vnops.c --- gnu/ext2fs/ext2_vnops.c 15 Aug 2002 20:55:01 -0000 1.68 +++ gnu/ext2fs/ext2_vnops.c 10 Sep 2002 03:09:33 -0000 @@ -827,7 +827,6 @@ struct vnode *vp = ap->a_vp; struct vnode *tdvp = ap->a_tdvp; struct componentname *cnp = ap->a_cnp; - struct thread *td = cnp->cn_thread; struct inode *ip; int error; @@ -837,19 +836,16 @@ #endif if (tdvp->v_mount != vp->v_mount) { error = EXDEV; - goto out2; - } - if (tdvp != vp && (error = vn_lock(vp, LK_EXCLUSIVE, td))) { - goto out2; + goto out; } ip = VTOI(vp); if ((nlink_t)ip->i_nlink >= LINK_MAX) { error = EMLINK; - goto out1; + goto out; } if (ip->i_flags & (IMMUTABLE | APPEND)) { error = EPERM; - goto out1; + goto out; } ip->i_nlink++; ip->i_flag |= IN_CHANGE; @@ -860,10 +856,7 @@ ip->i_nlink--; ip->i_flag |= IN_CHANGE; } -out1: - if (tdvp != vp) - VOP_UNLOCK(vp, 0, td); -out2: +out: return (error); } Index: kern/vfs_syscalls.c =================================================================== RCS file: /home/ncvs/src/sys/kern/vfs_syscalls.c,v retrieving revision 1.285 diff -u -r1.285 vfs_syscalls.c --- kern/vfs_syscalls.c 1 Sep 2002 20:37:28 -0000 1.285 +++ kern/vfs_syscalls.c 10 Sep 2002 03:00:50 -0000 @@ -1027,10 +1027,13 @@ if (nd.ni_vp != NULL) { vrele(nd.ni_vp); error = EEXIST; - } else { + } else if (nd.ni_dvp == vp || + (error = vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, td)) == 0) { VOP_LEASE(nd.ni_dvp, td, td->td_ucred, LEASE_WRITE); VOP_LEASE(vp, td, td->td_ucred, LEASE_WRITE); error = VOP_LINK(nd.ni_dvp, vp, &nd.ni_cnd); + if (nd.ni_dvp != vp) + VOP_UNLOCK(vp, 0, td); } NDFREE(&nd, NDF_ONLY_PNBUF); vput(nd.ni_dvp); Index: kern/vnode_if.src =================================================================== RCS file: /home/ncvs/src/sys/kern/vnode_if.src,v retrieving revision 1.56 diff -u -r1.56 vnode_if.src --- kern/vnode_if.src 5 Sep 2002 20:56:14 -0000 1.56 +++ kern/vnode_if.src 10 Sep 2002 02:29:56 -0000 @@ -261,7 +261,7 @@ # #% link tdvp L L L -#% link vp U U U +#% link vp L L L # vop_link { IN struct vnode *tdvp; Index: ufs/ufs/ufs_vnops.c =================================================================== RCS file: /home/ncvs/src/sys/ufs/ufs/ufs_vnops.c,v retrieving revision 1.204 diff -u -r1.204 ufs_vnops.c --- ufs/ufs/ufs_vnops.c 15 Aug 2002 20:55:08 -0000 1.204 +++ ufs/ufs/ufs_vnops.c 10 Sep 2002 03:08:48 -0000 @@ -814,7 +814,6 @@ struct vnode *vp = ap->a_vp; struct vnode *tdvp = ap->a_tdvp; struct componentname *cnp = ap->a_cnp; - struct thread *td = cnp->cn_thread; struct inode *ip; struct direct newdir; int error; @@ -825,19 +824,16 @@ #endif if (tdvp->v_mount != vp->v_mount) { error = EXDEV; - goto out2; - } - if (tdvp != vp && (error = vn_lock(vp, LK_EXCLUSIVE, td))) { - goto out2; + goto out; } ip = VTOI(vp); if ((nlink_t)ip->i_nlink >= LINK_MAX) { error = EMLINK; - goto out1; + goto out; } if (ip->i_flags & (IMMUTABLE | APPEND)) { error = EPERM; - goto out1; + goto out; } ip->i_effnlink++; ip->i_nlink++; @@ -859,10 +855,7 @@ if (DOINGSOFTDEP(vp)) softdep_change_linkcnt(ip); } -out1: - if (tdvp != vp) - VOP_UNLOCK(vp, 0, td); -out2: +out: VN_KNOTE(vp, NOTE_LINK); VN_KNOTE(tdvp, NOTE_WRITE); return (error); To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message