[rtems commit] score: Fix a release/cancel job race condition
Sebastian Huber
sebh at rtems.org
Fri Aug 5 07:26:51 UTC 2016
Module: rtems
Branch: master
Commit: ee0e41355d1faa5f74085effc4f57c3a7fc1f884
Changeset: http://git.rtems.org/rtems/commit/?id=ee0e41355d1faa5f74085effc4f57c3a7fc1f884
Author: Sebastian Huber <sebastian.huber at embedded-brains.de>
Date: Thu Aug 4 10:20:29 2016 +0200
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.
---
cpukit/rtems/include/rtems/rtems/ratemon.h | 5 +++++
cpukit/rtems/include/rtems/rtems/ratemonimpl.h | 12 ++++++------
cpukit/rtems/src/ratemoncancel.c | 13 ++++++-------
cpukit/rtems/src/ratemoncreate.c | 2 ++
cpukit/rtems/src/ratemongetstatistics.c | 6 ++----
cpukit/rtems/src/ratemongetstatus.c | 8 +++-----
cpukit/rtems/src/ratemonperiod.c | 17 ++++++++---------
cpukit/rtems/src/ratemonresetstatistics.c | 6 ++----
cpukit/rtems/src/ratemontimeout.c | 4 ++--
cpukit/score/include/rtems/score/scheduler.h | 12 ++++++++----
cpukit/score/include/rtems/score/schedulercbs.h | 15 +--------------
cpukit/score/include/rtems/score/scheduleredf.h | 17 ++---------------
cpukit/score/include/rtems/score/schedulerimpl.h | 16 ++++++++++++----
cpukit/score/src/schedulercbsreleasejob.c | 6 +++---
cpukit/score/src/schedulerdefaultreleasejob.c | 8 ++++++--
cpukit/score/src/scheduleredfreleasejob.c | 8 ++++----
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 3203eab..a2df13f 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 9963cab..b6b3ffd 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 41ba488..b4e899d 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 1a5c9b2..a86c6a1 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 a6a0525..7ffd2c5 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 82a950e..403c6ed 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 303fe17..75a80d8 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 b146e6a..0b0e6c6 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 bd38153..e514a31 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 c8a7f87..0a6d682 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 bfad633..c230e08 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 5d9b435..81b245e 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 91fa178..1c5a697 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 124c02b..d2169af 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 439f6b2..7272fc1 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 c109434..0fbf0f0 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,
More information about the vc
mailing list