From 8dec4efe47fa8772ded8d6816638703dbbe04379 Mon Sep 17 00:00:00 2001 From: Andreas Dilger Date: Tue, 6 Nov 2018 17:53:02 +0000 Subject: [PATCH] Revert "LU-8130 ptlrpc: convert conn_hash to rhashtable" This reverts commit 7b3f9e5d6c509fabcec3cbd71e541a84987db2ff due to crashes being seen on the MDS during client mount, as described in LU-11624. Change-Id: I3b39363ad1e41dee60f31466aa23d555ebe5135d Reviewed-on: https://review.whamcloud.com/33595 Tested-by: Jenkins Tested-by: Maloo Reviewed-by: Shuichi Ihara Reviewed-by: James Simmons Reviewed-by: Patrick Farrell Reviewed-by: Oleg Drokin --- lustre/include/lustre_net.h | 7 +- lustre/include/obd_support.h | 3 + lustre/ptlrpc/connection.c | 163 +++++++++++++++++++++++++++---------------- 3 files changed, 107 insertions(+), 66 deletions(-) diff --git a/lustre/include/lustre_net.h b/lustre/include/lustre_net.h index a8ae492..9f68ae7 100644 --- a/lustre/include/lustre_net.h +++ b/lustre/include/lustre_net.h @@ -51,11 +51,6 @@ * @{ */ #include -#ifdef HAVE_RHASHTABLE_LOOKUP_GET_INSERT_FAST -#include -#else -#include -#endif #include #include #include @@ -529,7 +524,7 @@ struct ptlrpc_replay_async_args { */ struct ptlrpc_connection { /** linkage for connections hash table */ - struct rhash_head c_hash; + struct hlist_node c_hash; /** Our own lnet nid for this connection */ lnet_nid_t c_self; /** Remote side nid for this connection */ diff --git a/lustre/include/obd_support.h b/lustre/include/obd_support.h index 6cbdad0..1aed5f9 100644 --- a/lustre/include/obd_support.h +++ b/lustre/include/obd_support.h @@ -90,6 +90,9 @@ extern char obd_jobid_var[]; #define HASH_LQE_BKT_BITS 5 #define HASH_LQE_CUR_BITS 7 #define HASH_LQE_MAX_BITS 12 +#define HASH_CONN_BKT_BITS 5 +#define HASH_CONN_CUR_BITS 5 +#define HASH_CONN_MAX_BITS 15 #define HASH_EXP_LOCK_BKT_BITS 5 #define HASH_EXP_LOCK_CUR_BITS 7 #define HASH_EXP_LOCK_MAX_BITS 16 diff --git a/lustre/ptlrpc/connection.c b/lustre/ptlrpc/connection.c index 21a24a2..369eace 100644 --- a/lustre/ptlrpc/connection.c +++ b/lustre/ptlrpc/connection.c @@ -37,41 +37,8 @@ #include "ptlrpc_internal.h" -static struct rhashtable conn_hash; - -/* - * struct lnet_process_id may contain unassigned bytes which might not - * be zero, so we cannot just hash and compare bytes. - */ - -static u32 lnet_process_id_hash(const void *data, u32 len, u32 seed) -{ - const struct lnet_process_id *lpi = data; - - seed = hash_32(seed ^ lpi->pid, 32); - seed ^= hash_64(lpi->nid, 32); - return seed; -} - -static int lnet_process_id_cmp(struct rhashtable_compare_arg *arg, - const void *obj) -{ - const struct lnet_process_id *lpi = arg->key; - const struct ptlrpc_connection *con = obj; - - if (lpi->nid == con->c_peer.nid && - lpi->pid == con->c_peer.pid) - return 0; - return -ESRCH; -} - -static const struct rhashtable_params conn_hash_params = { - .key_len = 1, /* actually variable-length */ - .key_offset = offsetof(struct ptlrpc_connection, c_peer), - .head_offset = offsetof(struct ptlrpc_connection, c_hash), - .hashfn = lnet_process_id_hash, - .obj_cmpfn = lnet_process_id_cmp, -}; +static struct cfs_hash *conn_hash; +static struct cfs_hash_ops conn_hash_ops; struct ptlrpc_connection * ptlrpc_connection_get(struct lnet_process_id peer, lnet_nid_t self, @@ -81,11 +48,9 @@ ptlrpc_connection_get(struct lnet_process_id peer, lnet_nid_t self, ENTRY; peer.nid = LNetPrimaryNID(peer.nid); - conn = rhashtable_lookup_fast(&conn_hash, &peer, conn_hash_params); - if (conn) { - ptlrpc_connection_addref(conn); + conn = cfs_hash_lookup(conn_hash, &peer); + if (conn) GOTO(out, conn); - } OBD_ALLOC_PTR(conn); if (!conn) @@ -93,6 +58,7 @@ ptlrpc_connection_get(struct lnet_process_id peer, lnet_nid_t self, conn->c_peer = peer; conn->c_self = self; + INIT_HLIST_NODE(&conn->c_hash); atomic_set(&conn->c_refcount, 1); if (uuid) obd_str2uuid(&conn->c_remote_uuid, uuid->uuid); @@ -101,17 +67,15 @@ ptlrpc_connection_get(struct lnet_process_id peer, lnet_nid_t self, * Add the newly created conn to the hash, on key collision we * lost a racing addition and must destroy our newly allocated * connection. The object which exists in the hash will be - * returned,otherwise NULL is returned on success. + * returned and may be compared against out object. */ - conn2 = rhashtable_lookup_get_insert_fast(&conn_hash, &conn->c_hash, - conn_hash_params); - if (conn2) { - /* insertion failed */ + /* In the function below, .hs_keycmp resolves to + * conn_keycmp() */ + /* coverity[overrun-buffer-val] */ + conn2 = cfs_hash_findadd_unique(conn_hash, &peer, &conn->c_hash); + if (conn != conn2) { OBD_FREE_PTR(conn); - if (IS_ERR(conn2)) - return NULL; conn = conn2; - ptlrpc_connection_addref(conn); } EXIT; out: @@ -129,7 +93,7 @@ int ptlrpc_connection_put(struct ptlrpc_connection *conn) if (!conn) RETURN(rc); - LASSERT(atomic_read(&conn->c_refcount) > 0); + LASSERT(atomic_read(&conn->c_refcount) > 1); /* * We do not remove connection from hashtable and @@ -147,7 +111,7 @@ int ptlrpc_connection_put(struct ptlrpc_connection *conn) * when ptlrpc_connection_fini()->lh_exit->conn_exit() * path is called. */ - if (atomic_dec_return(&conn->c_refcount) == 0) + if (atomic_dec_return(&conn->c_refcount) == 1) rc = 1; CDEBUG(D_INFO, "PUT conn=%p refcount %d to %s\n", @@ -170,11 +134,90 @@ ptlrpc_connection_addref(struct ptlrpc_connection *conn) RETURN(conn); } +int ptlrpc_connection_init(void) +{ + ENTRY; + + conn_hash = cfs_hash_create("CONN_HASH", + HASH_CONN_CUR_BITS, + HASH_CONN_MAX_BITS, + HASH_CONN_BKT_BITS, 0, + CFS_HASH_MIN_THETA, + CFS_HASH_MAX_THETA, + &conn_hash_ops, CFS_HASH_DEFAULT); + if (!conn_hash) + RETURN(-ENOMEM); + + RETURN(0); +} + +void ptlrpc_connection_fini(void) { + ENTRY; + cfs_hash_putref(conn_hash); + EXIT; +} + +/* + * Hash operations for net_peer<->connection + */ +static unsigned +conn_hashfn(struct cfs_hash *hs, const void *key, unsigned mask) +{ + return cfs_hash_djb2_hash(key, sizeof(struct lnet_process_id), mask); +} + +static int +conn_keycmp(const void *key, struct hlist_node *hnode) +{ + struct ptlrpc_connection *conn; + const struct lnet_process_id *conn_key; + + LASSERT(key != NULL); + conn_key = (struct lnet_process_id *)key; + conn = hlist_entry(hnode, struct ptlrpc_connection, c_hash); + + return conn_key->nid == conn->c_peer.nid && + conn_key->pid == conn->c_peer.pid; +} + +static void * +conn_key(struct hlist_node *hnode) +{ + struct ptlrpc_connection *conn; + conn = hlist_entry(hnode, struct ptlrpc_connection, c_hash); + return &conn->c_peer; +} + +static void * +conn_object(struct hlist_node *hnode) +{ + return hlist_entry(hnode, struct ptlrpc_connection, c_hash); +} + static void -conn_exit(void *vconn, void *data) +conn_get(struct cfs_hash *hs, struct hlist_node *hnode) { - struct ptlrpc_connection *conn = vconn; + struct ptlrpc_connection *conn; + conn = hlist_entry(hnode, struct ptlrpc_connection, c_hash); + atomic_inc(&conn->c_refcount); +} + +static void +conn_put_locked(struct cfs_hash *hs, struct hlist_node *hnode) +{ + struct ptlrpc_connection *conn; + + conn = hlist_entry(hnode, struct ptlrpc_connection, c_hash); + atomic_dec(&conn->c_refcount); +} + +static void +conn_exit(struct cfs_hash *hs, struct hlist_node *hnode) +{ + struct ptlrpc_connection *conn; + + conn = hlist_entry(hnode, struct ptlrpc_connection, c_hash); /* * Nothing should be left. Connection user put it and * connection also was deleted from table by this time @@ -186,12 +229,12 @@ conn_exit(void *vconn, void *data) OBD_FREE_PTR(conn); } -int ptlrpc_connection_init(void) -{ - return rhashtable_init(&conn_hash, &conn_hash_params); -} - -void ptlrpc_connection_fini(void) -{ - rhashtable_free_and_destroy(&conn_hash, conn_exit, NULL); -} +static struct cfs_hash_ops conn_hash_ops = { + .hs_hash = conn_hashfn, + .hs_keycmp = conn_keycmp, + .hs_key = conn_key, + .hs_object = conn_object, + .hs_get = conn_get, + .hs_put_locked = conn_put_locked, + .hs_exit = conn_exit, +}; -- 1.8.3.1