LU-13437 mdt: rename misses remote LOOKUP lock revoke

In rename, all objects but target may be remote, so to check whether
source is remote object on source parent, we need to compare which
MDTs they are located if both are remote. Add a helper function
mdt_rename_source_lock() to handle all possible combinations. If target
parent is remote, take remote LOOKUP for target on where target parent
is.

Add sanityn.sh 81c.

Signed-off-by: Lai Siyao <lai.siyao@whamcloud.com>
Change-Id: I2c134970d6abc8761528d01950b23495292cdf93
Reviewed-on: https://review.whamcloud.com/38181
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Mike Pershin <mpershin@whamcloud.com>
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/mdt/mdt_handler.c b/lustre/mdt/mdt_handler.c
index 7b4bc0f..efdbb07 100644 (file)
--- a/lustre/mdt/mdt_handler.c
+++ b/lustre/mdt/mdt_handler.c
@@ -3418,10 +3418,9 @@ int mdt_remote_object_lock(struct mdt_thread_info *mti, struct mdt_object *o,
                                          cache);
 }
 
                                          cache);
 }
 

-static int mdt_object_local_lock(struct mdt_thread_info *info,
-                                struct mdt_object *o,
-                                struct mdt_lock_handle *lh, __u64 *ibits,
-                                __u64 trybits, bool cos_incompat)
+int mdt_object_local_lock(struct mdt_thread_info *info, struct mdt_object *o,
+                         struct mdt_lock_handle *lh, __u64 *ibits,
+                         __u64 trybits, bool cos_incompat)

 {
        struct ldlm_namespace *ns = info->mti_mdt->mdt_namespace;
        union ldlm_policy_data *policy = &info->mti_policy;
 {
        struct ldlm_namespace *ns = info->mti_mdt->mdt_namespace;
        union ldlm_policy_data *policy = &info->mti_policy;

diff --git a/lustre/mdt/mdt_internal.h b/lustre/mdt/mdt_internal.h
index 5f099bc..ce00097 100644 (file)
--- a/lustre/mdt/mdt_internal.h
+++ b/lustre/mdt/mdt_internal.h
@@ -824,6 +824,9 @@ int mdt_remote_object_lock(struct mdt_thread_info *mti,
                           struct mdt_object *o, const struct lu_fid *fid,
                           struct lustre_handle *lh,
                           enum ldlm_mode mode, __u64 ibits, bool cache);
                           struct mdt_object *o, const struct lu_fid *fid,
                           struct lustre_handle *lh,
                           enum ldlm_mode mode, __u64 ibits, bool cache);

+int mdt_object_local_lock(struct mdt_thread_info *info, struct mdt_object *o,
+                         struct mdt_lock_handle *lh, __u64 *ibits,
+                         __u64 trybits, bool cos_incompat);

 int mdt_reint_striped_lock(struct mdt_thread_info *info,
                           struct mdt_object *o,
                           struct mdt_lock_handle *lh,
 int mdt_reint_striped_lock(struct mdt_thread_info *info,
                           struct mdt_object *o,
                           struct mdt_lock_handle *lh,

diff --git a/lustre/mdt/mdt_reint.c b/lustre/mdt/mdt_reint.c
index 7314162..5d37de3 100644 (file)
--- a/lustre/mdt/mdt_reint.c
+++ b/lustre/mdt/mdt_reint.c
@@ -2449,6 +2449,71 @@ static int mdt_rename_determine_lock_order(struct mdt_thread_info *info,
 }
 
 /*
 }
 
 /*

+ * lock rename source object.
+ *
+ * Both source and source parent may be remote, and source may be a remote
+ * object on source parent, to avoid overriding lock handle, store remote
+ * LOOKUP lock separately in @lhr.
+ *
+ * \retval     0 on success
+ * \retval     -ev negative errno upon error
+ */
+static int mdt_rename_source_lock(struct mdt_thread_info *info,
+                                 struct mdt_object *parent,
+                                 struct mdt_object *child,
+                                 struct mdt_lock_handle *lhc,
+                                 struct mdt_lock_handle *lhr,
+                                 __u64 ibits,
+                                 bool cos_incompat)
+{
+       int rc;
+
+       rc = mdt_is_remote_object(info, parent, child);
+       if (rc < 0)
+               return rc;
+
+       if (rc) {
+               /* enqueue remote LOOKUP lock from the parent MDT */
+               __u64 rmt_ibits = MDS_INODELOCK_LOOKUP;
+
+               if (mdt_object_remote(parent)) {
+                       rc = mdt_remote_object_lock(info, parent,
+                                                   mdt_object_fid(child),
+                                                   &lhr->mlh_rreg_lh,
+                                                   lhr->mlh_rreg_mode,
+                                                   rmt_ibits, false);
+                       if (rc != ELDLM_OK)
+                               return rc;
+               } else {
+                       LASSERT(mdt_object_remote(child));
+                       rc = mdt_object_local_lock(info, child, lhr,
+                                                  &rmt_ibits, 0, true);
+                       if (rc < 0)
+                               return rc;
+               }
+
+               ibits &= ~MDS_INODELOCK_LOOKUP;
+       }
+
+       if (mdt_object_remote(child)) {
+               rc = mdt_remote_object_lock(info, child, mdt_object_fid(child),
+                                           &lhc->mlh_rreg_lh,
+                                           lhc->mlh_rreg_mode,
+                                           ibits, false);
+               if (rc == ELDLM_OK)
+                       rc = 0;
+       } else {
+               rc = mdt_reint_object_lock(info, child, lhc, ibits,
+                                          cos_incompat);
+       }
+
+       if (!rc)
+               mdt_object_unlock(info, child, lhr, rc);
+
+       return rc;
+}
+
+/*

  * VBR: rename versions in reply: 0 - srcdir parent; 1 - tgtdir parent;
  * 2 - srcdir child; 3 - tgtdir child.
  * Update on disk version of srcdir child.
  * VBR: rename versions in reply: 0 - srcdir parent; 1 - tgtdir parent;
  * 2 - srcdir child; 3 - tgtdir child.
  * Update on disk version of srcdir child.


@@ -2468,6 +2533,7 @@ static int mdt_reint_rename(struct mdt_thread_info *info,
        struct mdt_lock_handle *lh_srcdirp;
        struct mdt_lock_handle *lh_tgtdirp;
        struct mdt_lock_handle *lh_oldp = NULL;
        struct mdt_lock_handle *lh_srcdirp;
        struct mdt_lock_handle *lh_tgtdirp;
        struct mdt_lock_handle *lh_oldp = NULL;

+       struct mdt_lock_handle *lh_rmt = NULL;

        struct mdt_lock_handle *lh_newp = NULL;
        struct lu_fid *old_fid = &info->mti_tmp_fid1;
        struct lu_fid *new_fid = &info->mti_tmp_fid2;
        struct mdt_lock_handle *lh_newp = NULL;
        struct lu_fid *old_fid = &info->mti_tmp_fid1;
        struct lu_fid *new_fid = &info->mti_tmp_fid2;


@@ -2700,26 +2766,14 @@ relock:
                        GOTO(out_put_new, rc = -EISDIR);
 
                lh_oldp = &info->mti_lh[MDT_LH_OLD];
                        GOTO(out_put_new, rc = -EISDIR);
 
                lh_oldp = &info->mti_lh[MDT_LH_OLD];

+               lh_rmt = &info->mti_lh[MDT_LH_RMT];

                mdt_lock_reg_init(lh_oldp, LCK_EX);
                mdt_lock_reg_init(lh_oldp, LCK_EX);

+               mdt_lock_reg_init(lh_rmt, LCK_EX);

                lock_ibits = MDS_INODELOCK_LOOKUP | MDS_INODELOCK_XATTR;
                lock_ibits = MDS_INODELOCK_LOOKUP | MDS_INODELOCK_XATTR;

-               if (mdt_object_remote(msrcdir)) {
-                       /* Enqueue lookup lock from the parent MDT */
-                       rc = mdt_remote_object_lock(info, msrcdir,
-                                                   mdt_object_fid(mold),
-                                                   &lh_oldp->mlh_rreg_lh,
-                                                   lh_oldp->mlh_rreg_mode,
-                                                   MDS_INODELOCK_LOOKUP,
-                                                   false);
-                       if (rc != ELDLM_OK)
-                               GOTO(out_put_new, rc);
-
-                       lock_ibits &= ~MDS_INODELOCK_LOOKUP;
-               }
-
-               rc = mdt_reint_object_lock(info, mold, lh_oldp, lock_ibits,
-                                          cos_incompat);
-               if (rc != 0)
-                       GOTO(out_unlock_old, rc);
+               rc = mdt_rename_source_lock(info, msrcdir, mold, lh_oldp,
+                                           lh_rmt, lock_ibits, cos_incompat);
+               if (rc < 0)
+                       GOTO(out_put_new, rc);

 
                /* Check if @msrcdir is subdir of @mnew, before locking child
                 * to avoid reverse locking.
 
                /* Check if @msrcdir is subdir of @mnew, before locking child
                 * to avoid reverse locking.


@@ -2742,39 +2796,38 @@ relock:
 
                lh_newp = &info->mti_lh[MDT_LH_NEW];
                mdt_lock_reg_init(lh_newp, LCK_EX);
 
                lh_newp = &info->mti_lh[MDT_LH_NEW];
                mdt_lock_reg_init(lh_newp, LCK_EX);

-               rc = mdt_reint_object_lock(info, mnew, lh_newp,
-                                          MDS_INODELOCK_LOOKUP |
-                                          MDS_INODELOCK_UPDATE,
+               lock_ibits = MDS_INODELOCK_LOOKUP | MDS_INODELOCK_UPDATE;
+               if (mdt_object_remote(mtgtdir)) {
+                       rc = mdt_remote_object_lock(info, mtgtdir,
+                                                   mdt_object_fid(mnew),
+                                                   &lh_newp->mlh_rreg_lh,
+                                                   lh_newp->mlh_rreg_mode,
+                                                   MDS_INODELOCK_LOOKUP,
+                                                   false);
+                       if (rc != ELDLM_OK)
+                               GOTO(out_unlock_old, rc);
+
+                       lock_ibits &= ~MDS_INODELOCK_LOOKUP;
+               }
+               rc = mdt_reint_object_lock(info, mnew, lh_newp, lock_ibits,

                                           cos_incompat);
                if (rc != 0)
                                           cos_incompat);
                if (rc != 0)

-                       GOTO(out_unlock_old, rc);
+                       GOTO(out_unlock_new, rc);

 
                /* get and save version after locking */
                mdt_version_get_save(info, mnew, 3);
 
                /* get and save version after locking */
                mdt_version_get_save(info, mnew, 3);

-       } else if (rc != -EREMOTE && rc != -ENOENT) {
+       } else if (rc != -ENOENT) {

                GOTO(out_put_old, rc);
        } else {
                lh_oldp = &info->mti_lh[MDT_LH_OLD];
                GOTO(out_put_old, rc);
        } else {
                lh_oldp = &info->mti_lh[MDT_LH_OLD];

+               lh_rmt = &info->mti_lh[MDT_LH_RMT];

                mdt_lock_reg_init(lh_oldp, LCK_EX);
                mdt_lock_reg_init(lh_oldp, LCK_EX);

+               mdt_lock_reg_init(lh_rmt, LCK_EX);

                lock_ibits = MDS_INODELOCK_LOOKUP | MDS_INODELOCK_XATTR;
                lock_ibits = MDS_INODELOCK_LOOKUP | MDS_INODELOCK_XATTR;

-               if (mdt_object_remote(msrcdir)) {
-                       /* Enqueue lookup lock from the parent MDT */
-                       rc = mdt_remote_object_lock(info, msrcdir,
-                                                   mdt_object_fid(mold),
-                                                   &lh_oldp->mlh_rreg_lh,
-                                                   lh_oldp->mlh_rreg_mode,
-                                                   MDS_INODELOCK_LOOKUP,
-                                                   false);
-                       if (rc != ELDLM_OK)
-                               GOTO(out_put_old, rc);
-
-                       lock_ibits &= ~MDS_INODELOCK_LOOKUP;
-               }
-
-               rc = mdt_reint_object_lock(info, mold, lh_oldp, lock_ibits,
-                                          cos_incompat);
+               rc = mdt_rename_source_lock(info, msrcdir, mold, lh_oldp,
+                                           lh_rmt, lock_ibits, cos_incompat);

                if (rc != 0)
                if (rc != 0)

-                       GOTO(out_unlock_old, rc);
+                       GOTO(out_put_old, rc);

 
                mdt_enoent_version_save(info, 3);
        }
 
                mdt_enoent_version_save(info, 3);
        }


@@ -2810,9 +2863,11 @@ relock:
        }
 
        EXIT;
        }
 
        EXIT;

+out_unlock_new:

        if (mnew != NULL)
                mdt_object_unlock(info, mnew, lh_newp, rc);
 out_unlock_old:
        if (mnew != NULL)
                mdt_object_unlock(info, mnew, lh_newp, rc);
 out_unlock_old:

+       mdt_object_unlock(info, NULL, lh_rmt, rc);

        mdt_object_unlock(info, mold, lh_oldp, rc);
 out_put_new:
        if (mnew && !discard)
        mdt_object_unlock(info, mold, lh_oldp, rc);
 out_put_new:
        if (mnew && !discard)

diff --git a/lustre/tests/sanityn.sh b/lustre/tests/sanityn.sh
index 642926e..1af29dd 100755 (executable)
--- a/lustre/tests/sanityn.sh
+++ b/lustre/tests/sanityn.sh
@@ -4494,6 +4494,52 @@ test_81b() {
 }
 run_test 81b "rename under striped directory doesn't deadlock"
 
 }
 run_test 81b "rename under striped directory doesn't deadlock"
 

+test_81c() {
+       [ $MDSCOUNT -lt 4 ] && skip_env "needs >= 4 MDTs"
+       [ $MDS1_VERSION -lt $(version_code 2.13.52) ] &&
+               skip "Need MDS version at least 2.13.52"
+
+       # source is local, source parent is remote
+       $LFS mkdir -i 0 $DIR1/${tdir}_src || error "mkdir ${tdir}_src"
+       $LFS mkdir -i 1 $DIR1/${tdir}_tgt || error "mkdir ${tdir}_tgt"
+       $LFS mkdir -i 3 $DIR1/${tdir}_src/sub || error "mkdir sub"
+       $LFS mkdir -i 3 $DIR1/${tdir}_tgt/sub || error "mkdir sub"
+       stat $DIR2/${tdir}_src/sub || error "stat sub failed"
+       mv $DIR1/${tdir}_src/sub $DIR1/${tdir}_tgt/ || error "mv failed"
+       [ -f $DIR2/${tdir}_src/sub ] && error "sub should be gone"
+       rm -rf $DIR1/${tdir}_src $DIR1/${tdir}_tgt
+
+       # source is remote, source parent is local
+       $LFS mkdir -i 3 $DIR1/${tdir}_src || error "mkdir ${tdir}_src"
+       $LFS mkdir -i 1 $DIR1/${tdir}_tgt || error "mkdir ${tdir}_tgt"
+       $LFS mkdir -i 0 $DIR1/${tdir}_src/sub || error "mkdir sub"
+       $LFS mkdir -i 3 $DIR1/${tdir}_tgt/sub || error "mkdir sub"
+       stat $DIR2/${tdir}_src/sub || error "stat sub failed"
+       mv $DIR1/${tdir}_src/sub $DIR1/${tdir}_tgt/ || error "mv failed"
+       [ -f $DIR2/${tdir}_src/sub ] && error "sub should be gone"
+       rm -rf $DIR1/${tdir}_src $DIR1/${tdir}_tgt
+
+       # source and source parent are remote
+       $LFS mkdir -i 0 $DIR1/${tdir}_src || error "mkdir ${tdir}_src"
+       $LFS mkdir -i 1 $DIR1/${tdir}_tgt || error "mkdir ${tdir}_tgt"
+       mkdir $DIR1/${tdir}_src/sub || error "mkdir sub"
+       $LFS mkdir -i 3 $DIR1/${tdir}_tgt/sub || error "mkdir sub"
+       stat $DIR2/${tdir}_src/sub || error "stat sub failed"
+       mv $DIR1/${tdir}_src/sub $DIR1/${tdir}_tgt/ || error "mv failed"
+       [ -f $DIR2/${tdir}_src/sub ] && error "sub should be gone"
+       rm -rf $DIR1/${tdir}_src $DIR1/${tdir}_tgt
+
+       # source and source parent are remote, and source is remote object
+       $LFS mkdir -i 0 $DIR1/${tdir}_src || error "mkdir ${tdir}_src"
+       $LFS mkdir -i 1 $DIR1/${tdir}_tgt || error "mkdir ${tdir}_tgt"
+       $LFS mkdir -i 2 $DIR1/${tdir}_src/sub || error "mkdir sub"
+       $LFS mkdir -i 3 $DIR1/${tdir}_tgt/sub || error "mkdir sub"
+       stat $DIR2/${tdir}_src/sub || error "stat sub failed"
+       mv $DIR1/${tdir}_src/sub $DIR1/${tdir}_tgt/ || error "mv failed"
+       [ -f $DIR2/${tdir}_src/sub ] && error "sub should be gone" || true
+}
+run_test 81c "rename revoke LOOKUP lock for remote object"
+

 test_82() {
        [[ "$MDS1_VERSION" -gt $(version_code 2.6.91) ]] ||
                skip "Need MDS version at least 2.6.92"
 test_82() {
        [[ "$MDS1_VERSION" -gt $(version_code 2.6.91) ]] ||
                skip "Need MDS version at least 2.6.92"