From fb983bbebf8127828dba545e7b778c12e5411f64 Mon Sep 17 00:00:00 2001 From: Mr NeilBrown Date: Mon, 18 Nov 2019 11:29:08 +1100 Subject: [PATCH] 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 Change-Id: I23dd07ef89c7abc14f5a5fef28468a62f7b2a35c Reviewed-on: https://review.whamcloud.com/36834 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Amir Shehata Reviewed-by: James Simmons Reviewed-by: Oleg Drokin --- lnet/klnds/socklnd/socklnd.c | 75 ++++++++++++++++++-------------------------- lnet/klnds/socklnd/socklnd.h | 8 +++-- 2 files changed, 35 insertions(+), 48 deletions(-) diff --git a/lnet/klnds/socklnd/socklnd.c b/lnet/klnds/socklnd/socklnd.c index 30cbf3f..e3e037a 100644 --- 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 --- 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 -- 1.8.3.1