[rtems commit] score: Avoid atomic fences for thread wait flags
Sebastian Huber
sebh at rtems.org
Thu Jun 30 05:58:51 UTC 2016
Module: rtems
Branch: master
Commit: 029877282edb8aa7a2095702742ce95c8246729e
Changeset: http://git.rtems.org/rtems/commit/?id=029877282edb8aa7a2095702742ce95c8246729e
Author: Sebastian Huber <sebastian.huber at embedded-brains.de>
Date: Wed Jun 29 15:33:26 2016 +0200
score: Avoid atomic fences for thread wait flags
The use of atomic fences is brittle and may break due to changes in
different areas which is hard to manage.
---
cpukit/rtems/src/eventseize.c | 8 +----
cpukit/rtems/src/eventsurrender.c | 5 +---
cpukit/rtems/src/ratemonperiod.c | 2 +-
cpukit/rtems/src/ratemontimeout.c | 2 +-
cpukit/score/include/rtems/score/threadimpl.h | 42 +++++++++++++++++++--------
cpukit/score/src/threadqenqueue.c | 4 +--
cpukit/score/src/threadtimeout.c | 8 +----
7 files changed, 37 insertions(+), 34 deletions(-)
diff --git a/cpukit/rtems/src/eventseize.c b/cpukit/rtems/src/eventseize.c
index c91d308..7f5903d 100644
--- a/cpukit/rtems/src/eventseize.c
+++ b/cpukit/rtems/src/eventseize.c
@@ -91,13 +91,7 @@ rtems_status_code _Event_Seize(
_Thread_Set_state( executing, block_state );
- /*
- * See _Event_Surrender() and _Thread_Timeout(), corresponding atomic
- * variable is Thread_Control::Wait::flags.
- */
- _Atomic_Fence( ATOMIC_ORDER_ACQUIRE );
-
- success = _Thread_Wait_flags_try_change(
+ success = _Thread_Wait_flags_try_change_acquire(
executing,
intend_to_block,
wait_class | THREAD_WAIT_STATE_BLOCKED
diff --git a/cpukit/rtems/src/eventsurrender.c b/cpukit/rtems/src/eventsurrender.c
index 576670b..a9bef59 100644
--- a/cpukit/rtems/src/eventsurrender.c
+++ b/cpukit/rtems/src/eventsurrender.c
@@ -91,11 +91,8 @@ rtems_status_code _Event_Surrender(
_Event_Satisfy( the_thread, event, pending_events, seized_events );
- /* See _Event_Seize() */
- _Atomic_Fence( ATOMIC_ORDER_RELEASE );
-
ready_again = wait_class | THREAD_WAIT_STATE_READY_AGAIN;
- success = _Thread_Wait_flags_try_change_critical(
+ success = _Thread_Wait_flags_try_change_release(
the_thread,
wait_class | THREAD_WAIT_STATE_INTEND_TO_BLOCK,
ready_again
diff --git a/cpukit/rtems/src/ratemonperiod.c b/cpukit/rtems/src/ratemonperiod.c
index 771f9c1..303fe17 100644
--- a/cpukit/rtems/src/ratemonperiod.c
+++ b/cpukit/rtems/src/ratemonperiod.c
@@ -221,7 +221,7 @@ static rtems_status_code _Rate_monotonic_Block_while_active(
_Thread_Set_state( executing, STATES_WAITING_FOR_PERIOD );
- success = _Thread_Wait_flags_try_change(
+ success = _Thread_Wait_flags_try_change_acquire(
executing,
RATE_MONOTONIC_INTEND_TO_BLOCK,
RATE_MONOTONIC_BLOCKED
diff --git a/cpukit/rtems/src/ratemontimeout.c b/cpukit/rtems/src/ratemontimeout.c
index 78c78e2..bd38153 100644
--- a/cpukit/rtems/src/ratemontimeout.c
+++ b/cpukit/rtems/src/ratemontimeout.c
@@ -43,7 +43,7 @@ void _Rate_monotonic_Timeout( Watchdog_Control *the_watchdog )
owner->Wait.return_argument = NULL;
- success = _Thread_Wait_flags_try_change_critical(
+ success = _Thread_Wait_flags_try_change_release(
owner,
RATE_MONOTONIC_INTEND_TO_BLOCK,
RATE_MONOTONIC_READY_AGAIN
diff --git a/cpukit/score/include/rtems/score/threadimpl.h b/cpukit/score/include/rtems/score/threadimpl.h
index a4e7469..b4d2f4f 100644
--- a/cpukit/score/include/rtems/score/threadimpl.h
+++ b/cpukit/score/include/rtems/score/threadimpl.h
@@ -1311,8 +1311,10 @@ RTEMS_INLINE_ROUTINE Thread_Wait_flags _Thread_Wait_flags_get(
}
/**
- * @brief Tries to change the thread wait flags inside a critical section
- * (interrupts disabled).
+ * @brief Tries to change the thread wait flags with release semantics in case
+ * of success.
+ *
+ * Must be called inside a critical section (interrupts disabled).
*
* In case the wait flags are equal to the expected wait flags, then the wait
* flags are set to the desired wait flags.
@@ -1324,22 +1326,24 @@ RTEMS_INLINE_ROUTINE Thread_Wait_flags _Thread_Wait_flags_get(
* @retval true The wait flags were equal to the expected wait flags.
* @retval false Otherwise.
*/
-RTEMS_INLINE_ROUTINE bool _Thread_Wait_flags_try_change_critical(
+RTEMS_INLINE_ROUTINE bool _Thread_Wait_flags_try_change_release(
Thread_Control *the_thread,
Thread_Wait_flags expected_flags,
Thread_Wait_flags desired_flags
)
{
+ _Assert( _ISR_Get_level() != 0 );
+
#if defined(RTEMS_SMP)
return _Atomic_Compare_exchange_uint(
&the_thread->Wait.flags,
&expected_flags,
desired_flags,
- ATOMIC_ORDER_RELAXED,
+ ATOMIC_ORDER_RELEASE,
ATOMIC_ORDER_RELAXED
);
#else
- bool success = the_thread->Wait.flags == expected_flags;
+ bool success = ( the_thread->Wait.flags == expected_flags );
if ( success ) {
the_thread->Wait.flags = desired_flags;
@@ -1350,30 +1354,44 @@ RTEMS_INLINE_ROUTINE bool _Thread_Wait_flags_try_change_critical(
}
/**
- * @brief Tries to change the thread wait flags.
+ * @brief Tries to change the thread wait flags with acquire semantics.
+ *
+ * In case the wait flags are equal to the expected wait flags, then the wait
+ * flags are set to the desired wait flags.
+ *
+ * @param[in] the_thread The thread.
+ * @param[in] expected_flags The expected wait flags.
+ * @param[in] desired_flags The desired wait flags.
*
- * @see _Thread_Wait_flags_try_change_critical().
+ * @retval true The wait flags were equal to the expected wait flags.
+ * @retval false Otherwise.
*/
-RTEMS_INLINE_ROUTINE bool _Thread_Wait_flags_try_change(
+RTEMS_INLINE_ROUTINE bool _Thread_Wait_flags_try_change_acquire(
Thread_Control *the_thread,
Thread_Wait_flags expected_flags,
Thread_Wait_flags desired_flags
)
{
bool success;
-#if !defined(RTEMS_SMP)
+#if defined(RTEMS_SMP)
+ return _Atomic_Compare_exchange_uint(
+ &the_thread->Wait.flags,
+ &expected_flags,
+ desired_flags,
+ ATOMIC_ORDER_ACQUIRE,
+ ATOMIC_ORDER_ACQUIRE
+ );
+#else
ISR_Level level;
_ISR_Local_disable( level );
-#endif
- success = _Thread_Wait_flags_try_change_critical(
+ success = _Thread_Wait_flags_try_change_release(
the_thread,
expected_flags,
desired_flags
);
-#if !defined(RTEMS_SMP)
_ISR_Local_enable( level );
#endif
diff --git a/cpukit/score/src/threadqenqueue.c b/cpukit/score/src/threadqenqueue.c
index f014fc5..3be7d58 100644
--- a/cpukit/score/src/threadqenqueue.c
+++ b/cpukit/score/src/threadqenqueue.c
@@ -101,7 +101,7 @@ void _Thread_queue_Enqueue_critical(
* as long as we are in the THREAD_QUEUE_INTEND_TO_BLOCK thread wait state,
* thus we have to cancel the blocking operation ourself if necessary.
*/
- success = _Thread_Wait_flags_try_change(
+ success = _Thread_Wait_flags_try_change_acquire(
the_thread,
THREAD_QUEUE_INTEND_TO_BLOCK,
THREAD_QUEUE_BLOCKED
@@ -144,7 +144,7 @@ bool _Thread_queue_Do_extract_locked(
* We must update the wait flags under protection of the current thread lock,
* otherwise a _Thread_Timeout() running on another processor may interfere.
*/
- success = _Thread_Wait_flags_try_change_critical(
+ success = _Thread_Wait_flags_try_change_release(
the_thread,
THREAD_QUEUE_INTEND_TO_BLOCK,
THREAD_QUEUE_READY_AGAIN
diff --git a/cpukit/score/src/threadtimeout.c b/cpukit/score/src/threadtimeout.c
index 9b5cfa6..a2ba61f 100644
--- a/cpukit/score/src/threadtimeout.c
+++ b/cpukit/score/src/threadtimeout.c
@@ -54,15 +54,9 @@ void _Thread_Timeout( Watchdog_Control *watchdog )
_Thread_Do_timeout( the_thread );
- /*
- * This fence is only necessary for the events, see _Event_Seize(). The
- * thread queues use the thread lock for synchronization.
- */
- _Atomic_Fence( ATOMIC_ORDER_RELEASE );
-
wait_class = wait_flags & THREAD_WAIT_CLASS_MASK;
ready_again = wait_class | THREAD_WAIT_STATE_READY_AGAIN;
- success = _Thread_Wait_flags_try_change_critical(
+ success = _Thread_Wait_flags_try_change_release(
the_thread,
wait_class | THREAD_WAIT_STATE_INTEND_TO_BLOCK,
ready_again
More information about the vc
mailing list