LU-12756 lnet: Avoid redundant peer NI lookups

Each caller of lnet_peer_ni_traffic_add() performs a subsequent call
to lnet_peer_ni_find_locked(). We can avoid the extra lookup by having
lnet_peer_ni_traffic_add() return a peer NI pointer (or ERR_PTR as
appropriate).

lnet_peer_ni_traffic_add() now takes a ref on the peer NI to mimic
the behavior of lnet_peer_ni_find_locked().

lnet_nid2peerni_ex() only has a single caller that always passes
LNET_LOCK_EX for the cpt argument, so this function argument is
removed.

Some duplicate code dealing with ln_state handling is removed from
lnet_peerni_by_nid_locked()

Test-Parameters: trivial testlist=sanity-lnet
Signed-off-by: Chris Horn <chris.horn@hpe.com>
Change-Id: I8e9e2449ef2b958b53abd59cd2c122e5492fbb34
Reviewed-on: https://review.whamcloud.com/36623
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Alexey Lyashkov <alexey.lyashkov@hpe.com>
Reviewed-by: Serguei Smirnov <ssmirnov@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lnet/include/lnet/lib-lnet.h b/lnet/include/lnet/lib-lnet.h
index f840fe5..b6dc74b 100644 (file)
--- a/lnet/include/lnet/lib-lnet.h
+++ b/lnet/include/lnet/lib-lnet.h
@@ -974,7 +974,7 @@ struct lnet_peer_ni *lnet_nid2peerni_locked(lnet_nid_t nid, lnet_nid_t pref,
 struct lnet_peer_ni *lnet_peerni_by_nid_locked(struct lnet_nid *nid,
                                               struct lnet_nid *pref,
                                               int cpt);
-struct lnet_peer_ni *lnet_nid2peerni_ex(struct lnet_nid *nid, int cpt);
+struct lnet_peer_ni *lnet_nid2peerni_ex(struct lnet_nid *nid);
 struct lnet_peer_ni *lnet_peer_get_ni_locked(struct lnet_peer *lp,
                                             lnet_nid_t nid);
 struct lnet_peer_ni *lnet_peer_ni_get_locked(struct lnet_peer *lp,

diff --git a/lnet/lnet/peer.c b/lnet/lnet/peer.c
index 50b64d2..e32423f 100644 (file)
--- a/lnet/lnet/peer.c
+++ b/lnet/lnet/peer.c
@@ -1844,19 +1844,22 @@ out:
 
 /*
  * lpni creation initiated due to traffic either sending or receiving.
+ * Callers must hold ln_api_mutex
+ * Ref taken on lnet_peer_ni returned by this function
  */
-static int
+static struct lnet_peer_ni *
 lnet_peer_ni_traffic_add(struct lnet_nid *nid, struct lnet_nid *pref)
+__must_hold(&the_lnet.ln_api_mutex)
 {
-       struct lnet_peer *lp;
-       struct lnet_peer_net *lpn;
+       struct lnet_peer *lp = NULL;
+       struct lnet_peer_net *lpn = NULL;
        struct lnet_peer_ni *lpni;
        unsigned flags = 0;
        int rc = 0;
 
        if (LNET_NID_IS_ANY(nid)) {
                rc = -EINVAL;
-               goto out;
+               goto out_err;
        }
 
        /* lnet_net_lock is not needed here because ln_api_lock is held */
@@ -1868,7 +1871,6 @@ lnet_peer_ni_traffic_add(struct lnet_nid *nid, struct lnet_nid *pref)
                 * traffic, we just assume everything is ok and
                 * return.
                 */
-               lnet_peer_ni_decref_locked(lpni);
                goto out;
        }
 
@@ -1876,24 +1878,31 @@ lnet_peer_ni_traffic_add(struct lnet_nid *nid, struct lnet_nid *pref)
        rc = -ENOMEM;
        lp = lnet_peer_alloc(nid);
        if (!lp)
-               goto out;
+               goto out_err;
        lpn = lnet_peer_net_alloc(LNET_NID_NET(nid));
        if (!lpn)
-               goto out_free_lp;
+               goto out_err;
        lpni = lnet_peer_ni_alloc(nid);
        if (!lpni)
-               goto out_free_lpn;
+               goto out_err;
        lnet_peer_ni_set_non_mr_pref_nid(lpni, pref);
 
-       return lnet_peer_attach_peer_ni(lp, lpn, lpni, flags);
+       /* lnet_peer_attach_peer_ni() always returns 0 */
+       rc = lnet_peer_attach_peer_ni(lp, lpn, lpni, flags);
 
-out_free_lpn:
-       LIBCFS_FREE(lpn, sizeof(*lpn));
-out_free_lp:
-       LIBCFS_FREE(lp, sizeof(*lp));
+       lnet_peer_ni_addref_locked(lpni);
+
+out_err:
+       if (rc) {
+               if (lpn)
+                       LIBCFS_FREE(lpn, sizeof(*lpn));
+               if (lp)
+                       LIBCFS_FREE(lp, sizeof(*lp));
+               lpni = ERR_PTR(rc);
+       }
 out:
        CDEBUG(D_NET, "peer %s: %d\n", libcfs_nidstr(nid), rc);
-       return rc;
+       return lpni;
 }
 
 /*
@@ -2062,10 +2071,10 @@ lnet_destroy_peer_ni_locked(struct kref *ref)
 }
 
 struct lnet_peer_ni *
-lnet_nid2peerni_ex(struct lnet_nid *nid, int cpt)
+lnet_nid2peerni_ex(struct lnet_nid *nid)
+__must_hold(&the_lnet.ln_api_mutex)
 {
        struct lnet_peer_ni *lpni = NULL;
-       int rc;
 
        if (the_lnet.ln_state != LNET_STATE_RUNNING)
                return ERR_PTR(-ESHUTDOWN);
@@ -2078,19 +2087,11 @@ lnet_nid2peerni_ex(struct lnet_nid *nid, int cpt)
        if (lpni)
                return lpni;
 
-       lnet_net_unlock(cpt);
-
-       rc = lnet_peer_ni_traffic_add(nid, NULL);
-       if (rc) {
-               lpni = ERR_PTR(rc);
-               goto out_net_relock;
-       }
+       lnet_net_unlock(LNET_LOCK_EX);
 
-       lpni = lnet_peer_ni_find_locked(nid);
-       LASSERT(lpni);
+       lpni = lnet_peer_ni_traffic_add(nid, NULL);
 
-out_net_relock:
-       lnet_net_lock(cpt);
+       lnet_net_lock(LNET_LOCK_EX);
 
        return lpni;
 }
@@ -2104,7 +2105,6 @@ lnet_peerni_by_nid_locked(struct lnet_nid *nid,
                        struct lnet_nid *pref, int cpt)
 {
        struct lnet_peer_ni *lpni = NULL;
-       int rc;
 
        if (the_lnet.ln_state != LNET_STATE_RUNNING)
                return ERR_PTR(-ESHUTDOWN);
@@ -2132,30 +2132,18 @@ lnet_peerni_by_nid_locked(struct lnet_nid *nid,
        lnet_net_unlock(cpt);
        mutex_lock(&the_lnet.ln_api_mutex);
        /*
-        * Shutdown is only set under the ln_api_lock, so a single
+        * the_lnet.ln_state is only modified under the ln_api_lock, so a single
         * check here is sufficent.
         */
-       if (the_lnet.ln_state != LNET_STATE_RUNNING) {
-               lpni = ERR_PTR(-ESHUTDOWN);
-               goto out_mutex_unlock;
-       }
-
-       rc = lnet_peer_ni_traffic_add(nid, pref);
-       if (rc) {
-               lpni = ERR_PTR(rc);
-               goto out_mutex_unlock;
-       }
-
-       lpni = lnet_peer_ni_find_locked(nid);
-       LASSERT(lpni);
+       if (the_lnet.ln_state == LNET_STATE_RUNNING)
+               lpni = lnet_peer_ni_traffic_add(nid, pref);
 
-out_mutex_unlock:
        mutex_unlock(&the_lnet.ln_api_mutex);
        lnet_net_lock(cpt);
 
        /* Lock has been dropped, check again for shutdown. */
        if (the_lnet.ln_state != LNET_STATE_RUNNING) {
-               if (!IS_ERR(lpni))
+               if (!IS_ERR_OR_NULL(lpni))
                        lnet_peer_ni_decref_locked(lpni);
                lpni = ERR_PTR(-ESHUTDOWN);
        }

diff --git a/lnet/lnet/router.c b/lnet/lnet/router.c
index 9ccf562..03502f3 100644 (file)
--- a/lnet/lnet/router.c
+++ b/lnet/lnet/router.c
@@ -726,7 +726,7 @@ lnet_add_route(__u32 net, __u32 hops, struct lnet_nid *gateway,
         * lnet_nid2peerni_ex() grabs a ref on the lpni. We will need to
         * lose that once we're done
         */
-       lpni = lnet_nid2peerni_ex(gateway, LNET_LOCK_EX);
+       lpni = lnet_nid2peerni_ex(gateway);
        if (IS_ERR(lpni)) {
                lnet_net_unlock(LNET_LOCK_EX);
 
@@ -740,7 +740,9 @@ lnet_add_route(__u32 net, __u32 hops, struct lnet_nid *gateway,
                return rc;
        }
 
-       LASSERT(lpni->lpni_peer_net && lpni->lpni_peer_net->lpn_peer);
+       LASSERT(lpni);
+       LASSERT(lpni->lpni_peer_net);
+       LASSERT(lpni->lpni_peer_net->lpn_peer);
        gw = lpni->lpni_peer_net->lpn_peer;
 
        route->lr_gateway = gw;