From: Alex Zhuravlev Date: Tue, 25 Feb 2020 16:44:18 +0000 (+0300) Subject: LU-11269 ptlrpc: request's counter in import X-Git-Tag: 2.13.54~174 X-Git-Url: https://git.whamcloud.com/?a=commitdiff_plain;h=refs%2Fchanges%2F22%2F37722%2F21;p=fs%2Flustre-release.git 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. Change-Id: I7c273a73e2b1bb43059c7ed003ee2b7d09273bfe Signed-off-by: Alex Zhuravlev Reviewed-on: https://review.whamcloud.com/37722 Tested-by: jenkins Reviewed-by: Andreas Dilger Tested-by: Maloo Reviewed-by: Mike Pershin Reviewed-by: Oleg Drokin --- diff --git a/lustre/include/lustre_import.h b/lustre/include/lustre_import.h index 9d1cf13..dd0fbdc 100644 --- a/lustre/include/lustre_import.h +++ b/lustre/include/lustre_import.h @@ -223,6 +223,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 97672f9..5ee07ed 100644 --- a/lustre/obdclass/genops.c +++ b/lustre/obdclass/genops.c @@ -1181,6 +1181,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; @@ -1267,6 +1269,7 @@ struct obd_import *class_new_import(struct obd_device *obd) refcount_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 c13eb73..6e16826 100644 --- a/lustre/ptlrpc/client.c +++ b/lustre/ptlrpc/client.c @@ -850,6 +850,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; @@ -894,6 +895,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"); } @@ -901,6 +903,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 @@ -918,32 +947,13 @@ ptlrpc_request_alloc_internal(struct obd_import *imp, if (!request) 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; } } @@ -2586,6 +2596,10 @@ static void __ptlrpc_free_req(struct ptlrpc_request *request, int locked) sptlrpc_cli_free_repbuf(request); if (request->rq_import) { + 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; } diff --git a/lustre/ptlrpc/import.c b/lustre/ptlrpc/import.c index 18a0a88..873d2e2 100644 --- a/lustre/ptlrpc/import.c +++ b/lustre/ptlrpc/import.c @@ -1788,7 +1788,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 2a80c45..0650c2e 100644 --- a/lustre/ptlrpc/pinger.c +++ b/lustre/ptlrpc/pinger.c @@ -101,14 +101,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 (refcount_read(&imp->imp_refcount) > 4) + if (atomic_read(&imp->imp_reqs) > 0) return false; /* any lock increases ns_bref being a resource holder */