From b7c72ec1ddeda2cf94ea151f974d3f94e3a7d1ed Mon Sep 17 00:00:00 2001 From: Vitaly Fertman Date: Thu, 5 Jun 2014 15:28:36 +0400 Subject: [PATCH] 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 Xyratex-bug-id: MRP-1700 Change-Id: I173e2fe3f87bc88fcc73c1813cfe4298fb515c50 Reviewed-on: http://review.whamcloud.com/10484 Tested-by: Jenkins Tested-by: Maloo Reviewed-by: wangdi Reviewed-by: John L. Hammond Tested-by: John L. Hammond Reviewed-by: Oleg Drokin --- lustre/include/obd_support.h | 1 + lustre/mdt/mdt_reint.c | 89 +++++++++++++++++--------------------------- lustre/tests/sanityn.sh | 38 ++++++++++++++++--- 3 files changed, 69 insertions(+), 59 deletions(-) diff --git a/lustre/include/obd_support.h b/lustre/include/obd_support.h index e99c1f5..3cedca7 100644 --- 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 --- 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 --- 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 -- 1.8.3.1