Module: xenomai-3
Branch: next
Commit: e3153cf2dcda718f49d859f6f539c03bb6297c54
URL:    
http://git.xenomai.org/?p=xenomai-3.git;a=commit;h=e3153cf2dcda718f49d859f6f539c03bb6297c54

Author: Philippe Gerum <r...@xenomai.org>
Date:   Sat May 16 16:18:08 2015 +0200

cobalt/mutex, lib/cobalt: detect attempts to sleep while holding a mutex

Going to sleep intentionally while holding a mutex is an application
issue. When user consistency checks are enabled (XENO_OPT_DEBUG_USER),
any attempt to do so will trigger a SIGDEBUG signal to the offending
thread, conveying the SIGDEBUG_RESCNT_SLEEP code.

This change implicitly disables fast locking optimizations for
user-space threads when XENO_OPT_DEBUG_USER is enabled. Since the
debugging code present or future may already add some overhead, this
seems acceptable anyway.

---

 include/cobalt/kernel/thread.h      |   21 +++++++++++++++++++++
 include/cobalt/uapi/kernel/thread.h |    3 ++-
 include/cobalt/uapi/signal.h        |    1 +
 kernel/cobalt/debug.c               |    1 +
 kernel/cobalt/posix/monitor.c       |    3 +--
 kernel/cobalt/posix/mutex.c         |    3 +--
 kernel/cobalt/posix/process.c       |   10 +++++-----
 kernel/cobalt/posix/syscall.c       |    2 +-
 kernel/cobalt/synch.c               |   21 ++++++++-------------
 kernel/cobalt/thread.c              |    9 +++++----
 lib/cobalt/internal.c               |   11 +++++++----
 lib/cobalt/mutex.c                  |   14 +++++++-------
 12 files changed, 60 insertions(+), 39 deletions(-)

diff --git a/include/cobalt/kernel/thread.h b/include/cobalt/kernel/thread.h
index bee24f2..335f2e1 100644
--- a/include/cobalt/kernel/thread.h
+++ b/include/cobalt/kernel/thread.h
@@ -29,6 +29,7 @@
 #include <cobalt/kernel/trace.h>
 #include <cobalt/kernel/synch.h>
 #include <cobalt/uapi/kernel/thread.h>
+#include <cobalt/uapi/signal.h>
 #include <asm/xenomai/machine.h>
 #include <asm/xenomai/thread.h>
 
@@ -478,6 +479,26 @@ int xnthread_map(struct xnthread *thread,
 
 void xnthread_call_mayday(struct xnthread *thread, int reason);
 
+static inline void xnthread_get_resource(struct xnthread *thread)
+{
+       if (xnthread_test_state(thread, XNWEAK|XNDEBUG))
+               thread->res_count++;
+}
+
+static inline int xnthread_put_resource(struct xnthread *thread)
+{
+       if (xnthread_test_state(thread, XNWEAK|XNDEBUG)) {
+               if (unlikely(thread->res_count == 0)) {
+                       xnthread_signal(thread, SIGDEBUG,
+                                       SIGDEBUG_RESCNT_IMBALANCE);
+                       return -EPERM;
+               }
+               thread->res_count--;
+       }
+
+       return 0;
+}
+
 #ifdef CONFIG_SMP
 int xnthread_migrate(int cpu);
 
diff --git a/include/cobalt/uapi/kernel/thread.h 
b/include/cobalt/uapi/kernel/thread.h
index 8f71c0d..eba9c01 100644
--- a/include/cobalt/uapi/kernel/thread.h
+++ b/include/cobalt/uapi/kernel/thread.h
@@ -41,7 +41,7 @@
 #define XNHELD    0x00000200 /**< Thread is held to process emergency. */
 
 #define XNBOOST   0x00000400 /**< Undergoes a PIP boost */
-#define XNDEBUG   0x00000800 /**< Hit a debugger breakpoint */
+#define XNSSTEP   0x00000800 /**< Single-stepped by debugger */
 #define XNLOCK    0x00001000 /**< Holds the scheduler lock (i.e. not 
preemptible) */
 #define XNRRB     0x00002000 /**< Undergoes a round-robin scheduling */
 #define XNWARN    0x00004000 /**< Issue SIGDEBUG on error detection */
@@ -51,6 +51,7 @@
 #define XNUSER    0x00040000 /**< Shadow thread running in userland */
 #define XNJOINED  0x00080000 /**< Another thread waits for joining this thread 
*/
 #define XNTRAPLB  0x00100000 /**< Trap lock break (i.e. may not sleep with 
XNLOCK) */
+#define XNDEBUG   0x00200000 /**< User-level debugging enabled */
 
 /** @} */
 
diff --git a/include/cobalt/uapi/signal.h b/include/cobalt/uapi/signal.h
index 49c4ba4..53e46ba 100644
--- a/include/cobalt/uapi/signal.h
+++ b/include/cobalt/uapi/signal.h
@@ -66,6 +66,7 @@
 #define SIGDEBUG_WATCHDOG              6
 #define SIGDEBUG_RESCNT_IMBALANCE      7
 #define SIGDEBUG_LOCK_BREAK            8
+#define SIGDEBUG_RESCNT_SLEEP          9
 
 #define COBALT_DELAYMAX                        2147483647U
 
diff --git a/kernel/cobalt/debug.c b/kernel/cobalt/debug.c
index 8cd32b3..6324ab7 100644
--- a/kernel/cobalt/debug.c
+++ b/kernel/cobalt/debug.c
@@ -410,6 +410,7 @@ static const char *reason_str[] = {
     [SIGDEBUG_NOMLOCK] = "mlock-check",
     [SIGDEBUG_WATCHDOG] = "runaway-break",
     [SIGDEBUG_RESCNT_IMBALANCE] = "resource-count-imbalance",
+    [SIGDEBUG_RESCNT_SLEEP] = "sleep-holding-resource",
     [SIGDEBUG_LOCK_BREAK] = "scheduler-lock-break",
 };
 
diff --git a/kernel/cobalt/posix/monitor.c b/kernel/cobalt/posix/monitor.c
index e928eeb..f3662b6 100644
--- a/kernel/cobalt/posix/monitor.c
+++ b/kernel/cobalt/posix/monitor.c
@@ -125,8 +125,7 @@ static int monitor_enter(xnhandle_t handle, struct xnthread 
*curr)
        ret = xnsynch_fast_acquire(mon->gate.fastlock, curr->handle);
        switch(ret) {
        case 0:
-               if (xnthread_test_state(curr, XNWEAK))
-                       curr->res_count++;
+               xnthread_get_resource(curr);
                break;
        default:
                /* Nah, we really have to wait. */
diff --git a/kernel/cobalt/posix/mutex.c b/kernel/cobalt/posix/mutex.c
index d659b46..3d31fa2 100644
--- a/kernel/cobalt/posix/mutex.c
+++ b/kernel/cobalt/posix/mutex.c
@@ -316,8 +316,7 @@ COBALT_SYSCALL(mutex_trylock, primary,
        err = xnsynch_fast_acquire(mutex->synchbase.fastlock, curr->handle);
        switch(err) {
        case 0:
-               if (xnthread_test_state(curr, XNWEAK))
-                       curr->res_count++;
+               xnthread_get_resource(curr);
                break;
 
 /* This should not happen, as recursive mutexes are handled in
diff --git a/kernel/cobalt/posix/process.c b/kernel/cobalt/posix/process.c
index bf684a8..4750fde 100644
--- a/kernel/cobalt/posix/process.c
+++ b/kernel/cobalt/posix/process.c
@@ -1014,7 +1014,7 @@ static int handle_taskexit_event(struct task_struct *p) 
/* p == current */
        XENO_BUG_ON(COBALT, thread == NULL);
        trace_cobalt_shadow_unmap(thread);
 
-       if (xnthread_test_state(thread, XNDEBUG))
+       if (xnthread_test_state(thread, XNSSTEP))
                unlock_timers();
 
        xnthread_run_handler_stack(thread, exit_thread);
@@ -1079,7 +1079,7 @@ static int handle_schedule_event(struct task_struct 
*next_task)
         * SIGSTOP and SIGINT in order to encompass both the NPTL and
         * LinuxThreads behaviours.
         */
-       if (xnthread_test_state(next, XNDEBUG)) {
+       if (xnthread_test_state(next, XNSSTEP)) {
                if (signal_pending(next_task)) {
                        /*
                         * Do not grab the sighand lock here: it's
@@ -1094,7 +1094,7 @@ static int handle_schedule_event(struct task_struct 
*next_task)
                            sigismember(&pending, SIGINT))
                                goto no_ptrace;
                }
-               xnthread_clear_state(next, XNDEBUG);
+               xnthread_clear_state(next, XNSSTEP);
                unlock_timers();
        }
 
@@ -1139,7 +1139,7 @@ static int handle_sigwake_event(struct task_struct *p)
         * state bit will be set right after we return, when the task
         * is woken up.
         */
-       if ((p->ptrace & PT_PTRACED) && !xnthread_test_state(thread, XNDEBUG)) {
+       if ((p->ptrace & PT_PTRACED) && !xnthread_test_state(thread, XNSSTEP)) {
                /* We already own the siglock. */
                sigorsets(&pending,
                          &p->pending.signal,
@@ -1148,7 +1148,7 @@ static int handle_sigwake_event(struct task_struct *p)
                if (sigismember(&pending, SIGTRAP) ||
                    sigismember(&pending, SIGSTOP)
                    || sigismember(&pending, SIGINT)) {
-                       xnthread_set_state(thread, XNDEBUG);
+                       xnthread_set_state(thread, XNSSTEP);
                        lock_timers();
                }
        }
diff --git a/kernel/cobalt/posix/syscall.c b/kernel/cobalt/posix/syscall.c
index bc33c96..cf93f07 100644
--- a/kernel/cobalt/posix/syscall.c
+++ b/kernel/cobalt/posix/syscall.c
@@ -90,7 +90,7 @@ static void prepare_for_signal(struct task_struct *p,
                        __xn_error_return(regs,
                                          (sysflags & __xn_exec_norestart) ?
                                          -EINTR : -ERESTARTSYS);
-                       notify = !xnthread_test_state(thread, XNDEBUG);
+                       notify = !xnthread_test_state(thread, XNSSTEP);
                        xnthread_clear_info(thread, XNBREAK);
                }
                xnthread_clear_info(thread, XNKICKED);
diff --git a/kernel/cobalt/synch.c b/kernel/cobalt/synch.c
index da827d0..ce7b206 100644
--- a/kernel/cobalt/synch.c
+++ b/kernel/cobalt/synch.c
@@ -171,6 +171,9 @@ int xnsynch_sleep_on(struct xnsynch *synch, xnticks_t 
timeout,
 
        thread = xnthread_current();
 
+       if (IS_ENABLED(CONFIG_XENO_OPT_DEBUG_USER) && thread->res_count > 0)
+               xnthread_signal(thread, SIGDEBUG, SIGDEBUG_RESCNT_SLEEP);
+       
        xnlock_get_irqsave(&nklock, s);
 
        trace_cobalt_synch_sleepon(synch, thread);
@@ -372,8 +375,7 @@ int xnsynch_try_acquire(struct xnsynch *synch)
                        -EDEADLK : -EBUSY;
 
        xnsynch_set_owner(synch, curr);
-       if (xnthread_test_state(curr, XNWEAK))
-               curr->res_count++;
+       xnthread_get_resource(curr);
 
        return 0;
 }
@@ -438,8 +440,7 @@ redo:
        h = atomic_cmpxchg(lockp, XN_NO_HANDLE, currh);
        if (likely(h == XN_NO_HANDLE)) {
                xnsynch_set_owner(synch, curr);
-               if (xnthread_test_state(curr, XNWEAK))
-                       curr->res_count++;
+               xnthread_get_resource(curr);
                xnthread_clear_info(curr, XNRMID | XNTIMEO | XNBREAK);
                return 0;
        }
@@ -544,8 +545,7 @@ block:
                goto out;
        }
  grab:
-       if (xnthread_test_state(curr, XNWEAK))
-               curr->res_count++;
+       xnthread_get_resource(curr);
 
        if (xnsynch_pended_p(synch))
                currh = xnsynch_fast_claimed(currh);
@@ -668,13 +668,8 @@ struct xnthread *xnsynch_release(struct xnsynch *synch,
 
        trace_cobalt_synch_release(synch);
 
-       if (unlikely(xnthread_test_state(thread, XNWEAK))) {
-               if (thread->res_count == 0)
-                       xnthread_signal(thread, SIGDEBUG,
-                                         SIGDEBUG_RESCNT_IMBALANCE);
-               else
-                       thread->res_count--;
-       }
+       if (xnthread_put_resource(thread))
+               return;
 
        lockp = xnsynch_fastlock(synch);
        XENO_BUG_ON(COBALT, lockp == NULL);
diff --git a/kernel/cobalt/thread.c b/kernel/cobalt/thread.c
index cd9e1c2..5adf44f 100644
--- a/kernel/cobalt/thread.c
+++ b/kernel/cobalt/thread.c
@@ -36,7 +36,6 @@
 #include <cobalt/kernel/select.h>
 #include <cobalt/kernel/lock.h>
 #include <cobalt/kernel/thread.h>
-#include <cobalt/uapi/signal.h>
 #include <trace/events/cobalt-core.h>
 #include <asm-generic/xenomai/mayday.h>
 #include "debug.h"
@@ -167,6 +166,9 @@ int __xnthread_init(struct xnthread *thread,
                ksformat(thread->name,
                         sizeof(thread->name), "@%p", thread);
 
+       if (IS_ENABLED(CONFIG_XENO_OPT_DEBUG_USER))
+               flags |= XNDEBUG;
+
        thread->personality = attr->personality;
        cpus_and(thread->affinity, attr->affinity, cobalt_cpu_affinity);
        thread->sched = sched;
@@ -187,8 +189,7 @@ int __xnthread_init(struct xnthread *thread,
        thread->entry = NULL;
        thread->cookie = NULL;
 
-       gravity = xnthread_test_state(thread, XNUSER) ?
-               XNTIMER_UGRAVITY : XNTIMER_KGRAVITY;
+       gravity = flags & XNUSER ? XNTIMER_UGRAVITY : XNTIMER_KGRAVITY;
        xntimer_init(&thread->rtimer, &nkclock, timeout_handler,
                     sched, gravity);
        xntimer_set_name(&thread->rtimer, thread->name);
@@ -1906,7 +1907,7 @@ int xnthread_harden(void)
         * is just silently queued up to here.
         */
        if (signal_pending(p)) {
-               xnthread_relax(!xnthread_test_state(thread, XNDEBUG),
+               xnthread_relax(!xnthread_test_state(thread, XNSSTEP),
                               SIGDEBUG_MIGRATE_SIGNAL);
                return -ERESTARTSYS;
        }
diff --git a/lib/cobalt/internal.c b/lib/cobalt/internal.c
index d154f26..0eda79a 100644
--- a/lib/cobalt/internal.c
+++ b/lib/cobalt/internal.c
@@ -186,7 +186,7 @@ int cobalt_monitor_enter(cobalt_monitor_t *mon)
         */
 
        status = cobalt_get_current_mode();
-       if (status & (XNRELAX|XNWEAK))
+       if (status & (XNRELAX|XNWEAK|XNDEBUG))
                goto syscall;
 
        state = get_monitor_state(mon);
@@ -226,7 +226,7 @@ int cobalt_monitor_exit(cobalt_monitor_t *mon)
                goto syscall;
 
        status = cobalt_get_current_mode();
-       if (status & XNWEAK)
+       if (status & (XNWEAK|XNDEBUG))
                goto syscall;
 
        cur = cobalt_get_current();
@@ -398,11 +398,14 @@ void cobalt_sigdebug_handler(int sig, siginfo_t *si, void 
*context)
 
        switch (sigdebug_reason(si)) {
        case SIGDEBUG_NOMLOCK:
-               raw_write_out("process memory not locked (missing mlockall?)");
+               raw_write_out("process memory not locked");
                _exit(4);
        case SIGDEBUG_RESCNT_IMBALANCE:
-               raw_write_out("internal resource count imbalance");
+               raw_write_out("resource locking imbalance");
                _exit(5);
+       case SIGDEBUG_RESCNT_SLEEP:
+               raw_write_out("sleeping while holding resource");
+               _exit(6);
        case SIGDEBUG_WATCHDOG:
                raw_write_out("watchdog triggered");
                break;
diff --git a/lib/cobalt/mutex.c b/lib/cobalt/mutex.c
index 46283cc..2f84bde 100644
--- a/lib/cobalt/mutex.c
+++ b/lib/cobalt/mutex.c
@@ -245,13 +245,13 @@ COBALT_IMPL(int, pthread_mutex_lock, (pthread_mutex_t 
*mutex))
                goto autoinit;
 
        /*
-        * We track resource ownership for non real-time shadows in
-        * order to handle the auto-relax feature, so we must always
-        * obtain them via a syscall.
+        * We track resource ownership for auto-relax of non real-time
+        * shadows and some debug features, so we must always obtain
+        * them via a syscall.
         */
   cont:
        status = cobalt_get_current_mode();
-       if ((status & (XNRELAX|XNWEAK)) == 0) {
+       if ((status & (XNRELAX|XNWEAK|XNDEBUG)) == 0) {
                ret = xnsynch_fast_acquire(mutex_get_ownerp(_mutex), cur);
                if (ret == 0) {
                        _mutex->lockcnt = 1;
@@ -342,7 +342,7 @@ COBALT_IMPL(int, pthread_mutex_timedlock, (pthread_mutex_t 
*mutex,
        /* See __cobalt_pthread_mutex_lock() */
   cont:
        status = cobalt_get_current_mode();
-       if ((status & (XNRELAX|XNWEAK)) == 0) {
+       if ((status & (XNRELAX|XNWEAK|XNDEBUG)) == 0) {
                ret = xnsynch_fast_acquire(mutex_get_ownerp(_mutex), cur);
                if (ret == 0) {
                        _mutex->lockcnt = 1;
@@ -425,7 +425,7 @@ COBALT_IMPL(int, pthread_mutex_trylock, (pthread_mutex_t 
*mutex))
 
   cont:
        status = cobalt_get_current_mode();
-       if ((status & (XNRELAX|XNWEAK)) == 0) {
+       if ((status & (XNRELAX|XNWEAK|XNDEBUG)) == 0) {
                err = xnsynch_fast_acquire(mutex_get_ownerp(_mutex), cur);
                if (err == 0) {
                        _mutex->lockcnt = 1;
@@ -520,7 +520,7 @@ COBALT_IMPL(int, pthread_mutex_unlock, (pthread_mutex_t 
*mutex))
        if ((state->flags & COBALT_MUTEX_COND_SIGNAL))
                goto do_syscall;
 
-       if (cobalt_get_current_mode() & XNWEAK)
+       if (cobalt_get_current_mode() & (XNWEAK|XNDEBUG))
                goto do_syscall;
 
        if (xnsynch_fast_release(&state->owner, cur))


_______________________________________________
Xenomai-git mailing list
Xenomai-git@xenomai.org
http://xenomai.org/mailman/listinfo/xenomai-git

Reply via email to