Skip to content

Commit 8eb629e

Browse files
shintaro-iwasakiawlauria
shintaro-iwasaki
authored and
awlauria
committed
opal/mca/threads/pthreads: remove opal_pthread_mutex_t
opal_pthread_mutex_t is not used. This patch removes it. Signed-off-by: Shintaro Iwasaki <siwasaki@anl.gov> opal/mca/threads/qthreads,argobots: remove m_recursive m_recursive is not used. This patch removes it. Signed-off-by: Shintaro Iwasaki <siwasaki@anl.gov> opal/mca/threads/qthreads,argobots: update wait_sync implementation This patch is corresponding to changes to the following commit for Pthreads: 6a406fb Signed-off-by: Shintaro Iwasaki <siwasaki@anl.gov> opal/mca/threads/pthreads: implement opal_thread_internal_mutex/cond_t opal_thread_internal_mutex_t and opal_thread_internal_cond_t are new objects that are directly mapped to raw mutex/condition variable implementations (e.g., pthread_mutex_t for Pthreads). This abstraction is different from opal_mutex_t, which has more functionalities. Introducing this abstraction helps removal of code duplication among other threading layers. Signed-off-by: Shintaro Iwasaki <siwasaki@anl.gov> opal/mca/threads/pthreads: use opal_thread_internal_mutex/cond_t opal_thread_internal_mutex/cond routines are static inline functions, so there should be no additional overhead. Signed-off-by: Shintaro Iwasaki <siwasaki@anl.gov> opal/mca/threads/argobots: implement opal_thread_internal_mutex/cond_t Signed-off-by: Shintaro Iwasaki <siwasaki@anl.gov> opal/mca/threads/argobots: use opal_thread_internal_mutex/cond_t Signed-off-by: Shintaro Iwasaki <siwasaki@anl.gov> opal/mca/threads/qthreads: implement opal_thread_internal_mutex/cond_t Signed-off-by: Shintaro Iwasaki <siwasaki@anl.gov> opal/mca/threads/qthreads: use opal_thread_internal_mutex/cond_t Signed-off-by: Shintaro Iwasaki <siwasaki@anl.gov> opal/mca/threads: move mutex implementation to threads/base Now implementations of opal_mutex_xxx() and opal_cond_xxx() are the same. This patch moves them to base/mutex.c and mutex.h and removes the duplication. Note that those implementations are copied from pthread/threads_pthreads_mutex.c and pthread/threads_pthreads_mutex.h. Signed-off-by: Shintaro Iwasaki <siwasaki@anl.gov> opal/mca/threads: use opal_thread_yield() in busy loop if requested This commit is prerequisite for the next commit that unifies the implementation of wait_sync. Nonpreemptive threads like Qthreads and Argobots need a yield in a busy loop to avoid a deadlock, while Pthreads does not. To unify the implementation in the next commit, this patch adds opal_thread_yield() if opal_progress_yield_when_idle is true. At the same time, this patch removes a yield operation after opal_progress() since opal_progress() internally yields when it does not make any progress. Signed-off-by: Shintaro Iwasaki <siwasaki@anl.gov> opal/mca/threads: move wait_sync implementation to threads/base Now implementations of wait_sync_xxx() functions and macros are the same. This patch moves them to base/wait_sync.c and wait_sync.h and removes the duplication. Note that those implementations are copied from pthread/threads_pthreads_wait_sync.c and pthread/threads_pthreads_wait_sync.h. Signed-off-by: Shintaro Iwasaki <siwasaki@anl.gov> opal/mca/threads: change Pthread-specific names This patch renames variables and functions that are no longer specific to Pthread. Signed-off-by: Shintaro Iwasaki <siwasaki@anl.gov> opal/mca/threads: remove MCA_threads_wait_sync_base_include_xxx MCA_threads_wait_sync_base_include_xxx is no longer used. This patch removes them. Signed-off-by: Shintaro Iwasaki <siwasaki@anl.gov> opal/mca/threads: minor cleanup - Remove unnecessary "include" - Remove weird empty lines - Clean up parentheses for macros - Remove EDEADLK check for trylock() and unlock() Signed-off-by: Shintaro Iwasaki <siwasaki@anl.gov> (cherry picked from commit bcd376a)
1 parent cbd24fc commit 8eb629e

21 files changed

+516
-1301
lines changed

‎opal/mca/threads/argobots/Makefile.am

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
# All rights reserved.
1212
# Copyright (c) 2008 Cisco Systems, Inc. All rights reserved.
1313
# Copyright (c) 2019 Sandia National Laboratories. All rights reserved.
14+
# Copyright (c) 2021 Argonne National Laboratory. All rights reserved.
15+
#
1416
# $COPYRIGHT$
1517
#
1618
# Additional copyrights may follow
@@ -25,12 +27,9 @@ sources = \
2527
threads_argobots_component.c \
2628
threads_argobots_condition.c \
2729
threads_argobots_module.c \
28-
threads_argobots_mutex.c \
2930
threads_argobots_mutex.h \
3031
threads_argobots_threads.h \
31-
threads_argobots_tsd.h \
32-
threads_argobots_wait_sync.c \
33-
threads_argobots_wait_sync.h
32+
threads_argobots_tsd.h
3433

3534
#lib = libmca_threads_argobots.la
3635
lib_sources = $(sources)

‎opal/mca/threads/argobots/configure.m4

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,6 @@ AC_DEFUN([MCA_opal_threads_argobots_POST_CONFIG],[
123123
AC_DEFINE_UNQUOTED([MCA_threads_tsd_base_include_HEADER],
124124
["opal/mca/threads/argobots/threads_argobots_tsd.h"],
125125
[Header to include for tsd implementation])
126-
AC_DEFINE_UNQUOTED([MCA_threads_wait_sync_base_include_HEADER],
127-
["opal/mca/threads/argobots/threads_argobots_wait_sync.h"],
128-
[Header to include for wait_sync implementation])
129126
THREAD_CFLAGS="$TPKG_CFLAGS"
130127
THREAD_FCFLAGS="$TPKG_FCFLAGS"
131128
THREAD_CXXFLAGS="$TPKG_CXXFLAGS"

‎opal/mca/threads/argobots/threads_argobots_mutex.h

Lines changed: 71 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -31,108 +31,71 @@
3131

3232
#include "opal_config.h"
3333

34-
#include <errno.h>
3534
#include <stdio.h>
35+
#include <string.h>
3636

3737
#include "opal/class/opal_object.h"
38+
#include "opal/constants.h"
3839
#include "opal/mca/threads/argobots/threads_argobots.h"
3940
#include "opal/mca/threads/mutex.h"
40-
#include "opal/sys/atomic.h"
4141
#include "opal/util/output.h"
4242

4343
BEGIN_C_DECLS
4444

45-
struct opal_mutex_t {
46-
opal_object_t super;
45+
typedef ABT_mutex_memory opal_thread_internal_mutex_t;
4746

48-
ABT_mutex_memory m_lock_argobots;
49-
int m_recursive;
47+
#define OPAL_THREAD_INTERNAL_MUTEX_INITIALIZER ABT_MUTEX_INITIALIZER
48+
#define OPAL_THREAD_INTERNAL_RECURSIVE_MUTEX_INITIALIZER ABT_RECURSIVE_MUTEX_INITIALIZER
5049

51-
#if OPAL_ENABLE_DEBUG
52-
int m_lock_debug;
53-
const char *m_lock_file;
54-
int m_lock_line;
55-
#endif
56-
57-
opal_atomic_lock_t m_lock_atomic;
58-
};
59-
60-
OPAL_DECLSPEC OBJ_CLASS_DECLARATION(opal_mutex_t);
61-
OPAL_DECLSPEC OBJ_CLASS_DECLARATION(opal_recursive_mutex_t);
62-
63-
#if OPAL_ENABLE_DEBUG
64-
# define OPAL_MUTEX_STATIC_INIT \
65-
{ \
66-
.super = OPAL_OBJ_STATIC_INIT(opal_mutex_t), .m_lock_argobots = ABT_MUTEX_INITIALIZER, \
67-
.m_recursive = 0, .m_lock_debug = 0, .m_lock_file = NULL, .m_lock_line = 0, \
68-
.m_lock_atomic = OPAL_ATOMIC_LOCK_INIT, \
69-
}
70-
#else
71-
# define OPAL_MUTEX_STATIC_INIT \
72-
{ \
73-
.super = OPAL_OBJ_STATIC_INIT(opal_mutex_t), .m_lock_argobots = ABT_MUTEX_INITIALIZER, \
74-
.m_recursive = 0, .m_lock_atomic = OPAL_ATOMIC_LOCK_INIT, \
75-
}
76-
#endif
50+
static inline int opal_thread_internal_mutex_init(opal_thread_internal_mutex_t *p_mutex,
51+
bool recursive)
52+
{
53+
if (recursive) {
54+
const ABT_mutex_memory init_mutex = ABT_RECURSIVE_MUTEX_INITIALIZER;
55+
memcpy(p_mutex, &init_mutex, sizeof(ABT_mutex_memory));
56+
} else {
57+
const ABT_mutex_memory init_mutex = ABT_MUTEX_INITIALIZER;
58+
memcpy(p_mutex, &init_mutex, sizeof(ABT_mutex_memory));
59+
}
60+
return OPAL_SUCCESS;
61+
}
7762

63+
static inline void opal_thread_internal_mutex_lock(opal_thread_internal_mutex_t *p_mutex)
64+
{
65+
ABT_mutex mutex = ABT_MUTEX_MEMORY_GET_HANDLE(p_mutex);
7866
#if OPAL_ENABLE_DEBUG
79-
# define OPAL_RECURSIVE_MUTEX_STATIC_INIT \
80-
{ \
81-
.super = OPAL_OBJ_STATIC_INIT(opal_mutex_t), \
82-
.m_lock_argobots = ABT_RECURSIVE_MUTEX_INITIALIZER, .m_recursive = 1, \
83-
.m_lock_debug = 0, .m_lock_file = NULL, .m_lock_line = 0, \
84-
.m_lock_atomic = OPAL_ATOMIC_LOCK_INIT, \
85-
}
67+
int ret = ABT_mutex_lock(mutex);
68+
if (ABT_SUCCESS != ret) {
69+
opal_output(0, "opal_thread_internal_mutex_lock()");
70+
}
8671
#else
87-
# define OPAL_RECURSIVE_MUTEX_STATIC_INIT \
88-
{ \
89-
.super = OPAL_OBJ_STATIC_INIT(opal_mutex_t), \
90-
.m_lock_argobots = ABT_RECURSIVE_MUTEX_INITIALIZER, .m_recursive = 1, \
91-
.m_lock_atomic = OPAL_ATOMIC_LOCK_INIT, \
92-
}
72+
ABT_mutex_lock(mutex);
9373
#endif
74+
}
9475

95-
/************************************************************************
96-
*
97-
* mutex operations (non-atomic versions)
98-
*
99-
************************************************************************/
100-
101-
static inline int opal_mutex_trylock(opal_mutex_t *m)
76+
static inline int opal_thread_internal_mutex_trylock(opal_thread_internal_mutex_t *p_mutex)
10277
{
103-
ABT_mutex mutex = ABT_MUTEX_MEMORY_GET_HANDLE(&m->m_lock_argobots);
78+
ABT_mutex mutex = ABT_MUTEX_MEMORY_GET_HANDLE(p_mutex);
10479
int ret = ABT_mutex_trylock(mutex);
10580
if (ABT_ERR_MUTEX_LOCKED == ret) {
10681
return 1;
10782
} else if (ABT_SUCCESS != ret) {
10883
#if OPAL_ENABLE_DEBUG
109-
opal_output(0, "opal_mutex_trylock()");
84+
opal_output(0, "opal_thread_internal_mutex_trylock()");
11085
#endif
11186
return 1;
87+
} else {
88+
return 0;
11289
}
113-
return 0;
11490
}
11591

116-
static inline void opal_mutex_lock(opal_mutex_t *m)
92+
static inline void opal_thread_internal_mutex_unlock(opal_thread_internal_mutex_t *p_mutex)
11793
{
118-
ABT_mutex mutex = ABT_MUTEX_MEMORY_GET_HANDLE(&m->m_lock_argobots);
119-
#if OPAL_ENABLE_DEBUG
120-
int ret = ABT_mutex_lock(mutex);
121-
if (ABT_SUCCESS != ret) {
122-
opal_output(0, "opal_mutex_lock()");
123-
}
124-
#else
125-
ABT_mutex_lock(mutex);
126-
#endif
127-
}
128-
129-
static inline void opal_mutex_unlock(opal_mutex_t *m)
130-
{
131-
ABT_mutex mutex = ABT_MUTEX_MEMORY_GET_HANDLE(&m->m_lock_argobots);
94+
ABT_mutex mutex = ABT_MUTEX_MEMORY_GET_HANDLE(p_mutex);
13295
#if OPAL_ENABLE_DEBUG
13396
int ret = ABT_mutex_unlock(mutex);
13497
if (ABT_SUCCESS != ret) {
135-
opal_output(0, "opal_mutex_unlock()");
98+
opal_output(0, "opal_thread_internal_mutex_unlock()");
13699
}
137100
#else
138101
ABT_mutex_unlock(mutex);
@@ -141,65 +104,62 @@ static inline void opal_mutex_unlock(opal_mutex_t *m)
141104
ABT_thread_yield();
142105
}
143106

144-
/************************************************************************
145-
*
146-
* mutex operations (atomic versions)
147-
*
148-
************************************************************************/
107+
static inline void opal_thread_internal_mutex_destroy(opal_thread_internal_mutex_t *p_mutex)
108+
{
109+
/* No specific operation is needed to destroy opal_thread_internal_mutex_t. */
110+
}
149111

150-
#if OPAL_HAVE_ATOMIC_SPINLOCKS
112+
typedef ABT_cond_memory opal_thread_internal_cond_t;
151113

152-
/************************************************************************
153-
* Spin Locks
154-
************************************************************************/
114+
#define OPAL_THREAD_INTERNAL_COND_INITIALIZER ABT_COND_INITIALIZER
155115

156-
static inline int opal_mutex_atomic_trylock(opal_mutex_t *m)
116+
static inline int opal_thread_internal_cond_init(opal_thread_internal_cond_t *p_cond)
157117
{
158-
return opal_atomic_trylock(&m->m_lock_atomic);
118+
const ABT_cond_memory init_cond = ABT_COND_INITIALIZER;
119+
memcpy(p_cond, &init_cond, sizeof(ABT_cond_memory));
120+
return OPAL_SUCCESS;
159121
}
160122

161-
static inline void opal_mutex_atomic_lock(opal_mutex_t *m)
123+
static inline void opal_thread_internal_cond_wait(opal_thread_internal_cond_t *p_cond,
124+
opal_thread_internal_mutex_t *p_mutex)
162125
{
163-
opal_atomic_lock(&m->m_lock_atomic);
126+
ABT_mutex mutex = ABT_MUTEX_MEMORY_GET_HANDLE(p_mutex);
127+
ABT_cond cond = ABT_COND_MEMORY_GET_HANDLE(p_cond);
128+
#if OPAL_ENABLE_DEBUG
129+
int ret = ABT_cond_wait(cond, mutex);
130+
assert(ABT_SUCCESS == ret);
131+
#else
132+
ABT_cond_wait(cond, mutex);
133+
#endif
164134
}
165135

166-
static inline void opal_mutex_atomic_unlock(opal_mutex_t *m)
136+
static inline void opal_thread_internal_cond_broadcast(opal_thread_internal_cond_t *p_cond)
167137
{
168-
opal_atomic_unlock(&m->m_lock_atomic);
169-
}
170-
138+
ABT_cond cond = ABT_COND_MEMORY_GET_HANDLE(p_cond);
139+
#if OPAL_ENABLE_DEBUG
140+
int ret = ABT_cond_broadcast(cond);
141+
assert(ABT_SUCCESS == ret);
171142
#else
172-
173-
/************************************************************************
174-
* Standard locking
175-
************************************************************************/
176-
177-
static inline int opal_mutex_atomic_trylock(opal_mutex_t *m)
178-
{
179-
return opal_mutex_trylock(m);
143+
ABT_cond_broadcast(cond);
144+
#endif
180145
}
181146

182-
static inline void opal_mutex_atomic_lock(opal_mutex_t *m)
147+
static inline void opal_thread_internal_cond_signal(opal_thread_internal_cond_t *p_cond)
183148
{
184-
opal_mutex_lock(m);
149+
ABT_cond cond = ABT_COND_MEMORY_GET_HANDLE(p_cond);
150+
#if OPAL_ENABLE_DEBUG
151+
int ret = ABT_cond_signal(cond);
152+
assert(ABT_SUCCESS == ret);
153+
#else
154+
ABT_cond_signal(cond);
155+
#endif
185156
}
186157

187-
static inline void opal_mutex_atomic_unlock(opal_mutex_t *m)
158+
static inline void opal_thread_internal_cond_destroy(opal_thread_internal_cond_t *p_cond)
188159
{
189-
opal_mutex_unlock(m);
160+
/* No destructor is needed. */
190161
}
191162

192-
#endif
193-
194-
typedef ABT_cond_memory opal_cond_t;
195-
#define OPAL_CONDITION_STATIC_INIT ABT_COND_INITIALIZER
196-
197-
int opal_cond_init(opal_cond_t *cond);
198-
int opal_cond_wait(opal_cond_t *cond, opal_mutex_t *lock);
199-
int opal_cond_broadcast(opal_cond_t *cond);
200-
int opal_cond_signal(opal_cond_t *cond);
201-
int opal_cond_destroy(opal_cond_t *cond);
202-
203163
END_C_DECLS
204164

205165
#endif /* OPAL_MCA_THREADS_ARGOBOTS_THREADS_ARGOBOTS_MUTEX_H */

0 commit comments

Comments
 (0)