[rtems commit] rtems: Avoid Giant lock for some task operations

Sebastian Huber sebh at rtems.org
Thu May 12 11:34:19 UTC 2016


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

Author:    Sebastian Huber <sebastian.huber at embedded-brains.de>
Date:      Wed May 11 10:21:57 2016 +0200

rtems: Avoid Giant lock for some task operations

Avoid Giant lock for rtems_task_set_priority(), rtems_task_suspend() and
rtems_task_resume().

Update #2555.

---

 cpukit/rtems/include/rtems/rtems/taskmp.h |  34 ++++-----
 cpukit/rtems/src/taskmp.c                 | 120 +++++++++++++++++++++---------
 cpukit/rtems/src/taskresume.c             |  39 +++++-----
 cpukit/rtems/src/tasksetpriority.c        |  57 +++++++-------
 cpukit/rtems/src/tasksuspend.c            |  39 +++++-----
 5 files changed, 158 insertions(+), 131 deletions(-)

diff --git a/cpukit/rtems/include/rtems/rtems/taskmp.h b/cpukit/rtems/include/rtems/rtems/taskmp.h
index 2a80f44..e2d70a9 100644
--- a/cpukit/rtems/include/rtems/rtems/taskmp.h
+++ b/cpukit/rtems/include/rtems/rtems/taskmp.h
@@ -57,17 +57,6 @@ typedef enum {
 }   RTEMS_tasks_MP_Remote_operations;
 
 /**
- *  The following data structure defines the packet used to perform
- *  remote task operations.
- */
-typedef struct {
-  rtems_packet_prefix               Prefix;
-  RTEMS_tasks_MP_Remote_operations  operation;
-  rtems_name                        name;
-  rtems_task_priority               the_priority;
-}   RTEMS_tasks_MP_Packet;
-
-/**
  *  @brief RTEMS Tasks MP Send Process Packet
  *
  *  Multiprocessing Support for the RTEMS Task Manager
@@ -82,18 +71,25 @@ void _RTEMS_tasks_MP_Send_process_packet (
 );
 
 /**
- *  @brief _RTEMS_tasks_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_task_set_priority() request.
  */
-rtems_status_code _RTEMS_tasks_MP_Send_request_packet (
-  RTEMS_tasks_MP_Remote_operations operation,
-  Objects_Id                       task_id,
-  rtems_task_priority              the_priority
+rtems_status_code _RTEMS_tasks_MP_Set_priority(
+  rtems_id             id,
+  rtems_task_priority  new_priority,
+  rtems_task_priority *old_priority
 );
 
 /**
+ * @brief Issues a remote rtems_task_suspend() request.
+ */
+rtems_status_code _RTEMS_tasks_MP_Suspend( rtems_id id );
+
+/**
+ * @brief Issues a remote rtems_task_resume() request.
+ */
+rtems_status_code _RTEMS_tasks_MP_Resume( rtems_id id );
+
+/**
  *  @brief _RTEMS_tasks_MP_Process_packet
  *
  *  This routine performs the actions specific to this package for
diff --git a/cpukit/rtems/src/taskmp.c b/cpukit/rtems/src/taskmp.c
index bca241f..d59db2e 100644
--- a/cpukit/rtems/src/taskmp.c
+++ b/cpukit/rtems/src/taskmp.c
@@ -24,6 +24,17 @@
 #include <rtems/score/threadimpl.h>
 #include <rtems/score/threadqimpl.h>
 
+/**
+ *  The following data structure defines the packet used to perform
+ *  remote task operations.
+ */
+typedef struct {
+  rtems_packet_prefix               Prefix;
+  RTEMS_tasks_MP_Remote_operations  operation;
+  rtems_name                        name;
+  rtems_task_priority               the_priority;
+}   RTEMS_tasks_MP_Packet;
+
 RTEMS_STATIC_ASSERT(
   sizeof(RTEMS_tasks_MP_Packet) <= MP_PACKET_MINIMUM_PACKET_SIZE,
   RTEMS_tasks_MP_Packet
@@ -34,6 +45,17 @@ static RTEMS_tasks_MP_Packet *_RTEMS_tasks_MP_Get_packet( void )
   return (RTEMS_tasks_MP_Packet *) _MPCI_Get_packet();
 }
 
+static RTEMS_tasks_MP_Packet *_RTEMS_tasks_MP_Get_request_packet(
+  Objects_Id id
+)
+{
+  if ( !_Thread_MP_Is_remote( id ) ) {
+    return NULL;
+  }
+
+  return _RTEMS_tasks_MP_Get_packet();
+}
+
 /*
  *  _RTEMS_tasks_MP_Send_process_packet
  *
@@ -73,54 +95,78 @@ void _RTEMS_tasks_MP_Send_process_packet (
   }
 }
 
-/*
- *  _RTEMS_tasks_MP_Send_request_packet
- *
- */
+static rtems_status_code _RTEMS_tasks_MP_Send_request_packet(
+  RTEMS_tasks_MP_Packet            *the_packet,
+  Objects_Id                        id,
+  RTEMS_tasks_MP_Remote_operations  operation
+)
+{
+  the_packet->Prefix.the_class  = MP_PACKET_TASKS;
+  the_packet->Prefix.length     = sizeof( *the_packet );
+  the_packet->Prefix.to_convert = sizeof( *the_packet );
+  the_packet->Prefix.id         = id;
+  the_packet->operation         = operation;
+
+  return _MPCI_Send_request_packet(
+    _Objects_Get_node( id ),
+    &the_packet->Prefix,
+    STATES_READY,    /* Not used */
+    RTEMS_TIMEOUT
+  );
+}
 
-rtems_status_code _RTEMS_tasks_MP_Send_request_packet (
-  RTEMS_tasks_MP_Remote_operations operation,
-  Objects_Id                       task_id,
-  rtems_task_priority              new_priority
+rtems_status_code _RTEMS_tasks_MP_Set_priority(
+  rtems_id             id,
+  rtems_task_priority  new_priority,
+  rtems_task_priority *old_priority
 )
 {
   RTEMS_tasks_MP_Packet *the_packet;
 
-  switch ( operation ) {
+  the_packet = _RTEMS_tasks_MP_Get_request_packet( id );
+  if ( the_packet == NULL ) {
+    return RTEMS_INVALID_ID;
+  }
 
-    case RTEMS_TASKS_MP_SUSPEND_REQUEST:
-    case RTEMS_TASKS_MP_RESUME_REQUEST:
-    case RTEMS_TASKS_MP_SET_PRIORITY_REQUEST:
+  the_packet->the_priority = new_priority;
+  _Thread_Executing->Wait.return_argument = old_priority;
+  return _RTEMS_tasks_MP_Send_request_packet(
+    the_packet,
+    id,
+    RTEMS_TASKS_MP_SET_PRIORITY_REQUEST
+  );
+}
 
-      the_packet                    = _RTEMS_tasks_MP_Get_packet();
-      the_packet->Prefix.the_class  = MP_PACKET_TASKS;
-      the_packet->Prefix.length     = sizeof ( RTEMS_tasks_MP_Packet );
-      the_packet->Prefix.to_convert = sizeof ( RTEMS_tasks_MP_Packet );
-      the_packet->operation         = operation;
-      the_packet->Prefix.id         = task_id;
-      the_packet->the_priority      = new_priority;
+rtems_status_code _RTEMS_tasks_MP_Suspend( rtems_id id )
+{
+  RTEMS_tasks_MP_Packet *the_packet;
 
-      return _MPCI_Send_request_packet(
-        _Objects_Get_node( task_id ),
-        &the_packet->Prefix,
-        STATES_READY,    /* Not used */
-        RTEMS_TIMEOUT
-      );
-      break;
+  the_packet = _RTEMS_tasks_MP_Get_request_packet( id );
+  if ( the_packet == NULL ) {
+    return RTEMS_INVALID_ID;
+  }
 
-    case RTEMS_TASKS_MP_ANNOUNCE_CREATE:
-    case RTEMS_TASKS_MP_ANNOUNCE_DELETE:
-    case RTEMS_TASKS_MP_SUSPEND_RESPONSE:
-    case RTEMS_TASKS_MP_RESUME_RESPONSE:
-    case RTEMS_TASKS_MP_SET_PRIORITY_RESPONSE:
-      break;
+  return _RTEMS_tasks_MP_Send_request_packet(
+    the_packet,
+    id,
+    RTEMS_TASKS_MP_SUSPEND_REQUEST
+  );
+}
 
+rtems_status_code _RTEMS_tasks_MP_Resume( rtems_id id )
+{
+  RTEMS_tasks_MP_Packet *the_packet;
+
+  the_packet = _RTEMS_tasks_MP_Get_request_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_SUCCESSFUL;
+
+  return _RTEMS_tasks_MP_Send_request_packet(
+    the_packet,
+    id,
+    RTEMS_TASKS_MP_RESUME_REQUEST
+  );
 }
 
 /*
diff --git a/cpukit/rtems/src/taskresume.c b/cpukit/rtems/src/taskresume.c
index fba605f..1bdf421 100644
--- a/cpukit/rtems/src/taskresume.c
+++ b/cpukit/rtems/src/taskresume.c
@@ -25,32 +25,27 @@ rtems_status_code rtems_task_resume(
   rtems_id id
 )
 {
-  Thread_Control          *the_thread;
-  Objects_Locations        location;
-  States_Control           previous_state;
+  Thread_Control   *the_thread;
+  ISR_lock_Context  lock_context;
+  Per_CPU_Control  *cpu_self;
+  States_Control    previous_state;
 
-  the_thread = _Thread_Get( id, &location );
-  switch ( location ) {
-
-    case OBJECTS_LOCAL:
-      previous_state = _Thread_Clear_state( the_thread, STATES_SUSPENDED );
-      _Objects_Put( &the_thread->Object );
-
-      return _States_Is_suspended( previous_state ) ?
-        RTEMS_SUCCESSFUL : RTEMS_INCORRECT_STATE;
+  the_thread = _Thread_Get_interrupt_disable( id, &lock_context );
 
+  if ( the_thread == NULL ) {
 #if defined(RTEMS_MULTIPROCESSING)
-    case OBJECTS_REMOTE:
-      return _RTEMS_tasks_MP_Send_request_packet(
-          RTEMS_TASKS_MP_RESUME_REQUEST,
-          id,
-          0          /* Not used */
-        );
+    return _RTEMS_tasks_MP_Resume( id );
+#else
+    return RTEMS_INVALID_ID;
 #endif
-
-    case OBJECTS_ERROR:
-      break;
   }
 
-  return RTEMS_INVALID_ID;
+  cpu_self = _Thread_Dispatch_disable_critical( &lock_context );
+  _ISR_lock_ISR_enable( &lock_context );
+
+  previous_state = _Thread_Clear_state( the_thread, STATES_SUSPENDED );
+
+  _Thread_Dispatch_enable( cpu_self );
+  return _States_Is_suspended( previous_state ) ?
+    RTEMS_SUCCESSFUL : RTEMS_INCORRECT_STATE;
 }
diff --git a/cpukit/rtems/src/tasksetpriority.c b/cpukit/rtems/src/tasksetpriority.c
index c6b2dc0..d343935 100644
--- a/cpukit/rtems/src/tasksetpriority.c
+++ b/cpukit/rtems/src/tasksetpriority.c
@@ -27,8 +27,9 @@ rtems_status_code rtems_task_set_priority(
   rtems_task_priority *old_priority
 )
 {
-  Thread_Control          *the_thread;
-  Objects_Locations        location;
+  Thread_Control   *the_thread;
+  ISR_lock_Context  lock_context;
+  Per_CPU_Control  *cpu_self;
 
   if ( new_priority != RTEMS_CURRENT_PRIORITY &&
        !_RTEMS_tasks_Priority_is_valid( new_priority ) )
@@ -37,39 +38,33 @@ rtems_status_code rtems_task_set_priority(
   if ( !old_priority )
     return RTEMS_INVALID_ADDRESS;
 
-  the_thread = _Thread_Get( id, &location );
-  switch ( location ) {
-
-    case OBJECTS_LOCAL:
-      if ( new_priority != RTEMS_CURRENT_PRIORITY ) {
-        _Thread_Set_priority(
-          the_thread,
-          _RTEMS_tasks_Priority_to_Core( new_priority ),
-          old_priority,
-          false
-        );
-        *old_priority = _RTEMS_tasks_Priority_from_Core( *old_priority );
-      } else {
-        *old_priority = _RTEMS_tasks_Priority_from_Core(
-          the_thread->current_priority
-        );
-      }
-      _Objects_Put( &the_thread->Object );
-      return RTEMS_SUCCESSFUL;
+  the_thread = _Thread_Get_interrupt_disable( id, &lock_context );
 
+  if ( the_thread == NULL ) {
 #if defined(RTEMS_MULTIPROCESSING)
-    case OBJECTS_REMOTE:
-      _Thread_Executing->Wait.return_argument = old_priority;
-      return _RTEMS_tasks_MP_Send_request_packet(
-          RTEMS_TASKS_MP_SET_PRIORITY_REQUEST,
-          id,
-          new_priority
-      );
+    return _RTEMS_tasks_MP_Set_priority( id, new_priority, old_priority );
+#else
+    return RTEMS_INVALID_ID;
 #endif
+  }
+
+  cpu_self = _Thread_Dispatch_disable_critical( &lock_context );
+  _ISR_lock_ISR_enable( &lock_context );
 
-    case OBJECTS_ERROR:
-      break;
+  if ( new_priority != RTEMS_CURRENT_PRIORITY ) {
+    _Thread_Set_priority(
+      the_thread,
+      _RTEMS_tasks_Priority_to_Core( new_priority ),
+      old_priority,
+      false
+    );
+    *old_priority = _RTEMS_tasks_Priority_from_Core( *old_priority );
+  } else {
+    *old_priority = _RTEMS_tasks_Priority_from_Core(
+      the_thread->current_priority
+    );
   }
 
-  return RTEMS_INVALID_ID;
+  _Thread_Dispatch_enable( cpu_self );
+  return RTEMS_SUCCESSFUL;
 }
diff --git a/cpukit/rtems/src/tasksuspend.c b/cpukit/rtems/src/tasksuspend.c
index 6b66161..bd9b89a 100644
--- a/cpukit/rtems/src/tasksuspend.c
+++ b/cpukit/rtems/src/tasksuspend.c
@@ -25,32 +25,27 @@ rtems_status_code rtems_task_suspend(
   rtems_id id
 )
 {
-  Thread_Control          *the_thread;
-  Objects_Locations        location;
-  States_Control           previous_state;
+  Thread_Control   *the_thread;
+  ISR_lock_Context  lock_context;
+  Per_CPU_Control  *cpu_self;
+  States_Control    previous_state;
 
-  the_thread = _Thread_Get( id, &location );
-  switch ( location ) {
-
-    case OBJECTS_LOCAL:
-      previous_state = _Thread_Set_state( the_thread, STATES_SUSPENDED );
-      _Objects_Put( &the_thread->Object );
-
-      return _States_Is_suspended( previous_state ) ?
-        RTEMS_ALREADY_SUSPENDED : RTEMS_SUCCESSFUL;
+  the_thread = _Thread_Get_interrupt_disable( id, &lock_context );
 
+  if ( the_thread == NULL ) {
 #if defined(RTEMS_MULTIPROCESSING)
-    case OBJECTS_REMOTE:
-      return _RTEMS_tasks_MP_Send_request_packet(
-        RTEMS_TASKS_MP_SUSPEND_REQUEST,
-        id,
-        0          /* Not used */
-      );
+    return _RTEMS_tasks_MP_Suspend( id );
+#else
+    return RTEMS_INVALID_ID;
 #endif
-
-    case OBJECTS_ERROR:
-      break;
   }
 
-  return RTEMS_INVALID_ID;
+  cpu_self = _Thread_Dispatch_disable_critical( &lock_context );
+  _ISR_lock_ISR_enable( &lock_context );
+
+  previous_state = _Thread_Set_state( the_thread, STATES_SUSPENDED );
+
+  _Thread_Dispatch_enable( cpu_self );
+  return _States_Is_suspended( previous_state ) ?
+    RTEMS_ALREADY_SUSPENDED : RTEMS_SUCCESSFUL;
 }




More information about the vc mailing list