Module: xenomai-3 Branch: condvar Commit: 375fa059824d99622f81db7e69a8f0e1d95078f3 URL: http://git.xenomai.org/?p=xenomai-3.git;a=commit;h=375fa059824d99622f81db7e69a8f0e1d95078f3
Author: Philippe Gerum <r...@xenomai.org> Date: Thu Jan 22 18:23:26 2015 +0100 copperplate/threadobj: mercury: work around glibc's PI bug with condvars We have a problem with all glibc releases to date with condition variables, which may suffer from priority inversion: https://sourceware.org/bugzilla/show_bug.cgi?id=11588 https://lwn.net/images/conf/rtlws11/papers/proc/p10.pdf --enable-condvar-workaround enables code to boost threads temporarily prior to signaling or waiting for an event on a condition variable protected by a PI mutex. Although this may be fairly costly, this prevents nasty CPU starvation issues which may be otherwise caused by priority inversions while blocking on the condvar's internal mutex as implemented by the glibc. This bug only affects Mercury-based applications which depend on strict PI to prevent from harmful priority inversion issues. The Cobalt implementation of condvars is PI-aware and needs no work around. --- configure | 28 +++++++++ configure.ac | 19 ++++++ include/copperplate/threadobj.h | 48 +++++++++++++++ include/xeno_config.h.in | 3 + lib/copperplate/syncobj.c | 18 +++--- lib/copperplate/threadobj.c | 123 +++++++++++++++++++++++++++++++++++++-- lib/copperplate/traceobj.c | 4 +- 7 files changed, 226 insertions(+), 17 deletions(-) diff --git a/configure b/configure index ef72f29..e4281b2 100755 --- a/configure +++ b/configure @@ -851,6 +851,7 @@ enable_lores_clock enable_clock_monotonic_raw enable_assert enable_async_cancel +enable_condvar_workaround enable_pshared enable_registry enable_smp @@ -1522,6 +1523,8 @@ Optional Features: Use CLOCK_MONOTONIC_RAW for timings --enable-assert Enable runtime assertions --enable-async-cancel Enable asynchronous cancellation + --enable-condvar-workaround + Enable workaround for broken PI with condvars --enable-pshared Enable shared multi-processing for capable skins --enable-registry Export real-time objects to a registry --enable-smp Enable SMP support @@ -13061,6 +13064,31 @@ $as_echo "#define CONFIG_XENO_ASYNC_CANCEL 1" >>confdefs.h fi +unset workaround_condvar_pi +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to enable the workaround for broken PI with condvars" >&5 +$as_echo_n "checking whether to enable the workaround for broken PI with condvars... " >&6; } +# Check whether --enable-condvar-workaround was given. +if test "${enable_condvar_workaround+set}" = set; then : + enableval=$enable_condvar_workaround; case "$enableval" in + y | yes) workaround_condvar_pi=y ;; + *) unset workaround_condvar_pi ;; + esac +fi + +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: ${workaround_condvar_pi:-no}" >&5 +$as_echo "${workaround_condvar_pi:-no}" >&6; } +if test x$workaround_condvar_pi = xy; then + if test x$rtcore_type = xmercury; then + +$as_echo "#define CONFIG_XENO_WORKAROUND_CONDVAR_PI 1" >>confdefs.h + + else + { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: PI workaround for condvars useless over Cobalt - ignoring" >&5 +$as_echo "$as_me: WARNING: PI workaround for condvars useless over Cobalt - ignoring" >&2;} + fi +fi + + use_pshared= { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether shared multi-processing should be supported" >&5 $as_echo_n "checking whether shared multi-processing should be supported... " >&6; } diff --git a/configure.ac b/configure.ac index c055f83..052b343 100644 --- a/configure.ac +++ b/configure.ac @@ -270,6 +270,25 @@ if test x$async_cancel = xy; then AC_DEFINE(CONFIG_XENO_ASYNC_CANCEL,1,[config]) fi +dnl Work-around for broken PI with condvars on Mercury (default: off) + +unset workaround_condvar_pi +AC_MSG_CHECKING(whether to enable the workaround for broken PI with condvars) +AC_ARG_ENABLE(condvar-workaround, + AS_HELP_STRING([--enable-condvar-workaround], [Enable workaround for broken PI with condvars in glibc]), + [case "$enableval" in + y | yes) workaround_condvar_pi=y ;; + *) unset workaround_condvar_pi ;; + esac]) +AC_MSG_RESULT(${workaround_condvar_pi:-no}) +if test x$workaround_condvar_pi = xy; then + if test x$rtcore_type = xmercury; then + AC_DEFINE(CONFIG_XENO_WORKAROUND_CONDVAR_PI,1,[config]) + else + AC_MSG_WARN([PI workaround for condvars useless over Cobalt - ignoring]) + fi +fi + dnl Enable shared multi-processing (default: off) use_pshared= diff --git a/include/copperplate/threadobj.h b/include/copperplate/threadobj.h index 05bc642..edc4d6c 100644 --- a/include/copperplate/threadobj.h +++ b/include/copperplate/threadobj.h @@ -86,6 +86,10 @@ struct threadobj_corespec { timer_t rr_timer; /** Timeout reported by sysregd. */ struct timespec timeout; +#ifdef CONFIG_XENO_WORKAROUND_CONDVAR_PI + int policy_unboosted; + struct sched_param_ex schedparam_unboosted; +#endif }; struct threadobj_stat { @@ -512,4 +516,48 @@ static inline pid_t threadobj_get_pid(struct threadobj *thobj) return thobj->pid; } +#ifdef CONFIG_XENO_WORKAROUND_CONDVAR_PI + +int threadobj_cond_timedwait(pthread_cond_t *cond, + pthread_mutex_t *lock, + const struct timespec *timeout); + +int threadobj_cond_wait(pthread_cond_t *cond, + pthread_mutex_t *lock); + +int threadobj_cond_signal(pthread_cond_t *cond); + +int threadobj_cond_broadcast(pthread_cond_t *cond); + +#else + +static inline +int threadobj_cond_timedwait(pthread_cond_t *cond, + pthread_mutex_t *lock, + const struct timespec *timeout) +{ + return __RT(pthread_cond_timedwait(cond, lock, timeout)); +} + +static inline +int threadobj_cond_wait(pthread_cond_t *cond, + pthread_mutex_t *lock) +{ + return __RT(pthread_cond_wait(cond, lock)); +} + +static inline +int threadobj_cond_signal(pthread_cond_t *cond) +{ + return __RT(pthread_cond_signal(cond)); +} + +static inline +int threadobj_cond_broadcast(pthread_cond_t *cond) +{ + return __RT(pthread_cond_broadcast(cond)); +} + +#endif /* !CONFIG_XENO_WORKAROUND_CONDVAR_PI */ + #endif /* _COPPERPLATE_THREADOBJ_H */ diff --git a/include/xeno_config.h.in b/include/xeno_config.h.in index 1a38e06..5771356 100644 --- a/include/xeno_config.h.in +++ b/include/xeno_config.h.in @@ -103,6 +103,9 @@ #undef CONFIG_XENO_VERSION_STRING /* config */ +#undef CONFIG_XENO_WORKAROUND_CONDVAR_PI + +/* config */ #undef CONFIG_XENO_X86_VSYSCALL #ifdef __IN_XENO__ diff --git a/lib/copperplate/syncobj.c b/lib/copperplate/syncobj.c index 9968a40..4716d46 100644 --- a/lib/copperplate/syncobj.c +++ b/lib/copperplate/syncobj.c @@ -144,10 +144,10 @@ int monitor_wait_grant(struct syncobj *sobj, const struct timespec *timeout) { if (timeout) - return -pthread_cond_timedwait(¤t->core.grant_sync, - &sobj->core.lock, timeout); + return -threadobj_cond_timedwait(¤t->core.grant_sync, + &sobj->core.lock, timeout); - return -pthread_cond_wait(¤t->core.grant_sync, &sobj->core.lock); + return -threadobj_cond_wait(¤t->core.grant_sync, &sobj->core.lock); } static inline @@ -156,23 +156,23 @@ int monitor_wait_drain(struct syncobj *sobj, const struct timespec *timeout) { if (timeout) - return -pthread_cond_timedwait(&sobj->core.drain_sync, - &sobj->core.lock, - timeout); + return -threadobj_cond_timedwait(&sobj->core.drain_sync, + &sobj->core.lock, + timeout); - return -pthread_cond_wait(&sobj->core.drain_sync, &sobj->core.lock); + return -threadobj_cond_wait(&sobj->core.drain_sync, &sobj->core.lock); } static inline void monitor_grant(struct syncobj *sobj, struct threadobj *thobj) { - pthread_cond_signal(&thobj->core.grant_sync); + threadobj_cond_signal(&thobj->core.grant_sync); } static inline void monitor_drain_all(struct syncobj *sobj) { - pthread_cond_broadcast(&sobj->core.drain_sync); + threadobj_cond_broadcast(&sobj->core.drain_sync); } /* diff --git a/lib/copperplate/threadobj.c b/lib/copperplate/threadobj.c index 16f23f8..1dcf9bd 100644 --- a/lib/copperplate/threadobj.c +++ b/lib/copperplate/threadobj.c @@ -561,6 +561,9 @@ static inline int threadobj_init_corespec(struct threadobj *thobj) ret = __bt(-pthread_cond_init(&thobj->core.grant_sync, &cattr)); pthread_condattr_destroy(&cattr); +#ifdef CONFIG_XENO_WORKAROUND_CONDVAR_PI + thobj->core.policy_unboosted = -1; +#endif return ret; } @@ -838,6 +841,114 @@ int threadobj_stat(struct threadobj *thobj, return 0; } +#ifdef CONFIG_XENO_WORKAROUND_CONDVAR_PI + +/* + * This workaround does NOT deal with concurrent updates of the caller + * priority by other threads while the former is boosted. If your code + * depends so much on strict PI to fix up CPU starvation, but you + * insist on using a broken glibc that does not implement PI properly + * nevertheless, then you have to refrain from issuing + * pthread_setschedparam() for threads which might be currently + * boosted. + */ +static void __threadobj_boost(void) +{ + struct threadobj *current = threadobj_current(); + struct sched_param param = { + .sched_priority = threadobj_irq_prio, /* Highest one. */ + }; + int ret; + + if (current == NULL) /* IRQ or invalid context */ + return; + + if (current->schedlock_depth > 0) { + current->core.policy_unboosted = SCHED_FIFO; + current->core.schedparam_unboosted.sched_priority = threadobj_lock_prio; + } else { + current->core.policy_unboosted = current->policy; + current->core.schedparam_unboosted = current->schedparam; + } + smp_mb(); + + ret = pthread_setschedparam(current->ptid, SCHED_FIFO, ¶m); + if (ret) { + current->core.policy_unboosted = -1; + warning("thread boost failed, %s", symerror(-ret)); + } +} + +static void __threadobj_unboost(void) + +{ + struct threadobj *current = threadobj_current(); + struct sched_param param; + int ret; + + if (current == NULL) /* IRQ or invalid context */ + return; + + param.sched_priority = current->core.schedparam_unboosted.sched_priority; + smp_mb(); + + ret = pthread_setschedparam(current->ptid, + current->core.policy_unboosted, ¶m); + if (ret) + warning("thread unboost failed, %s", symerror(-ret)); + + current->core.policy_unboosted = -1; +} + +int threadobj_cond_timedwait(pthread_cond_t *cond, + pthread_mutex_t *lock, + const struct timespec *timeout) +{ + int ret; + + __threadobj_boost(); + ret = pthread_cond_timedwait(cond, lock, timeout); + __threadobj_unboost(); + + return ret; +} + +int threadobj_cond_wait(pthread_cond_t *cond, + pthread_mutex_t *lock) +{ + int ret; + + __threadobj_boost(); + ret = pthread_cond_wait(cond, lock); + __threadobj_unboost(); + + return ret; +} + +int threadobj_cond_signal(pthread_cond_t *cond) +{ + int ret; + + __threadobj_boost(); + ret = pthread_cond_signal(cond); + __threadobj_unboost(); + + return ret; +} + +int threadobj_cond_broadcast(pthread_cond_t *cond) +{ + int ret; + + __threadobj_boost(); + ret = pthread_cond_broadcast(cond); + __threadobj_unboost(); + + return ret; +} + +#endif /* !CONFIG_XENO_WORKAROUND_CONDVAR_PI */ + #endif /* CONFIG_XENO_MERCURY */ static int request_setschedparam(struct threadobj *thobj, int policy, @@ -1042,7 +1153,7 @@ static int wait_on_barrier(struct threadobj *thobj, int mask) oldstate = thobj->cancel_state; push_cleanup_lock(&thobj->lock); __threadobj_tag_unlocked(thobj); - __RT(pthread_cond_wait(&thobj->barrier, &thobj->lock)); + threadobj_cond_wait(&thobj->barrier, &thobj->lock); __threadobj_tag_locked(thobj); pop_cleanup_lock(&thobj->lock); thobj->cancel_state = oldstate; @@ -1062,7 +1173,7 @@ int threadobj_start(struct threadobj *thobj) /* thobj->lock held. */ return 0; thobj->status |= __THREAD_S_STARTED; - __RT(pthread_cond_signal(&thobj->barrier)); + threadobj_cond_signal(&thobj->barrier); if (current && thobj->global_priority <= current->global_priority) return 0; @@ -1128,7 +1239,7 @@ void threadobj_notify_entry(void) /* current->lock free. */ threadobj_lock(current); current->status |= __THREAD_S_ACTIVE; current->run_state = __THREAD_S_RUNNING; - __RT(pthread_cond_signal(¤t->barrier)); + threadobj_cond_signal(¤t->barrier); threadobj_unlock(current); } @@ -1183,7 +1294,7 @@ int threadobj_prologue(struct threadobj *thobj, const char *name) threadobj_lock(thobj); thobj->status &= ~__THREAD_S_WARMUP; - __RT(pthread_cond_signal(&thobj->barrier)); + threadobj_cond_signal(&thobj->barrier); threadobj_unlock(thobj); #ifdef CONFIG_XENO_ASYNC_CANCEL @@ -1247,7 +1358,7 @@ static void cancel_sync(struct threadobj *thobj) /* thobj->lock held */ oldstate = thobj->cancel_state; push_cleanup_lock(&thobj->lock); __threadobj_tag_unlocked(thobj); - __RT(pthread_cond_wait(&thobj->barrier, &thobj->lock)); + threadobj_cond_wait(&thobj->barrier, &thobj->lock); __threadobj_tag_locked(thobj); pop_cleanup_lock(&thobj->lock); thobj->cancel_state = oldstate; @@ -1261,7 +1372,7 @@ static void cancel_sync(struct threadobj *thobj) /* thobj->lock held */ */ if ((thobj->status & __THREAD_S_STARTED) == 0) { thobj->status |= __THREAD_S_ABORTED; - __RT(pthread_cond_signal(&thobj->barrier)); + threadobj_cond_signal(&thobj->barrier); } threadobj_cancel_2_corespec(thobj); diff --git a/lib/copperplate/traceobj.c b/lib/copperplate/traceobj.c index 08fde68..29a4528 100644 --- a/lib/copperplate/traceobj.c +++ b/lib/copperplate/traceobj.c @@ -274,7 +274,7 @@ void traceobj_unwind(struct traceobj *trobj) write_lock_safe(&trobj->lock, state); if (--trobj->nr_threads <= 0) - __RT(pthread_cond_signal(&trobj->join)); + threadobj_cond_signal(&trobj->join); write_unlock_safe(&trobj->lock, state); @@ -301,7 +301,7 @@ void traceobj_join(struct traceobj *trobj) read_lock(&trobj->lock); while (trobj->nr_threads < 0 || trobj->nr_threads > 0) - __RT(pthread_cond_wait(&trobj->join, &trobj->lock)); + threadobj_cond_wait(&trobj->join, &trobj->lock); read_unlock(&trobj->lock); pop_cleanup_lock(&trobj->lock); _______________________________________________ Xenomai-git mailing list Xenomai-git@xenomai.org http://www.xenomai.org/mailman/listinfo/xenomai-git
