<div dir="ltr">My concern is that this will break existing programs that used them for <div>condition synchronization. </div><div><br></div><div>This just requires us to be vigilant to answer questions when things </div><div>break.<br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jun 3, 2016 at 8:37 AM, Sebastian Huber <span dir="ltr"><<a href="mailto:sebastian.huber@embedded-brains.de" target="_blank">sebastian.huber@embedded-brains.de</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">The Classic binary semaphores without a locking protocol<br>
</span>(RTEMS_BINARY_SEMAPHORE) could be released by everyone, e.g. in contrast<br>
<span class="">to the POSIX mutexes (all variants) or the Classic binary semphores with<br>
priority inheritance or ceiling there was no owner check in the release<br>
path.<br>
<br>
This behaviour was a bit unexpected and not documented. Add an owner<br>
check to the release path. Update sptests/sp42 accordingly.<br>
<br>
</span>This change has nothing to do with the simple binary semaphores<br>
(RTEMS_SIMPLE_BINARY_SEMAPHORE) which have no owner at all.<br>
<br>
Update #2725<br>
<div class="HOEnZb"><div class="h5">---<br>
cpukit/rtems/src/semrelease.c | 3 +-<br>
cpukit/score/include/rtems/score/coremuteximpl.h | 54 +++--------<br>
testsuites/sptests/sp42/init.c | 109 ++++++++++++++---------<br>
testsuites/sptests/sp42/sp42.scn | 4 +-<br>
4 files changed, 84 insertions(+), 86 deletions(-)<br>
<br>
diff --git a/cpukit/rtems/src/semrelease.c b/cpukit/rtems/src/semrelease.c<br>
index ccd4e63..39c467d 100644<br>
--- a/cpukit/rtems/src/semrelease.c<br>
+++ b/cpukit/rtems/src/semrelease.c<br>
@@ -64,9 +64,10 @@ rtems_status_code rtems_semaphore_release( rtems_id id )<br>
);<br>
break;<br>
case SEMAPHORE_VARIANT_MUTEX_NO_PROTOCOL:<br>
- _CORE_recursive_mutex_Surrender_no_protocol_classic(<br>
+ _CORE_recursive_mutex_Surrender_no_protocol(<br>
&the_semaphore->Core_control.Mutex.Recursive,<br>
_Semaphore_Get_operations( the_semaphore ),<br>
+ executing,<br>
&queue_context<br>
);<br>
status = STATUS_SUCCESSFUL;<br>
diff --git a/cpukit/score/include/rtems/score/coremuteximpl.h b/cpukit/score/include/rtems/score/coremuteximpl.h<br>
index decf770..956dfa8 100644<br>
--- a/cpukit/score/include/rtems/score/coremuteximpl.h<br>
+++ b/cpukit/score/include/rtems/score/coremuteximpl.h<br>
@@ -311,22 +311,29 @@ RTEMS_INLINE_ROUTINE Status_Control _CORE_recursive_mutex_Seize_no_protocol(<br>
);<br>
}<br>
<br>
-RTEMS_INLINE_ROUTINE void<br>
-_CORE_recursive_mutex_Surrender_no_protocol_finalize(<br>
+RTEMS_INLINE_ROUTINE Status_Control _CORE_recursive_mutex_Surrender_no_protocol(<br>
CORE_recursive_mutex_Control *the_mutex,<br>
const Thread_queue_Operations *operations,<br>
+ Thread_Control *executing,<br>
Thread_queue_Context *queue_context<br>
)<br>
{<br>
unsigned int nest_level;<br>
Thread_Control *new_owner;<br>
<br>
+ _CORE_mutex_Acquire_critical( &the_mutex->Mutex, queue_context );<br>
+<br>
+ if ( !_CORE_mutex_Is_owner( &the_mutex->Mutex, executing ) ) {<br>
+ _CORE_mutex_Release( &the_mutex->Mutex, queue_context );<br>
+ return STATUS_NOT_OWNER;<br>
+ }<br>
+<br>
nest_level = the_mutex->nest_level;<br>
<br>
if ( nest_level > 0 ) {<br>
the_mutex->nest_level = nest_level - 1;<br>
_CORE_mutex_Release( &the_mutex->Mutex, queue_context );<br>
- return;<br>
+ return STATUS_SUCCESSFUL;<br>
}<br>
<br>
new_owner = _Thread_queue_First_locked(<br>
@@ -337,7 +344,7 @@ _CORE_recursive_mutex_Surrender_no_protocol_finalize(<br>
<br>
if ( new_owner == NULL ) {<br>
_CORE_mutex_Release( &the_mutex->Mutex, queue_context );<br>
- return;<br>
+ return STATUS_SUCCESSFUL;<br>
}<br>
<br>
_Thread_queue_Extract_critical(<br>
@@ -346,48 +353,9 @@ _CORE_recursive_mutex_Surrender_no_protocol_finalize(<br>
new_owner,<br>
queue_context<br>
);<br>
-}<br>
-<br>
-RTEMS_INLINE_ROUTINE Status_Control _CORE_recursive_mutex_Surrender_no_protocol(<br>
- CORE_recursive_mutex_Control *the_mutex,<br>
- const Thread_queue_Operations *operations,<br>
- Thread_Control *executing,<br>
- Thread_queue_Context *queue_context<br>
-)<br>
-{<br>
- _CORE_mutex_Acquire_critical( &the_mutex->Mutex, queue_context );<br>
-<br>
- if ( !_CORE_mutex_Is_owner( &the_mutex->Mutex, executing ) ) {<br>
- _CORE_mutex_Release( &the_mutex->Mutex, queue_context );<br>
- return STATUS_NOT_OWNER;<br>
- }<br>
-<br>
- _CORE_recursive_mutex_Surrender_no_protocol_finalize(<br>
- the_mutex,<br>
- operations,<br>
- queue_context<br>
- );<br>
return STATUS_SUCCESSFUL;<br>
}<br>
<br>
-/*<br>
- * The Classic no protocol recursive mutex has the nice property that everyone<br>
- * can release it.<br>
- */<br>
-RTEMS_INLINE_ROUTINE void _CORE_recursive_mutex_Surrender_no_protocol_classic(<br>
- CORE_recursive_mutex_Control *the_mutex,<br>
- const Thread_queue_Operations *operations,<br>
- Thread_queue_Context *queue_context<br>
-)<br>
-{<br>
- _CORE_mutex_Acquire_critical( &the_mutex->Mutex, queue_context );<br>
- _CORE_recursive_mutex_Surrender_no_protocol_finalize(<br>
- the_mutex,<br>
- operations,<br>
- queue_context<br>
- );<br>
-}<br>
-<br>
RTEMS_INLINE_ROUTINE void _CORE_ceiling_mutex_Initialize(<br>
CORE_ceiling_mutex_Control *the_mutex,<br>
Priority_Control priority_ceiling<br>
diff --git a/testsuites/sptests/sp42/init.c b/testsuites/sptests/sp42/init.c<br>
index 55faa9b..6d96ede 100644<br>
--- a/testsuites/sptests/sp42/init.c<br>
+++ b/testsuites/sptests/sp42/init.c<br>
@@ -24,19 +24,12 @@ const char rtems_test_name[] = "SP 42";<br>
<br>
#define MAX_TASKS 20<br>
<br>
-rtems_task Init(rtems_task_argument argument);<br>
-rtems_task Locker_task(rtems_task_argument unused);<br>
-void do_test(<br>
- rtems_attribute attr,<br>
- bool extract /* TRUE if extract, not release */<br>
-);<br>
-<br>
/*<br>
* Carefully chosen to exercise threadq enqueue/dequeue priority logic.<br>
* Somewhat randomly sorted to ensure than if discipline is FIFO, run-time<br>
* behavior won't be the same when released.<br>
*/<br>
-rtems_task_priority Priorities_High[MAX_TASKS] = {<br>
+static const rtems_task_priority Priorities_High[MAX_TASKS] = {<br>
37, 37, 37, 37, /* backward - more 2-n */<br>
2, 2, 2, 2, /* forward - multiple are on 2-n chain */<br>
4, 3, /* forward - search forward arbitrary */<br>
@@ -45,7 +38,7 @@ rtems_task_priority Priorities_High[MAX_TASKS] = {<br>
34, 34, 34, 34, /* backward - multple on 2-n chain */<br>
};<br>
<br>
-rtems_task_priority Priorities_Low[MAX_TASKS] = {<br>
+static const rtems_task_priority Priorities_Low[MAX_TASKS] = {<br>
13, 13, 13, 13, /* backward - more 2-n */<br>
2, 2, 2, 2, /* forward - multiple are on 2-n chain */<br>
4, 3, /* forward - search forward arbitrary */<br>
@@ -54,24 +47,37 @@ rtems_task_priority Priorities_Low[MAX_TASKS] = {<br>
12, 12, 12, 12, /* backward - multple on 2-n chain */<br>
};<br>
<br>
-rtems_task_priority *Priorities;<br>
+static const int Obtain_order[2][MAX_TASKS] = {<br>
+ { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19 },<br>
+ { 4, 5, 6, 7, 9, 10, 11, 12, 13, 8, 16, 17, 18, 19, 0, 1, 2, 3, 15, 14 }<br>
+};<br>
+<br>
+static const rtems_task_priority *Priorities;<br>
+<br>
+static rtems_id Semaphore;<br>
+static rtems_id Master;<br>
+static rtems_id Task_id[ MAX_TASKS ];<br>
+static rtems_name Task_name[ MAX_TASKS ];<br>
+<br>
+static rtems_task_argument Obtain_counter;<br>
<br>
-rtems_id Semaphore;<br>
-rtems_id Task_id[ MAX_TASKS ];<br>
-rtems_name Task_name[ MAX_TASKS ];<br>
+static enum {<br>
+ FIFO,<br>
+ PRIORITY<br>
+} Variant;<br>
<br>
-rtems_task Locker_task(<br>
- rtems_task_argument unused<br>
+static rtems_task Locker_task(<br>
+ rtems_task_argument task_index<br>
)<br>
{<br>
- rtems_id tid;<br>
- uint32_t task_index;<br>
- rtems_status_code status;<br>
+ rtems_id tid;<br>
+ rtems_status_code status;<br>
+ rtems_task_argument my_obtain_counter;<br>
<br>
status = rtems_task_ident( RTEMS_SELF, RTEMS_SEARCH_ALL_NODES, &tid );<br>
directive_failed( status, "rtems_task_ident" );<br>
<br>
- task_index = task_number( tid ) - 1;<br>
+ rtems_test_assert( task_index == task_number( tid ) - 1 );<br>
<br>
status = rtems_semaphore_obtain( Semaphore, RTEMS_DEFAULT_OPTIONS, 0 );<br>
directive_failed( status, "rtems_semaphore_obtain" );<br>
@@ -79,28 +85,45 @@ rtems_task Locker_task(<br>
put_name( Task_name[ task_index ], FALSE );<br>
puts( " - unblocked - OK" );<br>
<br>
+ status = rtems_task_wake_after( 10 );<br>
+ directive_failed( status, "rtems_task_wake_after" );<br>
+<br>
+ my_obtain_counter = Obtain_counter;<br>
+ rtems_test_assert( task_index == Obtain_order[ Variant ][ Obtain_counter ] );<br>
+ ++Obtain_counter;<br>
+<br>
+ status = rtems_semaphore_release( Semaphore );<br>
+ directive_failed( status, "rtems_semaphore_release" );<br>
+<br>
+ if ( my_obtain_counter == MAX_TASKS - 1 ) {<br>
+ status = rtems_event_transient_send( Master );<br>
+ directive_failed( status, "rtems_event_transient_send" );<br>
+ }<br>
+<br>
(void) rtems_task_delete( RTEMS_SELF );<br>
}<br>
<br>
-void do_test(<br>
+static void do_test(<br>
rtems_attribute attr,<br>
bool extract /* TRUE if extract, not release */<br>
)<br>
{<br>
- rtems_status_code status;<br>
- int i;<br>
+ rtems_status_code status;<br>
+ rtems_task_argument i;<br>
+<br>
+ Variant = ( ( attr & RTEMS_PRIORITY ) != 0 ? PRIORITY : FIFO );<br>
+ Obtain_counter = 0;<br>
<br>
status = rtems_semaphore_create(<br>
rtems_build_name( 'S', 'E', 'M', '0' ), /* name = SEM0 */<br>
- 0, /* unlocked */<br>
+ 0, /* locked */<br>
RTEMS_BINARY_SEMAPHORE | attr, /* mutex w/desired discipline */<br>
0, /* IGNORED */<br>
&Semaphore<br>
);<br>
directive_failed( status, "rtems_semaphore_create" );<br>
<br>
- for (i=0 ; i< MAX_TASKS ; i++ ) {<br>
-<br>
+ for (i = 0 ; i < MAX_TASKS ; i++ ) {<br>
Task_name[ i ] = rtems_build_name(<br>
'T',<br>
'A',<br>
@@ -118,42 +141,42 @@ void do_test(<br>
);<br>
directive_failed( status, "rtems_task_create" );<br>
<br>
- status = rtems_task_start(<br>
- Task_id[ i ], Locker_task, (rtems_task_argument)i );<br>
+ status = rtems_task_start( Task_id[ i ], Locker_task, i );<br>
directive_failed( status, "rtems_task_start" );<br>
-<br>
- status = rtems_task_wake_after( 10 );<br>
- directive_failed( status, "rtems_task_wake_after" );<br>
}<br>
<br>
- for (i=0 ; i< MAX_TASKS ; i++ ) {<br>
- if ( extract == FALSE ) {<br>
- status = rtems_semaphore_release( Semaphore );<br>
- directive_failed( status, "rtems_semaphore_release" );<br>
-<br>
- status = rtems_task_wake_after( 100 );<br>
- directive_failed( status, "rtems_task_wake_after" );<br>
- } else {<br>
+ if ( extract ) {<br>
+ for (i = 0 ; i< MAX_TASKS ; i++ ) {<br>
status = rtems_task_delete( Task_id[ i ] );<br>
directive_failed( status, "rtems_task_delete" );<br>
}<br>
}<br>
<br>
- /* one extra release for the initial state */<br>
+ /* do the initial release */<br>
status = rtems_semaphore_release( Semaphore );<br>
directive_failed( status, "rtems_semaphore_release" );<br>
<br>
+ if ( !extract ) {<br>
+ status = rtems_event_transient_receive( RTEMS_WAIT, RTEMS_NO_TIMEOUT );<br>
+ directive_failed( status, "rtems_event_transient_receive" );<br>
+ }<br>
+<br>
/* now delete the semaphore since no one is waiting and it is unlocked */<br>
status = rtems_semaphore_delete( Semaphore );<br>
directive_failed( status, "rtems_semaphore_delete" );<br>
}<br>
<br>
-rtems_task Init(<br>
+static rtems_task Init(<br>
rtems_task_argument argument<br>
)<br>
{<br>
+ rtems_task_priority prio;<br>
+ rtems_status_code status;<br>
+<br>
TEST_BEGIN();<br>
<br>
+ Master = rtems_task_self();<br>
+<br>
if (RTEMS_MAXIMUM_PRIORITY == 255)<br>
Priorities = Priorities_High;<br>
else if (RTEMS_MAXIMUM_PRIORITY == 15)<br>
@@ -163,6 +186,10 @@ rtems_task Init(<br>
rtems_test_exit( 0 );<br>
}<br>
<br>
+ prio = RTEMS_MAXIMUM_PRIORITY - 1;<br>
+ status = rtems_task_set_priority(RTEMS_SELF, prio, &prio);<br>
+ directive_failed( status, "rtems_task_set_priority" );<br>
+<br>
if ( sizeof(Priorities_Low) / sizeof(rtems_task_priority) != MAX_TASKS ) {<br>
puts( "Priorities_Low table does not have right number of entries" );<br>
rtems_test_exit( 0 );<br>
@@ -201,6 +228,8 @@ rtems_task Init(<br>
#define CONFIGURE_MAXIMUM_TASKS MAX_TASKS+1<br>
#define CONFIGURE_MAXIMUM_SEMAPHORES 1<br>
<br>
+#define CONFIGURE_INIT_TASK_INITIAL_MODES RTEMS_DEFAULT_MODES<br>
+<br>
#define CONFIGURE_INITIAL_EXTENSIONS RTEMS_TEST_INITIAL_EXTENSION<br>
<br>
#define CONFIGURE_RTEMS_INIT_TASKS_TABLE<br>
diff --git a/testsuites/sptests/sp42/sp42.scn b/testsuites/sptests/sp42/sp42.scn<br>
index 01adcf7..206d05c 100644<br>
--- a/testsuites/sptests/sp42/sp42.scn<br>
+++ b/testsuites/sptests/sp42/sp42.scn<br>
@@ -1,4 +1,4 @@<br>
-*** START OF TEST 42 ***<br>
+*** BEGIN OF TEST SP 42 ***<br>
Exercising blocking discipline w/extract in FIFO order<br>
Exercising blocking discipline w/unblock in FIFO order<br>
TA00 - unblocked - OK<br>
@@ -44,4 +44,4 @@ TA02 - unblocked - OK<br>
TA03 - unblocked - OK<br>
TA15 - unblocked - OK<br>
TA14 - unblocked - OK<br>
-*** END OF TEST 42 ***<br>
+*** END OF TEST SP 42 ***<br>
--<br>
1.8.4.5<br>
<br>
_______________________________________________<br>
devel mailing list<br>
<a href="mailto:devel@rtems.org">devel@rtems.org</a><br>
<a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
</div></div></blockquote></div><br></div></div></div>