Skip to content

Commit 7991483

Browse files
jsquyresjsquyres
committed
mpi/finalized: revamp INITIALIZED/FINALIZED
Per MPI-3.1:8.7.1 p361:11-13, it's valid for MPI_FINALIZED to be invoked during an attribute destruction callback (e.g., during the destruction of keyvals on MPI_COMM_SELF during the very beginning of MPI_FINALIZE). In such cases, MPI_FINALIZED must return "false". Prior to this commit, we hung in FINALIZED if it were invoked during a COMM_SELF attribute destruction callback in FINALIZE. See open-mpi#5084. This commit converts the MPI_INITIALIZED / MPI_FINALIZED infrastructure to use a single enum (ompi_mpi_state, set atomically) to represent the state of MPI: - not initialized - init started - init completed - finalize started - finalize past COMM_SELF destruction - finalize completed The "finalize past COMM_SELF destruction" state is what allows us to return "false" from MPI_FINALIZED before COMM_SELF has been fully destroyed / all attribute callbacks have been invoked. Since this state is checked at nearly every MPI API call (to see if we're outside of the INIT/FINALIZE epoch), care was taken to use atomics to *set* the ompi_mpi_state value in ompi_mpi_init() and ompi_mpi_finalize(), but performance-critical code paths can simply read the variable without needing to use a slow call to an opal_atomic_*() function. Thanks to @AndrewGaspar for reporting the issue. Signed-off-by: Jeff Squyres <[email protected]>
1 parent 09f73f1 commit 7991483

19 files changed

+143
-91
lines changed

‎ompi/errhandler/errhandler.h

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* University of Stuttgart. All rights reserved.
1111
* Copyright (c) 2004-2005 The Regents of the University of California.
1212
* All rights reserved.
13-
* Copyright (c) 2008-2012 Cisco Systems, Inc. All rights reserved.
13+
* Copyright (c) 2008-2018 Cisco Systems, Inc. All rights reserved
1414
* Copyright (c) 2008-2009 Sun Microsystems, Inc. All rights reserved.
1515
* Copyright (c) 2015-2016 Intel, Inc. All rights reserved.
1616
* Copyright (c) 2016 Los Alamos National Security, LLC. All rights
@@ -193,11 +193,22 @@ struct ompi_request_t;
193193
* This macro directly invokes the ompi_mpi_errors_are_fatal_handler()
194194
* when an error occurs because MPI_COMM_WORLD does not exist (because
195195
* we're before MPI_Init() or after MPI_Finalize()).
196+
*
197+
* NOTE: The ompi_mpi_state variable is a volatile that is set
198+
* atomically in ompi_mpi_init() and ompi_mpi_finalize(). The
199+
* appropriate memory barriers are done in those 2 functions such that
200+
* we do not need to do a read memory barrier here (in
201+
* potentially-performance-critical code paths) before reading the
202+
* variable.
196203
*/
197-
#define OMPI_ERR_INIT_FINALIZE(name) \
198-
if( OPAL_UNLIKELY(!ompi_mpi_initialized || ompi_mpi_finalized) ) { \
199-
ompi_mpi_errors_are_fatal_comm_handler(NULL, NULL, name); \
200-
}
204+
#define OMPI_ERR_INIT_FINALIZE(name) \
205+
{ \
206+
int32_t state = ompi_mpi_state; \
207+
if (OPAL_UNLIKELY(state < OMPI_MPI_STATE_INIT_COMPLETED || \
208+
state > OMPI_MPI_STATE_FINALIZE_PAST_COMM_SELF_DESTRUCT)) { \
209+
ompi_mpi_errors_are_fatal_comm_handler(NULL, NULL, name); \
210+
} \
211+
}
201212

202213
/**
203214
* This is the macro to invoke to directly invoke an MPI error

‎ompi/errhandler/errhandler_predefined.c

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* Copyright (c) 2004-2005 The Regents of the University of California.
1111
* All rights reserved.
1212
* Copyright (c) 2006 University of Houston. All rights reserved.
13-
* Copyright (c) 2008-2013 Cisco Systems, Inc. All rights reserved.
13+
* Copyright (c) 2008-2018 Cisco Systems, Inc. All rights reserved
1414
* Copyright (c) 2009 Sun Microsystems, Inc. All rights reserved.
1515
* Copyright (c) 2010-2011 Oak Ridge National Labs. All rights reserved.
1616
* Copyright (c) 2012 Los Alamos National Security, LLC.
@@ -149,7 +149,9 @@ void ompi_mpi_errors_return_win_handler(struct ompi_win_t **win,
149149

150150
static void out(char *str, char *arg)
151151
{
152-
if (ompi_rte_initialized && !ompi_mpi_finalized) {
152+
int32_t state = ompi_mpi_state;
153+
if (ompi_rte_initialized &&
154+
state < OMPI_MPI_STATE_FINALIZE_PAST_COMM_SELF_DESTRUCT) {
153155
if (NULL != arg) {
154156
opal_output(0, str, arg);
155157
} else {
@@ -280,7 +282,9 @@ static void backend_fatal_no_aggregate(char *type,
280282
{
281283
char *arg;
282284

283-
assert(!ompi_mpi_initialized || ompi_mpi_finalized);
285+
int32_t state = ompi_mpi_state;
286+
assert(state < OMPI_MPI_STATE_INIT_COMPLETED ||
287+
state >= OMPI_MPI_STATE_FINALIZE_PAST_COMM_SELF_DESTRUCT);
284288

285289
fflush(stdout);
286290
fflush(stderr);
@@ -289,7 +293,7 @@ static void backend_fatal_no_aggregate(char *type,
289293

290294
/* Per #2152, print out in plain english if something was invoked
291295
before MPI_INIT* or after MPI_FINALIZE */
292-
if (!ompi_mpi_init_started && !ompi_mpi_initialized) {
296+
if (state < OMPI_MPI_STATE_INIT_STARTED) {
293297
if (NULL != arg) {
294298
out("*** The %s() function was called before MPI_INIT was invoked.\n"
295299
"*** This is disallowed by the MPI standard.\n", arg);
@@ -300,7 +304,7 @@ static void backend_fatal_no_aggregate(char *type,
300304
"*** function was invoked, sorry. :-(\n", NULL);
301305
}
302306
out("*** Your MPI job will now abort.\n", NULL);
303-
} else if (ompi_mpi_finalized) {
307+
} else if (state >= OMPI_MPI_STATE_FINALIZE_PAST_COMM_SELF_DESTRUCT) {
304308
if (NULL != arg) {
305309
out("*** The %s() function was called after MPI_FINALIZE was invoked.\n"
306310
"*** This is disallowed by the MPI standard.\n", arg);

‎ompi/mca/coll/fca/coll_fca_ops.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* Copyright (c) 2011 Mellanox Technologies. All rights reserved.
33
* Copyright (c) 2015 Research Organization for Information Science
44
* and Technology (RIST). All rights reserved.
5+
* Copyright (c) 2018 Cisco Systems, Inc. All rights reserved
56
* $COPYRIGHT$
67
*
78
* Additional copyrights may follow
@@ -159,7 +160,8 @@ int mca_coll_fca_barrier(struct ompi_communicator_t *comm,
159160
int ret;
160161

161162
FCA_VERBOSE(5,"Using FCA Barrier");
162-
if (OPAL_UNLIKELY(ompi_mpi_finalize_started)) {
163+
int32_t state = ompi_mpi_state;
164+
if (OPAL_UNLIKELY(state >= OMPI_MPI_STATE_FINALIZE_STARTED)) {
163165
FCA_VERBOSE(5, "In finalize, reverting to previous barrier");
164166
goto orig_barrier;
165167
}

‎ompi/mca/coll/hcoll/coll_hcoll_module.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
* Copyright (c) 2017 The University of Tennessee and The University
55
* of Tennessee Research Foundation. All rights
66
* reserved.
7+
* Copyright (c) 2018 Cisco Systems, Inc. All rights reserved
78
* $COPYRIGHT$
89
*
910
* Additional copyrights may follow
@@ -241,7 +242,8 @@ static int mca_coll_hcoll_module_enable(mca_coll_base_module_t *module,
241242

242243
int mca_coll_hcoll_progress(void)
243244
{
244-
if (ompi_mpi_finalized){
245+
int32_t state = ompi_mpi_state;
246+
if (state >= OMPI_MPI_STATE_FINALIZE_STARTED) {
245247
hcoll_rte_p2p_disabled_notify();
246248
}
247249

‎ompi/mca/coll/hcoll/coll_hcoll_ops.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
Copyright (c) 2011 Mellanox Technologies. All rights reserved.
33
Copyright (c) 2015 Research Organization for Information Science
44
and Technology (RIST). All rights reserved.
5+
Copyright (c) 2018 Cisco Systems, Inc. All rights reserved
56
$COPYRIGHT$
67
78
Additional copyrights may follow
@@ -21,7 +22,8 @@ int mca_coll_hcoll_barrier(struct ompi_communicator_t *comm,
2122
mca_coll_hcoll_module_t *hcoll_module = (mca_coll_hcoll_module_t*)module;
2223
HCOL_VERBOSE(20,"RUNNING HCOL BARRIER");
2324

24-
if (OPAL_UNLIKELY(ompi_mpi_finalize_started)) {
25+
int32_t state = ompi_mpi_state;
26+
if (OPAL_UNLIKELY(state >= OMPI_MPI_STATE_FINALIZE_STARTED)) {
2527
HCOL_VERBOSE(5, "In finalize, reverting to previous barrier");
2628
goto orig_barrier;
2729
}

‎ompi/mca/io/romio314/src/io_romio314_file_open.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
* Copyright (c) 2015 Research Organization for Information Science
1313
* and Technology (RIST). All rights reserved.
1414
* Copyright (c) 2016-2017 IBM Corporation. All rights reserved.
15+
* Copyright (c) 2018 Cisco Systems, Inc. All rights reserved
1516
* $COPYRIGHT$
1617
*
1718
* Additional copyrights may follow
@@ -69,7 +70,8 @@ mca_io_romio314_file_close (ompi_file_t *fh)
6970
which we obviously can't do if we've started to MPI_Finalize).
7071
The user didn't close the file, so they should expect
7172
unexpected behavior. */
72-
if (ompi_mpi_finalized) {
73+
int32_t state = ompi_mpi_state;
74+
if (state >= OMPI_MPI_STATE_FINALIZE_PAST_COMM_SELF_DESTRUCT) {
7375
return OMPI_SUCCESS;
7476
}
7577

‎ompi/mca/pml/yalla/pml_yalla.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* Copyright (C) 2001-2011 Mellanox Technologies Ltd. ALL RIGHTS RESERVED.
33
* Copyright (c) 2015 Research Organization for Information Science
44
* and Technology (RIST). All rights reserved.
5+
* Copyright (c) 2018 Cisco Systems, Inc. All rights reserved
56
* $COPYRIGHT$
67
*
78
* Additional copyrights may follow
@@ -264,8 +265,9 @@ int mca_pml_yalla_add_procs(struct ompi_proc_t **procs, size_t nprocs)
264265
int mca_pml_yalla_del_procs(struct ompi_proc_t **procs, size_t nprocs)
265266
{
266267
size_t i;
268+
int32_t state = ompi_mpi_state;
267269

268-
if (ompi_mpi_finalized) {
270+
if (state >= OMPI_MPI_STATE_FINALIZE_STARTED) {
269271
PML_YALLA_VERBOSE(3, "%s", "using bulk powerdown");
270272
mxm_ep_powerdown(ompi_pml_yalla.mxm_ep);
271273
}

‎ompi/mpi/c/finalized.c

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
* All rights reserved.
1212
* Copyright (c) 2015 Research Organization for Information Science
1313
* and Technology (RIST). All rights reserved.
14-
* Copyright (c) 2015 Cisco Systems, Inc. All rights reserved.
14+
* Copyright (c) 2015-2018 Cisco Systems, Inc. All rights reserved
1515
* Copyright (c) 2015 Intel, Inc. All rights reserved
1616
* $COPYRIGHT$
1717
*
@@ -44,13 +44,7 @@ int MPI_Finalized(int *flag)
4444

4545
ompi_hook_base_mpi_finalized_top(flag);
4646

47-
/* We must obtain the lock to guarnatee consistent values of
48-
ompi_mpi_initialized and ompi_mpi_finalized. Note, too, that
49-
this lock is held for the bulk of the duration of
50-
ompi_mpi_init() and ompi_mpi_finalize(), so when we get the
51-
lock, we are guaranteed that some other thread is not part way
52-
through initialization or finalization. */
53-
opal_mutex_lock(&ompi_mpi_bootstrap_mutex);
47+
int32_t state = ompi_mpi_state;
5448

5549
if (MPI_PARAM_CHECK) {
5650
if (NULL == flag) {
@@ -59,12 +53,11 @@ int MPI_Finalized(int *flag)
5953
whether we're currently (after MPI_Init and before
6054
MPI_Finalize) or not */
6155

62-
if (ompi_mpi_initialized && !ompi_mpi_finalized) {
63-
opal_mutex_unlock(&ompi_mpi_bootstrap_mutex);
56+
if (state >= OMPI_MPI_STATE_INIT_COMPLETED &&
57+
state < OMPI_MPI_STATE_FINALIZE_PAST_COMM_SELF_DESTRUCT) {
6458
return OMPI_ERRHANDLER_INVOKE(MPI_COMM_WORLD, MPI_ERR_ARG,
6559
FUNC_NAME);
6660
} else {
67-
opal_mutex_unlock(&ompi_mpi_bootstrap_mutex);
6861
/* We have no MPI object here so call ompi_errhandle_invoke
6962
* directly */
7063
return ompi_errhandler_invoke(NULL, NULL, -1,
@@ -74,8 +67,7 @@ int MPI_Finalized(int *flag)
7467
}
7568
}
7669

77-
*flag = ompi_mpi_finalized;
78-
opal_mutex_unlock(&ompi_mpi_bootstrap_mutex);
70+
*flag = (state >= OMPI_MPI_STATE_FINALIZE_PAST_COMM_SELF_DESTRUCT);
7971

8072
ompi_hook_base_mpi_finalized_bottom(flag);
8173

‎ompi/mpi/c/get_library_version.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
* University of Stuttgart. All rights reserved.
1010
* Copyright (c) 2004-2005 The Regents of the University of California.
1111
* All rights reserved.
12-
* Copyright (c) 2014-2015 Cisco Systems, Inc. All rights reserved.
12+
* Copyright (c) 2014-2018 Cisco Systems, Inc. All rights reserved
1313
* Copyright (c) 2015 Research Organization for Information Science
1414
* and Technology (RIST). All rights reserved.
1515
* Copyright (c) 2015 Intel, Inc. All rights reserved
@@ -58,7 +58,9 @@ int MPI_Get_library_version(char *version, int *resultlen)
5858
(i.e., use a NULL communicator, which will end up at the
5959
default errhandler, which is abort). */
6060

61-
if (ompi_mpi_initialized && !ompi_mpi_finalized) {
61+
int32_t state = ompi_mpi_state;
62+
if (state >= OMPI_MPI_STATE_INIT_COMPLETED &&
63+
state < OMPI_MPI_STATE_FINALIZE_PAST_COMM_SELF_DESTRUCT) {
6264
return OMPI_ERRHANDLER_INVOKE(MPI_COMM_WORLD, MPI_ERR_ARG,
6365
FUNC_NAME);
6466
} else {

‎ompi/mpi/c/get_version.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
* Copyright (c) 2015 Research Organization for Information Science
1313
* and Technology (RIST). All rights reserved.
1414
* Copyright (c) 2015 Intel, Inc. All rights reserved
15+
* Copyright (c) 2018 Cisco Systems, Inc. All rights reserved
1516
* $COPYRIGHT$
1617
*
1718
* Additional copyrights may follow
@@ -54,7 +55,9 @@ int MPI_Get_version(int *version, int *subversion)
5455
(i.e., use a NULL communicator, which will end up at the
5556
default errhandler, which is abort). */
5657

57-
if (ompi_mpi_initialized && !ompi_mpi_finalized) {
58+
int32_t state = ompi_mpi_state;
59+
if (state >= OMPI_MPI_STATE_INIT_COMPLETED &&
60+
state < OMPI_MPI_STATE_FINALIZE_PAST_COMM_SELF_DESTRUCT) {
5861
return OMPI_ERRHANDLER_INVOKE(MPI_COMM_WORLD, MPI_ERR_ARG,
5962
FUNC_NAME);
6063
} else {

0 commit comments

Comments
 (0)