[LU-8209] glimpse lock request does not engage ELC to drop unneeded locks

• Type: Bug
• Resolution: Fixed
• Priority: Major
• Fix Version/s: Lustre 2.9.0
• Severity: 3
• Assignee: Hongchao Zhang
• Reporter: Oleg Drokin

Description

It appears that with conversion to clio this important bit of funcionality that allows us to balance lock counts on clients was lost.

Vitaly tracked it to this patch:

commit fd908da92ccd9aab4ffc3d2463301831260c0474
Author: huanghua <huanghua>
Date:   Thu Feb 7 08:07:16 2008 +0000

    b=14149
    i=yong.fan
    i=rahul.deshmukh
    i=nikita.danilov
    
    - use req_capsule interface for client.
    - add some interoperability support on server side.

The removal seems to be accidental.
Where before all enqueues came via a single path and ELC was alwas engaged, this is no longer the case.
The patch was cli_enqueue() -> ldlm_prep_enqueue_req()
and this call to ldlm_prep_enqueue_req() is now missing for glimpse enqueue.

Issue Links

• is related to: LU-6529 Server side lock limits to avoid unnecessary memory exhaustion

Comments

[Peter Jones]

Landed for 2.9

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch http://review.whamcloud.com/21739/
Subject: LU-8209 ldlm: engage ELC for all ldlm enqueue req
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: e8f5f496ed41f237c4fb44cc7355f1ded4c33ac6

[Gerrit Updater]

Hongchao Zhang ([e-mail]) uploaded a new patch: http://review.whamcloud.com/21739
Subject: LU-8209 ldlm: engage ELC for all ldlm enqueue req
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: ab97c207d6310426d6872d2fa234ad4ed5dfeebb

[Vitaly Fertman]

err... the opposite. the glimpse works fine. others are not, because of:

@@ -646,7 +650,10 @@ int ldlm_cli_enqueue(struct obd_export *exp, struct ptlrpc_request **reqp,
         /* lock not sent to server yet */
 
         if (reqp == NULL || *reqp == NULL) {
-                req = ldlm_prep_enqueue_req(exp, 2, size, NULL, 0);
+                req = ptlrpc_request_alloc_pack(class_exp2cliimp(exp),
+                                                &RQF_LDLM_ENQUEUE,
+                                                LUSTRE_DLM_VERSION,
+                                                LDLM_ENQUEUE);
                 if (req == NULL) {
                         failed_lock_cleanup(ns, lock, lockh, einfo->ei_mode);
                         LDLM_LOCK_PUT(lock);

[Bruno Faccini (Inactive)]

Well, if I correctly understand, this means that presently in current master, ldlm_cli_enqueue() does not call ldlm_prep_[enqueue,elc]_req() but this is left to upper layer/callers responsibility to track for/submit eligible locks, right?

Based on this now, my understanding is that :
_ Client glimpse enqueue seems to start with cl_glimpse_lock(), calling cl_lock_request() with CEF_ASYNC flag set in cl_lock_descr struct. CEF_MUST flag is also set to ensure lock request will be issued and not be considered as lock-less.
_ cl_lock_request() will first call cl_lock_init()
_ cl_lock_init() will call ::coo_lock_init method() for each layer
_ thus osc_lock_init() will be called and will set LDLM_FL_HAS_INTENT flag due to CEF_ASYNC present (with help from osc_enq2ldlm_flags()), and will also set ols_glimpse bit
_ back in cl_lock _request(), it will then call cl_lock_enqueue()
_ cl_lock_enqueue() will call ::clo_enqueue() method for each layer, and thus osc_lock_enqueue() will be called
_ osc_lock_enqueue() will call osc_enqueue_base(), because ols_glimpse bit and not lock-less
_ osc_enqueue_base() will call ldlm_prep_enqueue_req() because of LDLM_FL_HAS_INTENT is set

What do you think? Did I miss something there ?