LU-11269 ptlrpc: request's counter in import

which is separate from imp_refcount as the latter can be
used for other purposes and it's hard to use to track
requests.

to verify the theory that imp_refcount should be checked.

Lustre-change: https://review.whamcloud.com/37722
Lustre-commit: b09afdf57643cbc1c437a42b4babb0837dd19e65

Change-Id: I7c273a73e2b1bb43059c7ed003ee2b7d09273bfe
Signed-off-by: Alex Zhuravlev <bzzz@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Mike Pershin <mpershin@whamcloud.com>
Signed-off-by: Minh Diep <mdiep@whamcloud.com>
Reviewed-on: https://review.whamcloud.com/38340
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/include/lustre_import.h b/lustre/include/lustre_import.h
index 927143b..6b436fd 100644 (file)
--- a/lustre/include/lustre_import.h
+++ b/lustre/include/lustre_import.h
@@ -218,6 +218,8 @@ struct obd_import {
        /** Wait queue for those who need to wait for recovery completion */
        wait_queue_head_t         imp_recovery_waitq;
 
+       /** Number of requests allocated */
+       atomic_t                  imp_reqs;
        /** Number of requests currently in-flight */
        atomic_t                  imp_inflight;
        /** Number of requests currently unregistering */

diff --git a/lustre/obdclass/genops.c b/lustre/obdclass/genops.c
index 6489c23..c057b82 100644 (file)
--- a/lustre/obdclass/genops.c
+++ b/lustre/obdclass/genops.c
@@ -1238,6 +1238,8 @@ static void obd_zombie_import_free(struct obd_import *imp)
         }
 
         LASSERT(imp->imp_sec == NULL);
+       LASSERTF(atomic_read(&imp->imp_reqs) == 0, "%s: imp_reqs = %d\n",
+                imp->imp_obd->obd_name, atomic_read(&imp->imp_reqs));
         class_decref(imp->imp_obd, "import", imp);
        OBD_FREE_PTR(imp);
        EXIT;
@@ -1324,6 +1326,7 @@ struct obd_import *class_new_import(struct obd_device *obd)
 
        atomic_set(&imp->imp_refcount, 2);
        atomic_set(&imp->imp_unregistering, 0);
+       atomic_set(&imp->imp_reqs, 0);
        atomic_set(&imp->imp_inflight, 0);
        atomic_set(&imp->imp_replay_inflight, 0);
        atomic_set(&imp->imp_inval_count, 0);

diff --git a/lustre/ptlrpc/client.c b/lustre/ptlrpc/client.c
index 4385153..03e016e 100644 (file)
--- a/lustre/ptlrpc/client.c
+++ b/lustre/ptlrpc/client.c
@@ -778,6 +778,7 @@ out_ctx:
        LASSERT(!request->rq_pool);
        sptlrpc_cli_ctx_put(request->rq_cli_ctx, 1);
 out_free:
+       atomic_dec(&imp->imp_reqs);
        class_import_put(imp);
 
        return rc;
@@ -846,6 +847,7 @@ struct ptlrpc_request *__ptlrpc_request_alloc(struct obd_import *imp,
                LASSERT(imp->imp_client != LP_POISON);
 
                request->rq_import = class_import_get(imp);
+               atomic_inc(&imp->imp_reqs);
        } else {
                CERROR("request allocation out of memory\n");
        }
@@ -853,6 +855,33 @@ struct ptlrpc_request *__ptlrpc_request_alloc(struct obd_import *imp,
        return request;
 }
 
+static int ptlrpc_reconnect_if_idle(struct obd_import *imp)
+{
+       int rc;
+
+       /*
+        * initiate connection if needed when the import has been
+        * referenced by the new request to avoid races with disconnect.
+        * serialize this check against conditional state=IDLE
+        * in ptlrpc_disconnect_idle_interpret()
+        */
+       spin_lock(&imp->imp_lock);
+       if (imp->imp_state == LUSTRE_IMP_IDLE) {
+               imp->imp_generation++;
+               imp->imp_initiated_at = imp->imp_generation;
+               imp->imp_state = LUSTRE_IMP_NEW;
+
+               /* connect_import_locked releases imp_lock */
+               rc = ptlrpc_connect_import_locked(imp);
+               if (rc)
+                       return rc;
+               ptlrpc_pinger_add_import(imp);
+       } else {
+               spin_unlock(&imp->imp_lock);
+       }
+       return 0;
+}
+
 /**
  * Helper function for creating a request.
  * Calls __ptlrpc_request_alloc to allocate new request sturcture and inits
@@ -870,29 +899,13 @@ ptlrpc_request_alloc_internal(struct obd_import *imp,
        if (request == NULL)
                return NULL;
 
-       /* initiate connection if needed when the import has been
-        * referenced by the new request to avoid races with disconnect */
-       if (unlikely(imp->imp_state == LUSTRE_IMP_IDLE)) {
-               int rc;
-               CDEBUG_LIMIT(imp->imp_idle_debug,
-                            "%s: reconnect after %llds idle\n",
-                            imp->imp_obd->obd_name, ktime_get_real_seconds() -
-                                                    imp->imp_last_reply_time);
-               spin_lock(&imp->imp_lock);
-               if (imp->imp_state == LUSTRE_IMP_IDLE) {
-                       imp->imp_generation++;
-                       imp->imp_initiated_at = imp->imp_generation;
-                       imp->imp_state = LUSTRE_IMP_NEW;
-
-                       /* connect_import_locked releases imp_lock */
-                       rc = ptlrpc_connect_import_locked(imp);
-                       if (rc < 0) {
-                               ptlrpc_request_free(request);
-                               return NULL;
-                       }
-                       ptlrpc_pinger_add_import(imp);
-               } else {
-                       spin_unlock(&imp->imp_lock);
+       /* don't make expensive check for idling connection
+        * if it's already connected */
+       if (unlikely(imp->imp_state != LUSTRE_IMP_FULL)) {
+               if (ptlrpc_reconnect_if_idle(imp) < 0) {
+                       atomic_dec(&imp->imp_reqs);
+                       ptlrpc_request_free(request);
+                       return NULL;
                }
        }
 
@@ -2474,9 +2487,13 @@ static void __ptlrpc_free_req(struct ptlrpc_request *request, int locked)
                 sptlrpc_cli_free_repbuf(request);
 
         if (request->rq_import != NULL) {
-                class_import_put(request->rq_import);
-                request->rq_import = NULL;
-        }
+               if (!ptlrpcd_check_work(request)) {
+                       LASSERT(atomic_read(&request->rq_import->imp_reqs) > 0);
+                       atomic_dec(&request->rq_import->imp_reqs);
+               }
+               class_import_put(request->rq_import);
+               request->rq_import = NULL;
+       }
        if (request->rq_bulk != NULL)
                ptlrpc_free_bulk(request->rq_bulk);
 

diff --git a/lustre/ptlrpc/import.c b/lustre/ptlrpc/import.c
index 0e51a7e..742b587 100644 (file)
--- a/lustre/ptlrpc/import.c
+++ b/lustre/ptlrpc/import.c
@@ -1762,7 +1762,7 @@ static int ptlrpc_disconnect_idle_interpret(const struct lu_env *env,
                memset(&imp->imp_remote_handle, 0,
                       sizeof(imp->imp_remote_handle));
                /* take our DISCONNECT into account */
-               if (atomic_read(&imp->imp_inflight) > 1) {
+               if (atomic_read(&imp->imp_reqs) > 1) {
                        imp->imp_generation++;
                        imp->imp_initiated_at = imp->imp_generation;
                        import_set_state_nolock(imp, LUSTRE_IMP_NEW);

diff --git a/lustre/ptlrpc/pinger.c b/lustre/ptlrpc/pinger.c
index 37cbffc..6cc7f34 100644 (file)
--- a/lustre/ptlrpc/pinger.c
+++ b/lustre/ptlrpc/pinger.c
@@ -100,13 +100,7 @@ static bool ptlrpc_check_import_is_idle(struct obd_import *imp)
        if (!imp->imp_idle_timeout)
                return false;
 
-       /* 4 comes from:
-        *  - client_obd_setup() - hashed import
-        *  - ptlrpcd_alloc_work()
-        *  - ptlrpcd_alloc_work()
-        *  - ptlrpc_pinger_add_import
-        */
-       if (atomic_read(&imp->imp_refcount) > 4)
+       if (atomic_read(&imp->imp_reqs) > 0)
                return false;
 
        /* any lock increases ns_bref being a resource holder */