Date: Tue, 02 Oct 2001 21:52:48 +0100 From: Ian Dowse <iedowse@maths.tcd.ie> To: Matt Dillon <dillon@earth.backplane.com> Cc: Yevgeniy Aleynikov <eugenea@infospace.com>, peter@FreeBSD.ORG, ache@FreeBSD.ORG, mckusick@mckusick.com, Ken Pizzini <kenp@infospace.com>, hackers@FreeBSD.ORG Subject: Re: bleh. Re: ufs_rename panic Message-ID: <200110022152.aa36964@salmon.maths.tcd.ie> In-Reply-To: Your message of "Tue, 02 Oct 2001 12:35:10 PDT." <200110021935.f92JZAm59707@earth.backplane.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi? <200110022152.aa36964>