Author: gleb
Date: Tue Apr 3 19:34:00 2012
New Revision: 233851
URL: http://svn.freebsd.org/changeset/base/233851
Log:
MFC r232959 and r232960:
Prevent tmpfs_rename() deadlock in a way similar to UFS. Unlock
vnodes and try to lock them one by one. Relookup fvp and tvp.
Don't enforce LK_RETRY to get existing vnode in tmpfs_alloc_vp().
Doomed vnode is hardly of any use here, besides all callers handle
error case. vfs_hash_get() does the same. Don't mess with vnode
holdcount, vget() takes care of it already.
Approved by: mdf (mentor)
Modified:
stable/9/sys/fs/tmpfs/tmpfs_subr.c
stable/9/sys/fs/tmpfs/tmpfs_vnops.c
Directory Properties:
stable/9/sys/ (props changed)
stable/9/sys/fs/ (props changed)
Modified: stable/9/sys/fs/tmpfs/tmpfs_subr.c
==============================================================================
--- stable/9/sys/fs/tmpfs/tmpfs_subr.c Tue Apr 3 18:38:00 2012
(r233850)
+++ stable/9/sys/fs/tmpfs/tmpfs_subr.c Tue Apr 3 19:34:00 2012
(r233851)
@@ -42,6 +42,7 @@ __FBSDID("$FreeBSD$");
#include <sys/proc.h>
#include <sys/stat.h>
#include <sys/systm.h>
+#include <sys/sysctl.h>
#include <sys/vnode.h>
#include <sys/vmmeter.h>
@@ -55,6 +56,8 @@ __FBSDID("$FreeBSD$");
#include <fs/tmpfs/tmpfs_fifoops.h>
#include <fs/tmpfs/tmpfs_vnops.h>
+SYSCTL_NODE(_vfs, OID_AUTO, tmpfs, CTLFLAG_RW, 0, "tmpfs file system");
+
/* --------------------------------------------------------------------- */
/*
@@ -319,9 +322,11 @@ loop:
MPASS((node->tn_vpstate & TMPFS_VNODE_DOOMED) == 0);
VI_LOCK(vp);
TMPFS_NODE_UNLOCK(node);
- vholdl(vp);
- (void) vget(vp, lkflag | LK_INTERLOCK | LK_RETRY, curthread);
- vdrop(vp);
+ error = vget(vp, lkflag | LK_INTERLOCK, curthread);
+ if (error != 0) {
+ vp = NULL;
+ goto out;
+ }
/*
* Make sure the vnode is still there after
@@ -419,11 +424,13 @@ unlock:
out:
*vpp = vp;
- MPASS(IFF(error == 0, *vpp != NULL && VOP_ISLOCKED(*vpp)));
#ifdef INVARIANTS
- TMPFS_NODE_LOCK(node);
- MPASS(*vpp == node->tn_vnode);
- TMPFS_NODE_UNLOCK(node);
+ if (error == 0) {
+ MPASS(*vpp != NULL && VOP_ISLOCKED(*vpp));
+ TMPFS_NODE_LOCK(node);
+ MPASS(*vpp == node->tn_vnode);
+ TMPFS_NODE_UNLOCK(node);
+ }
#endif
return error;
Modified: stable/9/sys/fs/tmpfs/tmpfs_vnops.c
==============================================================================
--- stable/9/sys/fs/tmpfs/tmpfs_vnops.c Tue Apr 3 18:38:00 2012
(r233850)
+++ stable/9/sys/fs/tmpfs/tmpfs_vnops.c Tue Apr 3 19:34:00 2012
(r233851)
@@ -46,6 +46,7 @@ __FBSDID("$FreeBSD$");
#include <sys/sf_buf.h>
#include <sys/stat.h>
#include <sys/systm.h>
+#include <sys/sysctl.h>
#include <sys/unistd.h>
#include <sys/vnode.h>
@@ -60,6 +61,13 @@ __FBSDID("$FreeBSD$");
#include <fs/tmpfs/tmpfs_vnops.h>
#include <fs/tmpfs/tmpfs.h>
+SYSCTL_DECL(_vfs_tmpfs);
+
+static volatile int tmpfs_rename_restarts;
+SYSCTL_INT(_vfs_tmpfs, OID_AUTO, rename_restarts, CTLFLAG_RD,
+ __DEVOLATILE(int *, &tmpfs_rename_restarts), 0,
+ "Times rename had to restart due to lock contention");
+
/* --------------------------------------------------------------------- */
static int
@@ -920,6 +928,139 @@ out:
/* --------------------------------------------------------------------- */
+/*
+ * We acquire all but fdvp locks using non-blocking acquisitions. If we
+ * fail to acquire any lock in the path we will drop all held locks,
+ * acquire the new lock in a blocking fashion, and then release it and
+ * restart the rename. This acquire/release step ensures that we do not
+ * spin on a lock waiting for release. On error release all vnode locks
+ * and decrement references the way tmpfs_rename() would do.
+ */
+static int
+tmpfs_rename_relock(struct vnode *fdvp, struct vnode **fvpp,
+ struct vnode *tdvp, struct vnode **tvpp,
+ struct componentname *fcnp, struct componentname *tcnp)
+{
+ struct vnode *nvp;
+ struct mount *mp;
+ struct tmpfs_dirent *de;
+ int error, restarts = 0;
+
+ VOP_UNLOCK(tdvp, 0);
+ if (*tvpp != NULL && *tvpp != tdvp)
+ VOP_UNLOCK(*tvpp, 0);
+ mp = fdvp->v_mount;
+
+relock:
+ restarts += 1;
+ error = vn_lock(fdvp, LK_EXCLUSIVE);
+ if (error)
+ goto releout;
+ if (vn_lock(tdvp, LK_EXCLUSIVE | LK_NOWAIT) != 0) {
+ VOP_UNLOCK(fdvp, 0);
+ error = vn_lock(tdvp, LK_EXCLUSIVE);
+ if (error)
+ goto releout;
+ VOP_UNLOCK(tdvp, 0);
+ goto relock;
+ }
+ /*
+ * Re-resolve fvp to be certain it still exists and fetch the
+ * correct vnode.
+ */
+ de = tmpfs_dir_lookup(VP_TO_TMPFS_DIR(fdvp), NULL, fcnp);
+ if (de == NULL) {
+ VOP_UNLOCK(fdvp, 0);
+ VOP_UNLOCK(tdvp, 0);
+ if ((fcnp->cn_flags & ISDOTDOT) != 0 ||
+ (fcnp->cn_namelen == 1 && fcnp->cn_nameptr[0] == '.'))
+ error = EINVAL;
+ else
+ error = ENOENT;
+ goto releout;
+ }
+ error = tmpfs_alloc_vp(mp, de->td_node, LK_EXCLUSIVE | LK_NOWAIT, &nvp);
+ if (error != 0) {
+ VOP_UNLOCK(fdvp, 0);
+ VOP_UNLOCK(tdvp, 0);
+ if (error != EBUSY)
+ goto releout;
+ error = tmpfs_alloc_vp(mp, de->td_node, LK_EXCLUSIVE, &nvp);
+ if (error != 0)
+ goto releout;
+ VOP_UNLOCK(nvp, 0);
+ /*
+ * Concurrent rename race.
+ */
+ if (nvp == tdvp) {
+ vrele(nvp);
+ error = EINVAL;
+ goto releout;
+ }
+ vrele(*fvpp);
+ *fvpp = nvp;
+ goto relock;
+ }
+ vrele(*fvpp);
+ *fvpp = nvp;
+ VOP_UNLOCK(*fvpp, 0);
+ /*
+ * Re-resolve tvp and acquire the vnode lock if present.
+ */
+ de = tmpfs_dir_lookup(VP_TO_TMPFS_DIR(tdvp), NULL, tcnp);
+ /*
+ * If tvp disappeared we just carry on.
+ */
+ if (de == NULL && *tvpp != NULL) {
+ vrele(*tvpp);
+ *tvpp = NULL;
+ }
+ /*
+ * Get the tvp ino if the lookup succeeded. We may have to restart
+ * if the non-blocking acquire fails.
+ */
+ if (de != NULL) {
+ nvp = NULL;
+ error = tmpfs_alloc_vp(mp, de->td_node,
+ LK_EXCLUSIVE | LK_NOWAIT, &nvp);
+ if (*tvpp != NULL)
+ vrele(*tvpp);
+ *tvpp = nvp;
+ if (error != 0) {
+ VOP_UNLOCK(fdvp, 0);
+ VOP_UNLOCK(tdvp, 0);
+ if (error != EBUSY)
+ goto releout;
+ error = tmpfs_alloc_vp(mp, de->td_node, LK_EXCLUSIVE,
+ &nvp);
+ if (error != 0)
+ goto releout;
+ VOP_UNLOCK(nvp, 0);
+ /*
+ * fdvp contains fvp, thus tvp (=fdvp) is not empty.
+ */
+ if (nvp == fdvp) {
+ error = ENOTEMPTY;
+ goto releout;
+ }
+ goto relock;
+ }
+ }
+ tmpfs_rename_restarts += restarts;
+
+ return (0);
+
+releout:
+ vrele(fdvp);
+ vrele(*fvpp);
+ vrele(tdvp);
+ if (*tvpp != NULL)
+ vrele(*tvpp);
+ tmpfs_rename_restarts += restarts;
+
+ return (error);
+}
+
static int
tmpfs_rename(struct vop_rename_args *v)
{
@@ -929,6 +1070,7 @@ tmpfs_rename(struct vop_rename_args *v)
struct vnode *tdvp = v->a_tdvp;
struct vnode *tvp = v->a_tvp;
struct componentname *tcnp = v->a_tcnp;
+ struct mount *mp = NULL;
char *newname;
int error;
@@ -944,8 +1086,6 @@ tmpfs_rename(struct vop_rename_args *v)
MPASS(fcnp->cn_flags & HASBUF);
MPASS(tcnp->cn_flags & HASBUF);
- tnode = (tvp == NULL) ? NULL : VP_TO_TMPFS_NODE(tvp);
-
/* Disallow cross-device renames.
* XXX Why isn't this done by the caller? */
if (fvp->v_mount != tdvp->v_mount ||
@@ -954,9 +1094,6 @@ tmpfs_rename(struct vop_rename_args *v)
goto out;
}
- tmp = VFS_TO_TMPFS(tdvp->v_mount);
- tdnode = VP_TO_TMPFS_DIR(tdvp);
-
/* If source and target are the same file, there is nothing to do. */
if (fvp == tvp) {
error = 0;
@@ -965,8 +1102,37 @@ tmpfs_rename(struct vop_rename_args *v)
/* If we need to move the directory between entries, lock the
* source so that we can safely operate on it. */
- if (fdvp != tdvp && fdvp != tvp)
- vn_lock(fdvp, LK_EXCLUSIVE | LK_RETRY);
+ if (fdvp != tdvp && fdvp != tvp) {
+ if (vn_lock(fdvp, LK_EXCLUSIVE | LK_NOWAIT) != 0) {
+ mp = tdvp->v_mount;
+ error = vfs_busy(mp, 0);
+ if (error != 0) {
+ mp = NULL;
+ goto out;
+ }
+ error = tmpfs_rename_relock(fdvp, &fvp, tdvp, &tvp,
+ fcnp, tcnp);
+ if (error != 0) {
+ vfs_unbusy(mp);
+ return (error);
+ }
+ ASSERT_VOP_ELOCKED(fdvp,
+ "tmpfs_rename: fdvp not locked");
+ ASSERT_VOP_ELOCKED(tdvp,
+ "tmpfs_rename: tdvp not locked");
+ if (tvp != NULL)
+ ASSERT_VOP_ELOCKED(tvp,
+ "tmpfs_rename: tvp not locked");
+ if (fvp == tvp) {
+ error = 0;
+ goto out_locked;
+ }
+ }
+ }
+
+ tmp = VFS_TO_TMPFS(tdvp->v_mount);
+ tdnode = VP_TO_TMPFS_DIR(tdvp);
+ tnode = (tvp == NULL) ? NULL : VP_TO_TMPFS_NODE(tvp);
fdnode = VP_TO_TMPFS_DIR(fdvp);
fnode = VP_TO_TMPFS_NODE(fvp);
de = tmpfs_dir_lookup(fdnode, fnode, fcnp);
@@ -1160,6 +1326,9 @@ out:
vrele(fdvp);
vrele(fvp);
+ if (mp != NULL)
+ vfs_unbusy(mp);
+
return error;
}
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "[email protected]"
