LU-10269 ldlm: fix the issues introduced by try bits

Try to grant optinal try_bits if lock is not blocked
and to be granted immediately otherwise trybits are cleared,
therefore there are no locks with trybits in both granted and
waiting queue.

Also mdt_object_lock_try() is changed to don't return negative for
try lock only case.

Test-Parameters: mdscount=1 mdtcount=1 testlist=racer,racer,racer
Signed-off-by: Mikhal Pershin <mike.pershin@intel.com>
Change-Id: If3f5734355511ad1af71d5996026d04c60f3e8c1
Reviewed-on: https://review.whamcloud.com/30246
Tested-by: Jenkins
Reviewed-by: Jinshan Xiong <jinshan.xiong@intel.com>
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>

diff --git a/lustre/ldlm/ldlm_inodebits.c b/lustre/ldlm/ldlm_inodebits.c
index cdae39e..85c4e31 100644 (file)
--- a/lustre/ldlm/ldlm_inodebits.c
+++ b/lustre/ldlm/ldlm_inodebits.c
@@ -133,8 +133,14 @@ ldlm_inodebits_compat_queue(struct list_head *queue, struct ldlm_lock *req,
                                          struct ldlm_lock,
                                          l_sl_policy)->l_res_link;
 
-                       /* drop try_bits used by other locks */
-                       *try_bits &= ~(lock->l_policy_data.l_inodebits.bits);
+                       /* no locks with trybits in either queues */
+                       LASSERT(lock->l_policy_data.l_inodebits.try_bits == 0);
+
+                       /* New lock's try_bits are filtered out by ibits
+                        * of all locks in both granted and waiting queues.
+                        */
+                       *try_bits &= ~lock->l_policy_data.l_inodebits.bits;
+
                        if ((req_bits | *try_bits) == 0)
                                RETURN(0);
 
@@ -148,6 +154,13 @@ ldlm_inodebits_compat_queue(struct list_head *queue, struct ldlm_lock *req,
                                    ldlm_is_cos_enabled(req) &&
                                    lock->l_client_cookie == req->l_client_cookie)
                                        goto not_conflicting;
+
+                               /* The conflicting lock will keep only mandatory
+                                * ibits and zero trybits in waiting queue.
+                                * All actual trybits are cleared.
+                                */
+                               *try_bits = 0;
+
                                /* Found a conflicting policy group. */
                                if (!work_list)
                                        RETURN(0);
@@ -217,6 +230,7 @@ int ldlm_process_inodebits_lock(struct ldlm_lock *lock, __u64 *flags,
                if (lock->l_policy_data.l_inodebits.try_bits != 0) {
                        lock->l_policy_data.l_inodebits.bits |=
                                lock->l_policy_data.l_inodebits.try_bits;
+                       lock->l_policy_data.l_inodebits.try_bits = 0;
                        *flags |= LDLM_FL_LOCK_CHANGED;
                }
                ldlm_resource_unlink_lock(lock);
@@ -249,6 +263,7 @@ restart:
                if (lock->l_policy_data.l_inodebits.try_bits != 0) {
                        lock->l_policy_data.l_inodebits.bits |=
                                lock->l_policy_data.l_inodebits.try_bits;
+                       lock->l_policy_data.l_inodebits.try_bits = 0;
                        *flags |= LDLM_FL_LOCK_CHANGED;
                }
                LASSERT(lock->l_policy_data.l_inodebits.bits);

diff --git a/lustre/mdt/mdt_handler.c b/lustre/mdt/mdt_handler.c
index ffd46af..8534589 100644 (file)
--- a/lustre/mdt/mdt_handler.c
+++ b/lustre/mdt/mdt_handler.c
@@ -2813,9 +2813,7 @@ static int mdt_object_local_lock(struct mdt_thread_info *info,
        }
 
        /* Only enqueue LOOKUP lock for remote object */
-       if (mdt_object_remote(o)) {
-               LASSERT(*ibits == MDS_INODELOCK_LOOKUP);
-       }
+       LASSERT(ergo(mdt_object_remote(o), *ibits == MDS_INODELOCK_LOOKUP));
 
        if (lh->mlh_type == MDT_PDO_LOCK) {
                 /* check for exists after object is locked */
@@ -2973,8 +2971,17 @@ int mdt_object_lock_try(struct mdt_thread_info *info, struct mdt_object *o,
                        struct mdt_lock_handle *lh, __u64 *ibits,
                        __u64 trybits, bool cos_incompat)
 {
-       return mdt_object_lock_internal(info, o, lh, ibits, trybits,
-                                       cos_incompat);
+       bool trylock_only = *ibits == 0;
+       int rc;
+
+       LASSERT(!(*ibits & trybits));
+       rc = mdt_object_lock_internal(info, o, lh, ibits, trybits,
+                                     cos_incompat);
+       if (rc && trylock_only) { /* clear error for try ibits lock only */
+               LASSERT(*ibits == 0);
+               rc = 0;
+       }
+       return rc;
 }
 
 /**

diff --git a/lustre/mdt/mdt_open.c b/lustre/mdt/mdt_open.c
index 3b684d4..4454500 100644 (file)
--- a/lustre/mdt/mdt_open.c
+++ b/lustre/mdt/mdt_open.c
@@ -873,7 +873,6 @@ static int mdt_object_open_lock(struct mdt_thread_info *info,
                } else if (dom_lock) {
                        lm = (open_flags & FMODE_WRITE) ? LCK_PW : LCK_PR;
                        *ibits = MDS_INODELOCK_DOM;
-                       try_layout = false;
                }
 
                CDEBUG(D_INODE, "normal open:"DFID" lease count: %d, lm: %d\n",
@@ -886,18 +885,11 @@ static int mdt_object_open_lock(struct mdt_thread_info *info,
        /* one problem to return layout lock on open is that it may result
         * in too many layout locks cached on the client side. */
        if (!OBD_FAIL_CHECK(OBD_FAIL_MDS_NO_LL_OPEN) && try_layout) {
-               /* 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 huge # of LOOKUP locks. */
-               trybits |= MDS_INODELOCK_LAYOUT | MDS_INODELOCK_LOOKUP;
+               trybits |= MDS_INODELOCK_LAYOUT;
        }
 
-       if (trybits != 0)
+       if (*ibits | trybits)
                rc = mdt_object_lock_try(info, obj, lhc, ibits, trybits, false);
-       else if (*ibits != 0)
-               rc = mdt_object_lock(info, obj, lhc, *ibits);
 
        CDEBUG(D_INODE, "%s: Requested bits lock:"DFID ", ibits = %#llx/%#llx"
               ", open_flags = %#llo, try_layout = %d : rc = %d\n",

diff --git a/lustre/mdt/mdt_reint.c b/lustre/mdt/mdt_reint.c
index 248efd1..7a7ca6c 100644 (file)
--- a/lustre/mdt/mdt_reint.c
+++ b/lustre/mdt/mdt_reint.c
@@ -1448,7 +1448,7 @@ again:
                rc = mdt_object_lock_try(info, mdt_pobj, &mll->mll_lh, &ibits,
                                         MDS_INODELOCK_UPDATE, true);
                if (!(ibits & MDS_INODELOCK_UPDATE)) {
-                       mdt_unlock_list(info, lock_list, rc);
+                       mdt_unlock_list(info, lock_list, 0);
 
                        CDEBUG(D_INFO, "%s: busy lock on "DFID" %s retry %d\n",
                               mdt_obd_name(mdt), PFID(&fid), name.ln_name,

diff --git a/lustre/tests/sanityn.sh b/lustre/tests/sanityn.sh
index b3c874d..fee301a 100755 (executable)
--- a/lustre/tests/sanityn.sh
+++ b/lustre/tests/sanityn.sh
@@ -2519,8 +2519,8 @@ test_51d() {
        local br=$(grep -A 10 $tfile /proc/$PID/smaps | awk '/^Rss/{print $2}')
        echo "Before revoking layout lock: $br KB mapped"
 
-       # delete the file will revoke layout lock
-       rm -f $DIR2/$tfile
+       # cancel layout lock manually
+       cancel_lru_locks mdc
 
        # rss after revoking
        local ar=$(grep -A 10 $tfile /proc/$PID/smaps | awk '/^Rss/{print $2}')