score: Fix a release/cancel job race condition

Split up the potential thread priority change in the scheduler
release/cancel job operation.  Protect the rate monotonic period state
with a dedicated SMP lock.  This avoids a race condition during
_Rate_monotonic_Timeout() while _Rate_monotonic_Cancel() is called on
another processor.

16 files changed, 72 insertions, 83 deletions

diff --git a/cpukit/rtems/include/rtems/rtems/ratemon.h b/cpukit/rtems/include/rtems/rtems/ratemon.h
index 3203eab394..a2df13f025 100644
--- a/cpukit/rtems/include/rtems/rtems/ratemon.h
+++ b/cpukit/rtems/include/rtems/rtems/ratemon.h
@@ -194,6 +194,11 @@ typedef struct {
   /** This field is the object management portion of a Period instance. */
   Objects_Control                         Object;
 
+  /**
+   * @brief Protects the rate monotonic period state.
+   */
+  ISR_LOCK_MEMBER(                        Lock )
+
   /** This is the timer used to provide the unblocking mechanism. */
   Watchdog_Control                        Timer;
 
diff --git a/cpukit/rtems/include/rtems/rtems/ratemonimpl.h b/cpukit/rtems/include/rtems/rtems/ratemonimpl.h
index 9963cab612..b6b3ffd404 100644
--- a/cpukit/rtems/include/rtems/rtems/ratemonimpl.h
+++ b/cpukit/rtems/include/rtems/rtems/ratemonimpl.h
@@ -69,19 +69,19 @@ RTEMS_INLINE_ROUTINE Rate_monotonic_Control *_Rate_monotonic_Allocate( void )
 }
 
 RTEMS_INLINE_ROUTINE void _Rate_monotonic_Acquire_critical(
-  Thread_Control   *the_thread,
-  ISR_lock_Context *lock_context
+  Rate_monotonic_Control *the_period,
+  ISR_lock_Context       *lock_context
 )
 {
-  _Thread_Wait_acquire_default_critical( the_thread, lock_context );
+  _ISR_lock_Acquire( &the_period->Lock, lock_context );
 }
 
 RTEMS_INLINE_ROUTINE void _Rate_monotonic_Release(
-  Thread_Control   *the_thread,
-  ISR_lock_Context *lock_context
+  Rate_monotonic_Control *the_period,
+  ISR_lock_Context       *lock_context
 )
 {
-  _Thread_Wait_release_default( the_thread, lock_context );
+  _ISR_lock_Release_and_ISR_enable( &the_period->Lock, lock_context );
 }
 
 RTEMS_INLINE_ROUTINE Rate_monotonic_Control *_Rate_monotonic_Get(
diff --git a/cpukit/rtems/src/ratemoncancel.c b/cpukit/rtems/src/ratemoncancel.c
index 41ba48856f..b4e899d296 100644
--- a/cpukit/rtems/src/ratemoncancel.c
+++ b/cpukit/rtems/src/ratemoncancel.c
@@ -28,18 +28,17 @@ void _Rate_monotonic_Cancel(
 )
 {
   Per_CPU_Control *cpu_self;
+  Thread_Control  *update_priority;
 
-  _Watchdog_Per_CPU_remove_relative( &the_period->Timer );
+  _Rate_monotonic_Acquire_critical( the_period, lock_context );
 
-  owner = the_period->owner;
-  _Rate_monotonic_Acquire_critical( owner, lock_context );
+  _Watchdog_Per_CPU_remove_relative( &the_period->Timer );
   the_period->state = RATE_MONOTONIC_INACTIVE;
+  update_priority = _Scheduler_Cancel_job( the_period->owner );
 
   cpu_self = _Thread_Dispatch_disable_critical( lock_context );
-  _Rate_monotonic_Release( owner, lock_context );
-
-  _Scheduler_Cancel_job( owner );
-
+  _Rate_monotonic_Release( the_period, lock_context );
+  _Thread_Update_priority( update_priority );
   _Thread_Dispatch_enable( cpu_self );
 }
 
diff --git a/cpukit/rtems/src/ratemoncreate.c b/cpukit/rtems/src/ratemoncreate.c
index 1a5c9b2615..a86c6a1eeb 100644
--- a/cpukit/rtems/src/ratemoncreate.c
+++ b/cpukit/rtems/src/ratemoncreate.c
@@ -62,6 +62,8 @@ rtems_status_code rtems_rate_monotonic_create(
     return RTEMS_TOO_MANY;
   }
 
+  _ISR_lock_Initialize( &the_period->Lock, "Rate Monotonic Period" );
+
   the_period->owner = _Thread_Get_executing();
   the_period->state = RATE_MONOTONIC_INACTIVE;
 
diff --git a/cpukit/rtems/src/ratemongetstatistics.c b/cpukit/rtems/src/ratemongetstatistics.c
index a6a0525fb0..7ffd2c5f5e 100644
--- a/cpukit/rtems/src/ratemongetstatistics.c
+++ b/cpukit/rtems/src/ratemongetstatistics.c
@@ -28,7 +28,6 @@ rtems_status_code rtems_rate_monotonic_get_statistics(
 {
   Rate_monotonic_Control          *the_period;
   ISR_lock_Context                 lock_context;
-  Thread_Control                  *owner;
   const Rate_monotonic_Statistics *src;
 
   if ( dst == NULL ) {
@@ -40,8 +39,7 @@ rtems_status_code rtems_rate_monotonic_get_statistics(
     return RTEMS_INVALID_ID;
   }
 
-  owner = the_period->owner;
-  _Rate_monotonic_Acquire_critical( owner, &lock_context );
+  _Rate_monotonic_Acquire_critical( the_period, &lock_context );
 
   src = &the_period->Statistics;
   dst->count        = src->count;
@@ -53,6 +51,6 @@ rtems_status_code rtems_rate_monotonic_get_statistics(
   _Timestamp_To_timespec( &src->max_wall_time,   &dst->max_wall_time );
   _Timestamp_To_timespec( &src->total_wall_time, &dst->total_wall_time );
 
-  _Rate_monotonic_Release( owner, &lock_context );
+  _Rate_monotonic_Release( the_period, &lock_context );
   return RTEMS_SUCCESSFUL;
 }
diff --git a/cpukit/rtems/src/ratemongetstatus.c b/cpukit/rtems/src/ratemongetstatus.c
index 82a950ec6a..403c6ed097 100644
--- a/cpukit/rtems/src/ratemongetstatus.c
+++ b/cpukit/rtems/src/ratemongetstatus.c
@@ -28,7 +28,6 @@ rtems_status_code rtems_rate_monotonic_get_status(
 {
   Rate_monotonic_Control *the_period;
   ISR_lock_Context        lock_context;
-  Thread_Control         *owner;
   rtems_status_code       status;
 
   if ( period_status == NULL ) {
@@ -40,10 +39,9 @@ rtems_status_code rtems_rate_monotonic_get_status(
     return RTEMS_INVALID_ID;
   }
 
-  owner = the_period->owner;
-  _Rate_monotonic_Acquire_critical( owner, &lock_context );
+  _Rate_monotonic_Acquire_critical( the_period, &lock_context );
 
-  period_status->owner = owner->Object.id;
+  period_status->owner = the_period->owner->Object.id;
   period_status->state = the_period->state;
 
   if ( the_period->state == RATE_MONOTONIC_INACTIVE ) {
@@ -81,6 +79,6 @@ rtems_status_code rtems_rate_monotonic_get_status(
     }
   }
 
-  _Rate_monotonic_Release( owner, &lock_context );
+  _Rate_monotonic_Release( the_period, &lock_context );
   return status;
 }
diff --git a/cpukit/rtems/src/ratemonperiod.c b/cpukit/rtems/src/ratemonperiod.c
index 303fe174e5..75a80d8088 100644
--- a/cpukit/rtems/src/ratemonperiod.c
+++ b/cpukit/rtems/src/ratemonperiod.c
@@ -71,21 +71,20 @@ static void _Rate_monotonic_Release_job(
 )
 {
   Per_CPU_Control *cpu_self;
-  uint64_t deadline;
+  Thread_Control  *update_priority;
+  uint64_t         deadline;
 
   cpu_self = _Thread_Dispatch_disable_critical( lock_context );
-  _Rate_monotonic_Release( owner, lock_context );
 
-  _ISR_lock_ISR_disable( lock_context );
   deadline = _Watchdog_Per_CPU_insert_relative(
     &the_period->Timer,
     cpu_self,
     next_length
   );
-  _ISR_lock_ISR_enable( lock_context );
-
-  _Scheduler_Release_job( owner, deadline );
+  update_priority = _Scheduler_Release_job( owner, deadline );
 
+  _Rate_monotonic_Release( the_period, lock_context );
+  _Thread_Update_priority( update_priority );
   _Thread_Dispatch_enable( cpu_self );
 }
 
@@ -217,7 +216,7 @@ static rtems_status_code _Rate_monotonic_Block_while_active(
   _Thread_Wait_flags_set( executing, RATE_MONOTONIC_INTEND_TO_BLOCK );
 
   cpu_self = _Thread_Dispatch_disable_critical( lock_context );
-  _Rate_monotonic_Release( executing, lock_context );
+  _Rate_monotonic_Release( the_period, lock_context );
 
   _Thread_Set_state( executing, STATES_WAITING_FOR_PERIOD );
 
@@ -278,13 +277,13 @@ rtems_status_code rtems_rate_monotonic_period(
     return RTEMS_NOT_OWNER_OF_RESOURCE;
   }
 
-  _Rate_monotonic_Acquire_critical( executing, &lock_context );
+  _Rate_monotonic_Acquire_critical( the_period, &lock_context );
 
   state = the_period->state;
 
   if ( length == RTEMS_PERIOD_STATUS ) {
     status = _Rate_monotonic_Get_status_for_state( state );
-    _Rate_monotonic_Release( executing, &lock_context );
+    _Rate_monotonic_Release( the_period, &lock_context );
   } else {
     switch ( state ) {
       case RATE_MONOTONIC_ACTIVE:
diff --git a/cpukit/rtems/src/ratemonresetstatistics.c b/cpukit/rtems/src/ratemonresetstatistics.c
index b146e6acb2..0b0e6c6369 100644
--- a/cpukit/rtems/src/ratemonresetstatistics.c
+++ b/cpukit/rtems/src/ratemonresetstatistics.c
@@ -26,16 +26,14 @@ rtems_status_code rtems_rate_monotonic_reset_statistics(
 {
   Rate_monotonic_Control *the_period;
   ISR_lock_Context        lock_context;
-  Thread_Control         *owner;
 
   the_period = _Rate_monotonic_Get( id, &lock_context );
   if ( the_period == NULL ) {
     return RTEMS_INVALID_ID;
   }
 
-  owner = the_period->owner;
-  _Rate_monotonic_Acquire_critical( owner, &lock_context );
+  _Rate_monotonic_Acquire_critical( the_period, &lock_context );
   _Rate_monotonic_Reset_statistics( the_period );
-  _Rate_monotonic_Release( owner, &lock_context );
+  _Rate_monotonic_Release( the_period, &lock_context );
   return RTEMS_SUCCESSFUL;
 }
diff --git a/cpukit/rtems/src/ratemontimeout.c b/cpukit/rtems/src/ratemontimeout.c
index bd38153b82..e514a314b3 100644
--- a/cpukit/rtems/src/ratemontimeout.c
+++ b/cpukit/rtems/src/ratemontimeout.c
@@ -31,7 +31,7 @@ void _Rate_monotonic_Timeout( Watchdog_Control *the_watchdog )
   owner = the_period->owner;
 
   _ISR_lock_ISR_disable( &lock_context );
-  _Rate_monotonic_Acquire_critical( owner, &lock_context );
+  _Rate_monotonic_Acquire_critical( the_period, &lock_context );
   wait_flags = _Thread_Wait_flags_get( owner );
 
   if (
@@ -63,6 +63,6 @@ void _Rate_monotonic_Timeout( Watchdog_Control *the_watchdog )
     }
   } else {
     the_period->state = RATE_MONOTONIC_EXPIRED;
-    _Rate_monotonic_Release( owner, &lock_context );
+    _Rate_monotonic_Release( the_period, &lock_context );
   }
 }
diff --git a/cpukit/score/include/rtems/score/scheduler.h b/cpukit/score/include/rtems/score/scheduler.h
index c8a7f87541..0a6d68208a 100644
--- a/cpukit/score/include/rtems/score/scheduler.h
+++ b/cpukit/score/include/rtems/score/scheduler.h
@@ -139,14 +139,14 @@ typedef struct {
   void ( *node_destroy )( const Scheduler_Control *, Scheduler_Node * );
 
   /** @see _Scheduler_Release_job() */
-  void ( *release_job ) (
+  Thread_Control *( *release_job ) (
     const Scheduler_Control *,
     Thread_Control *,
     uint64_t
   );
 
   /** @see _Scheduler_Cancel_job() */
-  void ( *cancel_job ) (
+  Thread_Control *( *cancel_job ) (
     const Scheduler_Control *,
     Thread_Control *
   );
@@ -533,8 +533,10 @@ void _Scheduler_default_Node_destroy(
  * @param[in] scheduler Unused.
  * @param[in] the_thread Unused.
  * @param[in] deadline Unused.
+ *
+ * @retval NULL Always.
  */
-void _Scheduler_default_Release_job(
+Thread_Control *_Scheduler_default_Release_job(
   const Scheduler_Control *scheduler,
   Thread_Control          *the_thread,
   uint64_t                 deadline
@@ -545,8 +547,10 @@ void _Scheduler_default_Release_job(
  *
  * @param[in] scheduler Unused.
  * @param[in] the_thread Unused.
+ *
+ * @retval NULL Always.
  */
-void _Scheduler_default_Cancel_job(
+Thread_Control *_Scheduler_default_Cancel_job(
   const Scheduler_Control *scheduler,
   Thread_Control          *the_thread
 );
diff --git a/cpukit/score/include/rtems/score/schedulercbs.h b/cpukit/score/include/rtems/score/schedulercbs.h
index bfad633987..c230e08d24 100644
--- a/cpukit/score/include/rtems/score/schedulercbs.h
+++ b/cpukit/score/include/rtems/score/schedulercbs.h
@@ -163,20 +163,7 @@ Scheduler_Void_or_thread _Scheduler_CBS_Unblock(
   Thread_Control          *the_thread
 );
 
-/**
- *  @brief Called when a new job of task is released.
- *
- *  This routine is called when a new job of task is released.
- *  It is called only from Rate Monotonic manager in the beginning
- *  of new period. Deadline has to be shifted and budget replenished.
- *
- *  @param[in] scheduler The scheduler instance.
- *  @param[in] the_thread is the owner of the job.
- *  @param[in] length of the new job from now. If equal to 0,
- *             the job was cancelled or deleted.
- */
-
-void _Scheduler_CBS_Release_job (
+Thread_Control *_Scheduler_CBS_Release_job(
   const Scheduler_Control *scheduler,
   Thread_Control          *the_thread,
   uint64_t                 length
diff --git a/cpukit/score/include/rtems/score/scheduleredf.h b/cpukit/score/include/rtems/score/scheduleredf.h
index 5d9b4359c4..81b245e391 100644
--- a/cpukit/score/include/rtems/score/scheduleredf.h
+++ b/cpukit/score/include/rtems/score/scheduleredf.h
@@ -215,26 +215,13 @@ Scheduler_Void_or_thread _Scheduler_EDF_Yield(
   Thread_Control          *the_thread
 );
 
-/**
- *  @brief Called when a new job of task is released.
- *
- *  This routine is called when a new job of task is released.
- *  It is called only from Rate Monotonic manager in the beginning
- *  of new period.
- *
- *  @param[in] scheduler The scheduler instance.
- *  @param[in] the_thread is the owner of the job.
- *  @param[in] deadline of the new job from now. If equal to 0,
- *             the job was cancelled or deleted, thus a running task
- *             has to be suspended.
- */
-void _Scheduler_EDF_Release_job(
+Thread_Control *_Scheduler_EDF_Release_job(
   const Scheduler_Control *scheduler,
   Thread_Control          *the_thread,
   uint64_t                 deadline
 );
 
-void _Scheduler_EDF_Cancel_job(
+Thread_Control *_Scheduler_EDF_Cancel_job(
   const Scheduler_Control *scheduler,
   Thread_Control          *the_thread
 );
diff --git a/cpukit/score/include/rtems/score/schedulerimpl.h b/cpukit/score/include/rtems/score/schedulerimpl.h
index 91fa178d20..1c5a697619 100644
--- a/cpukit/score/include/rtems/score/schedulerimpl.h
+++ b/cpukit/score/include/rtems/score/schedulerimpl.h
@@ -496,29 +496,37 @@ RTEMS_INLINE_ROUTINE void _Scheduler_Node_destroy(
  *
  * @param[in] the_thread The thread.
  * @param[in] deadline The deadline in watchdog ticks since boot.
+ *
+ * @return The thread to hand over to _Thread_Update_priority().
  */
-RTEMS_INLINE_ROUTINE void _Scheduler_Release_job(
+RTEMS_INLINE_ROUTINE Thread_Control *_Scheduler_Release_job(
   Thread_Control *the_thread,
   uint64_t        deadline
 )
 {
   const Scheduler_Control *scheduler = _Scheduler_Get( the_thread );
 
-  ( *scheduler->Operations.release_job )( scheduler, the_thread, deadline );
+  return ( *scheduler->Operations.release_job )(
+    scheduler,
+    the_thread,
+    deadline
+  );
 }
 
 /**
  * @brief Cancels a job of a thread with respect to the scheduler.
  *
  * @param[in] the_thread The thread.
+ *
+ * @return The thread to hand over to _Thread_Update_priority().
  */
-RTEMS_INLINE_ROUTINE void _Scheduler_Cancel_job(
+RTEMS_INLINE_ROUTINE Thread_Control *_Scheduler_Cancel_job(
   Thread_Control *the_thread
 )
 {
   const Scheduler_Control *scheduler = _Scheduler_Get( the_thread );
 
-  ( *scheduler->Operations.cancel_job )( scheduler, the_thread );
+  return ( *scheduler->Operations.cancel_job )( scheduler, the_thread );
 }
 
 /**
diff --git a/cpukit/score/src/schedulercbsreleasejob.c b/cpukit/score/src/schedulercbsreleasejob.c
index 124c02bac0..d2169af899 100644
--- a/cpukit/score/src/schedulercbsreleasejob.c
+++ b/cpukit/score/src/schedulercbsreleasejob.c
@@ -21,7 +21,7 @@
 
 #include <rtems/score/schedulercbsimpl.h>
 
-void _Scheduler_CBS_Release_job(
+Thread_Control *_Scheduler_CBS_Release_job(
   const Scheduler_Control *scheduler,
   Thread_Control          *the_thread,
   uint64_t                 deadline
@@ -30,8 +30,6 @@ void _Scheduler_CBS_Release_job(
   Scheduler_CBS_Node   *node;
   Scheduler_CBS_Server *serv_info;
 
-  _Scheduler_EDF_Release_job( scheduler, the_thread, deadline );
-
   node = _Scheduler_CBS_Thread_get_node( the_thread );
   serv_info = node->cbs_server;
 
@@ -39,4 +37,6 @@ void _Scheduler_CBS_Release_job(
   if ( serv_info != NULL ) {
     the_thread->cpu_time_budget = serv_info->parameters.budget;
   }
+
+  return _Scheduler_EDF_Release_job( scheduler, the_thread, deadline );
 }
diff --git a/cpukit/score/src/schedulerdefaultreleasejob.c b/cpukit/score/src/schedulerdefaultreleasejob.c
index 439f6b2649..7272fc1946 100644
--- a/cpukit/score/src/schedulerdefaultreleasejob.c
+++ b/cpukit/score/src/schedulerdefaultreleasejob.c
@@ -21,7 +21,7 @@
 
 #include <rtems/score/scheduler.h>
 
-void _Scheduler_default_Release_job(
+Thread_Control *_Scheduler_default_Release_job(
   const Scheduler_Control *scheduler,
   Thread_Control          *the_thread,
   uint64_t                 deadline
@@ -30,13 +30,17 @@ void _Scheduler_default_Release_job(
   (void) scheduler;
   (void) the_thread;
   (void) deadline;
+
+  return NULL;
 }
 
-void _Scheduler_default_Cancel_job(
+Thread_Control *_Scheduler_default_Cancel_job(
   const Scheduler_Control *scheduler,
   Thread_Control          *the_thread
 )
 {
   (void) scheduler;
   (void) the_thread;
+
+  return NULL;
 }
diff --git a/cpukit/score/src/scheduleredfreleasejob.c b/cpukit/score/src/scheduleredfreleasejob.c
index c10943499a..0fbf0f0f61 100644
--- a/cpukit/score/src/scheduleredfreleasejob.c
+++ b/cpukit/score/src/scheduleredfreleasejob.c
@@ -42,13 +42,13 @@ static bool _Scheduler_EDF_Release_job_filter(
     || !_Thread_Owns_resources( the_thread );
 }
 
-void _Scheduler_EDF_Release_job(
+Thread_Control *_Scheduler_EDF_Release_job(
   const Scheduler_Control *scheduler,
   Thread_Control          *the_thread,
   uint64_t                 deadline
 )
 {
-  _Thread_Change_priority(
+  return _Thread_Apply_priority(
     the_thread,
     deadline,
     NULL,
@@ -79,12 +79,12 @@ static bool _Scheduler_EDF_Cancel_job_filter(
     || !_Thread_Owns_resources( the_thread );
 }
 
-void _Scheduler_EDF_Cancel_job(
+Thread_Control *_Scheduler_EDF_Cancel_job(
   const Scheduler_Control *scheduler,
   Thread_Control          *the_thread
 )
 {
-  _Thread_Change_priority(
+  return _Thread_Apply_priority(
     the_thread,
     0,
     NULL,