Author: gleb
Date: Wed Mar 14 09:15:50 2012
New Revision: 232960
URL: http://svn.freebsd.org/changeset/base/232960
Log:
Prevent tmpfs_rename() deadlock in a way similar to UFS
Unlock vnodes and try to lock them one by one. Relookup fvp and tvp.
Approved by: mdf (mentor)
Modified:
head/sys/fs/tmpfs/tmpfs_subr.c
head/sys/fs/tmpfs/tmpfs_vnops.c
Modified: head/sys/fs/tmpfs/tmpfs_subr.c
==============================================================================
--- head/sys/fs/tmpfs/tmpfs_subr.c Wed Mar 14 08:29:21 2012
(r232959)
+++ head/sys/fs/tmpfs/tmpfs_subr.c Wed Mar 14 09:15:50 2012
(r232960)
@@ -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>
@@ -56,6 +57,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");
+
/* --------------------------------------------------------------------- */
/*
Modified: head/sys/fs/tmpfs/tmpfs_vnops.c
==============================================================================
--- head/sys/fs/tmpfs/tmpfs_vnops.c Wed Mar 14 08:29:21 2012
(r232959)
+++ head/sys/fs/tmpfs/tmpfs_vnops.c Wed Mar 14 09:15:50 2012
(r232960)
@@ -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>
@@ -57,6 +58,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
@@ -918,6 +926,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)
{
@@ -927,6 +1068,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;
@@ -942,8 +1084,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 ||
@@ -952,9 +1092,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;
@@ -963,8 +1100,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);
@@ -1158,6 +1324,9 @@ out:
vrele(fdvp);
vrele(fvp);
+ if (mp != NULL)
+ vfs_unbusy(mp);
+
return error;
}
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
