From 525abddbbd627a7736b5b1d66e1350759cd8d8c2 Mon Sep 17 00:00:00 2001 From: Ian Lumsden Date: Thu, 19 Oct 2023 15:52:35 -0400 Subject: [PATCH 1/5] Adds conditional compilation for UCX in dyad_dtl.c --- src/dtl/Makefile.am | 2 +- src/dtl/dyad_dtl_impl.c | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/dtl/Makefile.am b/src/dtl/Makefile.am index 085b7cb9..9877a412 100644 --- a/src/dtl/Makefile.am +++ b/src/dtl/Makefile.am @@ -23,7 +23,7 @@ libdyad_dtl_la_SOURCES += \ ucx_dtl.c \ ucx_dtl.h libdyad_dtl_la_LIBADD += $(UCX_LIBS) -libdyad_dtl_la_CFLAGS += $(UCX_CFLAGS) +libdyad_dtl_la_CFLAGS += $(UCX_CFLAGS) -DDYAD_ENABLE_UCX=1 endif include_HEADERS = dyad_rc.h dyad_flux_log.h dyad_dtl.h \ No newline at end of file diff --git a/src/dtl/dyad_dtl_impl.c b/src/dtl/dyad_dtl_impl.c index dd002909..8ef09c5d 100644 --- a/src/dtl/dyad_dtl_impl.c +++ b/src/dtl/dyad_dtl_impl.c @@ -14,6 +14,7 @@ dyad_rc_t dyad_dtl_init (dyad_dtl_t **dtl_handle, return DYAD_RC_SYSFAIL; } (*dtl_handle)->mode = mode; +#if DYAD_ENABLE_UCX if (mode == DYAD_DTL_UCX) { rc = dyad_dtl_ucx_init (*dtl_handle, mode, h, debug); if (DYAD_IS_ERROR (rc)) { @@ -21,6 +22,9 @@ dyad_rc_t dyad_dtl_init (dyad_dtl_t **dtl_handle, } return DYAD_RC_OK; } else if (mode == DYAD_DTL_FLUX_RPC) { +#else + if (mode == DYAD_DTL_FLUX_RPC) { +#endif rc = dyad_dtl_flux_init (*dtl_handle, mode, h, debug); if (DYAD_IS_ERROR (rc)) { return rc; @@ -39,6 +43,7 @@ dyad_rc_t dyad_dtl_finalize (dyad_dtl_t **dtl_handle) // this function should be treated as a no-op, and we // should return DYAD_RC_OK to indicate no error has occured return DYAD_RC_OK; +#if DYAD_ENABLE_UCX if ((*dtl_handle)->mode == DYAD_DTL_UCX) { if ((*dtl_handle)->private.ucx_dtl_handle != NULL) { rc = dyad_dtl_ucx_finalize (dtl_handle); @@ -47,6 +52,9 @@ dyad_rc_t dyad_dtl_finalize (dyad_dtl_t **dtl_handle) } } } else if ((*dtl_handle)->mode == DYAD_DTL_FLUX_RPC) { +#else + if ((*dtl_handle)->mode == DYAD_DTL_FLUX_RPC) { +#endif if ((*dtl_handle)->private.flux_dtl_handle != NULL) { rc = dyad_dtl_flux_finalize (dtl_handle); if (DYAD_IS_ERROR (rc)) { From 88444fab0b50f410bdf780c603270e1a9a7b1f34 Mon Sep 17 00:00:00 2001 From: Ian Lumsden Date: Fri, 20 Oct 2023 10:25:27 -0400 Subject: [PATCH 2/5] Makes the call to PKG_CHECK_VAR for UCX_LIBDIR optional --- configure.ac | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/configure.ac b/configure.ac index f232d497..d50132bf 100644 --- a/configure.ac +++ b/configure.ac @@ -132,20 +132,22 @@ PKG_CHECK_MODULES([UCX], if test "x$enable_ucx" = "xyes" && test "x$pkg_check_ucx_found" = "xno"; then AC_MSG_ERROR([requested UCX support, but cannot find UCX with pkg-config]) fi -PKG_CHECK_VAR([UCX_LIBDIR], - [ucx >= 1.6.0], - [libdir], - [], - [AC_MSG_FAILURE([Could not find libdir for UCX])] -) -AS_IF([test "x$UCX_LIBDIR" = "x"], - [AC_MSG_FAILURE([check_var succeeded, but value is incorrect])] -) -AS_IF([test "x$enable_ucx" = "xyes"], - [DYAD_MOD_RPATH="-Wl,-rpath,$UCX_LIBDIR"], - [DYAD_MOD_RPATH=""] -) -AC_SUBST([DYAD_MOD_RPATH]) +if test "x$enable_ucx" = "xyes"; then + PKG_CHECK_VAR([UCX_LIBDIR], + [ucx >= 1.6.0], + [libdir], + [], + [AC_MSG_FAILURE([Could not find libdir for UCX])] + ) + AS_IF([test "x$UCX_LIBDIR" = "x"], + [AC_MSG_FAILURE([check_var succeeded, but value is incorrect])] + ) + AS_IF([test "x$enable_ucx" = "xyes"], + [DYAD_MOD_RPATH="-Wl,-rpath,$UCX_LIBDIR"], + [DYAD_MOD_RPATH=""] + ) + AC_SUBST([DYAD_MOD_RPATH]) +fi ########################### # Checks for header files # From 66bd6fc0bf49a9e9da8e61abcc641c39da72e3d7 Mon Sep 17 00:00:00 2001 From: Ian Lumsden Date: Fri, 20 Oct 2023 10:55:00 -0400 Subject: [PATCH 3/5] Adds conditional compilation to the include of 'ucx_dtl.h' in dyad_dtl_impl.c --- src/dtl/dyad_dtl_impl.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/dtl/dyad_dtl_impl.c b/src/dtl/dyad_dtl_impl.c index 8ef09c5d..f4673a76 100644 --- a/src/dtl/dyad_dtl_impl.c +++ b/src/dtl/dyad_dtl_impl.c @@ -1,7 +1,10 @@ #include "dyad_dtl_impl.h" #include "flux_dtl.h" + +#if DYAD_ENABLE_UCX #include "ucx_dtl.h" +#endif dyad_rc_t dyad_dtl_init (dyad_dtl_t **dtl_handle, dyad_dtl_mode_t mode, From e86723a7d6d37b517194e22a44ece3b56e84f9ab Mon Sep 17 00:00:00 2001 From: Ian Lumsden Date: Fri, 20 Oct 2023 15:12:35 -0400 Subject: [PATCH 4/5] Removes unneeded rpath flags --- src/stream/Makefile.am | 1 - src/wrapper/Makefile.am | 1 - 2 files changed, 2 deletions(-) diff --git a/src/stream/Makefile.am b/src/stream/Makefile.am index a34ffab0..66851c31 100644 --- a/src/stream/Makefile.am +++ b/src/stream/Makefile.am @@ -2,7 +2,6 @@ lib_LTLIBRARIES = libdyad_fstream.la libdyad_fstream_la_SOURCES = dyad_stream_core.cpp libdyad_fstream_la_LDFLAGS = \ - -Wl,-rpath,'$(UCX_LIBDIR)' \ $(AM_LDFLAGS) \ -avoid-version \ -no-undefined diff --git a/src/wrapper/Makefile.am b/src/wrapper/Makefile.am index 5cf9c674..a4875231 100644 --- a/src/wrapper/Makefile.am +++ b/src/wrapper/Makefile.am @@ -1,7 +1,6 @@ lib_LTLIBRARIES = dyad_wrapper.la dyad_wrapper_la_SOURCES = wrapper.c dyad_wrapper_la_LDFLAGS = \ - -Wl,-rpath,'$(UCX_LIBDIR)' \ $(AM_LDFLAGS) \ -module \ -avoid-version \ From 6890cd93976938e1e56216f5cf586bb8d503ae72 Mon Sep 17 00:00:00 2001 From: Ian Lumsden Date: Mon, 30 Oct 2023 11:04:44 -0400 Subject: [PATCH 5/5] Fixes a bug in dyad_dtl_finalize that could cause the dtl handle to not be freed in some cases --- src/dtl/dyad_dtl_impl.c | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/src/dtl/dyad_dtl_impl.c b/src/dtl/dyad_dtl_impl.c index f4673a76..c035634c 100644 --- a/src/dtl/dyad_dtl_impl.c +++ b/src/dtl/dyad_dtl_impl.c @@ -14,44 +14,51 @@ dyad_rc_t dyad_dtl_init (dyad_dtl_t **dtl_handle, dyad_rc_t rc = DYAD_RC_OK; *dtl_handle = malloc (sizeof (struct dyad_dtl)); if (*dtl_handle == NULL) { - return DYAD_RC_SYSFAIL; + rc = DYAD_RC_SYSFAIL; + goto dtl_init_done; } (*dtl_handle)->mode = mode; #if DYAD_ENABLE_UCX if (mode == DYAD_DTL_UCX) { rc = dyad_dtl_ucx_init (*dtl_handle, mode, h, debug); if (DYAD_IS_ERROR (rc)) { - return rc; + goto dtl_init_done; } - return DYAD_RC_OK; } else if (mode == DYAD_DTL_FLUX_RPC) { #else if (mode == DYAD_DTL_FLUX_RPC) { #endif rc = dyad_dtl_flux_init (*dtl_handle, mode, h, debug); if (DYAD_IS_ERROR (rc)) { - return rc; + goto dtl_init_done; } - return DYAD_RC_OK; + } else { + rc = DYAD_RC_BADDTLMODE; + goto dtl_init_done; } - return DYAD_RC_BADDTLMODE; + rc = DYAD_RC_OK; + +dtl_init_done: + return rc; } dyad_rc_t dyad_dtl_finalize (dyad_dtl_t **dtl_handle) { dyad_rc_t rc = DYAD_RC_OK; - if (dtl_handle == NULL || *dtl_handle == NULL) + if (dtl_handle == NULL || *dtl_handle == NULL) { // We should only reach this line if the user has passed // in an already-finalized DTL handle. In that case, // this function should be treated as a no-op, and we // should return DYAD_RC_OK to indicate no error has occured - return DYAD_RC_OK; + rc = DYAD_RC_OK; + goto dtl_finalize_done; + } #if DYAD_ENABLE_UCX if ((*dtl_handle)->mode == DYAD_DTL_UCX) { if ((*dtl_handle)->private.ucx_dtl_handle != NULL) { rc = dyad_dtl_ucx_finalize (dtl_handle); if (DYAD_IS_ERROR (rc)) { - return rc; + goto dtl_finalize_done; } } } else if ((*dtl_handle)->mode == DYAD_DTL_FLUX_RPC) { @@ -61,13 +68,17 @@ dyad_rc_t dyad_dtl_finalize (dyad_dtl_t **dtl_handle) if ((*dtl_handle)->private.flux_dtl_handle != NULL) { rc = dyad_dtl_flux_finalize (dtl_handle); if (DYAD_IS_ERROR (rc)) { - return rc; + goto dtl_finalize_done; } } } else { - return DYAD_RC_BADDTLMODE; + rc = DYAD_RC_BADDTLMODE; + goto dtl_finalize_done; } + rc = DYAD_RC_OK; + +dtl_finalize_done: free (*dtl_handle); *dtl_handle = NULL; - return DYAD_RC_OK; + return rc; }