[PATCH 8/8] score: Rework thread priority management
Sebastian Huber
sebastian.huber at embedded-brains.de
Thu Sep 8 11:13:37 UTC 2016
Hello Gedare,
thanks for your quick review.
On 07/09/16 20:52, Gedare Bloom wrote:
> On Tue, Sep 6, 2016 at 8:40 AM, Sebastian Huber
> <sebastian.huber at embedded-brains.de> wrote:
>> Add priority nodes which contribute to the overall thread priority.
>>
>> The actual priority of a thread is now an aggregation of priority nodes.
>> The thread priority aggregation for the home scheduler instance of a
>> thread consists of at least one priority node, which is normally the
>> real priority of the thread. The locking protocols (e.g. priority
>> ceiling and priority inheritance), rate-monotonic period objects and the
>> POSIX sporadic server add, change and remove priority nodes.
>>
>> A thread changes its priority now immediately, e.g. priority changes are
>> not deferred until the thread releases its last resource.
>>
>> Replace the _Thread_Change_priority() function with
>>
>> * _Thread_Priority_perform_actions(),
>> * _Thread_Priority_add(),
>> * _Thread_Priority_remove(),
>> * _Thread_Priority_change(), and
>> * _Thread_Priority_update().
>>
>> Update #2412.
>> Update #2556.
>> [...]
>> + */
>> + RBTree_Control Contributors;
>> +
> Is it the case that a priority node is only ever on one of these
> "Contributors" trees at a time?
Yes, a priority node can be added to at most one tree at a time. This
priority aggregation is a container for priority nodes and is itself a
priority node.
> [...]
> + struct {
> + Priority_Actions Actions;
> +
> + /**
> + * @brief Count of threads to update the priority via
> + * _Thread_Priority_update().
> + */
> + size_t update_count;
> I'd prefer uint32_t for unsigned counters since usually size_t denotes
> an object's (byte) size.
It is related to the update table, so I think size_t is the right type.
>
>> +
>> + /**
>> + * @brief Threads to update the priority via _Thread_Priority_update().
>> + */
>> + Thread_Control *update[ 2 ];
> Why 2?
>
>> + } Priority;
>> +
>> /**
>> * @brief Invoked in case of a detected deadlock.
>> *
>> @@ -237,7 +274,7 @@ typedef struct {
>> /**
>> * @brief The actual thread priority queue.
>> */
>> - RBTree_Control Queue;
>> + Priority_Aggregation Queue;
>> } Thread_queue_Priority_queue;
>>
>> /**
>> @@ -289,6 +326,11 @@ typedef struct _Thread_queue_Heads {
>>
>> #if defined(RTEMS_SMP)
>> /**
>> + * @brief Boost priority.
> This note could be expanded (e.g. boosted by xxx). Thanks for
> improving the doxy along the way!
The boosting stuff will go away soon. The main goal of OMIP is to get
rid of priority boosting.
>
>> + */
>> + Priority_Node Boost_priority;
>> +
>> + /**
>> * @brief One priority queue per scheduler instance.
>> */
>> Thread_queue_Priority_queue Priority[ RTEMS_ZERO_LENGTH_ARRAY ];
>> @@ -337,34 +379,33 @@ struct Thread_queue_Queue {
>> };
>>
>> /**
>> - * @brief Thread queue priority change operation.
>> + * @brief Thread queue action operation.
>> *
>> * @param[in] queue The actual thread queue.
>> * @param[in] the_thread The thread.
>> - * @param[in] new_priority The new priority value.
>> - *
>> - * @see Thread_queue_Operations.
>> + * @param[in] queue_context The thread queue context providing the thread queue
>> + * action set to perform. Returns the thread queue action set to perform on
>> + * the thread queue owner or the empty set in case there is nothing to do.
>> */
>> -typedef void ( *Thread_queue_Priority_change_operation )(
>> - Thread_queue_Queue *queue,
>> - Thread_Control *the_thread,
>> - Priority_Control new_priority
>> +typedef void ( *Thread_queue_Priority_actions_operation )(
>> + Thread_queue_Queue *queue,
>> + Priority_Actions *priority_actions
>> );
>>
>> /**
>> * @brief Thread queue enqueue operation.
>> *
>> * A potential thread to update the priority due to priority inheritance is
>> - * returned via the thread queue path. This thread is handed over to
>> - * _Thread_Update_priority().
>> + * returned via the thread queue context. This thread is handed over to
>> + * _Thread_Priority_update().
>> *
>> * @param[in] queue The actual thread queue.
>> * @param[in] the_thread The thread to enqueue on the queue.
>> */
>> typedef void ( *Thread_queue_Enqueue_operation )(
>> - Thread_queue_Queue *queue,
>> - Thread_Control *the_thread,
>> - Thread_queue_Path *path
>> + Thread_queue_Queue *queue,
>> + Thread_Control *the_thread,
>> + Thread_queue_Context *queue_context
>> );
>>
>> /**
>> @@ -374,8 +415,9 @@ typedef void ( *Thread_queue_Enqueue_operation )(
>> * @param[in] the_thread The thread to extract from the thread queue.
>> */
>> typedef void ( *Thread_queue_Extract_operation )(
>> - Thread_queue_Queue *queue,
>> - Thread_Control *the_thread
>> + Thread_queue_Queue *queue,
>> + Thread_Control *the_thread,
>> + Thread_queue_Context *queue_context
>> );
>>
>> /**
>> @@ -390,9 +432,10 @@ typedef void ( *Thread_queue_Extract_operation )(
>> * @return The previous first thread on the queue.
>> */
>> typedef Thread_Control *( *Thread_queue_Surrender_operation )(
>> - Thread_queue_Queue *queue,
>> - Thread_queue_Heads *heads,
>> - Thread_Control *previous_owner
>> + Thread_queue_Queue *queue,
>> + Thread_queue_Heads *heads,
>> + Thread_Control *previous_owner,
>> + Thread_queue_Context *queue_context
>> );
>>
>> /**
>> @@ -415,16 +458,9 @@ typedef Thread_Control *( *Thread_queue_First_operation )(
>> */
>> struct Thread_queue_Operations {
>> /**
>> - * @brief Thread queue priority change operation.
>> - *
>> - * Called by _Thread_Change_priority() to notify a thread about a priority
>> - * change. In case this thread waits currently for a resource the handler
>> - * may adjust its data structures according to the new priority value. This
>> - * handler must not be NULL, instead the default handler
>> - * _Thread_Do_nothing_priority_change() should be used in case nothing needs
>> - * to be done during a priority change.
>> - */
>> - Thread_queue_Priority_change_operation priority_change;
>> + * @brief Thread queue priority actions operation.
> This note also should be expanded a bit.
The details are in the function pointer type documentation.
>
>> + */
>> + Thread_queue_Priority_actions_operation priority_actions;
>>
>> /**
>> * @brief Thread queue enqueue operation.
>> diff --git a/cpukit/score/include/rtems/score/threadqimpl.h b/cpukit/score/include/rtems/score/threadqimpl.h
>> index 977b0ce..3555b7f 100644
>> --- a/cpukit/score/include/rtems/score/threadqimpl.h
>> +++ b/cpukit/score/include/rtems/score/threadqimpl.h
>> @@ -21,7 +21,7 @@
>>
>> #include <rtems/score/threadq.h>
>> #include <rtems/score/chainimpl.h>
>> -#include <rtems/score/rbtreeimpl.h>
>> +#include <rtems/score/priorityimpl.h>
>> #include <rtems/score/scheduler.h>
>> #include <rtems/score/smp.h>
>> #include <rtems/score/thread.h>
>> @@ -39,38 +39,8 @@ extern "C" {
>> */
>> /**@{*/
>>
>> -/**
>> - * @brief Representation of a thread queue path from a start thread queue to
>> - * the terminal thread queue.
>> - *
>> - * The start thread queue is determined by the object on which a thread intends
>> - * to block. The terminal thread queue is the thread queue reachable via
>> - * thread queue links those owner is not blocked on a thread queue. The thread
>> - * queue links are determined by the thread queue owner and thread wait queue
>> - * relationships.
>> - */
>> -struct Thread_queue_Path {
>> -#if defined(RTEMS_SMP)
>> - /**
>> - * @brief The chain of thread queue links defining the thread queue path.
>> - */
>> - Chain_Control Links;
>> -
>> - /**
>> - * @brief The start of a thread queue path.
>> - */
>> - Thread_queue_Link Start;
>> -#endif
>> -
>> - /**
>> - * @brief A potential thread to update the priority via
>> - * _Thread_Update_priority().
>> - *
>> - * This thread is determined by thread queues which support priority
>> - * inheritance.
>> - */
>> - Thread_Control *update_priority;
>> -};
>> +#define THREAD_QUEUE_LINK_OF_PATH_NODE( node ) \
>> + RTEMS_CONTAINER_OF( node, Thread_queue_Link, Path_node );
>>
>> /**
>> * @brief Thread queue with a layout compatible to struct _Thread_queue_Queue
>> @@ -210,6 +180,42 @@ RTEMS_INLINE_ROUTINE void _Thread_queue_Context_set_deadlock_callout(
>> queue_context->deadlock_callout = deadlock_callout;
>> }
>>
>> +RTEMS_INLINE_ROUTINE void _Thread_queue_Context_clear_priority_updates(
>> + Thread_queue_Context *queue_context
>> +)
>> +{
>> + queue_context->Priority.update_count = 0;
>> +}
>> +
>> +RTEMS_INLINE_ROUTINE size_t _Thread_queue_Context_get_priority_updates(
>> + Thread_queue_Context *queue_context
>> +)
>> +{
>> + return queue_context->Priority.update_count;
>> +}
>> +
>> +RTEMS_INLINE_ROUTINE void _Thread_queue_Context_set_priority_updates(
>> + Thread_queue_Context *queue_context,
>> + size_t update_count
>> +)
>> +{
>> + queue_context->Priority.update_count = update_count;
>> +}
> Are there some rules for what can/should set the priority updates? Is
> this a monotonic counter until clear()?
I renamed these functions to
_Thread_queue_Context_save_priority_updates()
_Thread_queue_Context_restore_priority_updates()
The only user is _Thread_Priority_perform_actions(). I added a comment
why we do a save/restore.
> [...]
>
> -void _Thread_Restore_priority( Thread_Control *the_thread )
> +void _Thread_Priority_update( Thread_queue_Context *queue_context )
> {
> - _Thread_Change_priority(
> - the_thread,
> - 0,
> - NULL,
> - _Thread_Restore_priority_filter,
> - true
> - );
> + size_t i;
> + size_t n;
> +
> + n = queue_context->Priority.update_count;
> +
> + for ( i = 0; i < n ; ++i ) {
> + Thread_Control *the_thread;
> + ISR_lock_Context lock_context;
> +
> + the_thread = queue_context->Priority.update[ i ];
> + _Thread_State_acquire( the_thread, &lock_context );
> + _Scheduler_Update_priority( the_thread );
> + _Thread_State_release( the_thread, &lock_context );
> + }
> Why doesn't this decrement the update_count after completing the
> update? I'm missing some important detail about the "lifetime" of
> these update counters.
I added a comment.
--
Sebastian Huber, embedded brains GmbH
Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail : sebastian.huber at embedded-brains.de
PGP : Public key available on request.
Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
More information about the devel
mailing list