[rtems-libbsd commit] mutex: Use panic() after ISR lock release
Sebastian Huber
sebh at rtems.org
Tue Mar 13 10:31:39 UTC 2018
Module: rtems-libbsd
Branch: master
Commit: eae664ea8fdae9e514dc7970c6cf21a0b9767ddd
Changeset: http://git.rtems.org/rtems-libbsd/commit/?id=eae664ea8fdae9e514dc7970c6cf21a0b9767ddd
Author: Sebastian Huber <sebastian.huber at embedded-brains.de>
Date: Mon Mar 12 14:38:46 2018 +0100
mutex: Use panic() after ISR lock release
Using panic() with interrupts disabled could lead to an additional error
(INTERNAL_ERROR_BAD_THREAD_DISPATCH_ENVIRONMENT) due to a potentially
blocking output.
---
rtemsbsd/include/machine/rtems-bsd-muteximpl.h | 21 +++++-
rtemsbsd/rtems/rtems-kernel-muteximpl.c | 10 ++-
testsuite/mutex01/test_main.c | 100 ++++++++++++++++++++++++-
3 files changed, 124 insertions(+), 7 deletions(-)
diff --git a/rtemsbsd/include/machine/rtems-bsd-muteximpl.h b/rtemsbsd/include/machine/rtems-bsd-muteximpl.h
index 4706201..c947e2b 100644
--- a/rtemsbsd/include/machine/rtems-bsd-muteximpl.h
+++ b/rtemsbsd/include/machine/rtems-bsd-muteximpl.h
@@ -8,7 +8,7 @@
*/
/*
- * Copyright (c) 2014, 2017 embedded brains GmbH. All rights reserved.
+ * Copyright (c) 2014, 2018 embedded brains GmbH. All rights reserved.
*
* embedded brains GmbH
* Dornierstr. 4
@@ -46,6 +46,9 @@
#include <sys/types.h>
#include <sys/lock.h>
+#include <sys/systm.h>
+
+#include <inttypes.h>
#include <rtems/score/threadimpl.h>
#include <rtems/score/threadqimpl.h>
@@ -160,7 +163,12 @@ rtems_bsd_mutex_trylock(struct lock_object *lock, rtems_bsd_mutex *m)
_Thread_Resource_count_increment(executing);
success = 1;
} else if (owner == executing) {
- BSD_ASSERT(lock->lo_flags & LO_RECURSABLE);
+ if ((lock->lo_flags & LO_RECURSABLE) == 0) {
+ rtems_bsd_mutex_release(m, isr_level, &queue_context);
+ panic("mutex trylock: %s: not LO_RECURSABLE\n",
+ m->queue.Queue.name);
+ }
+
++m->nest_level;
success = 1;
} else {
@@ -178,6 +186,7 @@ rtems_bsd_mutex_unlock(rtems_bsd_mutex *m)
ISR_Level isr_level;
Thread_queue_Context queue_context;
Thread_Control *owner;
+ Thread_Control *executing;
int nest_level;
_Thread_queue_Context_initialize(&queue_context);
@@ -186,8 +195,14 @@ rtems_bsd_mutex_unlock(rtems_bsd_mutex *m)
nest_level = m->nest_level;
owner = m->queue.Queue.owner;
+ executing = _Thread_Executing;
- BSD_ASSERT(owner == _Thread_Executing);
+ if (__predict_false(owner != executing)) {
+ rtems_bsd_mutex_release(m, isr_level, &queue_context);
+ panic("mutex unlock: %s: owner 0x%08" PRIx32
+ " != executing 0x%08" PRIx32 "\n", m->queue.Queue.name,
+ owner->Object.id, executing->Object.id);
+ }
if (__predict_true(nest_level == 0)) {
Thread_queue_Heads *heads;
diff --git a/rtemsbsd/rtems/rtems-kernel-muteximpl.c b/rtemsbsd/rtems/rtems-kernel-muteximpl.c
index d240028..8a832b4 100644
--- a/rtemsbsd/rtems/rtems-kernel-muteximpl.c
+++ b/rtemsbsd/rtems/rtems-kernel-muteximpl.c
@@ -7,7 +7,7 @@
*/
/*
- * Copyright (c) 2014, 2016 embedded brains GmbH. All rights reserved.
+ * Copyright (c) 2014, 2018 embedded brains GmbH. All rights reserved.
*
* embedded brains GmbH
* Dornierstr. 4
@@ -48,9 +48,13 @@ rtems_bsd_mutex_lock_more(struct lock_object *lock, rtems_bsd_mutex *m,
Thread_queue_Context *queue_context)
{
if (owner == executing) {
- BSD_ASSERT(lock->lo_flags & LO_RECURSABLE);
- ++m->nest_level;
+ if ((lock->lo_flags & LO_RECURSABLE) == 0) {
+ _Thread_queue_Release(&m->queue, queue_context);
+ panic("mutex lock: %s: not LO_RECURSABLE\n",
+ m->queue.Queue.name);
+ }
+ ++m->nest_level;
_Thread_queue_Release(&m->queue, queue_context);
} else {
_Thread_queue_Context_set_thread_state(queue_context,
diff --git a/testsuite/mutex01/test_main.c b/testsuite/mutex01/test_main.c
index caaf118..c3e193d 100644
--- a/testsuite/mutex01/test_main.c
+++ b/testsuite/mutex01/test_main.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2014 embedded brains GmbH. All rights reserved.
+ * Copyright (c) 2014, 2018 embedded brains GmbH. All rights reserved.
*
* embedded brains GmbH
* Dornierstr. 4
@@ -66,6 +66,7 @@ typedef struct {
int timo;
rtems_id worker_task[WORKER_COUNT];
bool done[WORKER_COUNT];
+ rtems_id panic_task;
} test_context;
static test_context test_instance;
@@ -478,6 +479,102 @@ test_mtx_sleep_timeout(test_context *ctx)
assert(!mtx_initialized(mtx));
}
+#define PANIC_DO_LOCK 0
+
+#define PANIC_DO_TRYLOCK 1
+
+#define PANIC_DO_UNLOCK 2
+
+static void
+panic_task(rtems_task_argument arg)
+{
+ test_context *ctx = &test_instance;
+ struct mtx *mtx = &ctx->mtx;
+
+ switch (arg) {
+ case PANIC_DO_LOCK:
+ mtx_lock(mtx);
+ mtx_lock(mtx);
+ break;
+ case PANIC_DO_TRYLOCK:
+ mtx_lock(mtx);
+ mtx_trylock(mtx);
+ break;
+ case PANIC_DO_UNLOCK:
+ mtx_unlock(mtx);
+ break;
+ default:
+ assert(0);
+ break;
+ }
+
+ rtems_task_suspend(RTEMS_SELF);
+}
+
+static void
+test_mtx_panic(test_context *ctx)
+{
+ struct mtx *mtx = &ctx->mtx;
+ rtems_status_code sc;
+
+ puts("test mtx panic");
+
+ sc = rtems_task_create(rtems_build_name('P', 'A', 'N', 'I'),
+ get_self_prio(), RTEMS_MINIMUM_STACK_SIZE, RTEMS_DEFAULT_MODES,
+ RTEMS_FLOATING_POINT, &ctx->panic_task);
+ assert(sc == RTEMS_SUCCESSFUL);
+
+ assert(!mtx_initialized(mtx));
+ mtx_init(mtx, "test", NULL, MTX_DEF);
+ assert(mtx_initialized(mtx));
+
+ /* Lock recursive panic */
+
+ sc = rtems_task_start(ctx->panic_task, panic_task, PANIC_DO_LOCK);
+ assert(sc == RTEMS_SUCCESSFUL);
+
+ sc = rtems_task_wake_after(RTEMS_YIELD_PROCESSOR);
+ assert(sc == RTEMS_SUCCESSFUL);
+
+ sc = rtems_task_restart(ctx->panic_task, PANIC_DO_UNLOCK);
+ assert(sc == RTEMS_SUCCESSFUL);
+
+ sc = rtems_task_wake_after(RTEMS_YIELD_PROCESSOR);
+ assert(sc == RTEMS_SUCCESSFUL);
+
+ /* Try lock recursive panic */
+
+ sc = rtems_task_restart(ctx->panic_task, PANIC_DO_TRYLOCK);
+ assert(sc == RTEMS_SUCCESSFUL);
+
+ sc = rtems_task_wake_after(RTEMS_YIELD_PROCESSOR);
+ assert(sc == RTEMS_SUCCESSFUL);
+
+ sc = rtems_task_restart(ctx->panic_task, PANIC_DO_UNLOCK);
+ assert(sc == RTEMS_SUCCESSFUL);
+
+ sc = rtems_task_wake_after(RTEMS_YIELD_PROCESSOR);
+ assert(sc == RTEMS_SUCCESSFUL);
+
+ /* Unlock not owner panic */
+
+ mtx_lock(mtx);
+
+ sc = rtems_task_restart(ctx->panic_task, PANIC_DO_UNLOCK);
+ assert(sc == RTEMS_SUCCESSFUL);
+
+ sc = rtems_task_wake_after(RTEMS_YIELD_PROCESSOR);
+ assert(sc == RTEMS_SUCCESSFUL);
+
+ mtx_unlock(mtx);
+
+ mtx_destroy(mtx);
+ assert(!mtx_initialized(mtx));
+
+ sc = rtems_task_delete(ctx->panic_task);
+ assert(sc == RTEMS_SUCCESSFUL);
+}
+
static void
alloc_basic_resources(void)
{
@@ -504,6 +601,7 @@ test_main(void)
test_mtx_recursive(ctx);
test_mtx_trylock(ctx);
test_mtx_lock(ctx);
+ test_mtx_panic(ctx);
assert(rtems_resource_snapshot_check(&snapshot_1));
More information about the vc
mailing list