From d5651d52185486e7531f9dfa41a40ab89032905a Mon Sep 17 00:00:00 2001 From: Serguei Smirnov Date: Tue, 26 Sep 2023 16:57:46 -0700 Subject: [PATCH] LU-17103 lnet: use workqueue for lnd ping buffer updates Introduce workqueue for handling lnd-initiated ping buffer update requests. This is done to avoid the possibility of monitor thread lock up waiting for the "old" ping buffer refcount to get decremented during the update, while the message which triggers the decrement is on the monitor thread's own queue waiting to be processed. Lustre-change: https://review.whamcloud.com/52522/ Lustre-commit: TBD (from 1200e9ce1b8272f4affb20386570a9a6e79ceeb4) Test-Parameters: trivial Test-Parameters: testlist=sanity-lnet env=ONLY="207 500",ONLY_REPEAT=50 Fixes: 7ac399c5 ("LU-16949 lnet: get monitor thread to update ping buffer") Signed-off-by: Serguei Smirnov Change-Id: I5176581703e52f4adbfff417040bebcc2489b79e Reviewed-on: https://review.whamcloud.com/c/ex/lustre-release/+/52936 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Andreas Dilger --- lnet/include/lnet/lib-lnet.h | 2 +- lnet/include/lnet/lib-types.h | 6 ++++++ lnet/lnet/api-ni.c | 26 +++++++++++++++++++++----- lnet/lnet/lib-move.c | 25 ++++++++++++++++++++++++- lnet/lnet/peer.c | 4 ++++ lustre/tests/sanity-lnet.sh | 29 +++++++++++++++++++++++++++++ 6 files changed, 85 insertions(+), 7 deletions(-) diff --git a/lnet/include/lnet/lib-lnet.h b/lnet/include/lnet/lib-lnet.h index 68793d2..b23cf32 100644 --- a/lnet/include/lnet/lib-lnet.h +++ b/lnet/include/lnet/lib-lnet.h @@ -1093,7 +1093,7 @@ lnet_set_route_aliveness(struct lnet_route *route, bool alive) old ? "up" : "down", alive ? "up" : "down"); } -void lnet_update_ping_buffer(void); void lnet_mark_ping_buffer_for_update(void); +void lnet_queue_ping_buffer_update(void); #endif diff --git a/lnet/include/lnet/lib-types.h b/lnet/include/lnet/lib-types.h index fac6be2..dc872a8 100644 --- a/lnet/include/lnet/lib-types.h +++ b/lnet/include/lnet/lib-types.h @@ -1212,6 +1212,12 @@ struct lnet { /* for LNDs to signal that ping buffer needs updating */ atomic_t ln_update_ping_buf; + + /* workqueue for serving lnd ping buffer update requests */ + struct workqueue_struct *ln_pb_update_wq; + struct work_struct ln_pb_update_work; + + atomic_t ln_pb_update_ready; }; #endif diff --git a/lnet/lnet/api-ni.c b/lnet/lnet/api-ni.c index dc84c67..a3839f6 100644 --- a/lnet/lnet/api-ni.c +++ b/lnet/lnet/api-ni.c @@ -3570,24 +3570,40 @@ void lnet_mark_ping_buffer_for_update(void) } EXPORT_SYMBOL(lnet_mark_ping_buffer_for_update); -void lnet_update_ping_buffer(void) +void lnet_update_ping_buffer(struct work_struct *work) { struct lnet_ping_buffer *pbuf; struct lnet_handle_md ping_mdh; - if (atomic_dec_if_positive(&the_lnet.ln_update_ping_buf) < 0) - return; - mutex_lock(&the_lnet.ln_api_mutex); - if (!lnet_ping_target_setup(&pbuf, &ping_mdh, + atomic_set(&the_lnet.ln_pb_update_ready, 1); + + if ((the_lnet.ln_state == LNET_STATE_RUNNING) && + !lnet_ping_target_setup(&pbuf, &ping_mdh, lnet_get_ni_count(), false)) lnet_ping_target_update(pbuf, ping_mdh); + mutex_unlock(&the_lnet.ln_api_mutex); } + +void lnet_queue_ping_buffer_update(void) +{ + /* don't queue pb update if it is not needed */ + if (atomic_dec_if_positive(&the_lnet.ln_update_ping_buf) < 0) + return; + + /* don't queue pb update if already queued and not processed */ + if (atomic_dec_if_positive(&the_lnet.ln_pb_update_ready) < 0) + return; + + INIT_WORK(&the_lnet.ln_pb_update_work, lnet_update_ping_buffer); + queue_work(the_lnet.ln_pb_update_wq, &the_lnet.ln_pb_update_work); +} + void lnet_incr_dlc_seq(void) { atomic_inc(&lnet_dlc_seq_no); diff --git a/lnet/lnet/lib-move.c b/lnet/lnet/lib-move.c index f58a1bc..1b50525 100644 --- a/lnet/lnet/lib-move.c +++ b/lnet/lnet/lib-move.c @@ -3779,7 +3779,8 @@ lnet_monitor_thread(void *arg) lnet_recover_peer_nis(); recovery_timeout = now + lnet_recovery_interval; } - lnet_update_ping_buffer(); + + lnet_queue_ping_buffer_update(); /* * TODO do we need to check if we should sleep without @@ -4024,6 +4025,16 @@ int lnet_monitor_thr_start(void) if (rc) goto clean_queues; + the_lnet.ln_pb_update_wq = alloc_workqueue("lnetpb_wq", + WQ_UNBOUND, + 1); + if (!the_lnet.ln_pb_update_wq) { + rc = -ENOMEM; + CERROR("Failed to allocate LNet ping buffer workqueue\n"); + goto clean_queues; + } + atomic_set(&the_lnet.ln_pb_update_ready, 1); + sema_init(&the_lnet.ln_mt_signal, 0); lnet_net_lock(LNET_LOCK_EX); @@ -4068,6 +4079,18 @@ void lnet_monitor_thr_stop(void) return; LASSERT(the_lnet.ln_mt_state == LNET_MT_STATE_RUNNING); + + /* clean up the ping buffer update workqueue before telling + * the monitor thread to shut down to avoid getting stuck + * on pending messages + */ + mutex_unlock(&the_lnet.ln_api_mutex); + flush_workqueue(the_lnet.ln_pb_update_wq); + destroy_workqueue(the_lnet.ln_pb_update_wq); + atomic_set(&the_lnet.ln_pb_update_ready, 0); + the_lnet.ln_pb_update_wq = NULL; + mutex_lock(&the_lnet.ln_api_mutex); + lnet_net_lock(LNET_LOCK_EX); the_lnet.ln_mt_state = LNET_MT_STATE_STOPPING; lnet_net_unlock(LNET_LOCK_EX); diff --git a/lnet/lnet/peer.c b/lnet/lnet/peer.c index f6c5fd8..7833576 100644 --- a/lnet/lnet/peer.c +++ b/lnet/lnet/peer.c @@ -926,6 +926,9 @@ lnet_push_update_to_peers(int force) int lncpt; int cpt; + if (the_lnet.ln_dc_state != LNET_DC_STATE_RUNNING) + return; + lnet_net_lock(LNET_LOCK_EX); if (lnet_peer_discovery_disabled) force = 0; @@ -3308,6 +3311,7 @@ __must_hold(&lp->lp_lock) */ mutex_lock(&the_lnet.ln_api_mutex); if (the_lnet.ln_state != LNET_STATE_RUNNING) { + lnet_ping_buffer_decref(pbuf); rc = -ESHUTDOWN; goto out; } diff --git a/lustre/tests/sanity-lnet.sh b/lustre/tests/sanity-lnet.sh index 6c09736..0f6858d 100755 --- a/lustre/tests/sanity-lnet.sh +++ b/lustre/tests/sanity-lnet.sh @@ -2927,6 +2927,35 @@ EOF } run_test 304 "Check locked primary peer nid consolidation" +test_500() { + reinit_dlc || return $? + + cleanup_netns || error "Failed to cleanup netns before test execution" + setup_fakeif || error "Failed to add fake IF" + have_interface "$FAKE_IF" || + error "Expect $FAKE_IF configured but not found" + + add_net "tcp" "${INTERFACES[0]}" + add_net "tcp" "${FAKE_IF}" + + do_lnetctl discover $($LCTL list_nids | head -n 1) || + error "Failed to discover self" + + $LCTL net_delay_add -s *@tcp -d *@tcp -r 1 -l 1 -m PUT || + error "Failed to add delay rule" + + $LCTL net_drop_add -s *@tcp -d $($LCTL list_nids | head -n 1) -m PUT -e local_timeout -r 1 + $LCTL net_drop_add -s *@tcp -d $($LCTL list_nids | tail -n 1) -m PUT -e local_timeout -r 1 + + ip link set $FAKE_IF down || + error "Failed to set link down" + ip link set $FAKE_IF up || + error "Failed to set link up" + + unload_modules +} +run_test 500 "Check deadlock on ping target update" + complete_test $SECONDS cleanup_testsuite -- 1.8.3.1