From owner-freebsd-hackers Tue Oct 2 13:53: 6 2001 Delivered-To: freebsd-hackers@freebsd.org Received: from salmon.maths.tcd.ie (salmon.maths.tcd.ie [134.226.81.11]) by hub.freebsd.org (Postfix) with SMTP id 0160937B401; Tue, 2 Oct 2001 13:52:51 -0700 (PDT) Received: from walton.maths.tcd.ie by salmon.maths.tcd.ie with SMTP id ; 2 Oct 2001 21:52:49 +0100 (BST) To: Matt Dillon Cc: Yevgeniy Aleynikov , peter@FreeBSD.ORG, ache@FreeBSD.ORG, mckusick@mckusick.com, Ken Pizzini , hackers@FreeBSD.ORG Subject: Re: bleh. Re: ufs_rename panic In-Reply-To: Your message of "Tue, 02 Oct 2001 12:35:10 PDT." <200110021935.f92JZAm59707@earth.backplane.com> Date: Tue, 02 Oct 2001 21:52:48 +0100 From: Ian Dowse Message-ID: <200110022152.aa36964@salmon.maths.tcd.ie> Sender: owner-freebsd-hackers@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG In message <200110021935.f92JZAm59707@earth.backplane.com>, Matt Dillon writes: > What I've done is add a SOFTLOCKLEAF capability to namei(). If set, and > the file/directory exists, namei() will generate an extra VREF() on > the vnode and set the VSOFTLOCK flag in vp->v_flag. If the vnode already > has VSOFTLOCK set, namei() will return EINVAL. I just tried a more direct approach, which is to implement a flag at the vnode layer that is roughly equivalent to UFS's IN_RENAME flag. This keeps the changes local to vfs_syscalls.c except for the addition of a new vnode flag in vnode.h. A patch is below. It doesn't include the changes to remove IN_RENAME etc, but these could be done later anyway. The basic idea is that the rename syscall locks the source node just for long enough to mark it with VRENAME. It then keeps an extra reference on the source node so that it can clear VRENAME before returning. The syscalls unlink(), rmdir() and rename() also check for VRENAME before proceeding with the operation, and act appropriately if it is found set. One case that is not being handled well is where the target of a rename has VRENAME set; the patch just causes rename to return EINVAL, but a better approach would be to unlock everything and try again. I don't know how to deal with the case of vn_lock(fvp, ...) failing at the end of rename() either. Only lightly tested, so expect lots of bugs... Ian Index: sys/vnode.h =================================================================== RCS file: /dump/FreeBSD-CVS/src/sys/sys/vnode.h,v retrieving revision 1.157 diff -u -r1.157 vnode.h --- sys/vnode.h 13 Sep 2001 22:52:42 -0000 1.157 +++ sys/vnode.h 2 Oct 2001 19:06:41 -0000 @@ -163,8 +163,8 @@ #define VXLOCK 0x00100 /* vnode is locked to change underlying type */ #define VXWANT 0x00200 /* thread is waiting for vnode */ #define VBWAIT 0x00400 /* waiting for output to complete */ +#define VRENAME 0x00800 /* rename operation on progress */ #define VNOSYNC 0x01000 /* unlinked, stop syncing */ -/* open for business 0x01000 */ #define VOBJBUF 0x02000 /* Allocate buffers in VM object */ #define VCOPYONWRITE 0x04000 /* vnode is doing copy-on-write */ #define VAGE 0x08000 /* Insert vnode at head of free list */ Index: kern/vfs_syscalls.c =================================================================== RCS file: /dump/FreeBSD-CVS/src/sys/kern/vfs_syscalls.c,v retrieving revision 1.206 diff -u -r1.206 vfs_syscalls.c --- kern/vfs_syscalls.c 22 Sep 2001 03:07:41 -0000 1.206 +++ kern/vfs_syscalls.c 2 Oct 2001 20:29:54 -0000 @@ -1573,6 +1573,9 @@ if (vp->v_flag & VROOT) error = EBUSY; } + /* Claim that the node is already gone if it is being renamed. */ + if (vp->v_flag & VRENAME) + error = ENOENT; if (vn_start_write(nd.ni_dvp, &mp, V_NOWAIT) != 0) { NDFREE(&nd, NDF_ONLY_PNBUF); vrele(vp); @@ -2879,20 +2882,29 @@ struct mount *mp; struct vnode *tvp, *fvp, *tdvp; struct nameidata fromnd, tond; - int error; + int err1, error; bwillwrite(); - NDINIT(&fromnd, DELETE, WANTPARENT | SAVESTART, UIO_USERSPACE, - SCARG(uap, from), td); + NDINIT(&fromnd, DELETE, WANTPARENT | LOCKLEAF | SAVESTART, + UIO_USERSPACE, SCARG(uap, from), td); if ((error = namei(&fromnd)) != 0) return (error); fvp = fromnd.ni_vp; - if ((error = vn_start_write(fvp, &mp, V_WAIT | PCATCH)) != 0) { + if (fvp->v_flag & VRENAME) + /* The node is being renamed; claim it has already gone. */ + error = ENOENT; + if (!error) + error = vn_start_write(fvp, &mp, V_WAIT | PCATCH); + if (error) { NDFREE(&fromnd, NDF_ONLY_PNBUF); vrele(fromnd.ni_dvp); - vrele(fvp); + vput(fvp); + fvp = NULL; goto out1; } + fvp->v_flag |= VRENAME; + vref(fvp); + VOP_UNLOCK(fvp, 0, td); NDINIT(&tond, RENAME, LOCKPARENT | LOCKLEAF | NOCACHE | SAVESTART | NOOBJ, UIO_USERSPACE, SCARG(uap, to), td); if (fromnd.ni_vp->v_type == VDIR) @@ -2929,6 +2941,10 @@ !bcmp(fromnd.ni_cnd.cn_nameptr, tond.ni_cnd.cn_nameptr, fromnd.ni_cnd.cn_namelen)) error = -1; + if (tvp != NULL && (tvp->v_flag & VRENAME)) { + /* XXX, should just unlock everything and retry. */ + error = EINVAL; + } out: if (!error) { VOP_LEASE(tdvp, td, td->td_proc->p_ucred, LEASE_WRITE); @@ -2961,6 +2977,18 @@ ASSERT_VOP_UNLOCKED(tond.ni_dvp, "rename"); ASSERT_VOP_UNLOCKED(tond.ni_vp, "rename"); out1: + if (fvp != NULL) { + /* We set the VRENAME flag and did an extra vref(fvp) above. */ + if ((err1 = vn_lock(fvp, LK_EXCLUSIVE, td)) == 0) { + KASSERT(fvp->v_flag & VRENAME, + ("rename: lost VRENAME")); + fvp->v_flag &= ~VRENAME; + vput(fvp); + } else { + printf("rename: failed to clear VRENAME! (%d)\n", err1); + vrele(fvp); + } + } if (fromnd.ni_startdir) vrele(fromnd.ni_startdir); if (error == -1) @@ -3082,6 +3110,11 @@ */ if (vp->v_flag & VROOT) { error = EBUSY; + goto out; + } + /* Claim that the node is already gone if it is being renamed. */ + if (vp->v_flag & VRENAME) { + error = ENOENT; goto out; } if (vn_start_write(nd.ni_dvp, &mp, V_NOWAIT) != 0) { To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-hackers" in the body of the message