From 63be52b6cdaf6f1c1a81c940d90d66db22e76227 Mon Sep 17 00:00:00 2001 From: Joseph Moore Date: Thu, 29 Aug 2024 14:40:42 +0000 Subject: [PATCH 01/18] DAOS-16448 mercury: Add patch to check ep for null in UCX key resolve. Signed-off-by: Joseph Moore --- debian/changelog | 4 ++++ mercury.spec | 4 ++++ na_ucx_keyres_epchk.patch | 15 +++++++++++++++ 3 files changed, 23 insertions(+) create mode 100644 na_ucx_keyres_epchk.patch diff --git a/debian/changelog b/debian/changelog index 1295323e..2a77bc5c 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,7 @@ +mercury (2.4.0~rc5-2) unstable; urgency=medium + [ Joseph Moore ] + * Add patch to na_ucx.c to check ep in key_resolve + mercury (2.4.0~rc5-1) unstable; urgency=medium [ Jerome Soumagne ] * Update to 2.4.0rc5 diff --git a/mercury.spec b/mercury.spec index 1d082e0f..4258c350 100644 --- a/mercury.spec +++ b/mercury.spec @@ -12,6 +12,7 @@ License: BSD Group: Development/Libraries URL: http://mercury-hpc.github.io/ Source0: https://github.com/mercury-hpc/%{name}/releases/download/v%{dl_version}/%{name}-%{dl_version}.tar.bz2 +Patch0: na_ucx_keyres_epchk.patch BuildRequires: libfabric-devel >= 1.15.0 BuildRequires: cmake @@ -117,6 +118,9 @@ Mercury plugin to support the UCX transport. %{_libdir}/cmake/ %changelog +* Thu Aug 29 2024 Joseph Moore - 2.4.0~rc5-2 +- Add patch to na_ucx.c to check ep in key_resolve. + * Mon Aug 26 2024 Jerome Soumagne - 2.4.0~rc5-1 - Update to 2.4.0rc5 diff --git a/na_ucx_keyres_epchk.patch b/na_ucx_keyres_epchk.patch new file mode 100644 index 00000000..8d5dd8f7 --- /dev/null +++ b/na_ucx_keyres_epchk.patch @@ -0,0 +1,15 @@ +diff --git a/src/na/na_ucx.c b/src/na/na_ucx.c +index 84eb8b0..656bd0c 100644 +--- a/src/na/na_ucx.c ++++ b/src/na/na_ucx.c +@@ -3061,6 +3061,10 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, + + hg_thread_mutex_lock(&na_ucx_mem_handle->rkey_unpack_lock); + ++ if (!ep) ++ NA_GOTO_SUBSYS_ERROR( ++ mem, error, ret, NA_INVALID_ARG, "Invalid endpoint (%p)", ep); ++ + switch (hg_atomic_get32(&na_ucx_mem_handle->type)) { + case NA_UCX_MEM_HANDLE_REMOTE_PACKED: { + ucs_status_t status = ucp_ep_rkey_unpack(ep, From e5d851390fd8bc6a93457b114d8a9d6a133cb7d9 Mon Sep 17 00:00:00 2001 From: Joseph Moore Date: Thu, 29 Aug 2024 15:05:38 +0000 Subject: [PATCH 02/18] DAOS-16271 mercury: Add patch to check ep for null in UCX key resolve. Signed-off-by: Joseph Moore --- na_ucx_keyres_epchk.patch | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/na_ucx_keyres_epchk.patch b/na_ucx_keyres_epchk.patch index 8d5dd8f7..d3148d32 100644 --- a/na_ucx_keyres_epchk.patch +++ b/na_ucx_keyres_epchk.patch @@ -1,14 +1,13 @@ diff --git a/src/na/na_ucx.c b/src/na/na_ucx.c -index 84eb8b0..656bd0c 100644 +index 84eb8b0..290dfe8 100644 --- a/src/na/na_ucx.c +++ b/src/na/na_ucx.c -@@ -3061,6 +3061,10 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, +@@ -3061,6 +3061,9 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, hg_thread_mutex_lock(&na_ucx_mem_handle->rkey_unpack_lock); -+ if (!ep) -+ NA_GOTO_SUBSYS_ERROR( -+ mem, error, ret, NA_INVALID_ARG, "Invalid endpoint (%p)", ep); ++ NA_CHECK_SUBSYS_ERROR( ++ mem, ep == NULL, error, ret, NA_INVALID_ARG, "Invalid endpoint (%p)", ep); + switch (hg_atomic_get32(&na_ucx_mem_handle->type)) { case NA_UCX_MEM_HANDLE_REMOTE_PACKED: { From 3fa5324ccbb65897e6b491585f10eb431f54ed96 Mon Sep 17 00:00:00 2001 From: Joseph Moore Date: Thu, 29 Aug 2024 15:31:21 +0000 Subject: [PATCH 03/18] DAOS-16271 mercury: Add patch to check ep for null in UCX key resolve. Signed-off-by: Joseph Moore --- debian/changelog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/debian/changelog b/debian/changelog index 2a77bc5c..782e5deb 100644 --- a/debian/changelog +++ b/debian/changelog @@ -2,6 +2,8 @@ mercury (2.4.0~rc5-2) unstable; urgency=medium [ Joseph Moore ] * Add patch to na_ucx.c to check ep in key_resolve + -- Joseph Moore Thu, 29 Aug 2024 09:00:00 -0600 + mercury (2.4.0~rc5-1) unstable; urgency=medium [ Jerome Soumagne ] * Update to 2.4.0rc5 From fd9ae31386a65184eb1365bba9ed4f7924a844a6 Mon Sep 17 00:00:00 2001 From: Joseph Moore Date: Fri, 30 Aug 2024 20:21:53 +0000 Subject: [PATCH 04/18] DAOS-16271 mercury: Add patch to check ep for null in UCX key resolve. Signed-off-by: Joseph Moore --- mercury.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mercury.spec b/mercury.spec index 4258c350..5278b31d 100644 --- a/mercury.spec +++ b/mercury.spec @@ -1,6 +1,6 @@ Name: mercury Version: 2.4.0~rc5 -Release: 1%{?dist} +Release: 2%{?dist} # dl_version is version with ~ removed %{lua: From e1765607f6c78fe759656b8d9103a5aa800a8c66 Mon Sep 17 00:00:00 2001 From: Joseph Moore Date: Tue, 3 Sep 2024 16:20:31 +0000 Subject: [PATCH 05/18] DAOS-16481 mercury: Fixes and debug logging for customer testing. Signed-off-by: Joseph Moore --- na_ucx_keyres_epchk.patch | 102 +++++++++++++++++++++++++++++++++++++- 1 file changed, 100 insertions(+), 2 deletions(-) diff --git a/na_ucx_keyres_epchk.patch b/na_ucx_keyres_epchk.patch index d3148d32..163e8309 100644 --- a/na_ucx_keyres_epchk.patch +++ b/na_ucx_keyres_epchk.patch @@ -1,8 +1,106 @@ diff --git a/src/na/na_ucx.c b/src/na/na_ucx.c -index 84eb8b0..290dfe8 100644 +index 84eb8b0..e092e50 100644 --- a/src/na/na_ucx.c +++ b/src/na/na_ucx.c -@@ -3061,6 +3061,9 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, +@@ -1666,6 +1666,9 @@ na_ucp_listener_destroy(ucp_listener_h listener) + static void + na_ucp_listener_conn_cb(ucp_conn_request_h conn_request, void *arg) + { ++ ++ char host_string[NI_MAXHOST]; ++ char serv_string[NI_MAXSERV]; + struct na_ucx_class *na_ucx_class = (struct na_ucx_class *) arg; + ucp_conn_request_attr_t conn_request_attrs = { + .field_mask = UCP_CONN_REQUEST_ATTR_FIELD_CLIENT_ADDR}; +@@ -1683,13 +1686,24 @@ na_ucp_listener_conn_cb(ucp_conn_request_h conn_request, void *arg) + UCP_CONN_REQUEST_ATTR_FIELD_CLIENT_ADDR) == 0, + error, "conn attributes contain no client addr"); + ++ int rc = getnameinfo((const struct sockaddr *) &conn_request_attrs.client_address, ++ sizeof(struct sockaddr), host_string, ++ sizeof(host_string), serv_string, ++ sizeof(serv_string), NI_NUMERICHOST | NI_NUMERICSERV); ++ + /* Lookup address from table */ + addr_key = (ucs_sock_addr_t){ + .addr = (const struct sockaddr *) &conn_request_attrs.client_address, + .addrlen = sizeof(conn_request_attrs.client_address)}; + na_ucx_addr = na_ucx_addr_map_lookup(&na_ucx_class->addr_map, &addr_key); +- NA_CHECK_SUBSYS_ERROR_NORET(addr, na_ucx_addr != NULL, error, +- "An entry is already present for this address"); ++ ++ ++ if (na_ucx_addr != NULL) { ++ NA_LOG_SUBSYS_WARNING(addr, ++ "An entry is already present for this address: %s:%s, refcnt: %d", ++ host_string, serv_string, na_ucx_addr->refcount); ++ na_ucx_addr_map_remove(&na_ucx_class->addr_map, &addr_key); ++ } + + /* Insert new entry and create new address */ + na_ret = na_ucx_addr_map_insert(na_ucx_class, &na_ucx_class->addr_map, +@@ -1919,17 +1933,29 @@ error: + /*---------------------------------------------------------------------------*/ + static void + na_ucp_ep_error_cb( +- void *arg, ucp_ep_h NA_UNUSED ep, ucs_status_t NA_DEBUG_LOG_USED status) ++ void *arg, ucp_ep_h NA_UNUSED ep, ucs_status_t status) + { ++ char host_string[NI_MAXHOST]; ++ char serv_string[NI_MAXSERV]; + struct na_ucx_addr *na_ucx_addr = (struct na_ucx_addr *) arg; + +- NA_LOG_SUBSYS_DEBUG(addr, "ep_err_handler() returned (%s) for address (%p)", +- ucs_status_string(status), (void *) na_ucx_addr); ++ int rc = getnameinfo((const struct sockaddr *) &na_ucx_addr->ss_addr, ++ sizeof(struct sockaddr), host_string, ++ sizeof(host_string), serv_string, ++ sizeof(serv_string), NI_NUMERICHOST | NI_NUMERICSERV); ++ ++ NA_LOG_SUBSYS_WARNING(addr, ++ "ep_err_handler() returned (%s) for address (%s:%s), ep (%p) with refcnt: %d", ++ ucs_status_string(status), host_string, serv_string, ep, na_ucx_addr->refcount); + + /* Mark addr as no longer resolved to force reconnection */ + hg_atomic_and32(&na_ucx_addr->status, ~NA_UCX_ADDR_RESOLVED); + + /* Will schedule removal of address */ ++ if (status == UCS_ERR_CONNECTION_RESET) ++ while (na_ucx_addr->refcount > 1) ++ na_ucx_addr_ref_decr(na_ucx_addr); ++ + na_ucx_addr_ref_decr(na_ucx_addr); + } + +@@ -2993,7 +3019,7 @@ na_ucx_unexpected_info_free( + + /*---------------------------------------------------------------------------*/ + static na_return_t +-na_ucx_rma(struct na_ucx_class NA_UNUSED *na_ucx_class, na_context_t *context, ++na_ucx_rma(struct na_ucx_class *na_ucx_class, na_context_t *context, + na_cb_type_t cb_type, na_cb_t callback, void *arg, + struct na_ucx_mem_handle *local_mem_handle, na_offset_t local_offset, + struct na_ucx_mem_handle *remote_mem_handle, na_offset_t remote_offset, +@@ -3023,6 +3049,18 @@ na_ucx_rma(struct na_ucx_class NA_UNUSED *na_ucx_class, na_context_t *context, + + /* There is no need to have a fully resolved address to start an RMA. + * This is only necessary for two-sided communication. */ ++ /* The above assumption is now in question, so the following will resolve ++ * the address if required. */ ++ ++ /* Check addr to ensure the EP for that addr is still valid */ ++ if (!(hg_atomic_get32(&na_ucx_addr->status) & NA_UCX_ADDR_RESOLVED)) { ++ ret = na_ucx_addr_map_update( ++ na_ucx_class, &na_ucx_class->addr_map, na_ucx_addr); ++ NA_CHECK_SUBSYS_NA_ERROR( ++ addr, error, ret, "Could not update NA UCX address"); ++ } ++ NA_CHECK_SUBSYS_ERROR(msg, na_ucx_addr->ucp_ep == NULL, error, ret, ++ NA_ADDRNOTAVAIL, "UCP endpoint is NULL for that address"); + + /* TODO UCX requires the remote key to be bound to the origin, do we need a + * new API? */ +@@ -3061,6 +3099,9 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, hg_thread_mutex_lock(&na_ucx_mem_handle->rkey_unpack_lock); From d8ac0aa8405657c1bca6b8384ddc214a040bbb5d Mon Sep 17 00:00:00 2001 From: Joseph Moore Date: Wed, 4 Sep 2024 13:45:57 +0000 Subject: [PATCH 06/18] DAOS-16481 mercury: Enhanced logging for customer testing. Signed-off-by: Joseph Moore --- na_ucx_keyres_epchk.patch | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/na_ucx_keyres_epchk.patch b/na_ucx_keyres_epchk.patch index 163e8309..b603556d 100644 --- a/na_ucx_keyres_epchk.patch +++ b/na_ucx_keyres_epchk.patch @@ -1,5 +1,5 @@ diff --git a/src/na/na_ucx.c b/src/na/na_ucx.c -index 84eb8b0..e092e50 100644 +index 84eb8b0..78e63d6 100644 --- a/src/na/na_ucx.c +++ b/src/na/na_ucx.c @@ -1666,6 +1666,9 @@ na_ucp_listener_destroy(ucp_listener_h listener) @@ -39,7 +39,7 @@ index 84eb8b0..e092e50 100644 /* Insert new entry and create new address */ na_ret = na_ucx_addr_map_insert(na_ucx_class, &na_ucx_class->addr_map, -@@ -1919,17 +1933,29 @@ error: +@@ -1919,12 +1933,20 @@ error: /*---------------------------------------------------------------------------*/ static void na_ucp_ep_error_cb( @@ -63,16 +63,7 @@ index 84eb8b0..e092e50 100644 /* Mark addr as no longer resolved to force reconnection */ hg_atomic_and32(&na_ucx_addr->status, ~NA_UCX_ADDR_RESOLVED); - - /* Will schedule removal of address */ -+ if (status == UCS_ERR_CONNECTION_RESET) -+ while (na_ucx_addr->refcount > 1) -+ na_ucx_addr_ref_decr(na_ucx_addr); -+ - na_ucx_addr_ref_decr(na_ucx_addr); - } - -@@ -2993,7 +3019,7 @@ na_ucx_unexpected_info_free( +@@ -2993,7 +3015,7 @@ na_ucx_unexpected_info_free( /*---------------------------------------------------------------------------*/ static na_return_t @@ -81,7 +72,7 @@ index 84eb8b0..e092e50 100644 na_cb_type_t cb_type, na_cb_t callback, void *arg, struct na_ucx_mem_handle *local_mem_handle, na_offset_t local_offset, struct na_ucx_mem_handle *remote_mem_handle, na_offset_t remote_offset, -@@ -3023,6 +3049,18 @@ na_ucx_rma(struct na_ucx_class NA_UNUSED *na_ucx_class, na_context_t *context, +@@ -3023,6 +3045,18 @@ na_ucx_rma(struct na_ucx_class NA_UNUSED *na_ucx_class, na_context_t *context, /* There is no need to have a fully resolved address to start an RMA. * This is only necessary for two-sided communication. */ @@ -100,7 +91,7 @@ index 84eb8b0..e092e50 100644 /* TODO UCX requires the remote key to be bound to the origin, do we need a * new API? */ -@@ -3061,6 +3099,9 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, +@@ -3061,6 +3095,9 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, hg_thread_mutex_lock(&na_ucx_mem_handle->rkey_unpack_lock); From b7bd193e082f8c0d76ed084d64db240a7b1b68e0 Mon Sep 17 00:00:00 2001 From: Joseph Moore Date: Wed, 4 Sep 2024 16:25:43 +0000 Subject: [PATCH 07/18] DAOS-16481 mercury: Update patch to check ep in key_resolve. Signed-off-by: Joseph Moore --- mercury.spec | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mercury.spec b/mercury.spec index 5278b31d..41b4e6d4 100644 --- a/mercury.spec +++ b/mercury.spec @@ -1,6 +1,6 @@ Name: mercury Version: 2.4.0~rc5 -Release: 2%{?dist} +Release: 3%{?dist} # dl_version is version with ~ removed %{lua: @@ -118,6 +118,9 @@ Mercury plugin to support the UCX transport. %{_libdir}/cmake/ %changelog +* Wed Sep 04 2024 Joseph Moore - 2.4.0~rc5-3 +- Update patch to na_ucx.c to check reconnect state before key_resolve. + * Thu Aug 29 2024 Joseph Moore - 2.4.0~rc5-2 - Add patch to na_ucx.c to check ep in key_resolve. From 91d41e0361ac9db06d3dcc9f3d659caaaf8c053d Mon Sep 17 00:00:00 2001 From: Joseph Moore Date: Thu, 5 Sep 2024 17:36:26 +0000 Subject: [PATCH 08/18] DAOS-16481 mercury: Add logging to UCX interface code. Signed-off-by: Joseph Moore --- na_ucx_keyres_epchk.patch | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/na_ucx_keyres_epchk.patch b/na_ucx_keyres_epchk.patch index b603556d..7c3cbc2a 100644 --- a/na_ucx_keyres_epchk.patch +++ b/na_ucx_keyres_epchk.patch @@ -1,5 +1,5 @@ diff --git a/src/na/na_ucx.c b/src/na/na_ucx.c -index 84eb8b0..78e63d6 100644 +index 84eb8b0..3289357 100644 --- a/src/na/na_ucx.c +++ b/src/na/na_ucx.c @@ -1666,6 +1666,9 @@ na_ucp_listener_destroy(ucp_listener_h listener) @@ -12,7 +12,7 @@ index 84eb8b0..78e63d6 100644 struct na_ucx_class *na_ucx_class = (struct na_ucx_class *) arg; ucp_conn_request_attr_t conn_request_attrs = { .field_mask = UCP_CONN_REQUEST_ATTR_FIELD_CLIENT_ADDR}; -@@ -1683,13 +1686,24 @@ na_ucp_listener_conn_cb(ucp_conn_request_h conn_request, void *arg) +@@ -1683,20 +1686,35 @@ na_ucp_listener_conn_cb(ucp_conn_request_h conn_request, void *arg) UCP_CONN_REQUEST_ATTR_FIELD_CLIENT_ADDR) == 0, error, "conn attributes contain no client addr"); @@ -39,7 +39,18 @@ index 84eb8b0..78e63d6 100644 /* Insert new entry and create new address */ na_ret = na_ucx_addr_map_insert(na_ucx_class, &na_ucx_class->addr_map, -@@ -1919,12 +1933,20 @@ error: + &addr_key, conn_request, &na_ucx_addr); ++ + NA_CHECK_SUBSYS_NA_ERROR( + addr, error, na_ret, "Could not insert new address"); + ++ NA_LOG_SUBSYS_WARNING(addr, "Accepted connection req from %s:%s, ep: (%p)", host_string, serv_string, ++ na_ucx_addr->ucp_ep); ++ + return; + + error: +@@ -1919,12 +1937,20 @@ error: /*---------------------------------------------------------------------------*/ static void na_ucp_ep_error_cb( @@ -63,7 +74,16 @@ index 84eb8b0..78e63d6 100644 /* Mark addr as no longer resolved to force reconnection */ hg_atomic_and32(&na_ucx_addr->status, ~NA_UCX_ADDR_RESOLVED); -@@ -2993,7 +3015,7 @@ na_ucx_unexpected_info_free( +@@ -2085,7 +2111,7 @@ na_ucp_am_recv_cb(void *arg, const void *header, size_t header_length, + na_ucx_addr_ep_lookup(&na_ucx_class->addr_map, param->reply_ep); + NA_CHECK_SUBSYS_ERROR(addr, source_addr == NULL, error, ret, + UCS_ERR_INVALID_PARAM, +- "No entry found for previously inserted src addr"); ++ "No entry found for previously inserted src addr, ep (%p)", param->reply_ep); + + /* Pop op ID from queue */ + hg_thread_spin_lock(&unexpected_op_queue->lock); +@@ -2993,7 +3019,7 @@ na_ucx_unexpected_info_free( /*---------------------------------------------------------------------------*/ static na_return_t @@ -72,7 +92,7 @@ index 84eb8b0..78e63d6 100644 na_cb_type_t cb_type, na_cb_t callback, void *arg, struct na_ucx_mem_handle *local_mem_handle, na_offset_t local_offset, struct na_ucx_mem_handle *remote_mem_handle, na_offset_t remote_offset, -@@ -3023,6 +3045,18 @@ na_ucx_rma(struct na_ucx_class NA_UNUSED *na_ucx_class, na_context_t *context, +@@ -3023,6 +3049,18 @@ na_ucx_rma(struct na_ucx_class NA_UNUSED *na_ucx_class, na_context_t *context, /* There is no need to have a fully resolved address to start an RMA. * This is only necessary for two-sided communication. */ @@ -91,7 +111,7 @@ index 84eb8b0..78e63d6 100644 /* TODO UCX requires the remote key to be bound to the origin, do we need a * new API? */ -@@ -3061,6 +3095,9 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, +@@ -3061,6 +3099,9 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, hg_thread_mutex_lock(&na_ucx_mem_handle->rkey_unpack_lock); From ebfea50473d9baabb3ecf962329aa6f0cdc30d2d Mon Sep 17 00:00:00 2001 From: Jerome Soumagne Date: Thu, 5 Sep 2024 15:14:36 -0500 Subject: [PATCH 09/18] DAOS-16481 mercury: add patch for hg_first benchmark Signed-off-by: Jerome Soumagne --- mercury.spec | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mercury.spec b/mercury.spec index 41b4e6d4..8cbfd883 100644 --- a/mercury.spec +++ b/mercury.spec @@ -13,6 +13,7 @@ Group: Development/Libraries URL: http://mercury-hpc.github.io/ Source0: https://github.com/mercury-hpc/%{name}/releases/download/v%{dl_version}/%{name}-%{dl_version}.tar.bz2 Patch0: na_ucx_keyres_epchk.patch +Patch1: https://patch-diff.githubusercontent.com/raw/mercury-hpc/mercury/pull/760.patch BuildRequires: libfabric-devel >= 1.15.0 BuildRequires: cmake @@ -120,6 +121,7 @@ Mercury plugin to support the UCX transport. %changelog * Wed Sep 04 2024 Joseph Moore - 2.4.0~rc5-3 - Update patch to na_ucx.c to check reconnect state before key_resolve. +- Add patch that adds hg_first perf benchmark * Thu Aug 29 2024 Joseph Moore - 2.4.0~rc5-2 - Add patch to na_ucx.c to check ep in key_resolve. From 887a462a0414ec6f902cec385fce8beec0cfb855 Mon Sep 17 00:00:00 2001 From: Joseph Moore Date: Thu, 5 Sep 2024 20:38:35 +0000 Subject: [PATCH 10/18] DAOS-16481 mercury: Update logging and change reconnect handling. Signed-off-by: Joseph Moore --- na_ucx_keyres_epchk.patch | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/na_ucx_keyres_epchk.patch b/na_ucx_keyres_epchk.patch index 7c3cbc2a..22b87a2c 100644 --- a/na_ucx_keyres_epchk.patch +++ b/na_ucx_keyres_epchk.patch @@ -1,8 +1,8 @@ diff --git a/src/na/na_ucx.c b/src/na/na_ucx.c -index 84eb8b0..3289357 100644 +index 84eb8b0..27b9622 100644 --- a/src/na/na_ucx.c +++ b/src/na/na_ucx.c -@@ -1666,6 +1666,9 @@ na_ucp_listener_destroy(ucp_listener_h listener) +@@ -1666,11 +1666,15 @@ na_ucp_listener_destroy(ucp_listener_h listener) static void na_ucp_listener_conn_cb(ucp_conn_request_h conn_request, void *arg) { @@ -12,7 +12,13 @@ index 84eb8b0..3289357 100644 struct na_ucx_class *na_ucx_class = (struct na_ucx_class *) arg; ucp_conn_request_attr_t conn_request_attrs = { .field_mask = UCP_CONN_REQUEST_ATTR_FIELD_CLIENT_ADDR}; -@@ -1683,20 +1686,35 @@ na_ucp_listener_conn_cb(ucp_conn_request_h conn_request, void *arg) + struct na_ucx_addr *na_ucx_addr = NULL; + ucs_sock_addr_t addr_key; ++ bool was_present = false; + ucs_status_t status; + na_return_t na_ret; + +@@ -1683,20 +1687,39 @@ na_ucp_listener_conn_cb(ucp_conn_request_h conn_request, void *arg) UCP_CONN_REQUEST_ATTR_FIELD_CLIENT_ADDR) == 0, error, "conn attributes contain no client addr"); @@ -35,6 +41,7 @@ index 84eb8b0..3289357 100644 + "An entry is already present for this address: %s:%s, refcnt: %d", + host_string, serv_string, na_ucx_addr->refcount); + na_ucx_addr_map_remove(&na_ucx_class->addr_map, &addr_key); ++ was_present = true; + } /* Insert new entry and create new address */ @@ -44,13 +51,16 @@ index 84eb8b0..3289357 100644 NA_CHECK_SUBSYS_NA_ERROR( addr, error, na_ret, "Could not insert new address"); ++ if (was_present) ++ na_ucx_addr_ref_incr(na_ucx_addr); ++ + NA_LOG_SUBSYS_WARNING(addr, "Accepted connection req from %s:%s, ep: (%p)", host_string, serv_string, + na_ucx_addr->ucp_ep); + return; error: -@@ -1919,12 +1937,20 @@ error: +@@ -1919,12 +1942,20 @@ error: /*---------------------------------------------------------------------------*/ static void na_ucp_ep_error_cb( @@ -74,7 +84,7 @@ index 84eb8b0..3289357 100644 /* Mark addr as no longer resolved to force reconnection */ hg_atomic_and32(&na_ucx_addr->status, ~NA_UCX_ADDR_RESOLVED); -@@ -2085,7 +2111,7 @@ na_ucp_am_recv_cb(void *arg, const void *header, size_t header_length, +@@ -2085,7 +2116,7 @@ na_ucp_am_recv_cb(void *arg, const void *header, size_t header_length, na_ucx_addr_ep_lookup(&na_ucx_class->addr_map, param->reply_ep); NA_CHECK_SUBSYS_ERROR(addr, source_addr == NULL, error, ret, UCS_ERR_INVALID_PARAM, @@ -83,7 +93,7 @@ index 84eb8b0..3289357 100644 /* Pop op ID from queue */ hg_thread_spin_lock(&unexpected_op_queue->lock); -@@ -2993,7 +3019,7 @@ na_ucx_unexpected_info_free( +@@ -2993,7 +3024,7 @@ na_ucx_unexpected_info_free( /*---------------------------------------------------------------------------*/ static na_return_t @@ -92,7 +102,7 @@ index 84eb8b0..3289357 100644 na_cb_type_t cb_type, na_cb_t callback, void *arg, struct na_ucx_mem_handle *local_mem_handle, na_offset_t local_offset, struct na_ucx_mem_handle *remote_mem_handle, na_offset_t remote_offset, -@@ -3023,6 +3049,18 @@ na_ucx_rma(struct na_ucx_class NA_UNUSED *na_ucx_class, na_context_t *context, +@@ -3023,6 +3054,18 @@ na_ucx_rma(struct na_ucx_class NA_UNUSED *na_ucx_class, na_context_t *context, /* There is no need to have a fully resolved address to start an RMA. * This is only necessary for two-sided communication. */ @@ -111,7 +121,7 @@ index 84eb8b0..3289357 100644 /* TODO UCX requires the remote key to be bound to the origin, do we need a * new API? */ -@@ -3061,6 +3099,9 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, +@@ -3061,6 +3104,9 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, hg_thread_mutex_lock(&na_ucx_mem_handle->rkey_unpack_lock); From bfe5b3a582efc54a4ddafad1d3e1a66470bffe85 Mon Sep 17 00:00:00 2001 From: Joseph Moore Date: Thu, 5 Sep 2024 20:52:37 +0000 Subject: [PATCH 11/18] DAOS-16481 mercury: Update logging. Signed-off-by: Joseph Moore --- mercury.spec | 1 - 1 file changed, 1 deletion(-) diff --git a/mercury.spec b/mercury.spec index 8cbfd883..bb4e1940 100644 --- a/mercury.spec +++ b/mercury.spec @@ -13,7 +13,6 @@ Group: Development/Libraries URL: http://mercury-hpc.github.io/ Source0: https://github.com/mercury-hpc/%{name}/releases/download/v%{dl_version}/%{name}-%{dl_version}.tar.bz2 Patch0: na_ucx_keyres_epchk.patch -Patch1: https://patch-diff.githubusercontent.com/raw/mercury-hpc/mercury/pull/760.patch BuildRequires: libfabric-devel >= 1.15.0 BuildRequires: cmake From 2307932da0f64d718f438940cf4dacf48d405e69 Mon Sep 17 00:00:00 2001 From: Joseph Moore Date: Thu, 5 Sep 2024 21:05:04 +0000 Subject: [PATCH 12/18] DAOS-16481 mercury: Add logging and change reconnect handling. Signed-off-by: Joseph Moore --- na_ucx_keyres_epchk.patch | 7 ------- 1 file changed, 7 deletions(-) diff --git a/na_ucx_keyres_epchk.patch b/na_ucx_keyres_epchk.patch index 7fd7e05a..22b87a2c 100644 --- a/na_ucx_keyres_epchk.patch +++ b/na_ucx_keyres_epchk.patch @@ -1,5 +1,4 @@ diff --git a/src/na/na_ucx.c b/src/na/na_ucx.c -<<<<<<< HEAD index 84eb8b0..27b9622 100644 --- a/src/na/na_ucx.c +++ b/src/na/na_ucx.c @@ -123,12 +122,6 @@ index 84eb8b0..27b9622 100644 /* TODO UCX requires the remote key to be bound to the origin, do we need a * new API? */ @@ -3061,6 +3104,9 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, -======= -index 84eb8b0..290dfe8 100644 ---- a/src/na/na_ucx.c -+++ b/src/na/na_ucx.c -@@ -3061,6 +3061,9 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, ->>>>>>> master hg_thread_mutex_lock(&na_ucx_mem_handle->rkey_unpack_lock); From 24b8f801c7c39b0a692b131d4dadb053b12a3f6a Mon Sep 17 00:00:00 2001 From: Joseph Moore Date: Fri, 6 Sep 2024 13:39:13 +0000 Subject: [PATCH 13/18] DAOS-16481 mercury: Add logging for debugging. Signed-off-by: Joseph Moore --- mercury.spec | 1 + 1 file changed, 1 insertion(+) diff --git a/mercury.spec b/mercury.spec index bb4e1940..8cbfd883 100644 --- a/mercury.spec +++ b/mercury.spec @@ -13,6 +13,7 @@ Group: Development/Libraries URL: http://mercury-hpc.github.io/ Source0: https://github.com/mercury-hpc/%{name}/releases/download/v%{dl_version}/%{name}-%{dl_version}.tar.bz2 Patch0: na_ucx_keyres_epchk.patch +Patch1: https://patch-diff.githubusercontent.com/raw/mercury-hpc/mercury/pull/760.patch BuildRequires: libfabric-devel >= 1.15.0 BuildRequires: cmake From 336e0f8b19115a566d4b1ebc44a20b4315720acd Mon Sep 17 00:00:00 2001 From: Joseph Moore Date: Fri, 6 Sep 2024 15:44:37 +0000 Subject: [PATCH 14/18] DAOS-16481 mercury: Add logging for address tracking. Signed-off-by: Joseph Moore --- na_ucx_keyres_epchk.patch | 35 ++++++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/na_ucx_keyres_epchk.patch b/na_ucx_keyres_epchk.patch index 22b87a2c..34e161dd 100644 --- a/na_ucx_keyres_epchk.patch +++ b/na_ucx_keyres_epchk.patch @@ -1,5 +1,5 @@ diff --git a/src/na/na_ucx.c b/src/na/na_ucx.c -index 84eb8b0..27b9622 100644 +index 84eb8b0..a6d0485 100644 --- a/src/na/na_ucx.c +++ b/src/na/na_ucx.c @@ -1666,11 +1666,15 @@ na_ucp_listener_destroy(ucp_listener_h listener) @@ -65,7 +65,7 @@ index 84eb8b0..27b9622 100644 static void na_ucp_ep_error_cb( - void *arg, ucp_ep_h NA_UNUSED ep, ucs_status_t NA_DEBUG_LOG_USED status) -+ void *arg, ucp_ep_h NA_UNUSED ep, ucs_status_t status) ++ void *arg, ucp_ep_h ep, ucs_status_t status) { + char host_string[NI_MAXHOST]; + char serv_string[NI_MAXSERV]; @@ -93,7 +93,32 @@ index 84eb8b0..27b9622 100644 /* Pop op ID from queue */ hg_thread_spin_lock(&unexpected_op_queue->lock); -@@ -2993,7 +3024,7 @@ na_ucx_unexpected_info_free( +@@ -2930,6 +2961,14 @@ static NA_INLINE void + na_ucx_addr_ref_decr(struct na_ucx_addr *na_ucx_addr) + { + int32_t refcount = hg_atomic_decr32(&na_ucx_addr->refcount); ++ char host_string[NI_MAXHOST]; ++ char serv_string[NI_MAXSERV]; ++ ++ int rc = getnameinfo((const struct sockaddr *) &na_ucx_addr->ss_addr, ++ sizeof(struct sockaddr), host_string, ++ sizeof(host_string), serv_string, ++ sizeof(serv_string), NI_NUMERICHOST | NI_NUMERICSERV); ++ + NA_LOG_SUBSYS_DEBUG(addr, "Refcount for address (%p) is: %" PRId32, + (void *) na_ucx_addr, refcount); + +@@ -2938,7 +2977,8 @@ na_ucx_addr_ref_decr(struct na_ucx_addr *na_ucx_addr) + struct na_ucx_addr_pool *addr_pool = + &na_ucx_addr->na_ucx_class->addr_pool; + +- NA_LOG_SUBSYS_DEBUG(addr, "Releasing address %p", (void *) na_ucx_addr); ++ NA_LOG_SUBSYS_WARNING(addr, "Releasing address (%s:%s) ep: (%p)", ++ host_string, serv_string, na_ucx_addr->ucp_ep); + na_ucx_addr_release(na_ucx_addr); + + /* Push address back to addr pool */ +@@ -2993,7 +3033,7 @@ na_ucx_unexpected_info_free( /*---------------------------------------------------------------------------*/ static na_return_t @@ -102,7 +127,7 @@ index 84eb8b0..27b9622 100644 na_cb_type_t cb_type, na_cb_t callback, void *arg, struct na_ucx_mem_handle *local_mem_handle, na_offset_t local_offset, struct na_ucx_mem_handle *remote_mem_handle, na_offset_t remote_offset, -@@ -3023,6 +3054,18 @@ na_ucx_rma(struct na_ucx_class NA_UNUSED *na_ucx_class, na_context_t *context, +@@ -3023,6 +3063,18 @@ na_ucx_rma(struct na_ucx_class NA_UNUSED *na_ucx_class, na_context_t *context, /* There is no need to have a fully resolved address to start an RMA. * This is only necessary for two-sided communication. */ @@ -121,7 +146,7 @@ index 84eb8b0..27b9622 100644 /* TODO UCX requires the remote key to be bound to the origin, do we need a * new API? */ -@@ -3061,6 +3104,9 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, +@@ -3061,6 +3113,9 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, hg_thread_mutex_lock(&na_ucx_mem_handle->rkey_unpack_lock); From 30d3ca1eb56fd874ff3f307216d7608918316c38 Mon Sep 17 00:00:00 2001 From: Joseph Moore Date: Mon, 9 Sep 2024 16:34:19 +0000 Subject: [PATCH 15/18] DAOS-16481 mercury: Add logging for debug at customer site. Signed-off-by: Joseph Moore --- na_ucx_keyres_epchk.patch | 39 ++++++++++----------------------------- 1 file changed, 10 insertions(+), 29 deletions(-) diff --git a/na_ucx_keyres_epchk.patch b/na_ucx_keyres_epchk.patch index 34e161dd..ce75b482 100644 --- a/na_ucx_keyres_epchk.patch +++ b/na_ucx_keyres_epchk.patch @@ -1,8 +1,8 @@ diff --git a/src/na/na_ucx.c b/src/na/na_ucx.c -index 84eb8b0..a6d0485 100644 +index 84eb8b0..734909b 100644 --- a/src/na/na_ucx.c +++ b/src/na/na_ucx.c -@@ -1666,11 +1666,15 @@ na_ucp_listener_destroy(ucp_listener_h listener) +@@ -1666,6 +1666,9 @@ na_ucp_listener_destroy(ucp_listener_h listener) static void na_ucp_listener_conn_cb(ucp_conn_request_h conn_request, void *arg) { @@ -12,13 +12,7 @@ index 84eb8b0..a6d0485 100644 struct na_ucx_class *na_ucx_class = (struct na_ucx_class *) arg; ucp_conn_request_attr_t conn_request_attrs = { .field_mask = UCP_CONN_REQUEST_ATTR_FIELD_CLIENT_ADDR}; - struct na_ucx_addr *na_ucx_addr = NULL; - ucs_sock_addr_t addr_key; -+ bool was_present = false; - ucs_status_t status; - na_return_t na_ret; - -@@ -1683,20 +1687,39 @@ na_ucp_listener_conn_cb(ucp_conn_request_h conn_request, void *arg) +@@ -1683,20 +1686,35 @@ na_ucp_listener_conn_cb(ucp_conn_request_h conn_request, void *arg) UCP_CONN_REQUEST_ATTR_FIELD_CLIENT_ADDR) == 0, error, "conn attributes contain no client addr"); @@ -41,7 +35,6 @@ index 84eb8b0..a6d0485 100644 + "An entry is already present for this address: %s:%s, refcnt: %d", + host_string, serv_string, na_ucx_addr->refcount); + na_ucx_addr_map_remove(&na_ucx_class->addr_map, &addr_key); -+ was_present = true; + } /* Insert new entry and create new address */ @@ -51,16 +44,13 @@ index 84eb8b0..a6d0485 100644 NA_CHECK_SUBSYS_NA_ERROR( addr, error, na_ret, "Could not insert new address"); -+ if (was_present) -+ na_ucx_addr_ref_incr(na_ucx_addr); -+ + NA_LOG_SUBSYS_WARNING(addr, "Accepted connection req from %s:%s, ep: (%p)", host_string, serv_string, + na_ucx_addr->ucp_ep); + return; error: -@@ -1919,12 +1942,20 @@ error: +@@ -1919,12 +1937,20 @@ error: /*---------------------------------------------------------------------------*/ static void na_ucp_ep_error_cb( @@ -84,7 +74,7 @@ index 84eb8b0..a6d0485 100644 /* Mark addr as no longer resolved to force reconnection */ hg_atomic_and32(&na_ucx_addr->status, ~NA_UCX_ADDR_RESOLVED); -@@ -2085,7 +2116,7 @@ na_ucp_am_recv_cb(void *arg, const void *header, size_t header_length, +@@ -2085,7 +2111,7 @@ na_ucp_am_recv_cb(void *arg, const void *header, size_t header_length, na_ucx_addr_ep_lookup(&na_ucx_class->addr_map, param->reply_ep); NA_CHECK_SUBSYS_ERROR(addr, source_addr == NULL, error, ret, UCS_ERR_INVALID_PARAM, @@ -93,32 +83,23 @@ index 84eb8b0..a6d0485 100644 /* Pop op ID from queue */ hg_thread_spin_lock(&unexpected_op_queue->lock); -@@ -2930,6 +2961,14 @@ static NA_INLINE void +@@ -2930,6 +2956,7 @@ static NA_INLINE void na_ucx_addr_ref_decr(struct na_ucx_addr *na_ucx_addr) { int32_t refcount = hg_atomic_decr32(&na_ucx_addr->refcount); -+ char host_string[NI_MAXHOST]; -+ char serv_string[NI_MAXSERV]; -+ -+ int rc = getnameinfo((const struct sockaddr *) &na_ucx_addr->ss_addr, -+ sizeof(struct sockaddr), host_string, -+ sizeof(host_string), serv_string, -+ sizeof(serv_string), NI_NUMERICHOST | NI_NUMERICSERV); + NA_LOG_SUBSYS_DEBUG(addr, "Refcount for address (%p) is: %" PRId32, (void *) na_ucx_addr, refcount); -@@ -2938,7 +2977,8 @@ na_ucx_addr_ref_decr(struct na_ucx_addr *na_ucx_addr) +@@ -2938,7 +2965,6 @@ na_ucx_addr_ref_decr(struct na_ucx_addr *na_ucx_addr) struct na_ucx_addr_pool *addr_pool = &na_ucx_addr->na_ucx_class->addr_pool; - NA_LOG_SUBSYS_DEBUG(addr, "Releasing address %p", (void *) na_ucx_addr); -+ NA_LOG_SUBSYS_WARNING(addr, "Releasing address (%s:%s) ep: (%p)", -+ host_string, serv_string, na_ucx_addr->ucp_ep); na_ucx_addr_release(na_ucx_addr); /* Push address back to addr pool */ -@@ -2993,7 +3033,7 @@ na_ucx_unexpected_info_free( +@@ -2993,7 +3019,7 @@ na_ucx_unexpected_info_free( /*---------------------------------------------------------------------------*/ static na_return_t @@ -127,7 +108,7 @@ index 84eb8b0..a6d0485 100644 na_cb_type_t cb_type, na_cb_t callback, void *arg, struct na_ucx_mem_handle *local_mem_handle, na_offset_t local_offset, struct na_ucx_mem_handle *remote_mem_handle, na_offset_t remote_offset, -@@ -3023,6 +3063,18 @@ na_ucx_rma(struct na_ucx_class NA_UNUSED *na_ucx_class, na_context_t *context, +@@ -3023,6 +3049,18 @@ na_ucx_rma(struct na_ucx_class NA_UNUSED *na_ucx_class, na_context_t *context, /* There is no need to have a fully resolved address to start an RMA. * This is only necessary for two-sided communication. */ @@ -146,7 +127,7 @@ index 84eb8b0..a6d0485 100644 /* TODO UCX requires the remote key to be bound to the origin, do we need a * new API? */ -@@ -3061,6 +3113,9 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, +@@ -3061,6 +3099,9 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, hg_thread_mutex_lock(&na_ucx_mem_handle->rkey_unpack_lock); From 7dae4f03c4800745e27560007b70d8aad651c9c8 Mon Sep 17 00:00:00 2001 From: Joseph Moore Date: Mon, 9 Sep 2024 17:35:11 +0000 Subject: [PATCH 16/18] DAOS-16481 mercury: Add log entries and check for ep on disconnect. Signed-off-by: Joseph Moore --- na_ucx_keyres_epchk.patch | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/na_ucx_keyres_epchk.patch b/na_ucx_keyres_epchk.patch index ce75b482..814f2346 100644 --- a/na_ucx_keyres_epchk.patch +++ b/na_ucx_keyres_epchk.patch @@ -1,5 +1,5 @@ diff --git a/src/na/na_ucx.c b/src/na/na_ucx.c -index 84eb8b0..734909b 100644 +index 84eb8b0..2401afd 100644 --- a/src/na/na_ucx.c +++ b/src/na/na_ucx.c @@ -1666,6 +1666,9 @@ na_ucp_listener_destroy(ucp_listener_h listener) @@ -50,7 +50,7 @@ index 84eb8b0..734909b 100644 return; error: -@@ -1919,12 +1937,20 @@ error: +@@ -1919,12 +1937,24 @@ error: /*---------------------------------------------------------------------------*/ static void na_ucp_ep_error_cb( @@ -60,21 +60,25 @@ index 84eb8b0..734909b 100644 + char host_string[NI_MAXHOST]; + char serv_string[NI_MAXSERV]; struct na_ucx_addr *na_ucx_addr = (struct na_ucx_addr *) arg; - -- NA_LOG_SUBSYS_DEBUG(addr, "ep_err_handler() returned (%s) for address (%p)", -- ucs_status_string(status), (void *) na_ucx_addr); ++ struct na_ucx_addr *source_addr = NULL; ++ + int rc = getnameinfo((const struct sockaddr *) &na_ucx_addr->ss_addr, + sizeof(struct sockaddr), host_string, + sizeof(host_string), serv_string, + sizeof(serv_string), NI_NUMERICHOST | NI_NUMERICSERV); -+ ++ source_addr = ++ na_ucx_addr_ep_lookup(&na_ucx_addr->na_ucx_class->addr_map, ep); + +- NA_LOG_SUBSYS_DEBUG(addr, "ep_err_handler() returned (%s) for address (%p)", +- ucs_status_string(status), (void *) na_ucx_addr); + NA_LOG_SUBSYS_WARNING(addr, -+ "ep_err_handler() returned (%s) for address (%s:%s), ep (%p) with refcnt: %d", -+ ucs_status_string(status), host_string, serv_string, ep, na_ucx_addr->refcount); ++ "ep_err_handler() returned (%s) for address (%s:%s), ep (%p) with refcnt: %d, found: %s", ++ ucs_status_string(status), host_string, serv_string, ep, na_ucx_addr->refcount, ++ source_addr ? "true" : "false"); /* Mark addr as no longer resolved to force reconnection */ hg_atomic_and32(&na_ucx_addr->status, ~NA_UCX_ADDR_RESOLVED); -@@ -2085,7 +2111,7 @@ na_ucp_am_recv_cb(void *arg, const void *header, size_t header_length, +@@ -2085,7 +2115,7 @@ na_ucp_am_recv_cb(void *arg, const void *header, size_t header_length, na_ucx_addr_ep_lookup(&na_ucx_class->addr_map, param->reply_ep); NA_CHECK_SUBSYS_ERROR(addr, source_addr == NULL, error, ret, UCS_ERR_INVALID_PARAM, @@ -83,7 +87,7 @@ index 84eb8b0..734909b 100644 /* Pop op ID from queue */ hg_thread_spin_lock(&unexpected_op_queue->lock); -@@ -2930,6 +2956,7 @@ static NA_INLINE void +@@ -2930,6 +2960,7 @@ static NA_INLINE void na_ucx_addr_ref_decr(struct na_ucx_addr *na_ucx_addr) { int32_t refcount = hg_atomic_decr32(&na_ucx_addr->refcount); @@ -91,7 +95,7 @@ index 84eb8b0..734909b 100644 NA_LOG_SUBSYS_DEBUG(addr, "Refcount for address (%p) is: %" PRId32, (void *) na_ucx_addr, refcount); -@@ -2938,7 +2965,6 @@ na_ucx_addr_ref_decr(struct na_ucx_addr *na_ucx_addr) +@@ -2938,7 +2969,6 @@ na_ucx_addr_ref_decr(struct na_ucx_addr *na_ucx_addr) struct na_ucx_addr_pool *addr_pool = &na_ucx_addr->na_ucx_class->addr_pool; @@ -99,7 +103,7 @@ index 84eb8b0..734909b 100644 na_ucx_addr_release(na_ucx_addr); /* Push address back to addr pool */ -@@ -2993,7 +3019,7 @@ na_ucx_unexpected_info_free( +@@ -2993,7 +3023,7 @@ na_ucx_unexpected_info_free( /*---------------------------------------------------------------------------*/ static na_return_t @@ -108,7 +112,7 @@ index 84eb8b0..734909b 100644 na_cb_type_t cb_type, na_cb_t callback, void *arg, struct na_ucx_mem_handle *local_mem_handle, na_offset_t local_offset, struct na_ucx_mem_handle *remote_mem_handle, na_offset_t remote_offset, -@@ -3023,6 +3049,18 @@ na_ucx_rma(struct na_ucx_class NA_UNUSED *na_ucx_class, na_context_t *context, +@@ -3023,6 +3053,18 @@ na_ucx_rma(struct na_ucx_class NA_UNUSED *na_ucx_class, na_context_t *context, /* There is no need to have a fully resolved address to start an RMA. * This is only necessary for two-sided communication. */ @@ -127,7 +131,7 @@ index 84eb8b0..734909b 100644 /* TODO UCX requires the remote key to be bound to the origin, do we need a * new API? */ -@@ -3061,6 +3099,9 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, +@@ -3061,6 +3103,9 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, hg_thread_mutex_lock(&na_ucx_mem_handle->rkey_unpack_lock); From a4828f46604dc936d65d9b46032312a2a53e99cb Mon Sep 17 00:00:00 2001 From: Joseph Moore Date: Mon, 16 Sep 2024 16:50:46 +0000 Subject: [PATCH 17/18] DAOS-16481 mercury: Add potential fix for IOR hang issue Signed-off-by: Joseph Moore --- na_ucx_keyres_epchk.patch | 115 ++++++++++++++++++++++++-------------- 1 file changed, 73 insertions(+), 42 deletions(-) diff --git a/na_ucx_keyres_epchk.patch b/na_ucx_keyres_epchk.patch index 814f2346..8541023c 100644 --- a/na_ucx_keyres_epchk.patch +++ b/na_ucx_keyres_epchk.patch @@ -1,18 +1,26 @@ diff --git a/src/na/na_ucx.c b/src/na/na_ucx.c -index 84eb8b0..2401afd 100644 +index 84eb8b0..953a4c2 100644 --- a/src/na/na_ucx.c +++ b/src/na/na_ucx.c -@@ -1666,6 +1666,9 @@ na_ucp_listener_destroy(ucp_listener_h listener) +@@ -614,7 +614,7 @@ na_ucx_addr_map_update(struct na_ucx_class *na_ucx_class, + */ + static na_return_t + na_ucx_addr_map_remove( +- struct na_ucx_map *na_ucx_map, ucs_sock_addr_t *addr_key); ++ struct na_ucx_map *na_ucx_map, struct na_ucx_addr *remove_addr); + + /** + * Hash connection ID. +@@ -1666,6 +1666,8 @@ na_ucp_listener_destroy(ucp_listener_h listener) static void na_ucp_listener_conn_cb(ucp_conn_request_h conn_request, void *arg) { -+ + char host_string[NI_MAXHOST]; + char serv_string[NI_MAXSERV]; struct na_ucx_class *na_ucx_class = (struct na_ucx_class *) arg; ucp_conn_request_attr_t conn_request_attrs = { .field_mask = UCP_CONN_REQUEST_ATTR_FIELD_CLIENT_ADDR}; -@@ -1683,20 +1686,35 @@ na_ucp_listener_conn_cb(ucp_conn_request_h conn_request, void *arg) +@@ -1683,17 +1685,29 @@ na_ucp_listener_conn_cb(ucp_conn_request_h conn_request, void *arg) UCP_CONN_REQUEST_ATTR_FIELD_CLIENT_ADDR) == 0, error, "conn attributes contain no client addr"); @@ -34,7 +42,7 @@ index 84eb8b0..2401afd 100644 + NA_LOG_SUBSYS_WARNING(addr, + "An entry is already present for this address: %s:%s, refcnt: %d", + host_string, serv_string, na_ucx_addr->refcount); -+ na_ucx_addr_map_remove(&na_ucx_class->addr_map, &addr_key); ++ na_ucx_addr_map_remove(&na_ucx_class->addr_map, na_ucx_addr); + } /* Insert new entry and create new address */ @@ -44,41 +52,16 @@ index 84eb8b0..2401afd 100644 NA_CHECK_SUBSYS_NA_ERROR( addr, error, na_ret, "Could not insert new address"); -+ NA_LOG_SUBSYS_WARNING(addr, "Accepted connection req from %s:%s, ep: (%p)", host_string, serv_string, -+ na_ucx_addr->ucp_ep); -+ - return; - - error: -@@ -1919,12 +1937,24 @@ error: +@@ -1919,7 +1933,7 @@ error: /*---------------------------------------------------------------------------*/ static void na_ucp_ep_error_cb( - void *arg, ucp_ep_h NA_UNUSED ep, ucs_status_t NA_DEBUG_LOG_USED status) + void *arg, ucp_ep_h ep, ucs_status_t status) { -+ char host_string[NI_MAXHOST]; -+ char serv_string[NI_MAXSERV]; struct na_ucx_addr *na_ucx_addr = (struct na_ucx_addr *) arg; -+ struct na_ucx_addr *source_addr = NULL; -+ -+ int rc = getnameinfo((const struct sockaddr *) &na_ucx_addr->ss_addr, -+ sizeof(struct sockaddr), host_string, -+ sizeof(host_string), serv_string, -+ sizeof(serv_string), NI_NUMERICHOST | NI_NUMERICSERV); -+ source_addr = -+ na_ucx_addr_ep_lookup(&na_ucx_addr->na_ucx_class->addr_map, ep); - -- NA_LOG_SUBSYS_DEBUG(addr, "ep_err_handler() returned (%s) for address (%p)", -- ucs_status_string(status), (void *) na_ucx_addr); -+ NA_LOG_SUBSYS_WARNING(addr, -+ "ep_err_handler() returned (%s) for address (%s:%s), ep (%p) with refcnt: %d, found: %s", -+ ucs_status_string(status), host_string, serv_string, ep, na_ucx_addr->refcount, -+ source_addr ? "true" : "false"); - - /* Mark addr as no longer resolved to force reconnection */ - hg_atomic_and32(&na_ucx_addr->status, ~NA_UCX_ADDR_RESOLVED); -@@ -2085,7 +2115,7 @@ na_ucp_am_recv_cb(void *arg, const void *header, size_t header_length, + +@@ -2085,7 +2099,7 @@ na_ucp_am_recv_cb(void *arg, const void *header, size_t header_length, na_ucx_addr_ep_lookup(&na_ucx_class->addr_map, param->reply_ep); NA_CHECK_SUBSYS_ERROR(addr, source_addr == NULL, error, ret, UCS_ERR_INVALID_PARAM, @@ -87,15 +70,63 @@ index 84eb8b0..2401afd 100644 /* Pop op ID from queue */ hg_thread_spin_lock(&unexpected_op_queue->lock); -@@ -2930,6 +2960,7 @@ static NA_INLINE void - na_ucx_addr_ref_decr(struct na_ucx_addr *na_ucx_addr) +@@ -2722,22 +2736,24 @@ unlock: + + /*---------------------------------------------------------------------------*/ + static na_return_t +-na_ucx_addr_map_remove(struct na_ucx_map *na_ucx_map, ucs_sock_addr_t *addr_key) ++na_ucx_addr_map_remove(struct na_ucx_map *na_ucx_map, struct na_ucx_addr *remove_addr) { - int32_t refcount = hg_atomic_decr32(&na_ucx_addr->refcount); + struct na_ucx_addr *na_ucx_addr = NULL; + na_return_t ret = NA_SUCCESS; + int rc; +- ++ ++ + hg_thread_rwlock_wrlock(&na_ucx_map->lock); + + na_ucx_addr = hg_hash_table_lookup( +- na_ucx_map->key_map, (hg_hash_table_key_t) addr_key); +- if (na_ucx_addr == HG_HASH_TABLE_NULL) ++ na_ucx_map->key_map, (hg_hash_table_key_t) &remove_addr->addr_key); + - NA_LOG_SUBSYS_DEBUG(addr, "Refcount for address (%p) is: %" PRId32, - (void *) na_ucx_addr, refcount); ++ if (na_ucx_addr == HG_HASH_TABLE_NULL || na_ucx_addr->ucp_ep != remove_addr->ucp_ep) + goto unlock; + + /* Remove addr key from primary map */ + rc = hg_hash_table_remove( +- na_ucx_map->key_map, (hg_hash_table_key_t) addr_key); ++ na_ucx_map->key_map, (hg_hash_table_key_t) &na_ucx_addr->addr_key); + NA_CHECK_SUBSYS_ERROR(addr, rc != 1, unlock, ret, NA_NOENTRY, + "hg_hash_table_remove() failed"); + +@@ -2841,7 +2857,7 @@ na_ucx_addr_release(struct na_ucx_addr *na_ucx_addr) + NA_UCX_PRINT_ADDR_KEY_INFO("Removing address", &na_ucx_addr->addr_key); + + na_ucx_addr_map_remove( +- &na_ucx_addr->na_ucx_class->addr_map, &na_ucx_addr->addr_key); ++ &na_ucx_addr->na_ucx_class->addr_map, na_ucx_addr); + } + + if (na_ucx_addr->ucp_ep != NULL) { +@@ -2927,18 +2943,18 @@ na_ucx_addr_ref_incr(struct na_ucx_addr *na_ucx_addr) + + /*---------------------------------------------------------------------------*/ + static NA_INLINE void +-na_ucx_addr_ref_decr(struct na_ucx_addr *na_ucx_addr) +-{ +- int32_t refcount = hg_atomic_decr32(&na_ucx_addr->refcount); +- NA_LOG_SUBSYS_DEBUG(addr, "Refcount for address (%p) is: %" PRId32, +- (void *) na_ucx_addr, refcount); ++ na_ucx_addr_ref_decr(struct na_ucx_addr *na_ucx_addr) ++ { ++ int32_t refcount = hg_atomic_decr32(&na_ucx_addr->refcount); ++ ++ NA_LOG_SUBSYS_DEBUG(addr, "Refcount for address (%p) is: %" PRId32, ++ (void *) na_ucx_addr, refcount); -@@ -2938,7 +2969,6 @@ na_ucx_addr_ref_decr(struct na_ucx_addr *na_ucx_addr) + if (refcount == 0) { + #ifdef NA_UCX_HAS_ADDR_POOL struct na_ucx_addr_pool *addr_pool = &na_ucx_addr->na_ucx_class->addr_pool; @@ -103,7 +134,7 @@ index 84eb8b0..2401afd 100644 na_ucx_addr_release(na_ucx_addr); /* Push address back to addr pool */ -@@ -2993,7 +3023,7 @@ na_ucx_unexpected_info_free( +@@ -2993,7 +3009,7 @@ na_ucx_unexpected_info_free( /*---------------------------------------------------------------------------*/ static na_return_t @@ -112,7 +143,7 @@ index 84eb8b0..2401afd 100644 na_cb_type_t cb_type, na_cb_t callback, void *arg, struct na_ucx_mem_handle *local_mem_handle, na_offset_t local_offset, struct na_ucx_mem_handle *remote_mem_handle, na_offset_t remote_offset, -@@ -3023,6 +3053,18 @@ na_ucx_rma(struct na_ucx_class NA_UNUSED *na_ucx_class, na_context_t *context, +@@ -3023,6 +3039,18 @@ na_ucx_rma(struct na_ucx_class NA_UNUSED *na_ucx_class, na_context_t *context, /* There is no need to have a fully resolved address to start an RMA. * This is only necessary for two-sided communication. */ @@ -131,7 +162,7 @@ index 84eb8b0..2401afd 100644 /* TODO UCX requires the remote key to be bound to the origin, do we need a * new API? */ -@@ -3061,6 +3103,9 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, +@@ -3061,6 +3089,9 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, hg_thread_mutex_lock(&na_ucx_mem_handle->rkey_unpack_lock); From abd169a356eb5fd08e66a614069da3e26512c78d Mon Sep 17 00:00:00 2001 From: Joseph Moore Date: Fri, 20 Sep 2024 14:02:19 +0000 Subject: [PATCH 18/18] DAOS-16481: mercury: Change ep close for UCX. Signed-off-by: Joseph Moore --- na_ucx_keyres_epchk.patch | 100 +++++++++++++++++++++++++++++++++----- 1 file changed, 89 insertions(+), 11 deletions(-) diff --git a/na_ucx_keyres_epchk.patch b/na_ucx_keyres_epchk.patch index 8541023c..ca097dce 100644 --- a/na_ucx_keyres_epchk.patch +++ b/na_ucx_keyres_epchk.patch @@ -1,7 +1,16 @@ diff --git a/src/na/na_ucx.c b/src/na/na_ucx.c -index 84eb8b0..953a4c2 100644 +index 84eb8b0..d82784b 100644 --- a/src/na/na_ucx.c +++ b/src/na/na_ucx.c +@@ -444,7 +444,7 @@ na_ucp_ep_error_cb(void *arg, ucp_ep_h ep, ucs_status_t status); + /** + * Close endpoint. + */ +-static void ++static ucs_status_ptr_t + na_ucp_ep_close(ucp_ep_h ep); + + #ifndef NA_UCX_HAS_MEM_POOL @@ -614,7 +614,7 @@ na_ucx_addr_map_update(struct na_ucx_class *na_ucx_class, */ static na_return_t @@ -11,16 +20,17 @@ index 84eb8b0..953a4c2 100644 /** * Hash connection ID. -@@ -1666,6 +1666,8 @@ na_ucp_listener_destroy(ucp_listener_h listener) +@@ -1666,6 +1666,9 @@ na_ucp_listener_destroy(ucp_listener_h listener) static void na_ucp_listener_conn_cb(ucp_conn_request_h conn_request, void *arg) { ++ + char host_string[NI_MAXHOST]; + char serv_string[NI_MAXSERV]; struct na_ucx_class *na_ucx_class = (struct na_ucx_class *) arg; ucp_conn_request_attr_t conn_request_attrs = { .field_mask = UCP_CONN_REQUEST_ATTR_FIELD_CLIENT_ADDR}; -@@ -1683,17 +1685,29 @@ na_ucp_listener_conn_cb(ucp_conn_request_h conn_request, void *arg) +@@ -1683,17 +1686,29 @@ na_ucp_listener_conn_cb(ucp_conn_request_h conn_request, void *arg) UCP_CONN_REQUEST_ATTR_FIELD_CLIENT_ADDR) == 0, error, "conn attributes contain no client addr"); @@ -52,16 +62,46 @@ index 84eb8b0..953a4c2 100644 NA_CHECK_SUBSYS_NA_ERROR( addr, error, na_ret, "Could not insert new address"); -@@ -1919,7 +1933,7 @@ error: +@@ -1919,13 +1934,12 @@ error: /*---------------------------------------------------------------------------*/ static void na_ucp_ep_error_cb( - void *arg, ucp_ep_h NA_UNUSED ep, ucs_status_t NA_DEBUG_LOG_USED status) + void *arg, ucp_ep_h ep, ucs_status_t status) { ++ char host_string[NI_MAXHOST]; ++ char serv_string[NI_MAXSERV]; struct na_ucx_addr *na_ucx_addr = (struct na_ucx_addr *) arg; -@@ -2085,7 +2099,7 @@ na_ucp_am_recv_cb(void *arg, const void *header, size_t header_length, +- NA_LOG_SUBSYS_DEBUG(addr, "ep_err_handler() returned (%s) for address (%p)", +- ucs_status_string(status), (void *) na_ucx_addr); +- + /* Mark addr as no longer resolved to force reconnection */ + hg_atomic_and32(&na_ucx_addr->status, ~NA_UCX_ADDR_RESOLVED); + +@@ -1934,14 +1948,19 @@ na_ucp_ep_error_cb( + } + + /*---------------------------------------------------------------------------*/ +-static void ++static ucs_status_ptr_t + na_ucp_ep_close(ucp_ep_h ep) + { +- ucs_status_ptr_t status_ptr = ucp_ep_close_nb(ep, UCP_EP_CLOSE_MODE_FORCE); ++ const ucp_request_param_t close_params = { ++ .op_attr_mask = UCP_OP_ATTR_FIELD_FLAGS, ++ .flags = UCP_EP_CLOSE_FLAG_FORCE}; ++ ucs_status_ptr_t status_ptr = ucp_ep_close_nbx(ep, &close_params); ++ + NA_CHECK_SUBSYS_ERROR_DONE(addr, + status_ptr != NULL && UCS_PTR_IS_ERR(status_ptr), + "ucp_ep_close_nb() failed (%s)", + ucs_status_string(UCS_PTR_STATUS(status_ptr))); ++ return status_ptr; + } + + /*---------------------------------------------------------------------------*/ +@@ -2085,7 +2104,7 @@ na_ucp_am_recv_cb(void *arg, const void *header, size_t header_length, na_ucx_addr_ep_lookup(&na_ucx_class->addr_map, param->reply_ep); NA_CHECK_SUBSYS_ERROR(addr, source_addr == NULL, error, ret, UCS_ERR_INVALID_PARAM, @@ -70,7 +110,7 @@ index 84eb8b0..953a4c2 100644 /* Pop op ID from queue */ hg_thread_spin_lock(&unexpected_op_queue->lock); -@@ -2722,22 +2736,24 @@ unlock: +@@ -2722,22 +2741,25 @@ unlock: /*---------------------------------------------------------------------------*/ static na_return_t @@ -93,6 +133,7 @@ index 84eb8b0..953a4c2 100644 + if (na_ucx_addr == HG_HASH_TABLE_NULL || na_ucx_addr->ucp_ep != remove_addr->ucp_ep) goto unlock; ++ /* Remove addr key from primary map */ rc = hg_hash_table_remove( - na_ucx_map->key_map, (hg_hash_table_key_t) addr_key); @@ -100,7 +141,24 @@ index 84eb8b0..953a4c2 100644 NA_CHECK_SUBSYS_ERROR(addr, rc != 1, unlock, ret, NA_NOENTRY, "hg_hash_table_remove() failed"); -@@ -2841,7 +2857,7 @@ na_ucx_addr_release(struct na_ucx_addr *na_ucx_addr) +@@ -2747,6 +2769,7 @@ na_ucx_addr_map_remove(struct na_ucx_map *na_ucx_map, ucs_sock_addr_t *addr_key) + NA_CHECK_SUBSYS_ERROR(addr, rc != 1, unlock, ret, NA_NOENTRY, + "hg_hash_table_remove() failed"); + ++ + unlock: + hg_thread_rwlock_release_wrlock(&na_ucx_map->lock); + +@@ -2804,7 +2827,7 @@ na_ucx_addr_alloc(struct na_ucx_class *na_ucx_class) + static void + na_ucx_addr_destroy(struct na_ucx_addr *na_ucx_addr) + { +- NA_LOG_SUBSYS_DEBUG(addr, "Destroying address %p", (void *) na_ucx_addr); ++ NA_LOG_SUBSYS_DEBUG(addr, "Destroying address %p", (void *) na_ucx_addr); + + na_ucx_addr_release(na_ucx_addr); + free(na_ucx_addr); +@@ -2841,14 +2864,25 @@ na_ucx_addr_release(struct na_ucx_addr *na_ucx_addr) NA_UCX_PRINT_ADDR_KEY_INFO("Removing address", &na_ucx_addr->addr_key); na_ucx_addr_map_remove( @@ -109,7 +167,27 @@ index 84eb8b0..953a4c2 100644 } if (na_ucx_addr->ucp_ep != NULL) { -@@ -2927,18 +2943,18 @@ na_ucx_addr_ref_incr(struct na_ucx_addr *na_ucx_addr) + /* NB. for deserialized addresses that are not "connected" addresses, do + * not close the EP */ +- if (na_ucx_addr->worker_addr == NULL) +- na_ucp_ep_close(na_ucx_addr->ucp_ep); ++ if (na_ucx_addr->worker_addr == NULL) { ++ ucs_status_ptr_t status_ptr = na_ucp_ep_close(na_ucx_addr->ucp_ep); ++ ++ if (UCS_PTR_IS_PTR(status_ptr) && !na_ucx_addr->na_ucx_class->ucp_listener) { ++ ucs_status_t status; ++ ++ do { ++ ucp_worker_progress(na_ucx_addr->na_ucx_class->ucp_worker); ++ status = ucp_request_check_status(status_ptr); ++ } while (status == UCS_INPROGRESS); ++ ucp_request_free(status_ptr); ++ } ++ } + na_ucx_addr->ucp_ep = NULL; + } + +@@ -2927,18 +2961,18 @@ na_ucx_addr_ref_incr(struct na_ucx_addr *na_ucx_addr) /*---------------------------------------------------------------------------*/ static NA_INLINE void @@ -134,7 +212,7 @@ index 84eb8b0..953a4c2 100644 na_ucx_addr_release(na_ucx_addr); /* Push address back to addr pool */ -@@ -2993,7 +3009,7 @@ na_ucx_unexpected_info_free( +@@ -2993,7 +3027,7 @@ na_ucx_unexpected_info_free( /*---------------------------------------------------------------------------*/ static na_return_t @@ -143,7 +221,7 @@ index 84eb8b0..953a4c2 100644 na_cb_type_t cb_type, na_cb_t callback, void *arg, struct na_ucx_mem_handle *local_mem_handle, na_offset_t local_offset, struct na_ucx_mem_handle *remote_mem_handle, na_offset_t remote_offset, -@@ -3023,6 +3039,18 @@ na_ucx_rma(struct na_ucx_class NA_UNUSED *na_ucx_class, na_context_t *context, +@@ -3023,6 +3057,18 @@ na_ucx_rma(struct na_ucx_class NA_UNUSED *na_ucx_class, na_context_t *context, /* There is no need to have a fully resolved address to start an RMA. * This is only necessary for two-sided communication. */ @@ -162,7 +240,7 @@ index 84eb8b0..953a4c2 100644 /* TODO UCX requires the remote key to be bound to the origin, do we need a * new API? */ -@@ -3061,6 +3089,9 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, +@@ -3061,6 +3107,9 @@ na_ucx_rma_key_resolve(ucp_ep_h ep, struct na_ucx_mem_handle *na_ucx_mem_handle, hg_thread_mutex_lock(&na_ucx_mem_handle->rkey_unpack_lock);