LU-12678 lnet: discard ksnn_lock

This lock in 'struct ksock_net' is being taken in places where it
isn't needed, so it is worth cleaning up.

It isn't needed when checking if ksnn_npeers has reached
0 yet, as at that point in the code, the value can only
decrement to zero and then stay there.

It is only needed:
 - to ensure concurrent updates to ksnn_npeers don't race, and
 - to ensure that no more peers are added after the net is shutdown.

The first is best achieved using atomic_t.
The second is more easily achieved by replacing the ksnn_shutdown
flag with a large negative bias on ksnn_npeers, and using
atomic_inc_unless_negative().

So change ksnn_npeers to atomic_t and discard ksnn_lock
and ksnn_shutdown.

Signed-off-by: Mr NeilBrown <neilb@suse.de>
Change-Id: I23dd07ef89c7abc14f5a5fef28468a62f7b2a35c
Reviewed-on: https://review.whamcloud.com/36834
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Amir Shehata <ashehata@whamcloud.com>
Reviewed-by: James Simmons <jsimmons@infradead.org>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lnet/klnds/socklnd/socklnd.c b/lnet/klnds/socklnd/socklnd.c
index 30cbf3f..e3e037a 100644 (file)
--- a/lnet/klnds/socklnd/socklnd.c
+++ b/lnet/klnds/socklnd/socklnd.c
@@ -107,9 +107,16 @@ ksocknal_create_peer(struct lnet_ni *ni, struct lnet_process_id id)
        LASSERT(id.pid != LNET_PID_ANY);
        LASSERT(!in_interrupt());
 
+       if (!atomic_inc_unless_negative(&net->ksnn_npeers)) {
+               CERROR("Can't create peer_ni: network shutdown\n");
+               return ERR_PTR(-ESHUTDOWN);
+       }
+
        LIBCFS_CPT_ALLOC(peer_ni, lnet_cpt_table(), cpt, sizeof(*peer_ni));
-       if (peer_ni == NULL)
+       if (!peer_ni) {
+               atomic_dec(&net->ksnn_npeers);
                return ERR_PTR(-ENOMEM);
+       }
 
        peer_ni->ksnp_ni = ni;
        peer_ni->ksnp_id = id;
@@ -126,20 +133,6 @@ ksocknal_create_peer(struct lnet_ni *ni, struct lnet_process_id id)
        INIT_LIST_HEAD(&peer_ni->ksnp_zc_req_list);
        spin_lock_init(&peer_ni->ksnp_lock);
 
-       spin_lock_bh(&net->ksnn_lock);
-
-       if (net->ksnn_shutdown) {
-               spin_unlock_bh(&net->ksnn_lock);
-
-               LIBCFS_FREE(peer_ni, sizeof(*peer_ni));
-               CERROR("Can't create peer_ni: network shutdown\n");
-               return ERR_PTR(-ESHUTDOWN);
-       }
-
-       net->ksnn_npeers++;
-
-       spin_unlock_bh(&net->ksnn_lock);
-
        return peer_ni;
 }
 
@@ -160,13 +153,12 @@ ksocknal_destroy_peer(struct ksock_peer_ni *peer_ni)
 
        LIBCFS_FREE(peer_ni, sizeof(*peer_ni));
 
-        /* NB a peer_ni's connections and routes keep a reference on their peer_ni
-         * until they are destroyed, so we can be assured that _all_ state to
-         * do with this peer_ni has been cleaned up when its refcount drops to
-         * zero. */
-       spin_lock_bh(&net->ksnn_lock);
-       net->ksnn_npeers--;
-       spin_unlock_bh(&net->ksnn_lock);
+       /* NB a peer_ni's connections and routes keep a reference on their
+        * peer_ni until they are destroyed, so we can be assured that _all_
+        * state to do with this peer_ni has been cleaned up when its refcount
+        * drops to zero.
+        */
+       atomic_dec(&net->ksnn_npeers);
 }
 
 struct ksock_peer_ni *
@@ -469,7 +461,8 @@ ksocknal_add_peer(struct lnet_ni *ni, struct lnet_process_id id, __u32 ipaddr,
        write_lock_bh(&ksocknal_data.ksnd_global_lock);
 
         /* always called with a ref on ni, so shutdown can't have started */
-       LASSERT(((struct ksock_net *) ni->ni_data)->ksnn_shutdown == 0);
+       LASSERT(atomic_read(&((struct ksock_net *)ni->ni_data)->ksnn_npeers)
+               >= 0);
 
        peer2 = ksocknal_find_peer_locked(ni, id);
        if (peer2 != NULL) {
@@ -1133,7 +1126,7 @@ ksocknal_create_conn(struct lnet_ni *ni, struct ksock_route *route,
                write_lock_bh(global_lock);
 
                /* called with a ref on ni, so shutdown can't have started */
-               LASSERT(((struct ksock_net *) ni->ni_data)->ksnn_shutdown == 0);
+               LASSERT(atomic_read(&((struct ksock_net *)ni->ni_data)->ksnn_npeers) >= 0);
 
                peer2 = ksocknal_find_peer_locked(ni, peerid);
                if (peer2 == NULL) {
@@ -2522,39 +2515,32 @@ ksocknal_shutdown(struct lnet_ni *ni)
        };
        int i;
 
-        LASSERT(ksocknal_data.ksnd_init == SOCKNAL_INIT_ALL);
-        LASSERT(ksocknal_data.ksnd_nnets > 0);
+       LASSERT(ksocknal_data.ksnd_init == SOCKNAL_INIT_ALL);
+       LASSERT(ksocknal_data.ksnd_nnets > 0);
 
-       spin_lock_bh(&net->ksnn_lock);
-       net->ksnn_shutdown = 1;                 /* prevent new peers */
-       spin_unlock_bh(&net->ksnn_lock);
+       /* prevent new peers */
+       atomic_add(SOCKNAL_SHUTDOWN_BIAS, &net->ksnn_npeers);
 
        /* Delete all peers */
        ksocknal_del_peer(ni, anyid, 0);
 
        /* Wait for all peer_ni state to clean up */
        i = 2;
-       spin_lock_bh(&net->ksnn_lock);
-       while (net->ksnn_npeers != 0) {
-               spin_unlock_bh(&net->ksnn_lock);
-
+       while (atomic_read(&net->ksnn_npeers) > SOCKNAL_SHUTDOWN_BIAS) {
                i++;
                CDEBUG(((i & (-i)) == i) ? D_WARNING : D_NET, /* power of 2? */
                       "waiting for %d peers to disconnect\n",
-                      net->ksnn_npeers);
+                      atomic_read(&net->ksnn_npeers) - SOCKNAL_SHUTDOWN_BIAS);
                set_current_state(TASK_UNINTERRUPTIBLE);
                schedule_timeout(cfs_time_seconds(1));
 
                ksocknal_debug_peerhash(ni);
-
-               spin_lock_bh(&net->ksnn_lock);
        }
-       spin_unlock_bh(&net->ksnn_lock);
 
-        for (i = 0; i < net->ksnn_ninterfaces; i++) {
-                LASSERT (net->ksnn_interfaces[i].ksni_npeers == 0);
-                LASSERT (net->ksnn_interfaces[i].ksni_nroutes == 0);
-        }
+       for (i = 0; i < net->ksnn_ninterfaces; i++) {
+               LASSERT(net->ksnn_interfaces[i].ksni_npeers == 0);
+               LASSERT(net->ksnn_interfaces[i].ksni_nroutes == 0);
+       }
 
        list_del(&net->ksnn_list);
        LIBCFS_FREE(net, sizeof(*net));
@@ -2696,11 +2682,10 @@ ksocknal_startup(struct lnet_ni *ni)
                         return rc;
         }
 
-        LIBCFS_ALLOC(net, sizeof(*net));
-        if (net == NULL)
-                goto fail_0;
+       LIBCFS_ALLOC(net, sizeof(*net));
+       if (net == NULL)
+               goto fail_0;
 
-       spin_lock_init(&net->ksnn_lock);
        net->ksnn_incarnation = ktime_get_real_ns();
        ni->ni_data = net;
        net_tunables = &ni->ni_net->net_tunables;

diff --git a/lnet/klnds/socklnd/socklnd.h b/lnet/klnds/socklnd/socklnd.h
index bc77359..b1ce888 100644 (file)
--- a/lnet/klnds/socklnd/socklnd.h
+++ b/lnet/klnds/socklnd/socklnd.h
@@ -156,13 +156,15 @@ struct ksock_tunables {
 
 struct ksock_net {
        __u64             ksnn_incarnation;     /* my epoch */
-       spinlock_t        ksnn_lock;            /* serialise */
        struct list_head  ksnn_list;            /* chain on global list */
-       int               ksnn_npeers;          /* # peers */
-       int               ksnn_shutdown;        /* shutting down? */
+       atomic_t          ksnn_npeers;          /* # peers */
        int               ksnn_ninterfaces;     /* IP interfaces */
        struct ksock_interface ksnn_interfaces[LNET_INTERFACES_NUM];
 };
+/* When the ksock_net is shut down, this (negative) bias is added to
+ * ksnn_npeers, which prevents new peers from being added.
+ */
+#define SOCKNAL_SHUTDOWN_BIAS  (INT_MIN+1)
 
 /** connd timeout */
 #define SOCKNAL_CONND_TIMEOUT  120