[PATCH 15/27] score: Add SMP scheduler make/clean sticky
Gedare Bloom
gedare at rtems.org
Sat Nov 20 21:24:04 UTC 2021
On Mon, Nov 15, 2021 at 10:13 AM Sebastian Huber
<sebastian.huber at embedded-brains.de> wrote:
>
> This patch fixes the following broken behaviour:
>
> While a thread is scheduled on a helping scheduler, while it does not
> own a MrsP semaphore, if it obtains a MrsP semaphore, then no
> scheduler node using an idle thread and the ceiling priority of the
> semaphore is unblocked for the home scheduler.
>
> This could lead to priority inversion issues and is not in line
> with the MrsP protocol.
>
> Introduce two new scheduler operations which are only enabled if
> RTEMS_SMP is defined. The operations are used to make the scheduler
> node of the home scheduler sticky and to clean the sticky property.
> This helps to keep the sticky handing out of the frequently used
> priority update operation.
>
> Close #4532.
> ---
> cpukit/include/rtems/score/mrspimpl.h | 19 ++-
> cpukit/include/rtems/score/scheduler.h | 54 +++++++
> cpukit/include/rtems/score/scheduleredfsmp.h | 32 +++++
> cpukit/include/rtems/score/schedulerimpl.h | 59 --------
> .../score/schedulerpriorityaffinitysmp.h | 32 +++++
> .../rtems/score/schedulerprioritysmp.h | 32 +++++
> .../include/rtems/score/schedulersimplesmp.h | 32 +++++
> cpukit/include/rtems/score/schedulersmpimpl.h | 84 +++++++++++
> .../include/rtems/score/schedulerstrongapa.h | 62 ++++++++
> cpukit/include/rtems/score/threadimpl.h | 28 ++--
> .../src/schedulerdefaultmakecleansticky.c | 52 +++++++
> cpukit/score/src/scheduleredfsmp.c | 36 ++++-
> .../score/src/schedulerpriorityaffinitysmp.c | 39 +++++-
> cpukit/score/src/schedulerprioritysmp.c | 37 ++++-
> cpukit/score/src/schedulersimplesmp.c | 37 ++++-
> cpukit/score/src/schedulerstrongapa.c | 37 ++++-
> cpukit/score/src/threadchangepriority.c | 132 +++++++++++++++++-
> cpukit/score/src/threadqenqueue.c | 6 +-
> spec/build/cpukit/objsmp.yml | 1 +
> 19 files changed, 720 insertions(+), 91 deletions(-)
> create mode 100644 cpukit/score/src/schedulerdefaultmakecleansticky.c
>
> diff --git a/cpukit/include/rtems/score/mrspimpl.h b/cpukit/include/rtems/score/mrspimpl.h
> index 3e64ad94e6..444586b4ab 100644
> --- a/cpukit/include/rtems/score/mrspimpl.h
> +++ b/cpukit/include/rtems/score/mrspimpl.h
> @@ -268,7 +268,7 @@ RTEMS_INLINE_ROUTINE Status_Control _MRSP_Claim_ownership(
> _MRSP_Set_owner( mrsp, executing );
> cpu_self = _Thread_queue_Dispatch_disable( queue_context );
> _MRSP_Release( mrsp, queue_context );
> - _Thread_Priority_and_sticky_update( executing, 1 );
> + _Thread_Priority_update_and_make_sticky( executing );
> _Thread_Dispatch_enable( cpu_self );
> return STATUS_SUCCESSFUL;
> }
> @@ -384,13 +384,6 @@ RTEMS_INLINE_ROUTINE Status_Control _MRSP_Wait_for_ownership(
> _MRSP_Replace_priority( mrsp, executing, &ceiling_priority );
> } else {
> Per_CPU_Control *cpu_self;
> - int sticky_level_change;
> -
> - if ( status != STATUS_DEADLOCK ) {
> - sticky_level_change = -1;
> - } else {
> - sticky_level_change = 0;
> - }
>
> _ISR_lock_ISR_disable( &queue_context->Lock_context.Lock_context );
> _MRSP_Remove_priority( executing, &ceiling_priority, queue_context );
> @@ -398,7 +391,13 @@ RTEMS_INLINE_ROUTINE Status_Control _MRSP_Wait_for_ownership(
> &queue_context->Lock_context.Lock_context
> );
> _ISR_lock_ISR_enable( &queue_context->Lock_context.Lock_context );
> - _Thread_Priority_and_sticky_update( executing, sticky_level_change );
> +
> + if ( status != STATUS_DEADLOCK ) {
> + _Thread_Priority_update_and_clean_sticky( executing );
> + } else {
> + _Thread_Priority_update_only( executing );
> + }
> +
> _Thread_Dispatch_enable( cpu_self );
> }
>
> @@ -493,7 +492,7 @@ RTEMS_INLINE_ROUTINE Status_Control _MRSP_Surrender(
> &queue_context->Lock_context.Lock_context
> );
> _MRSP_Release( mrsp, queue_context );
> - _Thread_Priority_and_sticky_update( executing, -1 );
> + _Thread_Priority_update_and_clean_sticky( executing );
> _Thread_Dispatch_enable( cpu_self );
> return STATUS_SUCCESSFUL;
> }
> diff --git a/cpukit/include/rtems/score/scheduler.h b/cpukit/include/rtems/score/scheduler.h
> index ad9d630023..2fd1fc567a 100644
> --- a/cpukit/include/rtems/score/scheduler.h
> +++ b/cpukit/include/rtems/score/scheduler.h
> @@ -134,6 +134,40 @@ typedef struct {
> Thread_Scheduler_state next_state
> );
>
> + /**
> + * @brief Makes the node sticky.
> + *
> + * This operation is used by _Thread_Priority_update_and_make_sticky().
> + *
i think the "sticky" property needs more documentation now as it is
mandatory to implement in SMP schedulers (even if they all share the
same).
[...]
> diff --git a/cpukit/include/rtems/score/schedulersmpimpl.h b/cpukit/include/rtems/score/schedulersmpimpl.h
> index 54ed976e85..68ae3718a0 100644
> --- a/cpukit/include/rtems/score/schedulersmpimpl.h
> +++ b/cpukit/include/rtems/score/schedulersmpimpl.h
> @@ -1672,6 +1672,90 @@ static inline void _Scheduler_SMP_Withdraw_node(
> }
> }
>
> +/**
> + * @brief Makes the node sticky.
> + *
> + * @param scheduler is the scheduler of the node.
> + *
> + * @param[in, out] the_thread is the thread owning the node.
> + *
> + * @param[in, out] node is the scheduler node to make sticky.
> + */
> +static inline void _Scheduler_SMP_Make_sticky(
> + const Scheduler_Control *scheduler,
> + Thread_Control *the_thread,
> + Scheduler_Node *node,
> + Scheduler_SMP_Update update,
> + Scheduler_SMP_Enqueue enqueue
> +)
> +{
> + Scheduler_SMP_Node_state node_state;
> +
> + node_state = _Scheduler_SMP_Node_state( node );
> +
> + if ( node_state == SCHEDULER_SMP_NODE_BLOCKED ) {
> + Scheduler_Context *context;
> + Priority_Control insert_priority;
> + Priority_Control priority;
> +
> + context = _Scheduler_Get_context( scheduler );
> + priority = _Scheduler_Node_get_priority( node );
> + priority = SCHEDULER_PRIORITY_PURIFY( priority );
> +
> + if ( priority != _Scheduler_SMP_Node_priority( node ) ) {
> + ( *update )( context, node, priority );
> + }
> +
> + _Scheduler_SMP_Node_change_state( node, SCHEDULER_SMP_NODE_READY );
> + insert_priority = SCHEDULER_PRIORITY_APPEND( priority );
> + (void) ( *enqueue )( context, node, insert_priority );
> + }
> +}
> +
> +/**
> + * @brief Cleans the sticky property from the node.
> + *
> + * @param scheduler is the scheduler of the node.
> + *
> + * @param[in, out] the_thread is the thread owning the node.
> + *
> + * @param[in, out] node is the scheduler node to clean the sticky property.
> + */
> +static inline void _Scheduler_SMP_Clean_sticky(
> + const Scheduler_Control *scheduler,
> + Thread_Control *the_thread,
> + Scheduler_Node *node,
> + Scheduler_SMP_Extract extract_from_scheduled,
> + Scheduler_SMP_Extract extract_from_ready,
> + Scheduler_SMP_Get_highest_ready get_highest_ready,
> + Scheduler_SMP_Move move_from_ready_to_scheduled,
> + Scheduler_SMP_Allocate_processor allocate_processor
> +)
> +{
> + Scheduler_SMP_Node_state node_state;
> +
> + node_state = _Scheduler_SMP_Node_state( node );
> +
> + if ( node_state == SCHEDULER_SMP_NODE_SCHEDULED && node->idle != NULL ) {
> + Scheduler_Context *context;
> +
> + context = _Scheduler_Get_context( scheduler );
> + _Scheduler_SMP_Node_change_state( node, SCHEDULER_SMP_NODE_BLOCKED );
> +
> + ( *extract_from_scheduled )( context, node );
> +
> + _Scheduler_SMP_Schedule_highest_ready(
> + context,
> + node,
> + _Thread_Get_CPU( node->idle ),
> + extract_from_ready,
> + get_highest_ready,
> + move_from_ready_to_scheduled,
> + allocate_processor
> + );
Not super important, but this many arguments are probably going to
pass through the stack in pretty much any ISA (except maybe sparc). At
some point it may be worth refactoring these _Scheduler_ calls to pass
a structure of the operations by pointer instead of as individual
arguments to reduce the overhead of spilling/filling args to stack
frames. Just a random thought, maybe some kind of
Scheduler_SMP_Operations;
[...]
> diff --git a/cpukit/include/rtems/score/threadimpl.h b/cpukit/include/rtems/score/threadimpl.h
> index 934b56468a..e35a13abd6 100644
> --- a/cpukit/include/rtems/score/threadimpl.h
> +++ b/cpukit/include/rtems/score/threadimpl.h
> @@ -790,17 +790,29 @@ void _Thread_Priority_replace(
> */
> void _Thread_Priority_update( Thread_queue_Context *queue_context );
>
> +#if defined(RTEMS_SMP)
> /**
> - * @brief Updates the priority of the thread and changes it sticky level.
> + * @brief Updates the priority of the thread and makes its home scheduler node
> + * sticky.
> *
> - * @param the_thread The thread.
> - * @param sticky_level_change The new value for the sticky level.
> + * @param the_thread is the thread to work on.
> */
> -#if defined(RTEMS_SMP)
> -void _Thread_Priority_and_sticky_update(
> - Thread_Control *the_thread,
> - int sticky_level_change
> -);
> +void _Thread_Priority_update_and_make_sticky( Thread_Control *the_thread );
> +
> +/**
> + * @brief Updates the priority of the thread and cleans the sticky property of
> + * its home scheduler node.
> + *
> + * @param the_thread is the thread to work on.
> + */
> +void _Thread_Priority_update_and_clean_sticky( Thread_Control *the_thread );
> +
> +/**
> + * @brief Updates the priority of the thread.
> + *
> + * @param the_thread is the thread to update the priority.
> + */
> +void _Thread_Priority_update_only( Thread_Control *the_thread );
I wonder if it will be more clear like _Thread_Priority_update_ignore_sticky()?
[...]
> diff --git a/cpukit/score/src/scheduleredfsmp.c b/cpukit/score/src/scheduleredfsmp.c
> index 27be08ac40..0b0ee6ed21 100644
> --- a/cpukit/score/src/scheduleredfsmp.c
> +++ b/cpukit/score/src/scheduleredfsmp.c
> @@ -11,7 +11,8 @@
> * _Scheduler_EDF_SMP_Remove_processor(), _Scheduler_EDF_SMP_Set_affinity(),
> * _Scheduler_EDF_SMP_Start_idle(), _Scheduler_EDF_SMP_Unblock(),
> * _Scheduler_EDF_SMP_Unpin(), _Scheduler_EDF_SMP_Update_priority(),
> - * _Scheduler_EDF_SMP_Withdraw_node(), and _Scheduler_EDF_SMP_Yield().
> + * _Scheduler_EDF_SMP_Withdraw_node(), _Scheduler_EDF_SMP_Make_sticky(),
> + * _Scheduler_EDF_SMP_Clean_sticky(), and _Scheduler_EDF_SMP_Yield().
> */
>
> /*
> @@ -619,6 +620,39 @@ void _Scheduler_EDF_SMP_Withdraw_node(
> );
> }
>
> +void _Scheduler_EDF_SMP_Make_sticky(
> + const Scheduler_Control *scheduler,
> + Thread_Control *the_thread,
> + Scheduler_Node *node
> +)
> +{
> + _Scheduler_SMP_Make_sticky(
> + scheduler,
> + the_thread,
> + node,
> + _Scheduler_EDF_SMP_Do_update,
> + _Scheduler_EDF_SMP_Enqueue
> + );
> +}
> +
> +void _Scheduler_EDF_SMP_Clean_sticky(
> + const Scheduler_Control *scheduler,
> + Thread_Control *the_thread,
> + Scheduler_Node *node
> +)
> +{
> + _Scheduler_SMP_Clean_sticky(
> + scheduler,
> + the_thread,
> + node,
> + _Scheduler_EDF_SMP_Extract_from_scheduled,
> + _Scheduler_EDF_SMP_Extract_from_ready,
> + _Scheduler_EDF_SMP_Get_highest_ready,
> + _Scheduler_EDF_SMP_Move_from_ready_to_scheduled,
> + _Scheduler_EDF_SMP_Allocate_processor
> + );
I never really noticed before, but these functions are all declared
static inline. It may be better to just declare them as static. I
don't think it matters practically.
More information about the devel
mailing list