Skip to content

v5.0.x: Add opal_recursive_mutex_t support to qthreads #9731

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 10, 2021

Conversation

awlauria
Copy link
Contributor

@awlauria awlauria commented Dec 6, 2021

Signed-off-by: Jan Ciesko jciesko@sandia.gov

(cherry picked from commit 8c499a3)

Signed-off-by: Jan Ciesko <jciesko@sandia.gov>

(cherry picked from commit 8c499a3)
@awlauria awlauria requested a review from hppritcha December 6, 2021 19:41
@awlauria awlauria changed the title Add opal_recursive_mutex_t support to qthreads v5.0.x: Add opal_recursive_mutex_t support to qthreads Dec 6, 2021
@awlauria awlauria added this to the v5.0.0 milestone Dec 6, 2021
@awlauria awlauria requested a review from gpaulsen December 6, 2021 19:42

opal_atomic_lock_init(&m->m_lock, 0);
opal_atomic_lock_init(&m->m_lock_atomic, 0);
m->m_recursive = 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't there be some check in opal_mutex_trylock whether m_recursive == 1 and which thread owns the mutex?

@devreal
Copy link
Contributor

devreal commented Dec 6, 2021

Recursive mutex support was eventually removed in bcd376a from the argobots and qthreads modules. @awlauria Are you planning to bring that back to 5.0.x too?

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)
@awlauria
Copy link
Contributor Author

awlauria commented Dec 7, 2021

@devreal done. the cherry-pick was easier to do here instead of its own commit.

@awlauria awlauria merged commit b6dbae2 into open-mpi:v5.0.x Dec 10, 2021
@awlauria awlauria deleted the opal_recursive_mutex_t_v5.0.x branch December 10, 2021 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants