From aeee2ce6793ace7e617e1df49735d473957dd06e Mon Sep 17 00:00:00 2001 From: Alex Zhuravlev Date: Tue, 25 Feb 2020 19:44:18 +0300 Subject: [PATCH] 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 Reviewed-by: Andreas Dilger Reviewed-by: Mike Pershin Signed-off-by: Minh Diep Reviewed-on: https://review.whamcloud.com/38340 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Oleg Drokin --- lustre/include/lustre_import.h | 2 ++ lustre/obdclass/genops.c | 3 ++ lustre/ptlrpc/client.c | 69 ++++++++++++++++++++++++++---------------- lustre/ptlrpc/import.c | 2 +- lustre/ptlrpc/pinger.c | 8 +---- 5 files changed, 50 insertions(+), 34 deletions(-) diff --git a/lustre/include/lustre_import.h b/lustre/include/lustre_import.h index 927143b..6b436fd 100644 --- 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 --- 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 --- 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 --- 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 --- 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 */ -- 1.8.3.1