LU-12080 lnet: clean mt_eqh properly

There is a scenario where you have a peer on your recovery queue
that's down. So you keep pinging it, but every ping times out
after 10 seconds. In the middle of these 10 seconds you perform a
shutdown. First you try to do the rsp_tracker_clean. It goes through
and calls MDUnlink on the MD related to that ping. But because the
message has a ref count on the MD, it doesn't go away. The MD gets
zombied. And just waits for lnet_md_unlink to be called in
lnet_finalize(). Then you hit clean_peer_ni_recovery. We see the peer
on the queue, we try to call Unlink on it, but when we lookup the
MD using lnet_handle2md() we can't find it. Afterwards we try to clean
up the EQ and it asserts. Even if we remove the assert we end up with
a resource leak since the EQ is not actually freed since we won't call
LNetEQFree() again.

The solution is to pull the EQ create in the LNetNIInit() and deletion
happens in lnet_unprepare. By this point all the remaining messages
would've been finalized and all references on the EQ are gone,
allowing us to clean it up properly

Lustre-change: https://review.whamcloud.com/34477
Lustre-commit: 1065c8888e96fef9e98676bd3a71b46f7910b085

Signed-off-by: Amir Shehata <ashehata@whamcloud.com>
Change-Id: I7fd6018ee2e57f82c649fc3658352e89a4309986
Reviewed-by: Olaf Weber <olaf.weber@hpe.com>
Reviewed-by: Chris Horn <hornc@cray.com>
Signed-off-by: Minh Diep <mdiep@whamcloud.com>
Reviewed-on: https://review.whamcloud.com/36029
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@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 9fe5cba..26f6c38 100644 (file)
--- a/lnet/include/lnet/lib-lnet.h
+++ b/lnet/include/lnet/lib-lnet.h
@@ -571,6 +571,8 @@ extern unsigned int lnet_peer_discovery_disabled;
 extern unsigned int lnet_drop_asym_route;
 extern int portal_rotor;
 
+void lnet_mt_event_handler(struct lnet_event *event);
+
 int lnet_notify(struct lnet_ni *ni, lnet_nid_t peer, int alive,
                time64_t when);
 void lnet_notify_locked(struct lnet_peer_ni *lp, int notifylnd, int alive,

diff --git a/lnet/lnet/api-ni.c b/lnet/lnet/api-ni.c
index 9e53940..3f8b5de 100644 (file)
--- a/lnet/lnet/api-ni.c
+++ b/lnet/lnet/api-ni.c
@@ -1130,6 +1130,7 @@ lnet_prepare(lnet_pid_t requested_pid)
        INIT_LIST_HEAD(&the_lnet.ln_mt_localNIRecovq);
        INIT_LIST_HEAD(&the_lnet.ln_mt_peerNIRecovq);
        init_waitqueue_head(&the_lnet.ln_dc_waitq);
+       LNetInvalidateEQHandle(&the_lnet.ln_mt_eqh);
 
        rc = lnet_descriptor_setup();
        if (rc != 0)
@@ -1198,6 +1199,8 @@ lnet_prepare(lnet_pid_t requested_pid)
 static int
 lnet_unprepare (void)
 {
+       int rc;
+
        /* NB no LNET_LOCK since this is the last reference.  All LND instances
         * have shut down already, so it is safe to unlink and free all
         * descriptors, even those that appear committed to a network op (eg MD
@@ -1209,6 +1212,12 @@ lnet_unprepare (void)
        LASSERT(list_empty(&the_lnet.ln_test_peers));
        LASSERT(list_empty(&the_lnet.ln_nets));
 
+       if (!LNetEQHandleIsInvalid(the_lnet.ln_mt_eqh)) {
+               rc = LNetEQFree(the_lnet.ln_mt_eqh);
+               LNetInvalidateEQHandle(&the_lnet.ln_mt_eqh);
+               LASSERT(rc == 0);
+       }
+
        lnet_portals_destroy();
 
        if (the_lnet.ln_md_containers != NULL) {
@@ -2586,6 +2595,12 @@ LNetNIInit(lnet_pid_t requested_pid)
 
        lnet_ping_target_update(pbuf, ping_mdh);
 
+       rc = LNetEQAlloc(0, lnet_mt_event_handler, &the_lnet.ln_mt_eqh);
+       if (rc != 0) {
+               CERROR("Can't allocate monitor thread EQ: %d\n", rc);
+               goto err_stop_ping;
+       }
+
        rc = lnet_monitor_thr_start();
        if (rc != 0)
                goto err_stop_ping;

diff --git a/lnet/lnet/lib-eq.c b/lnet/lnet/lib-eq.c
index 3bca6b7..354c976 100644 (file)
--- a/lnet/lnet/lib-eq.c
+++ b/lnet/lnet/lib-eq.c
@@ -159,8 +159,6 @@ LNetEQFree(struct lnet_handle_eq eqh)
        int             size = 0;
        int             i;
 
-       LASSERT(the_lnet.ln_refcount > 0);
-
        lnet_res_lock(LNET_LOCK_EX);
        /* NB: hold lnet_eq_wait_lock for EQ link/unlink, so we can do
         * both EQ lookup and poll event with only lnet_eq_wait_lock */

diff --git a/lnet/lnet/lib-move.c b/lnet/lnet/lib-move.c
index e9139ce..81492a4 100644 (file)
--- a/lnet/lnet/lib-move.c
+++ b/lnet/lnet/lib-move.c
@@ -3521,7 +3521,7 @@ lnet_handle_recovery_reply(struct lnet_mt_event_info *ev_info,
        }
 }
 
-static void
+void
 lnet_mt_event_handler(struct lnet_event *event)
 {
        struct lnet_mt_event_info *ev_info = event->md.user_ptr;
@@ -3599,12 +3599,6 @@ int lnet_monitor_thr_start(void)
        if (rc)
                goto clean_queues;
 
-       rc = LNetEQAlloc(0, lnet_mt_event_handler, &the_lnet.ln_mt_eqh);
-       if (rc != 0) {
-               CERROR("Can't allocate monitor thread EQ: %d\n", rc);
-               goto clean_queues;
-       }
-
        /* Pre monitor thread start processing */
        rc = lnet_router_pre_mt_start();
        if (rc)
@@ -3643,7 +3637,6 @@ free_mem:
        lnet_clean_local_ni_recoveryq();
        lnet_clean_peer_ni_recoveryq();
        lnet_clean_resendqs();
-       LNetEQFree(the_lnet.ln_mt_eqh);
        LNetInvalidateEQHandle(&the_lnet.ln_mt_eqh);
        return rc;
 clean_queues:
@@ -3656,8 +3649,6 @@ clean_queues:
 
 void lnet_monitor_thr_stop(void)
 {
-       int rc;
-
        if (the_lnet.ln_mt_state == LNET_MT_STATE_SHUTDOWN)
                return;
 
@@ -3679,8 +3670,7 @@ void lnet_monitor_thr_stop(void)
        lnet_clean_local_ni_recoveryq();
        lnet_clean_peer_ni_recoveryq();
        lnet_clean_resendqs();
-       rc = LNetEQFree(the_lnet.ln_mt_eqh);
-       LASSERT(rc == 0);
+
        return;
 }