From 23fa920b0ceef8672d0ee6af9c023f96039c61e3 Mon Sep 17 00:00:00 2001 From: Lai Siyao Date: Wed, 8 Apr 2020 22:55:22 +0800 Subject: [PATCH] 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. Lustre-change: https://review.whamcloud.com/38181 Lustre-commit: 4918fe40db262b19093436caca688c75eb632496 Signed-off-by: Lai Siyao Change-Id: I2c134970d6abc8761528d01950b23495292cdf93 Reviewed-by: Andreas Dilger Reviewed-by: Mike Pershin Signed-off-by: Minh Diep Reviewed-on: https://review.whamcloud.com/39601 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Oleg Drokin --- lustre/mdt/mdt_handler.c | 7 ++- lustre/mdt/mdt_internal.h | 3 ++ lustre/mdt/mdt_reint.c | 135 ++++++++++++++++++++++++++++++++-------------- lustre/tests/sanityn.sh | 46 ++++++++++++++++ 4 files changed, 147 insertions(+), 44 deletions(-) diff --git a/lustre/mdt/mdt_handler.c b/lustre/mdt/mdt_handler.c index cdeb0cd..7afbf3e 100644 --- a/lustre/mdt/mdt_handler.c +++ b/lustre/mdt/mdt_handler.c @@ -3175,10 +3175,9 @@ int mdt_remote_object_lock(struct mdt_thread_info *mti, struct mdt_object *o, 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; diff --git a/lustre/mdt/mdt_internal.h b/lustre/mdt/mdt_internal.h index 91f7da7..1c671f3 100644 --- a/lustre/mdt/mdt_internal.h +++ b/lustre/mdt/mdt_internal.h @@ -766,6 +766,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); +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, diff --git a/lustre/mdt/mdt_reint.c b/lustre/mdt/mdt_reint.c index 80764c0..42657c6 100644 --- a/lustre/mdt/mdt_reint.c +++ b/lustre/mdt/mdt_reint.c @@ -2243,6 +2243,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. @@ -2262,6 +2327,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_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; @@ -2489,26 +2555,14 @@ relock: 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_rmt, LCK_EX); 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. */ @@ -2529,39 +2583,38 @@ relock: 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) - GOTO(out_unlock_old, rc); + GOTO(out_unlock_new, rc); /* 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]; + lh_rmt = &info->mti_lh[MDT_LH_RMT]; mdt_lock_reg_init(lh_oldp, LCK_EX); + mdt_lock_reg_init(lh_rmt, LCK_EX); 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) - GOTO(out_unlock_old, rc); + GOTO(out_put_old, rc); mdt_enoent_version_save(info, 3); } @@ -2595,9 +2648,11 @@ relock: } EXIT; +out_unlock_new: 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) diff --git a/lustre/tests/sanityn.sh b/lustre/tests/sanityn.sh index 6a5a466..b058c8b 100755 --- a/lustre/tests/sanityn.sh +++ b/lustre/tests/sanityn.sh @@ -4183,6 +4183,52 @@ test_81b() { } 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() { [[ $(lustre_version_code $SINGLEMDS) -gt $(version_code 2.6.91) ]] || { skip "Need MDS version at least 2.6.92"; return 0; } -- 1.8.3.1