[pypy-commit] stmgc default: First simplification step: unify all condition variables into one,
arigo
noreply at buildbot.pypy.org
Fri Feb 28 19:54:03 CET 2014
Author: Armin Rigo <arigo at tunes.org>
Branch:
Changeset: r903:a14d95357717
Date: 2014-02-28 19:53 +0100
http://bitbucket.org/pypy/stmgc/changeset/a14d95357717/
Log: First simplification step: unify all condition variables into one,
again. Should fix obscure synchronization bugs that are
theoretically possible at least with more than two threads.
diff --git a/c7/stm/contention.c b/c7/stm/contention.c
--- a/c7/stm/contention.c
+++ b/c7/stm/contention.c
@@ -44,7 +44,7 @@
cond_wait(C_SAFE_POINT). Thus broadcasting C_SAFE_POINT is
enough to wake it up in the second case.
*/
- cond_broadcast(C_SAFE_POINT);
+ cond_broadcast();
}
}
@@ -78,10 +78,10 @@
/* wait, hopefully until the other thread broadcasts "I'm
done aborting" (spurious wake-ups are ok). */
dprintf(("contention: wait C_SAFE_POINT...\n"));
- cond_wait(C_SAFE_POINT);
+ cond_wait();
dprintf(("contention: done\n"));
- cond_broadcast(C_RESUME);
+ cond_broadcast();
/* now we return into _stm_write_slowpath() and will try again
to acquire the write lock on our object. */
diff --git a/c7/stm/core.c b/c7/stm/core.c
--- a/c7/stm/core.c
+++ b/c7/stm/core.c
@@ -326,7 +326,6 @@
/* signal all the threads blocked in wait_for_other_safe_points() */
if (STM_SEGMENT->nursery_end == NSE_SIGNAL) {
STM_SEGMENT->nursery_end = NURSERY_END;
- cond_broadcast(C_SAFE_POINT);
}
STM_PSEGMENT->safe_point = SP_NO_TRANSACTION;
@@ -339,6 +338,9 @@
stm_thread_local_t *tl = STM_SEGMENT->running_thread;
release_thread_segment(tl);
/* cannot access STM_SEGMENT or STM_PSEGMENT from here ! */
+
+ /* wake up other threads waiting. */
+ cond_broadcast();
}
void stm_commit_transaction(void)
@@ -387,16 +389,9 @@
STM_PSEGMENT->overflow_number = highest_overflow_number;
}
- /* if we were inevitable, signal */
- if (STM_PSEGMENT->transaction_state == TS_INEVITABLE)
- cond_broadcast(C_INEVITABLE_DONE);
-
/* done */
_finish_transaction();
- /* wake up one other thread waiting for a segment. */
- cond_signal(C_RELEASE_THREAD_SEGMENT);
-
mutex_unlock();
}
@@ -478,18 +473,6 @@
_finish_transaction();
- /* wake up one other thread waiting for a segment. In order to support
- contention.c, we use a broadcast, to make sure that all threads are
- signalled, including the one that requested an abort, if any.
- Moreover, we wake up any thread waiting for this one to do a safe
- point, if any (in _finish_transaction above). Finally, it's
- possible that we reach this place from the middle of a piece of
- code like wait_for_other_safe_points() which ends in broadcasting
- C_RESUME; we must make sure to broadcast it.
- */
- cond_broadcast(C_RELEASE_THREAD_SEGMENT);
- cond_broadcast(C_RESUME);
-
mutex_unlock();
/* It seems to be a good idea, at least in some examples, to sleep
diff --git a/c7/stm/sync.c b/c7/stm/sync.c
--- a/c7/stm/sync.c
+++ b/c7/stm/sync.c
@@ -16,7 +16,7 @@
static union {
struct {
pthread_mutex_t global_mutex;
- pthread_cond_t cond[_C_TOTAL];
+ pthread_cond_t global_cond;
/* some additional pieces of global state follow */
uint8_t in_use[NB_SEGMENTS]; /* 1 if running a pthread */
uint64_t global_time;
@@ -30,11 +30,8 @@
if (pthread_mutex_init(&sync_ctl.global_mutex, NULL) != 0)
stm_fatalerror("mutex initialization: %m\n");
- long i;
- for (i = 0; i < _C_TOTAL; i++) {
- if (pthread_cond_init(&sync_ctl.cond[i], NULL) != 0)
- stm_fatalerror("cond initialization: %m\n");
- }
+ if (pthread_cond_init(&sync_ctl.global_cond, NULL) != 0)
+ stm_fatalerror("cond initialization: %m\n");
}
static void teardown_sync(void)
@@ -42,11 +39,8 @@
if (pthread_mutex_destroy(&sync_ctl.global_mutex) != 0)
stm_fatalerror("mutex destroy: %m\n");
- long i;
- for (i = 0; i < _C_TOTAL; i++) {
- if (pthread_cond_destroy(&sync_ctl.cond[i]) != 0)
- stm_fatalerror("cond destroy: %m\n");
- }
+ if (pthread_cond_destroy(&sync_ctl.global_cond) != 0)
+ stm_fatalerror("cond destroy: %m\n");
memset(&sync_ctl, 0, sizeof(sync_ctl.in_use));
}
@@ -91,35 +85,29 @@
assert((_has_mutex_here = false, 1));
}
-static inline void cond_wait_no_abort(enum cond_type_e ctype)
+static inline void cond_wait_no_abort(void)
{
#ifdef STM_NO_COND_WAIT
- stm_fatalerror("*** cond_wait/%d called!\n", (int)ctype);
+ stm_fatalerror("*** cond_wait called!\n");
#endif
assert(_has_mutex_here);
- if (UNLIKELY(pthread_cond_wait(&sync_ctl.cond[ctype],
+ if (UNLIKELY(pthread_cond_wait(&sync_ctl.global_cond,
&sync_ctl.global_mutex) != 0))
- stm_fatalerror("pthread_cond_wait/%d: %m\n", (int)ctype);
+ stm_fatalerror("pthread_cond_wait: %m\n");
}
-static inline void cond_wait(enum cond_type_e ctype)
+static inline void cond_wait(void)
{
- cond_wait_no_abort(ctype);
+ cond_wait_no_abort();
if (STM_PSEGMENT->transaction_state == TS_MUST_ABORT)
abort_with_mutex();
}
-static inline void cond_broadcast(enum cond_type_e ctype)
+static inline void cond_broadcast(void)
{
- if (UNLIKELY(pthread_cond_broadcast(&sync_ctl.cond[ctype]) != 0))
- stm_fatalerror("pthread_cond_broadcast/%d: %m\n", (int)ctype);
-}
-
-static inline void cond_signal(enum cond_type_e ctype)
-{
- if (UNLIKELY(pthread_cond_signal(&sync_ctl.cond[ctype]) != 0))
- stm_fatalerror("pthread_cond_signal/%d: %m\n", (int)ctype);
+ if (UNLIKELY(pthread_cond_broadcast(&sync_ctl.global_cond) != 0))
+ stm_fatalerror("pthread_cond_broadcast: %m\n");
}
static bool acquire_thread_segment(stm_thread_local_t *tl)
@@ -154,8 +142,8 @@
}
/* Wait and retry. It is guaranteed that any thread releasing its
segment will do so by acquiring the mutex and calling
- cond_signal(C_RELEASE_THREAD_SEGMENT). */
- cond_wait_no_abort(C_RELEASE_THREAD_SEGMENT);
+ cond_broadcast(). */
+ cond_wait_no_abort();
/* Return false to the caller, which will call us again */
return false;
@@ -192,10 +180,10 @@
/* XXX should we wait here? or abort? or a mix?
for now, always abort */
abort_with_mutex();
- //cond_wait(C_INEVITABLE_DONE);
+ //cond_wait();
}
else {
- cond_wait_no_abort(C_INEVITABLE_DONE);
+ cond_wait_no_abort();
}
goto restart;
}
@@ -288,7 +276,7 @@
}
if (wait) {
- cond_wait(C_SAFE_POINT);
+ cond_wait();
/* XXX think: I believe this can end in a busy-loop, with this thread
setting NSE_SIGNAL on the other thread; then the other thread
commits, sends C_SAFE_POINT, finish the transaction, start
@@ -300,7 +288,7 @@
/* all threads are at a safe-point now. Broadcast C_RESUME, which
will allow them to resume --- but only when we release the mutex. */
- cond_broadcast(C_RESUME);
+ cond_broadcast();
return true;
}
@@ -336,9 +324,9 @@
/* signal all the threads blocked in
wait_for_other_safe_points() */
- cond_broadcast(C_SAFE_POINT);
+ cond_broadcast();
- cond_wait(C_RESUME);
+ cond_wait();
STM_PSEGMENT->safe_point = SP_RUNNING;
}
diff --git a/c7/stm/sync.h b/c7/stm/sync.h
--- a/c7/stm/sync.h
+++ b/c7/stm/sync.h
@@ -3,19 +3,11 @@
static void setup_sync(void);
static void teardown_sync(void);
-/* all synchronization is done via a mutex and a few condition variables */
-enum cond_type_e {
- C_RELEASE_THREAD_SEGMENT,
- C_SAFE_POINT,
- C_RESUME,
- C_INEVITABLE_DONE,
- _C_TOTAL
-};
+/* all synchronization is done via a mutex and a condition variable */
static void mutex_lock(void);
static void mutex_unlock(void);
-static void cond_wait(enum cond_type_e);
-static void cond_broadcast(enum cond_type_e);
-static void cond_signal(enum cond_type_e);
+static void cond_wait(void);
+static void cond_broadcast(void);
#ifndef NDEBUG
static bool _has_mutex(void);
#endif
More information about the pypy-commit
mailing list