From 4b44204930cf4d312425dd95e51720fe5d3e801b Mon Sep 17 00:00:00 2001 From: Oleg Drokin Date: Tue, 4 Jun 2019 00:41:40 -0400 Subject: [PATCH] LU-11623 mdt: return UPDATE|PERM on open This patch includes the following changes: * try lock UPDATE|PERM ibits on open to speed up subsequent stat. * open returns PR lock to client by default, because CR mode UPDATE|PERM ibits don't make sense since all modifications take PW lock. * don't lock UPDATE|PERM ibits for PCC attach, otherwise these ibits revoke will cause file detach. * update sanity-pcc 13a to make it fail on single client test if anything went wrong. * update sanity-lfsck 31d because previously CR UPDATE lock is fetched, thus the test pass by mistake. This should help common workloads with open followed by a stat or other such operation. Benchmark results: This patch can significantly improve open-create + stat on the same client. This patch in combination with two others: https://review.whamcloud.com/32157 https://review.whamcloud.com/33584 Improves the 'stat' side of open-create + stat by >10x. Without patches (master branch commit 26a7abe): mpirun -np 24 --allow-run-as-root /work/tools/bin/mdtest -n 50000 -d /cache1/out/ -F -C -T -v -w 32k Operation Max Min Mean --------- --- --- ---- File creation : 3838.205 3838.204 3838.204 File stat : 33459.289 33459.249 33459.271 File read : 0.000 0.000 0.000 File removal : 0.000 0.000 0.000 Tree creation : 3146.841 3146.841 3146.841 Tree removal : 0.000 0.000 0.000 With the three patches: mpirun -np 24 --allow-run-as-root /work/tools/bin/mdtest -n 50000 -d /cache1/out/ -F -C -T -v -w 32k SUMMARY rate: (of 1 iterations) Operation Max Min Mean --------- --- --- ---- File creation : 3822.440 3822.439 3822.440 File stat : 350620.140 350615.980 350617.193 File read : 0.000 0.000 0.000 File removal : 0.000 0.000 0.000 Tree creation : 2076.727 2076.727 2076.727 Tree removal : 0.000 0.000 0.000 Note 33K stats/second vs 350K stats/second. ls -l time of the mdtest directory is also reduced from 23.5 seconds to 5.8 seconds. Change-Id: Ib3410629c190de6f74246a4a92f8216537fa2b95 Signed-off-by: Oleg Drokin Signed-off-by: Lai Siyao Signed-off-by: Qian Yingjiin Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/33585 Tested-by: jenkins Tested-by: Maloo Tested-by: Shuichi Ihara Reviewed-by: Mikhail Pershin Reviewed-by: James Simmons --- lustre/mdt/mdt_open.c | 19 ++++++++++++++++--- lustre/tests/sanity-lfsck.sh | 17 ++++++++++++++++- lustre/tests/sanity-pcc.sh | 2 +- 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/lustre/mdt/mdt_open.c b/lustre/mdt/mdt_open.c index ff8a078..a10b223 100644 --- a/lustre/mdt/mdt_open.c +++ b/lustre/mdt/mdt_open.c @@ -790,7 +790,7 @@ static int mdt_object_open_lock(struct mdt_thread_info *info, struct md_attr *ma = &info->mti_attr; __u64 open_flags = info->mti_spec.sp_cr_flags; __u64 trybits = 0; - enum ldlm_mode lm = LCK_CR; + enum ldlm_mode lm = LCK_PR; bool acq_lease = !!(open_flags & MDS_OPEN_LEASE); bool try_layout = false; bool create_layout = false; @@ -894,13 +894,26 @@ static int mdt_object_open_lock(struct mdt_thread_info *info, /* Return lookup lock to validate inode at the client side. * This is pretty important otherwise MDT will return layout * lock for each open. - * However this is a double-edged sword because changing - * permission will revoke a huge number of LOOKUP locks. + * Since trylocks are opportunistic now and we also have + * ability to selectively cancel lock bits from clients on conflict, + * we would also return other useful bits: PERM and UPDATE - both + * necessary to ensure subsequent stats from the client would not + * need to talk to us again. Historically speaking both permissions + * and other inode data rarely changes after creation anyway, so + * there should not be any downsides from doing it for normal + * operations now. */ if (!OBD_FAIL_CHECK(OBD_FAIL_MDS_NO_LL_OPEN) && try_layout) { if (!(*ibits & MDS_INODELOCK_LOOKUP)) trybits |= MDS_INODELOCK_LOOKUP; trybits |= MDS_INODELOCK_LAYOUT; + /* PERM|UPDATE ibits in CR mode don't make sense, since + * all modifications lock these ibits in PW mode. + * Don't lock UPDATE|PERM ibits upon PCC attach, otherwise + * PCC will be detached when these ibits are revoked. + */ + if (lm != LCK_CR && !(open_flags & MDS_OPEN_PCC)) + trybits |= MDS_INODELOCK_UPDATE | MDS_INODELOCK_PERM; } if (*ibits | trybits) diff --git a/lustre/tests/sanity-lfsck.sh b/lustre/tests/sanity-lfsck.sh index 1f305f1..ecdffae 100644 --- a/lustre/tests/sanity-lfsck.sh +++ b/lustre/tests/sanity-lfsck.sh @@ -5078,6 +5078,8 @@ test_31d() { do_facet $SINGLEMDS $LCTL set_param fail_loc=0x0 umount_client $MOUNT || error "(2) umount failed" + stop mds1 + start mds1 $(mdsdevname 1) $MDS_MOUNT_OPTS mount_client $MOUNT || error "(3) mount failed" touch $DIR/$tdir/striped_dir/dummy || @@ -5103,8 +5105,21 @@ test_31d() { chattr -i $DIR/$tdir/striped_dir || error "(10) Fail to chattr on the broken striped directory" + rm -f $DIR/$tdir/striped_dir/dummy || error "(11) Fail to remove dummy" + + # LFSCK again to regenerate master LMV + echo "Trigger namespace LFSCK to find out the inconsistency" + $START_NAMESPACE -r -A || + error "(12) Fail to start LFSCK for namespace" + + wait_all_targets_blocked namespace completed 6 + + # reload striped_dir to parse newly generated LMV + stop mds1 + start mds1 $(mdsdevname 1) $MDS_MOUNT_OPTS + rmdir $DIR/$tdir/striped_dir || - error "(11) Fail to remove the striped directory after LFSCK" + error "(13) Fail to remove the striped directory after LFSCK" } run_test 31d "Set broken striped directory (modified after broken) as read-only" diff --git a/lustre/tests/sanity-pcc.sh b/lustre/tests/sanity-pcc.sh index 8dc3ada..b0a28aa 100644 --- a/lustre/tests/sanity-pcc.sh +++ b/lustre/tests/sanity-pcc.sh @@ -449,7 +449,7 @@ test_1g() { check_lpcc_state $file "readwrite" do_facet $SINGLEAGT $RUNAS dd if=/dev/zero of=$file bs=1024 count=1 && error "non-root user can dd write to $file" - chmod 777 $file || error "chmod 777 $file failed" + chmod 777 $DIR2/$tfile || error "chmod 777 $DIR2/$tfile failed" do_facet $SINGLEAGT $RUNAS dd if=/dev/zero of=$file bs=1024 count=1 || error "non-root user cannot write $file with permission (777)" -- 1.8.3.1