Commit abb56764 authored by Pavan Balaji's avatar Pavan Balaji Committed by Halim Amer
Browse files

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: default avatarHalim Amer <aamer@anl.gov>
parent b39314a5
......@@ -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"
......
......@@ -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
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment