From 0a03e51b7e49a319a288c7c2be367c90e63d045d Mon Sep 17 00:00:00 2001 From: Mr NeilBrown Date: Mon, 6 Jul 2020 08:34:48 -0400 Subject: [PATCH] LU-9679 lustre: make ptlrpc_connection_put() static inline This function needs to be called from the obdclass modules, but is currently defined in a module that depends on that module. The get around this interdependence, a global variable ptlrpc_put_connection_superhack is used to make a pointer to the function available. Rather than this hack, we can make ptlrpc_connection_put() static-inline. This does expose some details of ptlrpc to obdclass, but there is already a fairly tight connection. Also change the return value to 'void' as it is never used, and don't bother checking for NULL before calling, as the function has its own test for NULL. Signed-off-by: Mr NeilBrown Change-Id: I37333b78e410e8c46bddc468c31bed61dd9e7b33 Reviewed-on: https://review.whamcloud.com/39291 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Sebastien Buisson Reviewed-by: Alex Zhuravlev Reviewed-by: James Simmons Reviewed-by: Oleg Drokin --- lustre/include/lustre_net.h | 32 +++++++++++++++++++++++++++++++- lustre/include/obd_class.h | 6 ------ lustre/ldlm/ldlm_lib.c | 3 +-- lustre/obdclass/genops.c | 24 ++++++++++-------------- lustre/ptlrpc/connection.c | 36 ------------------------------------ lustre/ptlrpc/import.c | 6 ++---- lustre/ptlrpc/ptlrpc_module.c | 2 -- 7 files changed, 44 insertions(+), 65 deletions(-) diff --git a/lustre/include/lustre_net.h b/lustre/include/lustre_net.h index 7f5fd29..2e631d2 100644 --- a/lustre/include/lustre_net.h +++ b/lustre/include/lustre_net.h @@ -1974,7 +1974,37 @@ extern void server_bulk_callback(struct lnet_event *ev); struct ptlrpc_connection *ptlrpc_connection_get(struct lnet_process_id peer, lnet_nid_t self, struct obd_uuid *uuid); -int ptlrpc_connection_put(struct ptlrpc_connection *c); + +static inline void ptlrpc_connection_put(struct ptlrpc_connection *conn) +{ + if (!conn) + return; + + LASSERT(atomic_read(&conn->c_refcount) > 0); + + /* + * We do not remove connection from hashtable and + * do not free it even if last caller released ref, + * as we want to have it cached for the case it is + * needed again. + * + * Deallocating it and later creating new connection + * again would be wastful. This way we also avoid + * expensive locking to protect things from get/put + * race when found cached connection is freed by + * ptlrpc_connection_put(). + * + * It will be freed later in module unload time, + * when ptlrpc_connection_fini()->lh_exit->conn_exit() + * path is called. + */ + atomic_dec(&conn->c_refcount); + + CDEBUG(D_INFO, "PUT conn=%p refcount %d to %s\n", + conn, atomic_read(&conn->c_refcount), + libcfs_nid2str(conn->c_peer.nid)); +} + struct ptlrpc_connection *ptlrpc_connection_addref(struct ptlrpc_connection *); int ptlrpc_connection_init(void); void ptlrpc_connection_fini(void); diff --git a/lustre/include/obd_class.h b/lustre/include/obd_class.h index d624c27..6e68c1e 100644 --- a/lustre/include/obd_class.h +++ b/lustre/include/obd_class.h @@ -1823,12 +1823,6 @@ struct lwp_register_item { char lri_name[MTI_NAME_MAXLEN]; }; -/* I'm as embarrassed about this as you are. - * - * // XXX do not look into _superhack with remaining eye - * // XXX if this were any uglier, I'd get my own show on MTV */ -extern int (*ptlrpc_put_connection_superhack)(struct ptlrpc_connection *c); - /* obd_mount.c */ #ifdef HAVE_SERVER_SUPPORT int lustre_register_lwp_item(const char *lwpname, struct obd_export **exp, diff --git a/lustre/ldlm/ldlm_lib.c b/lustre/ldlm/ldlm_lib.c index 62dc27e..4715718 100644 --- a/lustre/ldlm/ldlm_lib.c +++ b/lustre/ldlm/ldlm_lib.c @@ -997,8 +997,7 @@ static int rev_import_reconnect(struct obd_export *exp, /* avoid sending a request until import flags are changed */ ptlrpc_import_enter_resend(revimp); - if (revimp->imp_connection != NULL) - ptlrpc_connection_put(revimp->imp_connection); + ptlrpc_connection_put(revimp->imp_connection); /* * client from recovery don't have a handle so we need to take from diff --git a/lustre/obdclass/genops.c b/lustre/obdclass/genops.c index dfdc237..2223c14 100644 --- a/lustre/obdclass/genops.c +++ b/lustre/obdclass/genops.c @@ -62,9 +62,6 @@ static LIST_HEAD(obd_stale_exports); static DEFINE_SPINLOCK(obd_stale_export_lock); static atomic_t obd_stale_export_num = ATOMIC_INIT(0); -int (*ptlrpc_put_connection_superhack)(struct ptlrpc_connection *c); -EXPORT_SYMBOL(ptlrpc_put_connection_superhack); - /* * support functions: we could use inter-module communication, but this * is more portable to other OS's @@ -947,8 +944,7 @@ static void class_export_destroy(struct obd_export *exp) exp->exp_client_uuid.uuid, obd->obd_name); /* "Local" exports (lctl, LOV->{mdc,osc}) have no connection. */ - if (exp->exp_connection) - ptlrpc_put_connection_superhack(exp->exp_connection); + ptlrpc_connection_put(exp->exp_connection); LASSERT(list_empty(&exp->exp_outstanding_replies)); LASSERT(list_empty(&exp->exp_uncommitted_replies)); @@ -1161,14 +1157,14 @@ EXPORT_SYMBOL(class_unlink_export); /* Import management functions */ static void obd_zombie_import_free(struct obd_import *imp) { - ENTRY; + ENTRY; - CDEBUG(D_IOCTL, "destroying import %p for %s\n", imp, - imp->imp_obd->obd_name); + CDEBUG(D_IOCTL, "destroying import %p for %s\n", imp, + imp->imp_obd->obd_name); LASSERT(refcount_read(&imp->imp_refcount) == 0); - ptlrpc_put_connection_superhack(imp->imp_connection); + ptlrpc_connection_put(imp->imp_connection); while (!list_empty(&imp->imp_conn_list)) { struct obd_import_conn *imp_conn; @@ -1176,14 +1172,14 @@ static void obd_zombie_import_free(struct obd_import *imp) imp_conn = list_first_entry(&imp->imp_conn_list, struct obd_import_conn, oic_item); list_del_init(&imp_conn->oic_item); - ptlrpc_put_connection_superhack(imp_conn->oic_conn); - OBD_FREE(imp_conn, sizeof(*imp_conn)); - } + ptlrpc_connection_put(imp_conn->oic_conn); + OBD_FREE(imp_conn, sizeof(*imp_conn)); + } - LASSERT(imp->imp_sec == NULL); + 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); + class_decref(imp->imp_obd, "import", imp); OBD_FREE_PTR(imp); EXIT; } diff --git a/lustre/ptlrpc/connection.c b/lustre/ptlrpc/connection.c index 62db6aa..3cc71cf 100644 --- a/lustre/ptlrpc/connection.c +++ b/lustre/ptlrpc/connection.c @@ -132,42 +132,6 @@ out: return conn; } -int ptlrpc_connection_put(struct ptlrpc_connection *conn) -{ - int rc = 0; - ENTRY; - - if (!conn) - RETURN(rc); - - LASSERT(atomic_read(&conn->c_refcount) > 0); - - /* - * We do not remove connection from hashtable and - * do not free it even if last caller released ref, - * as we want to have it cached for the case it is - * needed again. - * - * Deallocating it and later creating new connection - * again would be wastful. This way we also avoid - * expensive locking to protect things from get/put - * race when found cached connection is freed by - * ptlrpc_connection_put(). - * - * It will be freed later in module unload time, - * when ptlrpc_connection_fini()->lh_exit->conn_exit() - * path is called. - */ - if (atomic_dec_return(&conn->c_refcount) == 0) - rc = 1; - - CDEBUG(D_INFO, "PUT conn=%p refcount %d to %s\n", - conn, atomic_read(&conn->c_refcount), - libcfs_nid2str(conn->c_peer.nid)); - - RETURN(rc); -} - struct ptlrpc_connection * ptlrpc_connection_addref(struct ptlrpc_connection *conn) { diff --git a/lustre/ptlrpc/import.c b/lustre/ptlrpc/import.c index 389e6c9..9674217 100644 --- a/lustre/ptlrpc/import.c +++ b/lustre/ptlrpc/import.c @@ -594,15 +594,13 @@ static int import_select_connection(struct obd_import *imp) imp_conn->oic_last_attempt = ktime_get_seconds(); /* switch connection, don't mind if it's same as the current one */ - if (imp->imp_connection) - ptlrpc_connection_put(imp->imp_connection); + ptlrpc_connection_put(imp->imp_connection); imp->imp_connection = ptlrpc_connection_addref(imp_conn->oic_conn); dlmexp = class_conn2export(&imp->imp_dlm_handle); if (!dlmexp) GOTO(out_unlock, rc = -EINVAL); - if (dlmexp->exp_connection) - ptlrpc_connection_put(dlmexp->exp_connection); + ptlrpc_connection_put(dlmexp->exp_connection); dlmexp->exp_connection = ptlrpc_connection_addref(imp_conn->oic_conn); class_export_put(dlmexp); diff --git a/lustre/ptlrpc/ptlrpc_module.c b/lustre/ptlrpc/ptlrpc_module.c index 2b46ba2..d4f8286 100644 --- a/lustre/ptlrpc/ptlrpc_module.c +++ b/lustre/ptlrpc/ptlrpc_module.c @@ -79,8 +79,6 @@ static __init int ptlrpc_init(void) if (rc) GOTO(err_portals, rc); - ptlrpc_put_connection_superhack = ptlrpc_connection_put; - rc = ptlrpc_start_pinger(); if (rc) GOTO(err_conn, rc); -- 1.8.3.1