Skip to content

Commit d9fb59b

Browse files
committed
Update the synchronization primitive
Add comments and make sure we correctly return the status of the synchronization primitive, especially if it was completed with error.
1 parent 2e1b1d3 commit d9fb59b

File tree

2 files changed

+23
-15
lines changed

2 files changed

+23
-15
lines changed

opal/threads/wait_sync.c

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,18 @@ int sync_wait_st(ompi_wait_sync_t *sync)
2727
while(sync->count > 0) {
2828
opal_progress();
2929
}
30-
return OPAL_SUCCESS;
30+
return (0 == sync->status) ? OPAL_SUCCESS : OPAL_ERROR;
3131
}
3232

3333
int sync_wait_mt(ompi_wait_sync_t *sync)
3434
{
3535
if(sync->count <= 0)
36-
return OPAL_SUCCESS;
36+
return (0 == sync->status) ? OPAL_SUCCESS : OPAL_ERROR;
3737

3838
/* lock so nobody can signal us during the list updating */
3939
pthread_mutex_lock(&sync->lock);
4040

41-
/* Insert sync to the list */
41+
/* Insert sync on the list of pending synchronization constructs */
4242
OPAL_THREAD_LOCK(&wait_sync_lock);
4343
if( NULL == wait_sync_list ) {
4444
sync->next = sync->prev = sync;
@@ -52,32 +52,34 @@ int sync_wait_mt(ompi_wait_sync_t *sync)
5252
OPAL_THREAD_UNLOCK(&wait_sync_lock);
5353

5454
/**
55-
* If we are not responsible for progresing, let's go silent until something worth noticing happen:
55+
* If we are not responsible for progresing, go silent until something worth noticing happen:
5656
* - this thread has been promoted to take care of the progress
5757
* - our sync has been triggered.
5858
*/
59+
check_status:
5960
if( sync != wait_sync_list ) {
6061
pthread_cond_wait(&sync->condition, &sync->lock);
6162

6263
/**
63-
* At this point either the sync was completed in which case we should remove it from the wait
64-
* list, or/and I was promoted as the progress manager.
64+
* At this point either the sync was completed in which case
65+
* we should remove it from the wait list, or/and I was
66+
* promoted as the progress manager.
6567
*/
6668

6769
if( sync->count <= 0 ) { /* Completed? */
6870
pthread_mutex_unlock(&sync->lock);
6971
goto i_am_done;
7072
}
71-
/* promoted ! */
72-
assert(sync == wait_sync_list);
73+
/* either promoted, or spurious wakeup ! */
74+
goto check_status;
7375
}
7476

7577
pthread_mutex_unlock(&sync->lock);
7678
while(sync->count > 0) { /* progress till completion */
7779
opal_progress(); /* don't progress with the sync lock locked or you'll deadlock */
7880
}
79-
8081
assert(sync == wait_sync_list);
82+
8183
i_am_done:
8284
/* My sync is now complete. Trim the list: remove self, wake next */
8385
OPAL_THREAD_LOCK(&wait_sync_lock);
@@ -91,5 +93,5 @@ int sync_wait_mt(ompi_wait_sync_t *sync)
9193
}
9294
OPAL_THREAD_UNLOCK(&wait_sync_lock);
9395

94-
return OPAL_SUCCESS;
96+
return (0 == sync->status) ? OPAL_SUCCESS : OPAL_ERROR;
9597
}

opal/threads/wait_sync.h

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -73,17 +73,23 @@ OPAL_DECLSPEC int sync_wait_st(ompi_wait_sync_t *sync);
7373
PTHREAD_MUTEX_INIT(&(sync)->lock, NULL); \
7474
} while(0)
7575

76-
static inline void wait_sync_update(ompi_wait_sync_t *sync, int updates, int req_status)
76+
/**
77+
* Update the status of the synchronization primitive. If an error is
78+
* reported the synchronization is completed and the signal
79+
* triggered. The status of the synchronization will be reported to
80+
* the waiting threads.
81+
*/
82+
static inline void wait_sync_update(ompi_wait_sync_t *sync, int updates, int status)
7783
{
78-
if( OPAL_LIKELY(OPAL_SUCCESS == req_status) ) {
79-
if( 0 == (OPAL_ATOMIC_ADD_32(&sync->count, -updates)) ) {
80-
WAIT_SYNC_SIGNAL(sync);
84+
if( OPAL_LIKELY(OPAL_SUCCESS == status) ) {
85+
if( 0 != (OPAL_ATOMIC_ADD_32(&sync->count, -updates)) ) {
86+
return;
8187
}
8288
} else {
8389
OPAL_ATOMIC_CMPSET_32(&(sync->count), 0, 0);
8490
sync->status = -1;
85-
WAIT_SYNC_SIGNAL(sync);
8691
}
92+
WAIT_SYNC_SIGNAL(sync);
8793
}
8894

8995
END_C_DECLS

0 commit comments

Comments
 (0)