From c0fedb52fa61cd4a847f5be7eb66629f9b7b6ab7 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 12 Oct 2024 00:35:13 +0200 Subject: [PATCH] mds/MDSDaemon: unlock `mds_lock` while shutting down Beacon and others This fixes a deadlock bug during MDS shutdown: - the "signal_handler" thread receives the shutdown signal and invokes MDSDaemon::suicide() while holding `mds_lock` - MDSDaemon::suicide() invokes Beacon::send_and_wait() while still holding `mds_lock` - meanwhile, all "ms_dispatch" threads get stuck waiting for `mds_lock`, for example in MDCache::upkeep_main() or MDSDaemon::ms_dispatch2() - Beacon::send_and_wait() waits for a `MSG_MDS_BEACON` packet to be dispatched (via `cvar` with a timeout) At this point, even if a `MSG_MDS_BEACON` packet is received by one of the worker threads, they will put it in the `DispatchQueue`, but no dispatcher thread will be able to handle it because they are all stuck. The cvar.wait_for() call in Beacon::send_and_wait() will therefore time out and the `MSG_MDS_BEACON` will never be processed. The proper solution is to unlock `mds_lock` to avoid the dispatchers from getting stuck. And in general, we should be holding a lock strictly only when it is needed and never do blocking calls while holding a lock. Fixes: https://tracker.ceph.com/issues/68760 Signed-off-by: Max Kellermann --- src/mds/MDSDaemon.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/mds/MDSDaemon.cc b/src/mds/MDSDaemon.cc index 1dc592c9757..b81b39a5816 100644 --- a/src/mds/MDSDaemon.cc +++ b/src/mds/MDSDaemon.cc @@ -923,6 +923,13 @@ void MDSDaemon::suicide() // to wait for us to go laggy. Only do this if we're actually in the MDSMap, // because otherwise the MDSMonitor will drop our message. beacon.set_want_state(*mdsmap, MDSMap::STATE_DNE); + + /* Unlock the mds_lock while waiting for beacon ACK to avoid a + * deadlock with the dispatcher thread which may try to acquire + * mds_lock, preventing it from receiving the beacon ACK. + */ + mds_lock.unlock(); + if (!mdsmap->is_dne_gid(mds_gid_t(monc->get_global_id()))) { beacon.send_and_wait(1); } @@ -931,6 +938,8 @@ void MDSDaemon::suicide() if (mgrc.is_initialized()) mgrc.shutdown(); + mds_lock.lock(); + if (mds_rank) { mds_rank->shutdown(); } else { -- 2.47.3