Page MenuHomeFreeBSD
Files F146302323

D44788.id137374.diff
No OneTemporary
Actions

D44788.id137374.diff
View Options

diff --git a/sys/fs/unionfs/union_vnops.c b/sys/fs/unionfs/union_vnops.c
--- a/sys/fs/unionfs/union_vnops.c
+++ b/sys/fs/unionfs/union_vnops.c
@@ -1167,7 +1167,6 @@
struct unionfs_mount *ump;
struct unionfs_node *unp;
int error;
- int needrelookup;
UNIONFS_INTERNAL_DEBUG("unionfs_rename: enter\n");
@@ -1185,7 +1184,6 @@
rfvp = fvp;
rtdvp = tdvp;
rtvp = tvp;
- needrelookup = 0;
/* check for cross device rename */
if (fvp->v_mount != tdvp->v_mount ||
@@ -1201,58 +1199,114 @@
if (fvp == tvp)
goto unionfs_rename_abort;
- /*
- * from/to vnode is unionfs node.
- */
-
- KASSERT_UNIONFS_VNODE(fdvp);
- KASSERT_UNIONFS_VNODE(fvp);
KASSERT_UNIONFS_VNODE(tdvp);
if (tvp != NULLVP)
KASSERT_UNIONFS_VNODE(tvp);
-
+ if (fdvp != tdvp)
+ VI_LOCK(fdvp);
unp = VTOUNIONFS(fdvp);
+ if (unp == NULL) {
+ if (fdvp != tdvp)
+ VI_UNLOCK(fdvp);
+ error = ENOENT;
+ goto unionfs_rename_abort;
+ }
#ifdef UNIONFS_IDBG_RENAME
UNIONFS_INTERNAL_DEBUG("fdvp=%p, ufdvp=%p, lfdvp=%p\n",
fdvp, unp->un_uppervp, unp->un_lowervp);
#endif
if (unp->un_uppervp == NULLVP) {
error = ENODEV;
- goto unionfs_rename_abort;
+ } else {
+ rfdvp = unp->un_uppervp;
+ vref(rfdvp);
}
- rfdvp = unp->un_uppervp;
- vref(rfdvp);
+ if (fdvp != tdvp)
+ VI_UNLOCK(fdvp);
+ if (error != 0)
+ goto unionfs_rename_abort;
+ VI_LOCK(fvp);
unp = VTOUNIONFS(fvp);
+ if (unp == NULL) {
+ VI_UNLOCK(fvp);
+ error = ENOENT;
+ goto unionfs_rename_abort;
+ }
+
#ifdef UNIONFS_IDBG_RENAME
UNIONFS_INTERNAL_DEBUG("fvp=%p, ufvp=%p, lfvp=%p\n",
fvp, unp->un_uppervp, unp->un_lowervp);
#endif
ump = MOUNTTOUNIONFSMOUNT(fvp->v_mount);
+ /*
+ * If we only have a lower vnode, copy the source file to the upper
+ * FS so that the rename operation can be issued against the upper FS.
+ */
if (unp->un_uppervp == NULLVP) {
- switch (fvp->v_type) {
- case VREG:
- if ((error = vn_lock(fvp, LK_EXCLUSIVE)) != 0)
- goto unionfs_rename_abort;
- error = unionfs_copyfile(unp, 1, fcnp->cn_cred, td);
- VOP_UNLOCK(fvp);
- if (error != 0)
- goto unionfs_rename_abort;
- break;
- case VDIR:
- if ((error = vn_lock(fvp, LK_EXCLUSIVE)) != 0)
- goto unionfs_rename_abort;
- error = unionfs_mkshadowdir(ump, rfdvp, unp, fcnp, td);
- VOP_UNLOCK(fvp);
- if (error != 0)
- goto unionfs_rename_abort;
- break;
- default:
- error = ENODEV;
- goto unionfs_rename_abort;
+ bool unlock_fdvp = false, relock_tdvp = false;
+ VI_UNLOCK(fvp);
+ if (tvp != NULLVP)
+ VOP_UNLOCK(tvp);
+ if (fvp->v_type == VREG) {
+ /*
+ * For regular files, unionfs_copyfile() will expect
+ * fdvp's upper parent directory vnode to be unlocked
+ * and will temporarily lock it. If fdvp == tdvp, we
+ * should unlock tdvp to avoid recursion on tdvp's
+ * lock. If fdvp != tdvp, we should also unlock tdvp
+ * to avoid potential deadlock due to holding tdvp's
+ * lock while locking unrelated vnodes associated with
+ * fdvp/fvp.
+ */
+ VOP_UNLOCK(tdvp);
+ relock_tdvp = true;
+ } else if (fvp->v_type == VDIR && tdvp != fdvp) {
+ /*
+ * For directories, unionfs_mkshadowdir() will expect
+ * fdvp's upper parent directory vnode to be locked
+ * and will temporarily unlock it. If fdvp == tdvp,
+ * we can therefore leave tdvp locked. If fdvp !=
+ * tdvp, we should exchange the lock on tdvp for a
+ * lock on fdvp.
+ */
+ VOP_UNLOCK(tdvp);
+ unlock_fdvp = true;
+ relock_tdvp = true;
+ vn_lock(fdvp, LK_EXCLUSIVE | LK_RETRY);
}
-
- needrelookup = 1;
+ vn_lock(fvp, LK_EXCLUSIVE | LK_RETRY);
+ unp = VTOUNIONFS(fvp);
+ if (unp == NULL)
+ error = ENOENT;
+ else if (unp->un_uppervp == NULLVP) {
+ switch (fvp->v_type) {
+ case VREG:
+ error = unionfs_copyfile(unp, 1, fcnp->cn_cred, td);
+ break;
+ case VDIR:
+ error = unionfs_mkshadowdir(ump, rfdvp, unp, fcnp, td);
+ break;
+ default:
+ error = ENODEV;
+ break;
+ }
+ }
+ VOP_UNLOCK(fvp);
+ if (unlock_fdvp)
+ VOP_UNLOCK(fdvp);
+ if (relock_tdvp)
+ vn_lock(tdvp, LK_EXCLUSIVE | LK_RETRY);
+ if (tvp != NULLVP)
+ vn_lock(tvp, LK_EXCLUSIVE | LK_RETRY);
+ /*
+ * Since we've dropped tdvp's lock at some point in the copy
+ * sequence above, force the caller to re-drive the lookup
+ * in case the relationship between tdvp and tvp has changed.
+ */
+ if (error == 0)
+ error = ERELOOKUP;
+ goto unionfs_rename_abort;
}
if (unp->un_lowervp != NULLVP)
@@ -1260,7 +1314,10 @@
rfvp = unp->un_uppervp;
vref(rfvp);
+ VI_UNLOCK(fvp);
+
unp = VTOUNIONFS(tdvp);
+
#ifdef UNIONFS_IDBG_RENAME
UNIONFS_INTERNAL_DEBUG("tdvp=%p, utdvp=%p, ltdvp=%p\n",
tdvp, unp->un_uppervp, unp->un_lowervp);
@@ -1273,11 +1330,12 @@
ltdvp = unp->un_lowervp;
vref(rtdvp);
- if (tdvp == tvp) {
- rtvp = rtdvp;
- vref(rtvp);
- } else if (tvp != NULLVP) {
+ if (tvp != NULLVP) {
unp = VTOUNIONFS(tvp);
+ if (unp == NULL) {
+ error = ENOENT;
+ goto unionfs_rename_abort;
+ }
#ifdef UNIONFS_IDBG_RENAME
UNIONFS_INTERNAL_DEBUG("tvp=%p, utvp=%p, ltvp=%p\n",
tvp, unp->un_uppervp, unp->un_lowervp);
@@ -1298,24 +1356,6 @@
if (rfvp == rtvp)
goto unionfs_rename_abort;
- if (needrelookup != 0) {
- if ((error = vn_lock(fdvp, LK_EXCLUSIVE)) != 0)
- goto unionfs_rename_abort;
- error = unionfs_relookup_for_delete(fdvp, fcnp, td);
- VOP_UNLOCK(fdvp);
- if (error != 0)
- goto unionfs_rename_abort;
-
- /* Lock of tvp is canceled in order to avoid recursive lock. */
- if (tvp != NULLVP && tvp != tdvp)
- VOP_UNLOCK(tvp);
- error = unionfs_relookup_for_rename(tdvp, tcnp, td);
- if (tvp != NULLVP && tvp != tdvp)
- vn_lock(tvp, LK_EXCLUSIVE | LK_RETRY);
- if (error != 0)
- goto unionfs_rename_abort;
- }
-
error = VOP_RENAME(rfdvp, rfvp, fcnp, rtdvp, rtvp, tcnp);
if (error == 0) {

File Metadata

Mime Type
text/plain
Expires
Mon, Mar 2, 1:45 PM (15 h, 13 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
29138787
Default Alt Text
D44788.id137374.diff (5 KB)

Event Timeline