[mpich-commits] [mpich] MPICH primary repository branch, master, updated. v3.2b1-96-gabb5676
Service Account
noreply at mpich.org
Wed Apr 22 19:54:27 CDT 2015
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "MPICH primary repository".
The branch, master has been updated
via abb56764b8dff75e636c9da985c43335742015aa (commit)
via b39314a5b24242907c5819a0e43dc4b19728fada (commit)
via f385680e392d999ad5e2f7ddcc625a0f6556f014 (commit)
from 5fb750b97cb0a95d13a1b31a1952100c58c7dfd6 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
http://git.mpich.org/mpich.git/commitdiff/abb56764b8dff75e636c9da985c43335742015aa
commit abb56764b8dff75e636c9da985c43335742015aa
Author: Pavan Balaji <balaji at anl.gov>
Date: Sat Apr 18 04:46:33 2015 -0500
Fix arbitrary poll count before yielding.
Instead of polling for an arbitrarily decided number of times in the
progress engine before yielding, we now moved the yielding
intelligence to the threading layer. The threading layer can keep
track of other threads that are waiting to enter the critical section
and only yield if another thread is waiting. In this way, if no
thread is waiting to get the lock, the main thread never yields. At
the same time, if another thread is waiting to get a lock, there is no
delay in yielding.
This change, however, introduces possible deadlocks. If a thread enters
MPIDI_CH3I_progress with is_blocking unset, it may set the
MPIDI_CH3I_progress_blocked flag and then will yield the critical section.
Another thread may enter with is_blocking set, find the flag
MPIDI_CH3I_progress_blocked set, and block in the conditional variable.
The first thread will wake up and leave the progress engine without
emitting any signal to wake up the second thread which may sleep forever.
A simple fix is to yield the critical section only if the current thread
entered the progress engine with is_blocking set.
Signed-off-by: Halim Amer <aamer at anl.gov>
diff --git a/src/mpid/ch3/channels/nemesis/include/mpid_nem_inline.h b/src/mpid/ch3/channels/nemesis/include/mpid_nem_inline.h
index 9601eac..e537dc2 100644
--- a/src/mpid/ch3/channels/nemesis/include/mpid_nem_inline.h
+++ b/src/mpid/ch3/channels/nemesis/include/mpid_nem_inline.h
@@ -7,8 +7,6 @@
#ifndef _MPID_NEM_INLINE_H
#define _MPID_NEM_INLINE_H
-#define MPID_NEM_THREAD_POLLS_BEFORE_YIELD 10
-
#include "my_papi_defs.h"
#include "mpiiov.h"
#include "mpidi_nem_statistics.h"
diff --git a/src/mpid/ch3/channels/nemesis/src/ch3_progress.c b/src/mpid/ch3/channels/nemesis/src/ch3_progress.c
index 422c533..351bf81 100644
--- a/src/mpid/ch3/channels/nemesis/src/ch3_progress.c
+++ b/src/mpid/ch3/channels/nemesis/src/ch3_progress.c
@@ -289,9 +289,6 @@ int MPIDI_CH3I_Shm_send_progress(void)
int MPIDI_CH3I_Progress (MPID_Progress_state *progress_state, int is_blocking)
{
int mpi_errno = MPI_SUCCESS;
-#ifdef MPICH_IS_THREADED
- int pollcount = 0;
-#endif
int made_progress = FALSE;
MPIDI_STATE_DECL(MPID_STATE_MPIDI_CH3I_PROGRESS);
@@ -503,28 +500,23 @@ int MPIDI_CH3I_Progress (MPID_Progress_state *progress_state, int is_blocking)
#ifdef MPICH_IS_THREADED
MPIU_THREAD_CHECK_BEGIN;
{
- /* In the case of threads, we poll for lesser number of
- * iterations than the case with only processes, as
- * threads contend for CPU and the lock, while processes
- * only contend for the CPU. */
- if (pollcount >= MPID_NEM_THREAD_POLLS_BEFORE_YIELD)
- {
- pollcount = 0;
+ if (is_blocking) {
MPIDI_CH3I_progress_blocked = TRUE;
MPIU_THREAD_CS_YIELD(ALLFUNC,);
- /* MPIDCOMM yield is needed because at least the send functions
- * acquire MPIDCOMM to put things into the send queues. Failure
- * to yield could result in a deadlock. This thread needs the
- * send from another thread to be posted, but the other thread
- * can't post it while this CS is held. */
+ /* MPIDCOMM yield is needed because at least the send
+ * functions acquire MPIDCOMM to put things into the send
+ * queues. Failure to yield could result in a deadlock.
+ * This thread needs the send from another thread to be
+ * posted, but the other thread can't post it while this
+ * CS is held. */
/* assertion: we currently do not hold any other critical
- * sections besides the MPIDCOMM CS at this point. Violating
- * this will probably lead to lock-ordering deadlocks. */
+ * sections besides the MPIDCOMM CS at this point.
+ * Violating this will probably lead to lock-ordering
+ * deadlocks. */
MPIU_THREAD_CS_YIELD(MPIDCOMM,);
MPIDI_CH3I_progress_blocked = FALSE;
MPIDI_CH3I_progress_wakeup_signalled = FALSE;
}
- ++pollcount;
}
MPIU_THREAD_CHECK_END;
#else
http://git.mpich.org/mpich.git/commitdiff/b39314a5b24242907c5819a0e43dc4b19728fada
commit b39314a5b24242907c5819a0e43dc4b19728fada
Author: Pavan Balaji <balaji at anl.gov>
Date: Sat Apr 18 04:37:09 2015 -0500
Initial version of the intelligent thread yielding.
Instead of a simple thread yield, this patch adds some additional
information to the yield about how many threads are waiting for it.
When a thread tries to acquire a lock, they increment a counter. When
a thread needs to yield, it can check this counter to see how many
threads are waiting to get the lock. If there are no threads waiting,
the yield can be skipped.
This patch contains various changes to make that happen:
1. We modify the mutex object to maintain additional information on
the number of queued threads.
2. We improve the yield call to include the unlock and lock as well,
since it needs to decide whether to do the unlock/lock based on how
many other threads are queued up.
Signed-off-by: Halim Amer <aamer at anl.gov>
diff --git a/src/include/mpiimplthreadpost.h b/src/include/mpiimplthreadpost.h
index be4578f..fe1cf99 100644
--- a/src/include/mpiimplthreadpost.h
+++ b/src/include/mpiimplthreadpost.h
@@ -69,9 +69,7 @@ MPIU_Thread_CS_yield_lockname_impl_(enum MPIU_Nest_mutexes kind,
MPIU_THREADPRIV_DECL;
MPIU_THREADPRIV_GET;
MPIU_THREAD_CHECKDEPTH(kind, lockname, 1);
- MPID_Thread_mutex_unlock(mutex);
- MPID_Thread_yield();
- MPID_Thread_mutex_lock(mutex);
+ MPID_Thread_yield(mutex);
}
/* ------------------------------------------------------------------- */
@@ -189,9 +187,7 @@ MPIU_Thread_CS_yield_lockname_recursive_impl_(enum MPIU_Nest_mutexes kind,
MPIU_Assert(depth > 0 && depth < 10); /* we must hold the mutex */
/* no need to update depth, this is a thread-local value */
- MPID_Thread_mutex_unlock(mutex);
- MPID_Thread_yield();
- MPID_Thread_mutex_lock(mutex);
+ MPID_Thread_yield(mutex);
}
/* undef for safety, this is a commonly-included header */
diff --git a/src/include/thread/mpiu_thread_posix_funcs.h b/src/include/thread/mpiu_thread_posix_funcs.h
index 10af9fc..f0d3b94 100644
--- a/src/include/thread/mpiu_thread_posix_funcs.h
+++ b/src/include/thread/mpiu_thread_posix_funcs.h
@@ -46,12 +46,17 @@ do { \
*(same_) = pthread_equal(*(id1_), *(id2_)) ? TRUE : FALSE; \
} while (0)
-#define MPIU_Thread_yield() \
-do { \
- MPIU_DBG_MSG(THREAD,VERBOSE,"enter MPIU_Thread_yield"); \
- MPIU_PW_Sched_yield(); \
- MPIU_DBG_MSG(THREAD,VERBOSE,"exit MPIU_Thread_yield"); \
-} while (0)
+#define MPIU_Thread_yield(mutex_ptr_) \
+ do { \
+ int err; \
+ MPIU_DBG_MSG(THREAD,VERBOSE,"enter MPIU_Thread_yield"); \
+ if (OPA_load_int(&(mutex_ptr_)->num_queued_threads) == 0) \
+ break; \
+ MPIU_Thread_mutex_unlock(mutex_ptr_, &err); \
+ MPIU_PW_Sched_yield(); \
+ MPIU_Thread_mutex_lock(mutex_ptr_, &err); \
+ MPIU_DBG_MSG(THREAD,VERBOSE,"exit MPIU_Thread_yield"); \
+ } while (0)
/*
@@ -70,7 +75,7 @@ do { \
do { \
int err__; \
\
- err__ = pthread_mutex_init((mutex_ptr_), NULL); \
+ err__ = pthread_mutex_init(&(mutex_ptr_)->mutex, NULL); \
/* FIXME: convert error to an MPIU_THREAD_ERR value */ \
*(int *)(err_ptr_) = err__; \
MPIU_DBG_MSG_P(THREAD,TYPICAL,"Created MPIU_Thread_mutex %p", (mutex_ptr_)); \
@@ -87,7 +92,7 @@ do { \
error checked versions with the recursive mutexes. */ \
pthread_mutexattr_init(&attr__); \
pthread_mutexattr_settype(&attr__, PTHREAD_MUTEX_ERRORCHECK_VALUE); \
- err__ = pthread_mutex_init((mutex_ptr_), &attr__); \
+ err__ = pthread_mutex_init(&(mutex_ptr_)->mutex, &attr__); \
if (err__) \
MPIU_Internal_sys_error_printf("pthread_mutex_init", err__, \
" %s:%d\n", __FILE__, __LINE__);\
@@ -102,7 +107,7 @@ do { \
int err__; \
\
MPIU_DBG_MSG_P(THREAD,TYPICAL,"About to destroy MPIU_Thread_mutex %p", (mutex_ptr_)); \
- err__ = pthread_mutex_destroy(mutex_ptr_); \
+ err__ = pthread_mutex_destroy(&(mutex_ptr_)->mutex); \
/* FIXME: convert error to an MPIU_THREAD_ERR value */ \
*(int *)(err_ptr_) = err__; \
} while (0)
@@ -112,7 +117,9 @@ do { \
do { \
int err__; \
MPIU_DBG_MSG_P(THREAD,VERBOSE,"enter MPIU_Thread_mutex_lock %p", (mutex_ptr_)); \
- err__ = pthread_mutex_lock(mutex_ptr_); \
+ OPA_add_int(&(mutex_ptr_)->num_queued_threads, 1); \
+ err__ = pthread_mutex_lock(&(mutex_ptr_)->mutex); \
+ OPA_add_int(&(mutex_ptr_)->num_queued_threads, -1); \
/* FIXME: convert error to an MPIU_THREAD_ERR value */ \
*(int *)(err_ptr_) = err__; \
MPIU_DBG_MSG_P(THREAD,VERBOSE,"exit MPIU_Thread_mutex_lock %p", (mutex_ptr_)); \
@@ -122,7 +129,9 @@ do { \
do { \
int err__; \
MPIU_DBG_MSG_P(THREAD,VERBOSE,"enter MPIU_Thread_mutex_lock %p", (mutex_ptr_)); \
- err__ = pthread_mutex_lock(mutex_ptr_); \
+ OPA_add_int(&(mutex_ptr_)->num_queued_threads, 1); \
+ err__ = pthread_mutex_lock(&(mutex_ptr_)->mutex); \
+ OPA_add_int(&(mutex_ptr_)->num_queued_threads, -1); \
if (err__) \
{ \
MPIU_DBG_MSG_S(THREAD,TERSE," mutex lock error: %s", MPIU_Strerror(err__)); \
@@ -141,7 +150,7 @@ do { \
int err__; \
\
MPIU_DBG_MSG_P(THREAD,TYPICAL,"MPIU_Thread_mutex_unlock %p", (mutex_ptr_)); \
- err__ = pthread_mutex_unlock(mutex_ptr_); \
+ err__ = pthread_mutex_unlock(&(mutex_ptr_)->mutex); \
/* FIXME: convert error to an MPIU_THREAD_ERR value */ \
*(int *)(err_ptr_) = err__; \
} while (0)
@@ -151,7 +160,7 @@ do { \
int err__; \
\
MPIU_DBG_MSG_P(THREAD,VERBOSE,"MPIU_Thread_mutex_unlock %p", (mutex_ptr_)); \
- err__ = pthread_mutex_unlock(mutex_ptr_); \
+ err__ = pthread_mutex_unlock(&(mutex_ptr_)->mutex); \
if (err__) \
{ \
MPIU_DBG_MSG_S(THREAD,TERSE," mutex unlock error: %s", MPIU_Strerror(err__)); \
@@ -168,7 +177,7 @@ do { \
do { \
int err__; \
\
- err__ = pthread_mutex_trylock(mutex_ptr_); \
+ err__ = pthread_mutex_trylock(&(mutex_ptr_)->mutex); \
*(flag_ptr_) = (err__ == 0) ? TRUE : FALSE; \
MPIU_DBG_MSG_FMT(THREAD,VERBOSE,(MPIU_DBG_FDEST, "MPIU_Thread_mutex_trylock mutex=%p result=%s", (mutex_ptr_), (*(flag_ptr_) ? "success" : "failure"))); \
*(int *)(err_ptr_) = (err__ == EBUSY) ? MPIU_THREAD_SUCCESS : err__; \
@@ -179,7 +188,7 @@ do { \
do { \
int err__; \
\
- err__ = pthread_mutex_trylock(mutex_ptr_); \
+ err__ = pthread_mutex_trylock(&(mutex_ptr_)->mutex); \
if (err__ && err__ != EBUSY) \
{ \
MPIU_DBG_MSG_S(THREAD,TERSE," mutex trylock error: %s", MPIU_Strerror(err__)); \
@@ -227,7 +236,9 @@ do { \
MPIU_DBG_MSG_FMT(THREAD,TYPICAL,(MPIU_DBG_FDEST,"Enter cond_wait on cond=%p mutex=%p",(cond_ptr_),(mutex_ptr_))) \
do \
{ \
- err__ = pthread_cond_wait((cond_ptr_), (mutex_ptr_)); \
+ OPA_add_int(&(mutex_ptr_)->num_queued_threads, 1); \
+ err__ = pthread_cond_wait((cond_ptr_), &(mutex_ptr_)->mutex); \
+ OPA_add_int(&(mutex_ptr_)->num_queued_threads, -1); \
} \
while (err__ == EINTR); \
\
diff --git a/src/include/thread/mpiu_thread_posix_types.h b/src/include/thread/mpiu_thread_posix_types.h
index b38f7a1..b9e07b4 100644
--- a/src/include/thread/mpiu_thread_posix_types.h
+++ b/src/include/thread/mpiu_thread_posix_types.h
@@ -7,8 +7,12 @@
#include <errno.h>
#include <pthread.h>
+#include "opa_primitives.h"
-typedef pthread_mutex_t MPIU_Thread_mutex_t;
+typedef struct {
+ pthread_mutex_t mutex;
+ OPA_int_t num_queued_threads;
+} MPIU_Thread_mutex_t;
typedef pthread_cond_t MPIU_Thread_cond_t;
typedef pthread_t MPIU_Thread_id_t;
typedef pthread_key_t MPIU_Thread_tls_t;
diff --git a/src/include/thread/mpiu_thread_solaris_funcs.h b/src/include/thread/mpiu_thread_solaris_funcs.h
index 3880b99..79dece6 100644
--- a/src/include/thread/mpiu_thread_solaris_funcs.h
+++ b/src/include/thread/mpiu_thread_solaris_funcs.h
@@ -26,9 +26,12 @@ do { \
*(same_ptr_) = (*(id1_ptr_) == *(id2_ptr_)) ? TRUE : FALSE; \
} while (0)
-#define MPIU_Thread_yield() \
+#define MPIU_Thread_yield(mutex_ptr_) \
do { \
- thr_yield(); \
+ int err; \
+ MPIU_Thread_mutex_unlock(mutex_ptr_, &err); \
+ thr_yield(); \
+ MPIU_Thread_mutex_lock(mutex_ptr_, &err); \
} while (0)
diff --git a/src/mpid/common/thread/mpid_thread.h b/src/mpid/common/thread/mpid_thread.h
index 8386cf2..969134f 100644
--- a/src/mpid/common/thread/mpid_thread.h
+++ b/src/mpid/common/thread/mpid_thread.h
@@ -278,9 +278,9 @@ do { \
MPIU_Thread_same((id1_), (id2_), (same_)); \
} while (0)
-#define MPID_Thread_yield() \
+#define MPID_Thread_yield(mutex_ptr) \
do { \
- MPIU_Thread_yield(); \
+ MPIU_Thread_yield(mutex_ptr); \
} while (0)
diff --git a/src/util/thread/mpiu_thread.c b/src/util/thread/mpiu_thread.c
index fa881f0..2c2599f 100644
--- a/src/util/thread/mpiu_thread.c
+++ b/src/util/thread/mpiu_thread.c
@@ -260,9 +260,13 @@ void MPIU_Thread_same(MPIU_Thread_id_t * id1, MPIU_Thread_id_t * id2, int * same
*same = (*id1 == *id2) ? TRUE : FALSE;
}
-void MPIU_Thread_yield()
+void MPIU_Thread_yield(MPIU_Thread_mutex_t * mutex)
{
+ int err;
+
+ MPIU_Thread_mutex_unlock(mutex, &err);
Sleep(0);
+ MPIU_Thread_mutex_lock(mutex, &err);
}
/*
http://git.mpich.org/mpich.git/commitdiff/f385680e392d999ad5e2f7ddcc625a0f6556f014
commit f385680e392d999ad5e2f7ddcc625a0f6556f014
Author: Pavan Balaji <balaji at anl.gov>
Date: Fri Apr 17 18:31:29 2015 -0500
Cleanup threaded progress.
The nemesis progress engine was written in a way so that if one thread
is inside a progress engine, other threads cannot enter the receive
progress. They can enter the send progress in some cases. There
doesn't seem to be a good reason for this behavior. This patch
combines this so threads would simply return for nonblocking
operations and wait for a signal before entering the progress engine
for blocking operations.
Signed-off-by: Halim Amer <aamer at anl.gov>
diff --git a/src/mpid/ch3/channels/nemesis/src/ch3_progress.c b/src/mpid/ch3/channels/nemesis/src/ch3_progress.c
index f022651..422c533 100644
--- a/src/mpid/ch3/channels/nemesis/src/ch3_progress.c
+++ b/src/mpid/ch3/channels/nemesis/src/ch3_progress.c
@@ -325,6 +325,40 @@ int MPIDI_CH3I_Progress (MPID_Progress_state *progress_state, int is_blocking)
}
#endif
+/* For threaded mode, if another thread is in the progress engine, we
+ * don't enter the progress engine */
+#ifdef MPICH_IS_THREADED
+ MPIU_THREAD_CHECK_BEGIN;
+ {
+ while (MPIDI_CH3I_progress_blocked == TRUE)
+ {
+ /* if this is a nonblocking call, and some other thread is
+ * going to poke progress, our job is done and we can
+ * return */
+ if (!is_blocking)
+ goto fn_exit;
+
+ /* if it's a blocking call, and some other thread is going
+ * to poke progress, our job might also be done. But
+ * there's no point returning from this call to see if the
+ * work is done and coming back in again if it's not done.
+ * We might as well wait for the other thread to be done
+ * before doing that. */
+ if (progress_state->ch.completion_count == OPA_load_int(&MPIDI_CH3I_progress_completion_count))
+ MPIDI_CH3I_Progress_delay(progress_state->ch.completion_count);
+ else {
+ /* if the completion count of our progress state is
+ * different from the current completion count, some
+ * progress happened. We reset the value for the next
+ * iteration and return from the progress engine. */
+ progress_state->ch.completion_count = OPA_load_int(&MPIDI_CH3I_progress_completion_count);
+ goto fn_exit;
+ }
+ }
+ }
+ MPIU_THREAD_CHECK_END;
+#endif
+
do
{
MPID_Request *rreq;
@@ -334,19 +368,6 @@ int MPIDI_CH3I_Progress (MPID_Progress_state *progress_state, int is_blocking)
do /* receive progress */
{
-
-#ifdef MPICH_IS_THREADED
- MPIU_THREAD_CHECK_BEGIN;
- {
- if (MPIDI_CH3I_progress_blocked == TRUE)
- {
- /* another thread is already blocking in the progress engine.*/
- break; /* break out of receive block */
- }
- }
- MPIU_THREAD_CHECK_END;
-#endif
-
/* make progress receiving */
/* check queue */
if (MPID_nem_safe_to_block_recv() && is_blocking
@@ -434,21 +455,6 @@ int MPIDI_CH3I_Progress (MPID_Progress_state *progress_state, int is_blocking)
if (MPIDI_CH3I_shm_active_send || MPIDI_CH3I_Sendq_head(MPIDI_CH3I_shm_sendq)) {
mpi_errno = MPIDI_CH3I_Shm_send_progress();
if (mpi_errno) MPIU_ERR_POP(mpi_errno);
- } else {
- /* there are no pending sends */
-#ifdef MPICH_IS_THREADED
- MPIU_THREAD_CHECK_BEGIN;
- {
- if (MPIDI_CH3I_progress_blocked == TRUE && is_blocking && !MPID_nem_local_lmt_pending)
- {
- /* There's nothing to send and there's another thread already blocking in the progress engine.*/
- MPIDI_CH3I_Progress_delay(progress_state->ch.completion_count);
- /* the progress_state count will be updated below at the
- * bottom of the outermost loop (see CC-1) */
- }
- }
- MPIU_THREAD_CHECK_END;
-#endif
}
/* make progress on LMTs */
@@ -563,13 +569,7 @@ static int MPIDI_CH3I_Progress_delay(unsigned int completion_count)
/* FIXME should be appropriately abstracted somehow */
# if defined(MPICH_IS_THREADED) && (MPIU_THREAD_GRANULARITY == MPIU_THREAD_GRANULARITY_GLOBAL)
{
- while (1)
- {
- if (completion_count != OPA_load_int(&MPIDI_CH3I_progress_completion_count) ||
- MPIDI_CH3I_progress_blocked != TRUE)
- break;
- MPID_Thread_cond_wait(&MPIDI_CH3I_progress_completion_cond, &MPIR_ThreadInfo.global_mutex/*MPIDCOMM*/);
- }
+ MPID_Thread_cond_wait(&MPIDI_CH3I_progress_completion_cond, &MPIR_ThreadInfo.global_mutex/*MPIDCOMM*/);
}
# endif
-----------------------------------------------------------------------
Summary of changes:
src/include/mpiimplthreadpost.h | 8 +-
src/include/thread/mpiu_thread_posix_funcs.h | 43 ++++++---
src/include/thread/mpiu_thread_posix_types.h | 6 +-
src/include/thread/mpiu_thread_solaris_funcs.h | 7 +-
.../ch3/channels/nemesis/include/mpid_nem_inline.h | 2 -
src/mpid/ch3/channels/nemesis/src/ch3_progress.c | 98 +++++++++-----------
src/mpid/common/thread/mpid_thread.h | 4 +-
src/util/thread/mpiu_thread.c | 6 +-
8 files changed, 91 insertions(+), 83 deletions(-)
hooks/post-receive
--
MPICH primary repository
More information about the commits
mailing list