[rtems commit] rtems: Avoid Giant lock for signals

Sebastian Huber sebh at rtems.org
Fri May 6 06:18:08 UTC 2016


Module:    rtems
Branch:    master
Commit:    bb2ad039a7246eecde65592a5116c86d3dede34b
Changeset: http://git.rtems.org/rtems/commit/?id=bb2ad039a7246eecde65592a5116c86d3dede34b

Author:    Sebastian Huber <sebastian.huber at embedded-brains.de>
Date:      Wed May  4 10:09:45 2016 +0200

rtems: Avoid Giant lock for signals

Update #2555.

---

 cpukit/rtems/include/rtems/rtems/asr.h      |  2 +-
 cpukit/rtems/include/rtems/rtems/asrimpl.h  | 66 ++++++++----------------
 cpukit/rtems/include/rtems/rtems/signalmp.h | 31 ++---------
 cpukit/rtems/src/signalcatch.c              | 12 ++---
 cpukit/rtems/src/signalmp.c                 | 79 ++++++++++++++++-------------
 cpukit/rtems/src/signalsend.c               | 74 +++++++++++++--------------
 cpukit/rtems/src/taskmode.c                 |  4 +-
 7 files changed, 115 insertions(+), 153 deletions(-)

diff --git a/cpukit/rtems/include/rtems/rtems/asr.h b/cpukit/rtems/include/rtems/rtems/asr.h
index 0110a40..4563750 100644
--- a/cpukit/rtems/include/rtems/rtems/asr.h
+++ b/cpukit/rtems/include/rtems/rtems/asr.h
@@ -76,7 +76,7 @@ typedef struct {
   /** This field indicates if nest level of signals being processed */
   uint32_t          nest_level;
   /** Lock to protect this structure */
-  ISR_lock_Control  Lock;
+  ISR_LOCK_MEMBER(  Lock )
 }   ASR_Information;
 
 /*
diff --git a/cpukit/rtems/include/rtems/rtems/asrimpl.h b/cpukit/rtems/include/rtems/rtems/asrimpl.h
index 7c1c648..892c58c 100644
--- a/cpukit/rtems/include/rtems/rtems/asrimpl.h
+++ b/cpukit/rtems/include/rtems/rtems/asrimpl.h
@@ -59,41 +59,29 @@ RTEMS_INLINE_ROUTINE void _ASR_Destroy( ASR_Information *asr )
   _ISR_lock_Destroy( &asr->Lock );
 }
 
-RTEMS_INLINE_ROUTINE void _ASR_Acquire(
+RTEMS_INLINE_ROUTINE void _ASR_Acquire_critical(
   ASR_Information  *asr,
   ISR_lock_Context *lock_context
 )
 {
-  _ISR_lock_ISR_disable_and_acquire( &asr->Lock, lock_context );
+  _ISR_lock_Acquire( &asr->Lock, lock_context );
 }
 
-RTEMS_INLINE_ROUTINE void _ASR_Release(
+RTEMS_INLINE_ROUTINE void _ASR_Acquire(
   ASR_Information  *asr,
   ISR_lock_Context *lock_context
 )
 {
-  _ISR_lock_Release_and_ISR_enable( &asr->Lock, lock_context );
+  _ISR_lock_ISR_disable( lock_context );
+  _ASR_Acquire_critical( asr, lock_context );
 }
 
-/**
- *  @brief ASR_Swap_signals
- *
- *  This routine atomically swaps the pending and posted signal
- *  sets.  This is done when the thread alters its mode in such a
- *  way that the RTEMS_ASR disable/enable flag changes.
- */
-RTEMS_INLINE_ROUTINE void _ASR_Swap_signals (
-  ASR_Information *asr
+RTEMS_INLINE_ROUTINE void _ASR_Release(
+  ASR_Information  *asr,
+  ISR_lock_Context *lock_context
 )
 {
-  rtems_signal_set _signals;
-  ISR_lock_Context lock_context;
-
-  _ASR_Acquire( asr, &lock_context );
-    _signals             = asr->signals_pending;
-    asr->signals_pending = asr->signals_posted;
-    asr->signals_posted  = _signals;
-  _ASR_Release( asr, &lock_context );
+  _ISR_lock_Release_and_ISR_enable( &asr->Lock, lock_context );
 }
 
 /**
@@ -109,38 +97,26 @@ RTEMS_INLINE_ROUTINE bool _ASR_Is_null_handler (
   return asr_handler == NULL;
 }
 
-/**
- *  @brief ASR_Are_signals_pending
- *
- *  This function returns TRUE if there are signals pending in the
- *  given RTEMS_ASR information record and FALSE otherwise.
- */
-RTEMS_INLINE_ROUTINE bool _ASR_Are_signals_pending (
-  ASR_Information *asr
-)
+RTEMS_INLINE_ROUTINE rtems_signal_set _ASR_Swap_signals( ASR_Information *asr )
 {
-  return asr->signals_posted != 0;
+  rtems_signal_set new_signals_posted;
+  ISR_lock_Context lock_context;
+
+  _ASR_Acquire( asr, &lock_context );
+    new_signals_posted   = asr->signals_pending;
+    asr->signals_pending = asr->signals_posted;
+    asr->signals_posted  = new_signals_posted;
+  _ASR_Release( asr, &lock_context );
+
+  return new_signals_posted;
 }
 
-/**
- *  @brief ASR_Post_signals
- *
- *  This routine posts the given signals into the signal_set
- *  passed in.  The result is returned to the user in signal_set.
- *
- *  NOTE:  This must be implemented as a macro.
- */
 RTEMS_INLINE_ROUTINE void _ASR_Post_signals(
-  ASR_Information  *asr,
   rtems_signal_set  signals,
   rtems_signal_set *signal_set
 )
 {
-  ISR_lock_Context lock_context;
-
-  _ASR_Acquire( asr, &lock_context );
-    *signal_set |= signals;
-  _ASR_Release( asr, &lock_context );
+  *signal_set |= signals;
 }
 
 RTEMS_INLINE_ROUTINE rtems_signal_set _ASR_Get_posted_signals(
diff --git a/cpukit/rtems/include/rtems/rtems/signalmp.h b/cpukit/rtems/include/rtems/rtems/signalmp.h
index 2f85ac6..57b8682 100644
--- a/cpukit/rtems/include/rtems/rtems/signalmp.h
+++ b/cpukit/rtems/include/rtems/rtems/signalmp.h
@@ -38,25 +38,6 @@ extern "C" {
  */
 /*{*/
 
-/**
- *  The following enumerated type defines the list of
- *  remote signal operations.
- */
-typedef enum {
-  SIGNAL_MP_SEND_REQUEST  = 0,
-  SIGNAL_MP_SEND_RESPONSE = 1
-}   Signal_MP_Remote_operations;
-
-/**
- *  The following data structure defines the packet used to perform
- *  remote signal operations.
- */
-typedef struct {
-  rtems_packet_prefix          Prefix;
-  Signal_MP_Remote_operations  operation;
-  rtems_signal_set             signal_in;
-}   Signal_MP_Packet;
-
 /*
  *  @brief Signal_MP_Send_process_packet
  *
@@ -68,15 +49,11 @@ typedef struct {
  */
 
 /**
- *  @brief Signal MP Send Request Packet
- *
- *  This routine performs a remote procedure call so that a
- *  directive operation can be initiated on another node.
+ * @brief Issues a remote rtems_signal_send() request.
  */
-rtems_status_code _Signal_MP_Send_request_packet (
-  Signal_MP_Remote_operations operation,
-  Objects_Id                  task_id,
-  rtems_signal_set            signal_in
+rtems_status_code _Signal_MP_Send(
+  rtems_id         id,
+  rtems_signal_set signal_set
 );
 
 /**
diff --git a/cpukit/rtems/src/signalcatch.c b/cpukit/rtems/src/signalcatch.c
index 7bf298d..73138a1 100644
--- a/cpukit/rtems/src/signalcatch.c
+++ b/cpukit/rtems/src/signalcatch.c
@@ -22,8 +22,6 @@
 #include <rtems/rtems/signalimpl.h>
 #include <rtems/rtems/asrimpl.h>
 #include <rtems/rtems/tasks.h>
-#include <rtems/score/isrlevel.h>
-#include <rtems/score/threaddispatch.h>
 #include <rtems/score/threadimpl.h>
 
 void _Signal_Action_handler(
@@ -52,8 +50,9 @@ void _Signal_Action_handler(
   asr = &api->Signal;
   signal_set = _ASR_Get_posted_signals( asr );
 
-  if ( !signal_set ) /* similar to _ASR_Are_signals_pending( asr ) */
+  if ( signal_set == 0 ) {
     return;
+  }
 
   asr->nest_level += 1;
   rtems_task_mode( asr->mode_set, RTEMS_ALL_MODE_MASKS, &prev_mode );
@@ -74,11 +73,12 @@ rtems_status_code rtems_signal_catch(
   ASR_Information    *asr;
   ISR_lock_Context    lock_context;
 
-  executing = _Thread_Get_executing();
-  api = (RTEMS_API_Control*)executing->API_Extensions[ THREAD_API_RTEMS ];
+  _ISR_lock_ISR_disable( &lock_context );
+  executing = _Thread_Executing;
+  api = executing->API_Extensions[ THREAD_API_RTEMS ];
   asr = &api->Signal;
 
-  _ASR_Acquire( asr, &lock_context );
+  _ASR_Acquire_critical( asr, &lock_context );
 
   if ( !_ASR_Is_null_handler( asr_handler ) ) {
     asr->mode_set = mode_set;
diff --git a/cpukit/rtems/src/signalmp.c b/cpukit/rtems/src/signalmp.c
index 8c63c2f..cd89e9f 100644
--- a/cpukit/rtems/src/signalmp.c
+++ b/cpukit/rtems/src/signalmp.c
@@ -24,13 +24,36 @@
 #include <rtems/score/threadimpl.h>
 #include <rtems/score/threadqimpl.h>
 
+/**
+ *  The following enumerated type defines the list of
+ *  remote signal operations.
+ */
+typedef enum {
+  SIGNAL_MP_SEND_REQUEST  = 0,
+  SIGNAL_MP_SEND_RESPONSE = 1
+}   Signal_MP_Remote_operations;
+
+/**
+ *  The following data structure defines the packet used to perform
+ *  remote signal operations.
+ */
+typedef struct {
+  rtems_packet_prefix          Prefix;
+  Signal_MP_Remote_operations  operation;
+  rtems_signal_set             signal_set;
+}   Signal_MP_Packet;
+
 RTEMS_STATIC_ASSERT(
   sizeof(Signal_MP_Packet) <= MP_PACKET_MINIMUM_PACKET_SIZE,
   Signal_MP_Packet
 );
 
-static Signal_MP_Packet *_Signal_MP_Get_packet( void )
+static Signal_MP_Packet *_Signal_MP_Get_packet( Objects_Id id )
 {
+  if ( !_Thread_MP_Is_remote( id ) ) {
+    return NULL;
+  }
+
   return (Signal_MP_Packet *) _MPCI_Get_packet();
 }
 
@@ -42,43 +65,31 @@ static Signal_MP_Packet *_Signal_MP_Get_packet( void )
  *
  */
 
-rtems_status_code _Signal_MP_Send_request_packet (
-  Signal_MP_Remote_operations operation,
-  Objects_Id                  task_id,
-  rtems_signal_set            signal_in
+rtems_status_code _Signal_MP_Send(
+  rtems_id         id,
+  rtems_signal_set signal_set
 )
 {
   Signal_MP_Packet *the_packet;
 
-  switch ( operation ) {
-
-    case SIGNAL_MP_SEND_REQUEST:
-
-      the_packet                    = _Signal_MP_Get_packet();
-      the_packet->Prefix.the_class  = MP_PACKET_SIGNAL;
-      the_packet->Prefix.length     = sizeof ( Signal_MP_Packet );
-      the_packet->Prefix.to_convert = sizeof ( Signal_MP_Packet );
-      the_packet->operation         = operation;
-      the_packet->Prefix.id         = task_id;
-      the_packet->signal_in         = signal_in;
-
-      return _MPCI_Send_request_packet(
-        _Objects_Get_node( task_id ),
-        &the_packet->Prefix,
-        STATES_READY,  /* Not used */
-        RTEMS_TIMEOUT
-      );
-      break;
-
-    case SIGNAL_MP_SEND_RESPONSE:
-      break;
-
+  the_packet = _Signal_MP_Get_packet( id );
+  if ( the_packet == NULL ) {
+    return RTEMS_INVALID_ID;
   }
-  /*
-   *  The following line is included to satisfy compilers which
-   *  produce warnings when a function does not end with a return.
-   */
-  return RTEMS_INTERNAL_ERROR;
+
+  the_packet->Prefix.the_class  = MP_PACKET_SIGNAL;
+  the_packet->Prefix.length     = sizeof( *the_packet );
+  the_packet->Prefix.to_convert = sizeof( *the_packet );
+  the_packet->operation         = SIGNAL_MP_SEND_REQUEST;
+  the_packet->Prefix.id         = id;
+  the_packet->signal_set        = signal_set;
+
+  return (rtems_status_code) _MPCI_Send_request_packet(
+    _Objects_Get_node( id ),
+    &the_packet->Prefix,
+    STATES_READY,
+    RTEMS_TIMEOUT
+  );
 }
 
 static void _Signal_MP_Send_response_packet (
@@ -127,7 +138,7 @@ void _Signal_MP_Process_packet (
 
       the_packet->Prefix.return_code = rtems_signal_send(
         the_packet->Prefix.id,
-        the_packet->signal_in
+        the_packet->signal_set
       );
 
       _Signal_MP_Send_response_packet(
diff --git a/cpukit/rtems/src/signalsend.c b/cpukit/rtems/src/signalsend.c
index 4153e40..162de5c 100644
--- a/cpukit/rtems/src/signalsend.c
+++ b/cpukit/rtems/src/signalsend.c
@@ -21,7 +21,7 @@
 #include <rtems/rtems/signalimpl.h>
 #include <rtems/rtems/asrimpl.h>
 #include <rtems/rtems/tasks.h>
-#include <rtems/score/isr.h>
+#include <rtems/score/threaddispatch.h>
 #include <rtems/score/threadimpl.h>
 
 rtems_status_code rtems_signal_send(
@@ -29,50 +29,48 @@ rtems_status_code rtems_signal_send(
   rtems_signal_set  signal_set
 )
 {
-  Thread_Control          *the_thread;
-  Objects_Locations        location;
-  RTEMS_API_Control       *api;
-  ASR_Information         *asr;
+  Thread_Control    *the_thread;
+  ISR_lock_Context   lock_context;
+  RTEMS_API_Control *api;
+  ASR_Information   *asr;
 
-  if ( !signal_set )
+  if ( signal_set == 0 ) {
     return RTEMS_INVALID_NUMBER;
+  }
 
-  the_thread = _Thread_Get( id, &location );
-  switch ( location ) {
-
-    case OBJECTS_LOCAL:
-      api = the_thread->API_Extensions[ THREAD_API_RTEMS ];
-      asr = &api->Signal;
-
-      if ( ! _ASR_Is_null_handler( asr->handler ) ) {
-        if ( asr->is_enabled ) {
-          _ASR_Post_signals( asr, signal_set, &asr->signals_posted );
-          _Thread_Add_post_switch_action(
-            the_thread,
-            &api->Signal_action,
-            _Signal_Action_handler
-          );
-        } else {
-          _ASR_Post_signals( asr, signal_set, &asr->signals_pending );
-        }
-        _Objects_Put( &the_thread->Object );
-        return RTEMS_SUCCESSFUL;
-      }
-      _Objects_Put( &the_thread->Object );
-      return RTEMS_NOT_DEFINED;
+  the_thread = _Thread_Get_interrupt_disable( id, &lock_context );
 
+  if ( the_thread == NULL ) {
 #if defined(RTEMS_MULTIPROCESSING)
-    case OBJECTS_REMOTE:
-      return _Signal_MP_Send_request_packet(
-        SIGNAL_MP_SEND_REQUEST,
-        id,
-        signal_set
-      );
+    return _Signal_MP_Send( id, signal_set );
+#else
+    return RTEMS_INVALID_ID;
 #endif
+  }
+
+  api = the_thread->API_Extensions[ THREAD_API_RTEMS ];
+  asr = &api->Signal;
+
+  _ASR_Acquire_critical( asr, &lock_context );
+
+  if ( _ASR_Is_null_handler( asr->handler ) ) {
+    _ASR_Release( asr, &lock_context );
+    return RTEMS_NOT_DEFINED;
+  }
 
-    case OBJECTS_ERROR:
-      break;
+  if ( asr->is_enabled ) {
+    _ASR_Post_signals( signal_set, &asr->signals_posted );
+    _ASR_Release( asr, &lock_context );
+    _Thread_Add_post_switch_action(
+      the_thread,
+      &api->Signal_action,
+      _Signal_Action_handler
+    );
+    _Thread_Dispatch();
+  } else {
+    _ASR_Post_signals( signal_set, &asr->signals_pending );
+    _ASR_Release( asr, &lock_context );
   }
 
-  return RTEMS_INVALID_ID;
+  return RTEMS_SUCCESSFUL;
 }
diff --git a/cpukit/rtems/src/taskmode.c b/cpukit/rtems/src/taskmode.c
index 0fc0924..e2dc7ed 100644
--- a/cpukit/rtems/src/taskmode.c
+++ b/cpukit/rtems/src/taskmode.c
@@ -99,8 +99,8 @@ rtems_status_code rtems_task_mode(
 
     if ( is_asr_enabled != asr->is_enabled ) {
       asr->is_enabled = is_asr_enabled;
-      _ASR_Swap_signals( asr );
-      if ( _ASR_Are_signals_pending( asr ) ) {
+
+      if ( _ASR_Swap_signals( asr ) != 0 ) {
         needs_asr_dispatching = true;
         _Thread_Add_post_switch_action(
           executing,




More information about the vc mailing list