Hello Joel,
the interrupt enable/disable must be replaced with ISR lock acquire/release for
SMP support. Please have a look at the attached patch.
On 2014-04-11 15:18, Joel Sherrill wrote:
Hi
I thought it would be quicker to simply ask and get insight than bang
heads on a Friday. Building RTEMS with debug enabled, running the
sample/capture, you get an assertion on a lock inconsistency.
Since Sebastian did the lock checking and Chris wrote this code, I was
hoping one of you would have a quick answer.
(gdb) r
Starting program:
/home/joel/rtems-4.11-work/b-sis/./sparc-rtems4.11/c/sis/testsuites/samples/capture/capture.exe
*** BEGIN OF TEST CAPTURE ENGINE ***
Press any key to start capture engine (20s remaining)
Press any key to start capture engine (19s remaining)
Press any key to start capture engine (18s remaining)
Press any key to start capture engine (17s remaining)
Press any key to start capture engine (16s remaining)
Press any key to start capture engine (15s remaining)
Monitor ready, press enter to login.
1-rtems $ cwceil 100
watch ceiling is 100.
1-rtems $ cwfloor 102
watch floor is 102.
1-rtems $ cwglob on
assertion "_Debug_Is_owner_of_giant()" failed: file
"../../../../../../rtems/c/src/../../cpukit/libmisc/capture/capture.c",
line 1372, function: rtems_capture_watch_global
Breakpoint 1, _Terminate
(the_source=the_source@entry=RTEMS_FATAL_SOURCE_ASSERT,
is_internal=is_internal@entry=false, the_error=the_error@entry=33825456)
at ../../../../../../rtems/c/src/../../cpukit/score/src/interr.c:36
36 {
(gdb) bt
#0 _Terminate (the_source=the_source@entry=RTEMS_FATAL_SOURCE_ASSERT,
is_internal=is_internal@entry=false, the_error=the_error@entry=33825456)
at ../../../../../../rtems/c/src/../../cpukit/score/src/interr.c:36
#1 0x02010aa0 in rtems_fatal
(source=source@entry=RTEMS_FATAL_SOURCE_ASSERT,
error=error@entry=33825456) at
../../../../../../rtems/c/src/../../cpukit/sapi/src/fatal2.c:34
#2 0x0200a66c in __assert_func (
file=0x20331e0
"../../../../../../rtems/c/src/../../cpukit/libmisc/capture/capture.c",
line=1372,
func=func@entry=0x2033330 <__FUNCTION__.6346>
"rtems_capture_watch_global",
failedexpr=failedexpr@entry=0x2032188 "_Debug_Is_owner_of_giant()")
at
../../../../../../rtems/c/src/../../cpukit/libcsupport/src/__assert.c:52
#3 0x02007e28 in rtems_capture_watch_global (enable=enable@entry=true)
at
../../../../../../rtems/c/src/../../cpukit/libmisc/capture/capture.c:1383
#4 0x02003d70 in rtems_capture_cli_watch_global (argc=2,
argv=<optimized out>,
command_arg=<optimized out>, verbose=<optimized out>)
at
../../../../../../rtems/c/src/../../cpukit/libmisc/capture/capture-cli.c:939
#5 0x02018574 in rtems_monitor_task (monitor_flags=0)
at
../../../../../../rtems/c/src/../../cpukit/libmisc/monitor/mon-editor.c:590
#6 0x02020d48 in _Thread_Handler ()
at
../../../../../../rtems/c/src/../../cpukit/score/src/threadhandler.c:192
#7 0x02020bdc in _Thread_Handler ()
at
../../../../../../rtems/c/src/../../cpukit/score/src/threadhandler.c:96
(gdb) q
A debugging session is active.
Inferior 1 [process 42000] will be killed.
Quit anyway? (y or n) y
--
Sebastian Huber, embedded brains GmbH
Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.
Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
>From 9afc24835a87f6337e189dbdad0b0244a815da30 Mon Sep 17 00:00:00 2001
From: Sebastian Huber <sebastian.hu...@embedded-brains.de>
Date: Fri, 11 Apr 2014 15:41:27 +0200
Subject: [PATCH] capture: Use ISR lock for SMP support
---
cpukit/libmisc/capture/capture.c | 143 +++++++++++++++++++-------------------
1 files changed, 72 insertions(+), 71 deletions(-)
diff --git a/cpukit/libmisc/capture/capture.c b/cpukit/libmisc/capture/capture.c
index 6bdec50..da9d785 100644
--- a/cpukit/libmisc/capture/capture.c
+++ b/cpukit/libmisc/capture/capture.c
@@ -85,6 +85,8 @@ static rtems_capture_timestamp capture_timestamp;
static rtems_task_priority capture_ceiling;
static rtems_task_priority capture_floor;
static rtems_id capture_reader;
+static rtems_interrupt_lock capture_lock =
+ RTEMS_INTERRUPT_LOCK_INITIALIZER("capture");
/*
* RTEMS Event text.
@@ -335,9 +337,9 @@ rtems_capture_find_control (rtems_name name, rtems_id id)
static inline rtems_capture_control_t*
rtems_capture_create_control (rtems_name name, rtems_id id)
{
- rtems_interrupt_level level;
- rtems_capture_control_t* control;
- rtems_capture_task_t* task;
+ rtems_interrupt_lock_context lock_context;
+ rtems_capture_control_t* control;
+ rtems_capture_task_t* task;
if ((name == 0) && (id == 0))
return NULL;
@@ -363,7 +365,7 @@ rtems_capture_create_control (rtems_name name, rtems_id id)
memset (control->by, 0, sizeof (control->by));
- rtems_interrupt_disable (level);
+ rtems_interrupt_lock_acquire (&capture_lock, &lock_context);
control->next = capture_controls;
capture_controls = control;
@@ -376,7 +378,7 @@ rtems_capture_create_control (rtems_name name, rtems_id id)
if (rtems_capture_match_name_id (name, id, task->name, task->id))
task->control = control;
- rtems_interrupt_enable (level);
+ rtems_interrupt_lock_release (&capture_lock, &lock_context);
}
return control;
@@ -393,12 +395,12 @@ rtems_capture_create_control (rtems_name name, rtems_id id)
static inline rtems_capture_task_t*
rtems_capture_create_capture_task (rtems_tcb* new_task)
{
- rtems_interrupt_level level;
- rtems_capture_task_t* task;
- rtems_capture_control_t* control;
- rtems_name name;
- rtems_capture_time_t time;
- bool ok;
+ rtems_interrupt_lock_context lock_context;
+ rtems_capture_task_t* task;
+ rtems_capture_control_t* control;
+ rtems_name name;
+ rtems_capture_time_t time;
+ bool ok;
ok = rtems_workspace_allocate (sizeof (*task), (void **) &task);
@@ -440,7 +442,7 @@ rtems_capture_create_capture_task (rtems_tcb* new_task)
task->stack_size = new_task->Start.Initial_stack.size;
task->stack_clean = task->stack_size;
- rtems_interrupt_disable (level);
+ rtems_interrupt_lock_acquire (&capture_lock, &lock_context);
task->forw = capture_tasks;
if (task->forw)
@@ -448,7 +450,7 @@ rtems_capture_create_capture_task (rtems_tcb* new_task)
task->back = NULL;
capture_tasks = task;
- rtems_interrupt_enable (level);
+ rtems_interrupt_lock_release (&capture_lock, &lock_context);
/*
* We need to scan the default control list to initialise
@@ -478,9 +480,9 @@ rtems_capture_destroy_capture_task (rtems_capture_task_t* task)
{
if (task)
{
- rtems_interrupt_level level;
+ rtems_interrupt_lock_context lock_context;
- rtems_interrupt_disable (level);
+ rtems_interrupt_lock_acquire (&capture_lock, &lock_context);
if (task->tcb || task->refcount)
task = 0;
@@ -495,7 +497,7 @@ rtems_capture_destroy_capture_task (rtems_capture_task_t* task)
capture_tasks = task->forw;
}
- rtems_interrupt_enable (level);
+ rtems_interrupt_lock_release (&capture_lock, &lock_context);
rtems_workspace_free (task);
}
@@ -538,9 +540,9 @@ rtems_capture_record (rtems_capture_task_t* task,
((capture_flags & RTEMS_CAPTURE_GLOBAL_WATCH) ||
(control && (control->flags & RTEMS_CAPTURE_WATCH)))))
{
- rtems_interrupt_level level;
+ rtems_interrupt_lock_context lock_context;
- rtems_interrupt_disable (level);
+ rtems_interrupt_lock_acquire (&capture_lock, &lock_context);
if (capture_count < capture_size)
{
@@ -564,7 +566,7 @@ rtems_capture_record (rtems_capture_task_t* task,
}
else
capture_flags |= RTEMS_CAPTURE_OVERFLOW;
- rtems_interrupt_enable (level);
+ rtems_interrupt_lock_release (&capture_lock, &lock_context);
}
}
}
@@ -1050,16 +1052,16 @@ rtems_capture_open (uint32_t size, rtems_capture_timestamp timestamp __attribu
rtems_status_code
rtems_capture_close (void)
{
- rtems_interrupt_level level;
- rtems_capture_task_t* task;
- rtems_capture_control_t* control;
- rtems_status_code sc;
+ rtems_interrupt_lock_context lock_context;
+ rtems_capture_task_t* task;
+ rtems_capture_control_t* control;
+ rtems_status_code sc;
- rtems_interrupt_disable (level);
+ rtems_interrupt_lock_acquire (&capture_lock, &lock_context);
if (!capture_records)
{
- rtems_interrupt_enable (level);
+ rtems_interrupt_lock_release (&capture_lock, &lock_context);
return RTEMS_SUCCESSFUL;
}
@@ -1067,7 +1069,7 @@ rtems_capture_close (void)
capture_records = NULL;
- rtems_interrupt_enable (level);
+ rtems_interrupt_lock_release (&capture_lock, &lock_context);
/*
* Delete the extension first. This means we are now able to
@@ -1126,13 +1128,13 @@ rtems_capture_task_setup (Thread_Control *tcb)
rtems_status_code
rtems_capture_control (bool enable)
{
- rtems_interrupt_level level;
+ rtems_interrupt_lock_context lock_context;
- rtems_interrupt_disable (level);
+ rtems_interrupt_lock_acquire (&capture_lock, &lock_context);
if (!capture_records)
{
- rtems_interrupt_enable (level);
+ rtems_interrupt_lock_release (&capture_lock, &lock_context);
return RTEMS_UNSATISFIED;
}
@@ -1143,7 +1145,7 @@ rtems_capture_control (bool enable)
rtems_iterate_over_all_threads (rtems_capture_task_setup);
- rtems_interrupt_enable (level);
+ rtems_interrupt_lock_release (&capture_lock, &lock_context);
return RTEMS_SUCCESSFUL;
}
@@ -1160,13 +1162,13 @@ rtems_capture_control (bool enable)
rtems_status_code
rtems_capture_monitor (bool enable)
{
- rtems_interrupt_level level;
+ rtems_interrupt_lock_context lock_context;
- rtems_interrupt_disable (level);
+ rtems_interrupt_lock_acquire (&capture_lock, &lock_context);
if (!capture_records)
{
- rtems_interrupt_enable (level);
+ rtems_interrupt_lock_release (&capture_lock, &lock_context);
return RTEMS_UNSATISFIED;
}
@@ -1175,7 +1177,7 @@ rtems_capture_monitor (bool enable)
else
capture_flags &= ~RTEMS_CAPTURE_ONLY_MONITOR;
- rtems_interrupt_enable (level);
+ rtems_interrupt_lock_release (&capture_lock, &lock_context);
return RTEMS_SUCCESSFUL;
}
@@ -1191,10 +1193,10 @@ rtems_capture_monitor (bool enable)
rtems_status_code
rtems_capture_flush (bool prime)
{
- rtems_interrupt_level level;
- rtems_capture_task_t* task;
+ rtems_interrupt_lock_context lock_context;
+ rtems_capture_task_t* task;
- rtems_interrupt_disable (level);
+ rtems_interrupt_lock_acquire (&capture_lock, &lock_context);
for (task = capture_tasks; task != NULL; task = task->forw)
{
@@ -1211,7 +1213,7 @@ rtems_capture_flush (bool prime)
capture_in = capture_records;
capture_out = 0;
- rtems_interrupt_enable (level);
+ rtems_interrupt_lock_release (&capture_lock, &lock_context);
task = capture_tasks;
@@ -1269,11 +1271,11 @@ rtems_capture_watch_add (rtems_name name, rtems_id id)
rtems_status_code
rtems_capture_watch_del (rtems_name name, rtems_id id)
{
- rtems_interrupt_level level;
- rtems_capture_control_t* control;
- rtems_capture_control_t** prev_control;
- rtems_capture_task_t* task;
- bool found = false;
+ rtems_interrupt_lock_context lock_context;
+ rtems_capture_control_t* control;
+ rtems_capture_control_t** prev_control;
+ rtems_capture_task_t* task;
+ bool found = false;
/*
* Should this test be for wildcards ?
@@ -1284,7 +1286,7 @@ rtems_capture_watch_del (rtems_name name, rtems_id id)
{
if (rtems_capture_match_name_id (control->name, control->id, name, id))
{
- rtems_interrupt_disable (level);
+ rtems_interrupt_lock_acquire (&capture_lock, &lock_context);
for (task = capture_tasks; task != NULL; task = task->forw)
if (task->control == control)
@@ -1292,7 +1294,7 @@ rtems_capture_watch_del (rtems_name name, rtems_id id)
*prev_control = control->next;
- rtems_interrupt_enable (level);
+ rtems_interrupt_lock_release (&capture_lock, &lock_context);
rtems_workspace_free (control);
@@ -1324,9 +1326,9 @@ rtems_capture_watch_del (rtems_name name, rtems_id id)
rtems_status_code
rtems_capture_watch_ctrl (rtems_name name, rtems_id id, bool enable)
{
- rtems_interrupt_level level;
- rtems_capture_control_t* control;
- bool found = false;
+ rtems_interrupt_lock_context lock_context;
+ rtems_capture_control_t* control;
+ bool found = false;
/*
* Find the control and then set the watch. It must exist before it can
@@ -1336,14 +1338,14 @@ rtems_capture_watch_ctrl (rtems_name name, rtems_id id, bool enable)
{
if (rtems_capture_match_name_id (control->name, control->id, name, id))
{
- rtems_interrupt_disable (level);
+ rtems_interrupt_lock_acquire (&capture_lock, &lock_context);
if (enable)
control->flags |= RTEMS_CAPTURE_WATCH;
else
control->flags &= ~RTEMS_CAPTURE_WATCH;
- rtems_interrupt_enable (level);
+ rtems_interrupt_lock_release (&capture_lock, &lock_context);
found = true;
}
@@ -1367,9 +1369,9 @@ rtems_capture_watch_ctrl (rtems_name name, rtems_id id, bool enable)
rtems_status_code
rtems_capture_watch_global (bool enable)
{
- rtems_interrupt_level level;
+ rtems_interrupt_lock_context lock_context;
- rtems_interrupt_disable (level);
+ rtems_interrupt_lock_acquire (&capture_lock, &lock_context);
/*
* We need to keep specific and global watches separate so
@@ -1380,7 +1382,7 @@ rtems_capture_watch_global (bool enable)
else
capture_flags &= ~RTEMS_CAPTURE_GLOBAL_WATCH;
- rtems_interrupt_enable (level);
+ rtems_interrupt_lock_release (&capture_lock, &lock_context);
return RTEMS_SUCCESSFUL;
}
@@ -1695,14 +1697,14 @@ rtems_capture_read (uint32_t threshold,
uint32_t* read,
rtems_capture_record_t** recs)
{
- rtems_interrupt_level level;
- rtems_status_code sc = RTEMS_SUCCESSFUL;
- uint32_t count;
+ rtems_interrupt_lock_context lock_context;
+ rtems_status_code sc = RTEMS_SUCCESSFUL;
+ uint32_t count;
*read = 0;
*recs = NULL;
- rtems_interrupt_disable (level);
+ rtems_interrupt_lock_acquire (&capture_lock, &lock_context);
/*
* Only one reader is allowed.
@@ -1710,14 +1712,14 @@ rtems_capture_read (uint32_t threshold,
if (capture_flags & RTEMS_CAPTURE_READER_ACTIVE)
{
- rtems_interrupt_enable (level);
+ rtems_interrupt_lock_release (&capture_lock, &lock_context);
return RTEMS_RESOURCE_IN_USE;
}
capture_flags |= RTEMS_CAPTURE_READER_ACTIVE;
*read = count = capture_count;
- rtems_interrupt_enable (level);
+ rtems_interrupt_lock_release (&capture_lock, &lock_context);
*recs = &capture_records[capture_out];
@@ -1744,11 +1746,11 @@ rtems_capture_read (uint32_t threshold,
rtems_task_ident (RTEMS_SELF, RTEMS_LOCAL, &capture_reader);
- rtems_interrupt_disable (level);
+ rtems_interrupt_lock_acquire (&capture_lock, &lock_context);
capture_flags |= RTEMS_CAPTURE_READER_WAITING;
- rtems_interrupt_enable (level);
+ rtems_interrupt_lock_release (&capture_lock, &lock_context);
sc = rtems_event_receive (RTEMS_EVENT_0,
RTEMS_WAIT | RTEMS_EVENT_ANY,
@@ -1763,11 +1765,11 @@ rtems_capture_read (uint32_t threshold,
if ((sc != RTEMS_SUCCESSFUL) && (sc != RTEMS_TIMEOUT))
break;
- rtems_interrupt_disable (level);
+ rtems_interrupt_lock_acquire (&capture_lock, &lock_context);
*read = count = capture_count;
- rtems_interrupt_enable (level);
+ rtems_interrupt_lock_release (&capture_lock, &lock_context);
continue;
}
@@ -1793,17 +1795,16 @@ rtems_capture_read (uint32_t threshold,
rtems_status_code
rtems_capture_release (uint32_t count)
{
- rtems_capture_record_t* rec;
- uint32_t counted;
-
- rtems_interrupt_level level;
+ rtems_interrupt_lock_context lock_context;
+ rtems_capture_record_t* rec;
+ uint32_t counted;
- rtems_interrupt_disable (level);
+ rtems_interrupt_lock_acquire (&capture_lock, &lock_context);
if (count > capture_count)
count = capture_count;
- rtems_interrupt_enable (level);
+ rtems_interrupt_lock_release (&capture_lock, &lock_context);
counted = count;
@@ -1816,7 +1817,7 @@ rtems_capture_release (uint32_t count)
rec++;
}
- rtems_interrupt_disable (level);
+ rtems_interrupt_lock_acquire (&capture_lock, &lock_context);
capture_count -= count;
@@ -1824,7 +1825,7 @@ rtems_capture_release (uint32_t count)
capture_flags &= ~RTEMS_CAPTURE_READER_ACTIVE;
- rtems_interrupt_enable (level);
+ rtems_interrupt_lock_release (&capture_lock, &lock_context);
return RTEMS_SUCCESSFUL;
}
--
1.7.7
_______________________________________________
rtems-devel mailing list
rtems-devel@rtems.org
http://www.rtems.org/mailman/listinfo/rtems-devel
