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

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

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 unne...

Comments

[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 ?

[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);

[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

[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

[Peter Jones]

Landed for 2.9