LU-4725 obd: lu_object_find_at hung

lu_object_find_at hungs if called 2 times and object is removed
in between. It makes mdt_rename_sanity not workable for rename.
Change mdt_rename_sanity to work on existing object.

Signed-off-by: Vitaly Fertman <vitaly_fertman@xyratex.com>
Xyratex-bug-id: MRP-1700
Change-Id: I173e2fe3f87bc88fcc73c1813cfe4298fb515c50
Reviewed-on: http://review.whamcloud.com/10484
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: wangdi <di.wang@intel.com>
Reviewed-by: John L. Hammond <john.hammond@intel.com>
Tested-by: John L. Hammond <john.hammond@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>

diff --git a/lustre/include/obd_support.h b/lustre/include/obd_support.h
index e99c1f5..3cedca7 100644 (file)
--- a/lustre/include/obd_support.h
+++ b/lustre/include/obd_support.h
@@ -244,6 +244,7 @@ int obd_alloc_fail(const void *ptr, const char *name, const char *type,
 #define OBD_FAIL_MDS_RENAME              0x153
 #define OBD_FAIL_MDS_RENAME2             0x154
 #define OBD_FAIL_MDS_RENAME3             0x155
+#define OBD_FAIL_MDS_RENAME4             0x156
 
 /* layout lock */
 #define OBD_FAIL_MDS_NO_LL_GETATTR      0x170

diff --git a/lustre/mdt/mdt_reint.c b/lustre/mdt/mdt_reint.c
index fc4461c..3150e60 100644 (file)
--- a/lustre/mdt/mdt_reint.c
+++ b/lustre/mdt/mdt_reint.c
@@ -1290,52 +1290,36 @@ static void mdt_rename_unlock(struct lustre_handle *lh)
  * target. Source should not be ancestor of target dir. May be other rename
  * checks can be moved here later.
  */
-static int mdt_rename_sanity(struct mdt_thread_info *info,
-                            const struct lu_fid *dir_fid,
-                            const struct lu_fid *fid)
+static int mdt_is_subdir(struct mdt_thread_info *info,
+                        struct mdt_object *dir,
+                        const struct lu_fid *fid)
 {
-        struct mdt_object *dst;
-       struct lu_fid dst_fid = *dir_fid;
+       struct lu_fid dir_fid = dir->mot_header.loh_fid;
         int rc = 0;
         ENTRY;
 
        /* If the source and target are in the same directory, they can not
         * be parent/child relationship, so subdir check is not needed */
-       if (lu_fid_eq(dir_fid, fid))
+       if (lu_fid_eq(&dir_fid, fid))
                return 0;
 
-       do {
-               LASSERT(fid_is_sane(&dst_fid));
-               dst = mdt_object_find(info->mti_env, info->mti_mdt, &dst_fid);
-               if (!IS_ERR(dst)) {
-                       /* XXX: this object might not be protected by LDLM lock
-                        * here, (see mdt_rename_parents_lock), but LOHA_EXISTS
-                        * will not change once it is being set, but LFSCK might
-                        * change this later.(LU-5069) */
-                       if (!mdt_object_exists(dst))
-                               RETURN(-ESTALE);
-
-                       rc = mdo_is_subdir(info->mti_env,
-                                          mdt_object_child(dst), fid,
-                                          &dst_fid);
-                       mdt_object_put(info->mti_env, dst);
-                       if (rc != -EREMOTE && rc < 0) {
-                               CERROR("%s: failed subdir check in "DFID" for "
-                                      DFID": rc = %d\n",
-                                      mdt_obd_name(info->mti_mdt),
-                                      PFID(dir_fid), PFID(fid), rc);
-                               /* Return EINVAL only if a parent is the @fid */
-                               if (rc == -EINVAL)
-                                       rc = -EIO;
-                       } else {
-                               /* check the found fid */
-                               if (lu_fid_eq(&dst_fid, fid))
-                                       rc = -EINVAL;
-                       }
-               } else {
-                       rc = PTR_ERR(dst);
-               }
-       } while (rc == -EREMOTE);
+       if (!mdt_object_exists(dir))
+               RETURN(-ENOENT);
+
+       rc = mdo_is_subdir(info->mti_env, mdt_object_child(dir),
+                          fid, &dir_fid);
+       if (rc < 0) {
+               CERROR("%s: failed subdir check in "DFID" for "DFID
+                      ": rc = %d\n", mdt_obd_name(info->mti_mdt),
+                      PFID(&dir_fid), PFID(fid), rc);
+               /* Return EINVAL only if a parent is the @fid */
+               if (rc == -EINVAL)
+                       rc = -EIO;
+       } else {
+               /* check the found fid */
+               if (lu_fid_eq(&dir_fid, fid))
+                       rc = -EINVAL;
+       }
 
         RETURN(rc);
 }
@@ -1688,18 +1672,6 @@ static int mdt_rename_parents_lock(struct mdt_thread_info *info,
        int                      rc;
        ENTRY;
 
-       /* Check if the @src is not a child of the @tgt, otherwise a
-        * reverse locking must take place.
-        *
-        * Note: cannot be called after object_find, because if the object
-        * is destroyed in between it gets stuck in lu_object_find_at(),
-        * waiting for the last ref. */
-       rc = mdt_rename_sanity(info, fid_src, fid_tgt);
-       if (rc == -EINVAL)
-               reverse = 1;
-       else if (rc)
-               RETURN(rc);
-
        /* find both parents. */
        src = mdt_object_find_check(info, fid_src, 0);
        if (IS_ERR(src))
@@ -1711,6 +1683,14 @@ static int mdt_rename_parents_lock(struct mdt_thread_info *info,
                tgt = src;
                mdt_object_get(info->mti_env, tgt);
        } else {
+               /* Check if the @src is not a child of the @tgt, otherwise a
+                * reverse locking must take place. */
+               rc = mdt_is_subdir(info, src, fid_tgt);
+               if (rc == -EINVAL)
+                       reverse = 1;
+               else if (rc)
+                       GOTO(err_src_put, rc);
+
                tgt = mdt_object_find_check(info, fid_tgt, 1);
                if (IS_ERR(tgt))
                        GOTO(err_src_put, rc = PTR_ERR(tgt));
@@ -1723,6 +1703,8 @@ static int mdt_rename_parents_lock(struct mdt_thread_info *info,
                }
        }
 
+       OBD_FAIL_TIMEOUT(OBD_FAIL_MDS_RENAME4, 5);
+
        /* lock parents in the proper order. */
        if (reverse) {
                rc = mdt_object_lock_save(info, tgt, lh_tgt, 1);
@@ -1750,8 +1732,7 @@ static int mdt_rename_parents_lock(struct mdt_thread_info *info,
        if (rc)
                GOTO(err_unlock, rc);
 
-       if (lu_object_is_dying(&tgt->mot_header))
-               GOTO(err_unlock, rc = -ENOENT);
+       OBD_FAIL_TIMEOUT(OBD_FAIL_MDS_RENAME4, 5);
 
        *srcp = src;
        *tgtp = tgt;
@@ -1838,7 +1819,7 @@ static int mdt_reint_rename_internal(struct mdt_thread_info *info,
 
        /* Check if @mtgtdir is subdir of @mold, before locking child
         * to avoid reverse locking. */
-       rc = mdt_rename_sanity(info, rr->rr_fid2, old_fid);
+       rc = mdt_is_subdir(info, mtgtdir, old_fid);
        if (rc)
                GOTO(out_put_old, rc);
 
@@ -1894,7 +1875,7 @@ static int mdt_reint_rename_internal(struct mdt_thread_info *info,
 
                /* Check if @msrcdir is subdir of @mnew, before locking child
                 * to avoid reverse locking. */
-               rc = mdt_rename_sanity(info, rr->rr_fid1, new_fid);
+               rc = mdt_is_subdir(info, msrcdir, new_fid);
                if (rc)
                        GOTO(out_unlock_old, rc);
 

diff --git a/lustre/tests/sanityn.sh b/lustre/tests/sanityn.sh
index bcf6199..449af52 100644 (file)
--- a/lustre/tests/sanityn.sh
+++ b/lustre/tests/sanityn.sh
@@ -2441,8 +2441,8 @@ run_test 54 "rename locking"
 test_55a() {
        mkdir -p $DIR/d1/d2 $DIR/d3 || error "(1) mkdir failed"
 
-#define OBD_FAIL_MDS_RENAME              0x153
-       do_facet mds $LCTL set_param fail_loc=0x80000153
+#define OBD_FAIL_MDS_RENAME4              0x156
+       do_facet mds $LCTL set_param fail_loc=0x80000156
 
        mv -T $DIR/d1/d2 $DIR/d3/d2 &
        PID1=$!
@@ -2459,8 +2459,8 @@ test_55b()
 {
        mkdir -p $DIR/d1/d2 $DIR/d3 || error "(1) mkdir failed"
 
-#define OBD_FAIL_MDS_RENAME              0x155
-       do_facet mds $LCTL set_param fail_loc=0x80000155
+#define OBD_FAIL_MDS_RENAME4             0x156
+       do_facet mds $LCTL set_param fail_loc=0x80000156
 
        mv -T $DIR/d1/d2 $DIR/d3/d2 &
        PID1=$!
@@ -2469,10 +2469,38 @@ test_55b()
        rm -r $DIR2/d1
        wait $PID1 && error "(2) mv succeeded"
 
-       rm -rf $DIR/d1
+       rm -rf $DIR/d3
 }
 run_test 55b "rename vs unlink source dir"
 
+test_55c()
+{
+       mkdir -p $DIR/d1/d2 $DIR/d3 || error "(1) mkdir failed"
+
+#define OBD_FAIL_MDS_RENAME4              0x156
+       do_facet mds $LCTL set_param fail_loc=0x156
+
+       mv -T $DIR/d1/d2 $DIR/d3/d2 &
+       PID1=$!
+       sleep 1
+
+       # while rename is sleeping, open and remove d3
+       $MULTIOP $DIR2/d3 D_c &
+       PID2=$!
+       sleep 1
+       rm -rf $DIR2/d3
+       sleep 5
+
+       # while rename is sleeping 2nd time, close d3
+       kill -USR1 $PID2
+       wait $PID2 || error "(3) multiop failed"
+
+       wait $PID1 && error "(2) mv succeeded"
+
+       rm -rf $DIR/d1
+}
+run_test 55c "rename vs unlink orphan target dir"
+
 test_55d()
 {
        touch $DIR/f1