Module: xenomai-3
Branch: next
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(&current->core.grant_sync,
-                                              &sobj->core.lock, timeout);
+               return -threadobj_cond_timedwait(&current->core.grant_sync,
+                                                &sobj->core.lock, timeout);
 
-       return -pthread_cond_wait(&current->core.grant_sync, &sobj->core.lock);
+       return -threadobj_cond_wait(&current->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, &param);
+       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, &param);
+       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(&current->barrier));
+       threadobj_cond_signal(&current->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

Reply via email to