From fe5d20eee3568500233eb16c38d53502ac13b50a Mon Sep 17 00:00:00 2001 From: Aurelien Bouteiller Date: Fri, 25 Apr 2025 11:01:13 -0400 Subject: [PATCH 1/2] bugfix: Setting OMPI_MPI_THREAD_LEVEL to a value different than `requested` in `MPI_Init_thread` would invoke the error handler, even though it is an useful override in some threaded library use cases. Signed-off-by: Aurelien Bouteiller --- ompi/mpi/c/init.c.in | 8 +++++++- ompi/mpi/c/init_thread.c.in | 32 ++++++++++++++++++++++++-------- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/ompi/mpi/c/init.c.in b/ompi/mpi/c/init.c.in index b9e5d513482..16a4d5525cc 100644 --- a/ompi/mpi/c/init.c.in +++ b/ompi/mpi/c/init.c.in @@ -15,6 +15,7 @@ * and Technology (RIST). All rights reserved. * Copyright (c) 2024 Triad National Security, LLC. All rights * reserved. + * Copyright (c) 2025 Advanced Micro Devices, Inc. All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -46,7 +47,12 @@ PROTOTYPE INT init(INT_OUT argc, ARGV argv) if (NULL != (env = getenv("OMPI_MPI_THREAD_LEVEL"))) { required = atoi(env); - if (required < MPI_THREAD_SINGLE || required > MPI_THREAD_MULTIPLE) { + /* In the future we may have to contend with non-sequential (MPI ABI) values + * If you are implementing MPI ABI changes please refer to + * https://github.com/open-mpi/ompi/pull/13211#discussion_r2085086844 + */ + if (required != MPI_THREAD_SINGLE && required != MPI_THREAD_FUNNELED && + required != MPI_THREAD_SERIALIZED && required != MPI_THREAD_MULTIPLE) { required = MPI_THREAD_MULTIPLE; } } diff --git a/ompi/mpi/c/init_thread.c.in b/ompi/mpi/c/init_thread.c.in index f56728ee262..1e18b66040f 100644 --- a/ompi/mpi/c/init_thread.c.in +++ b/ompi/mpi/c/init_thread.c.in @@ -18,6 +18,7 @@ * reserved. * Copyright (c) 2024 Triad National Security, LLC. All rights * reserved. + * Copyright (c) 2025 Advanced Micro Devices, Inc. All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -40,6 +41,7 @@ PROTOTYPE ERROR_CLASS init_thread(INT_OUT argc, ARGV argv, INT required, INT_OUT provided) { int err, safe_required = MPI_THREAD_SERIALIZED; + bool err_arg_required = false; char *env; ompi_hook_base_mpi_init_thread_top(argc, argv, required, provided); @@ -47,14 +49,28 @@ PROTOTYPE ERROR_CLASS init_thread(INT_OUT argc, ARGV argv, INT required, /* Detect an incorrect thread support level, but dont report until we have the minimum * infrastructure setup. */ - if( (MPI_THREAD_SINGLE == required) || (MPI_THREAD_SERIALIZED == required) || - (MPI_THREAD_FUNNELED == required) || (MPI_THREAD_MULTIPLE == required) ) { + err_arg_required = (required != MPI_THREAD_SINGLE && required != MPI_THREAD_FUNNELED && + required != MPI_THREAD_SERIALIZED && required != MPI_THREAD_MULTIPLE); + if (!err_arg_required) { + safe_required = required; + } - if (NULL != (env = getenv("OMPI_MPI_THREAD_LEVEL"))) { - safe_required = atoi(env); - } - else { - safe_required = required; + /* check for environment overrides for required thread level. If + * there is, check to see that it is a valid/supported thread level. + * If valid, the environment variable always override the provided thread + * level (even if lower than argument `required`). A user program can + * check `provided != required` to check if `required` has been overruled. + */ + if (NULL != (env = getenv("OMPI_MPI_THREAD_LEVEL"))) { + int env_required = atoi(env); + /* In the future we may have to contend with non-sequential (MPI ABI) values + * If you are implementing MPI ABI changes please refer to + * https://github.com/open-mpi/ompi/pull/13211#discussion_r2085086844 + */ + err_arg_required |= (env_required != MPI_THREAD_SINGLE && env_required != MPI_THREAD_FUNNELED && + env_required != MPI_THREAD_SERIALIZED && env_required != MPI_THREAD_MULTIPLE); + if (!err_arg_required) { + safe_required = env_required; } } @@ -70,7 +86,7 @@ PROTOTYPE ERROR_CLASS init_thread(INT_OUT argc, ARGV argv, INT required, err = ompi_mpi_init(0, NULL, safe_required, provided, false); } - if( safe_required != required ) { + if (err_arg_required) { /* Trigger the error handler for the incorrect argument. Keep it separate from the * check on the ompi_mpi_init return and report a nice, meaningful error message to * the user. */ From 505913435b8e1e8135fa2dadb91a78644a0bb4b5 Mon Sep 17 00:00:00 2001 From: Aurelien Bouteiller Date: Thu, 12 Jun 2025 23:42:06 -0400 Subject: [PATCH 2/2] OMPI_MPI_THREAD_LEVEL can now take 'multiple' 'MPI_THREAD_MULTIPLE' (single,etc) in addition to numeric 0-3 values Signed-off-by: Aurelien Bouteiller --- ompi/mpi/c/init.c.in | 14 ++------------ ompi/mpi/c/init_thread.c.in | 16 +++------------- ompi/runtime/mpiruntime.h | 22 +++++++++++++++++++++ ompi/runtime/ompi_mpi_init.c | 37 ++++++++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 25 deletions(-) diff --git a/ompi/mpi/c/init.c.in b/ompi/mpi/c/init.c.in index 16a4d5525cc..522347c7ece 100644 --- a/ompi/mpi/c/init.c.in +++ b/ompi/mpi/c/init.c.in @@ -38,23 +38,13 @@ PROTOTYPE INT init(INT_OUT argc, ARGV argv) { int err; int provided; - char *env; int required = MPI_THREAD_SINGLE; /* check for environment overrides for required thread level. If there is, check to see that it is a valid/supported thread level. If not, default to MPI_THREAD_MULTIPLE. */ - - if (NULL != (env = getenv("OMPI_MPI_THREAD_LEVEL"))) { - required = atoi(env); - /* In the future we may have to contend with non-sequential (MPI ABI) values - * If you are implementing MPI ABI changes please refer to - * https://github.com/open-mpi/ompi/pull/13211#discussion_r2085086844 - */ - if (required != MPI_THREAD_SINGLE && required != MPI_THREAD_FUNNELED && - required != MPI_THREAD_SERIALIZED && required != MPI_THREAD_MULTIPLE) { - required = MPI_THREAD_MULTIPLE; - } + if (OMPI_SUCCESS > ompi_getenv_mpi_thread_level(&required)) { + required = MPI_THREAD_MULTIPLE; } /* Call the back-end initialization function (we need to put as diff --git a/ompi/mpi/c/init_thread.c.in b/ompi/mpi/c/init_thread.c.in index 1e18b66040f..319448b82bc 100644 --- a/ompi/mpi/c/init_thread.c.in +++ b/ompi/mpi/c/init_thread.c.in @@ -42,12 +42,13 @@ PROTOTYPE ERROR_CLASS init_thread(INT_OUT argc, ARGV argv, INT required, { int err, safe_required = MPI_THREAD_SERIALIZED; bool err_arg_required = false; - char *env; ompi_hook_base_mpi_init_thread_top(argc, argv, required, provided); /* Detect an incorrect thread support level, but dont report until we have the minimum * infrastructure setup. + * In the future integer MPI_ABI values for MPI_THREAD_SINGLE-MULTIPLE + * may have gaps between them, so just checking the range is not enough. */ err_arg_required = (required != MPI_THREAD_SINGLE && required != MPI_THREAD_FUNNELED && required != MPI_THREAD_SERIALIZED && required != MPI_THREAD_MULTIPLE); @@ -61,18 +62,7 @@ PROTOTYPE ERROR_CLASS init_thread(INT_OUT argc, ARGV argv, INT required, * level (even if lower than argument `required`). A user program can * check `provided != required` to check if `required` has been overruled. */ - if (NULL != (env = getenv("OMPI_MPI_THREAD_LEVEL"))) { - int env_required = atoi(env); - /* In the future we may have to contend with non-sequential (MPI ABI) values - * If you are implementing MPI ABI changes please refer to - * https://github.com/open-mpi/ompi/pull/13211#discussion_r2085086844 - */ - err_arg_required |= (env_required != MPI_THREAD_SINGLE && env_required != MPI_THREAD_FUNNELED && - env_required != MPI_THREAD_SERIALIZED && env_required != MPI_THREAD_MULTIPLE); - if (!err_arg_required) { - safe_required = env_required; - } - } + err_arg_required |= (OMPI_SUCCESS > ompi_getenv_mpi_thread_level(&safe_required)); *provided = safe_required; diff --git a/ompi/runtime/mpiruntime.h b/ompi/runtime/mpiruntime.h index 5ce1ce53539..6fde46fb190 100644 --- a/ompi/runtime/mpiruntime.h +++ b/ompi/runtime/mpiruntime.h @@ -16,6 +16,7 @@ * Copyright (c) 2009 University of Houston. All rights reserved. * Copyright (c) 2014 Intel, Inc. All rights reserved. * Copyright (c) 2018 FUJITSU LIMITED. All rights reserved. + * Copyright (c) 2025 Advanced Micro Devices, Inc. All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -163,6 +164,27 @@ extern opal_hash_table_t ompi_mpi_f90_complex_hashtable; /** version string of ompi */ OMPI_DECLSPEC extern const char ompi_version_string[]; +/** + * Obtain the required thread level from environment (if any) + * + * @param requested Thread support that is requested (OUT) + * + * @returns Error code if environment exist but has an invalid value + * + * The function reads the environment variable OMPI_MPI_THREAD_LEVEL + * and set parameter requested accordingly. If the environment is not + * set, or has an invalid value, requested is left unchanged. + */ +int ompi_getenv_mpi_thread_level(int *requested); + +/** + * Determine the thread level + * + * @param requested Thread support that is requested (IN) + * @param provided Thread support that is provided (OUT) + */ +void ompi_mpi_thread_level(int requested, int *provided); + /** * Determine the thread level * diff --git a/ompi/runtime/ompi_mpi_init.c b/ompi/runtime/ompi_mpi_init.c index 19c0999d163..787e1e10249 100644 --- a/ompi/runtime/ompi_mpi_init.c +++ b/ompi/runtime/ompi_mpi_init.c @@ -29,6 +29,7 @@ * Copyright (c) 2021 Nanook Consulting. All rights reserved. * Copyright (c) 2021-2022 Triad National Security, LLC. All rights * reserved. + * Copyright (c) 2025 Advanced Micro Devices, Inc. All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -268,6 +269,42 @@ MPI_Fint *MPI_F08_STATUSES_IGNORE = NULL; #include "mpif-c-constants.h" +int ompi_getenv_mpi_thread_level(int *requested) +{ + char* env; + if (NULL != (env = getenv("OMPI_MPI_THREAD_LEVEL"))) { + /* deal with string values, int values (no atoi, it doesn't error check) */ + /* In the future integer MPI_ABI values for MPI_THREAD_SINGLE-MULTIPLE + * may be non-sequential (but ordered) integer values. + * If you are implementing MPI ABI changes please refer to + * https://github.com/open-mpi/ompi/pull/13211#discussion_r2085086844 + */ + if (0 == strcasecmp(env, "multiple") || + 0 == strcasecmp(env, "MPI_THREAD_MULTIPLE") || + 0 == strcmp(env, "3")) { + return *requested = MPI_THREAD_MULTIPLE; + } + if (0 == strcasecmp(env, "serialized") || + 0 == strcasecmp(env, "MPI_THREAD_SERIALIZED") || + 0 == strcmp(env, "2")) { + return *requested = MPI_THREAD_SERIALIZED; + } + if (0 == strcasecmp(env, "funneled") || + 0 == strcasecmp(env, "MPI_THREAD_FUNNELED") || + 0 == strcmp(env, "1")) { + return *requested = MPI_THREAD_FUNNELED; + } + if (0 == strcasecmp(env, "single") || + 0 == strcasecmp(env, "MPI_THREAD_SINGLE") || + 0 == strcmp(env, "0")) { + return *requested = MPI_THREAD_SINGLE; + } + /* the env value is invalid... */ + return OMPI_ERR_BAD_PARAM; + } + return OMPI_SUCCESS; +} + void ompi_mpi_thread_level(int requested, int *provided) { /**