Whamcloud - gitweb
LU-11623 mdt: return UPDATE|PERM on open 85/33585/30
authorOleg Drokin <green@whamcloud.com>
Tue, 4 Jun 2019 04:41:40 +0000 (00:41 -0400)
committerOleg Drokin <green@whamcloud.com>
Fri, 19 May 2023 07:00:03 +0000 (07:00 +0000)
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 <green@whamcloud.com>
Signed-off-by: Lai Siyao <lai.siyao@whamcloud.com>
Signed-off-by: Qian Yingjiin <qian@ddn.com>
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/33585
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Tested-by: Shuichi Ihara <sihara@ddn.com>
Reviewed-by: Mikhail Pershin <mpershin@whamcloud.com>
Reviewed-by: James Simmons <jsimmons@infradead.org>
lustre/mdt/mdt_open.c
lustre/tests/sanity-lfsck.sh
lustre/tests/sanity-pcc.sh

index ff8a078..a10b223 100644 (file)
@@ -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)
index 1f305f1..ecdffae 100644 (file)
@@ -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"
 
index 8dc3ada..b0a28aa 100644 (file)
@@ -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)"