sergiotarxz/nuttx
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

sched/semaphore: fix priority boost restoration for priority inheritance

Browse Source 
Signed-off-by: Petro Karashchenko <petro.karashchenko@gmail.com>
This commit is contained in:
Petro Karashchenko 2022-05-24 00:06:10 +02:00 committed by Alan Carvalho de Assis
parent 03bce705d5
commit d247e8d1d2
8 changed files with 87 additions and 395 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
Documentation/reference/user/05_counting_semaphore.rst
View File

@ -77,23 +77,6 @@ implementation:
slightly increased code size and around 6-12 bytes times the value of
``CONFIG_SEM_PREALLOCHOLDERS``.
- ``CONFIG_SEM_NNESTPRIO``: In addition, there may be multiple
threads of various priorities that need to wait for a count from the
semaphore. These, the lower priority thread holding the semaphore may
have to be boosted numerous times and, to make things more complex,
will have to keep track of all of the boost priorities values in
order to correctly restore the priorities after a count has been
handed out to the higher priority thread. The
``CONFIG_SEM_NNESTPRIO`` defines the size of an array, one array per
active thread. This setting is the maximum number of higher priority
threads (minus 1) than can be waiting for another thread to release a
count on a semaphore. This value may be set to zero if no more than
one thread is expected to wait for a semaphore.
The cost associated with setting ``CONFIG_SEM_NNESTPRIO`` is slightly
increased code size and (``CONFIG_SEM_PREALLOCHOLDERS`` + 1) times
the maximum number of active threads.
- **Increased Susceptibility to Bad Thread Behavior**. These various
structures tie the semaphore implementation more tightly to the
behavior of the implementation. For examples, if a thread executes

5
include/nuttx/sched.h
View File

@ -559,10 +559,7 @@ struct tcb_s
uint8_t task_state; /* Current state of the thread */
#ifdef CONFIG_PRIORITY_INHERITANCE
#if CONFIG_SEM_NNESTPRIO > 0
uint8_t npend_reprio; /* Number of nested reprioritizations */
uint8_t pend_reprios[CONFIG_SEM_NNESTPRIO];
#endif
uint8_t boost_priority; /* "Boosted" priority of the thread */
uint8_t base_priority; /* "Normal" priority of the thread */
FAR struct semholder_s *holdsem; /* List of held semaphores */
#endif

10
sched/Kconfig
View File

@ -1172,16 +1172,6 @@ config SEM_PREALLOCHOLDERS
are only using semaphores as mutexes (only one holder) OR if no more
than two threads participate using a counting semaphore.
config SEM_NNESTPRIO
int "Maximum number of higher priority threads"
default 16
---help---
If priority inheritance is enabled, then this setting is the
maximum number of higher priority threads (minus 1) than can be
waiting for another thread to release a count on a semaphore.
This value may be set to zero if no more than one thread is
expected to wait for a semaphore.
endif # PRIORITY_INHERITANCE
menu "RTOS hooks"

6
sched/sched/sched_reprioritize.c
View File

@ -79,11 +79,9 @@ int nxsched_reprioritize(FAR struct tcb_s *tcb, int sched_priority)
tcb->base_priority = (uint8_t)sched_priority;
/* Discard any pending reprioritizations as well */
/* Discard priority boost as well */
#if CONFIG_SEM_NNESTPRIO > 0
tcb->npend_reprio = 0;
#endif
tcb->boost_priority = 0;
}
return ret;

204
sched/semaphore/sem_holder.c
View File

@ -325,76 +325,6 @@ static int nxsem_boostholderprio(FAR struct semholder_s *pholder,
FAR struct tcb_s *htcb = (FAR struct tcb_s *)pholder->htcb;
FAR struct tcb_s *rtcb = (FAR struct tcb_s *)arg;
#if CONFIG_SEM_NNESTPRIO > 0
/* If the priority of the thread that is waiting for a count is greater
* than the base priority of the thread holding a count, then we may need
* to adjust the holder's priority now or later to that priority.
*/
if (rtcb->sched_priority > htcb->base_priority)
{
/* If the new priority is greater than the current, possibly already
* boosted priority of the holder thread, then we will have to raise
* the holder's priority now.
*/
if (rtcb->sched_priority > htcb->sched_priority)
{
/* If the current priority of holder thread has already been
* boosted, then add the boost priority to the list of restoration
* priorities. When the higher priority waiter thread gets its
* count, then we need to revert the holder thread to this saved
* priority (not to its base priority).
*/
if (htcb->sched_priority > htcb->base_priority)
{
/* Save the current, boosted priority of the holder thread. */
if (htcb->npend_reprio < CONFIG_SEM_NNESTPRIO)
{
htcb->pend_reprios[htcb->npend_reprio] =
htcb->sched_priority;
htcb->npend_reprio++;
}
else
{
serr("ERROR: CONFIG_SEM_NNESTPRIO exceeded\n");
DEBUGASSERT(htcb->npend_reprio < CONFIG_SEM_NNESTPRIO);
}
}
/* Raise the priority of the thread holding of the semaphore.
* This cannot cause a context switch because we have preemption
* disabled. The holder thread may be marked "pending" and the
* switch may occur during up_block_task() processing.
*/
nxsched_set_priority(htcb, rtcb->sched_priority);
}
else
{
/* The new priority is above the base priority of the holder,
* but not as high as its current working priority. Just put it
* in the list of pending restoration priorities so that when the
* higher priority thread gets its count, we can revert to this
* saved priority and not to the base priority.
*/
if (htcb->npend_reprio < CONFIG_SEM_NNESTPRIO)
{
htcb->pend_reprios[htcb->npend_reprio] = rtcb->sched_priority;
htcb->npend_reprio++;
}
else
{
serr("ERROR: CONFIG_SEM_NNESTPRIO exceeded\n");
DEBUGASSERT(htcb->npend_reprio < CONFIG_SEM_NNESTPRIO);
}
}
}
#else
/* If the priority of the thread that is waiting for a count is less than
* or equal to the priority of the thread holding a count, then do nothing
* because the thread is already running at a sufficient priority.
@ -410,7 +340,6 @@ static int nxsem_boostholderprio(FAR struct semholder_s *pholder,
nxsched_set_priority(htcb, rtcb->sched_priority);
}
#endif
return 0;
}
@ -435,9 +364,6 @@ static int nxsem_verifyholder(FAR struct semholder_s *pholder,
* In this case, the priority of the holder should not be boosted.
*/
#if CONFIG_SEM_NNESTPRIO > 0
DEBUGASSERT(htcb->npend_reprio == 0);
#endif
DEBUGASSERT(htcb->sched_priority == htcb->base_priority);
#endif
@ -473,12 +399,7 @@ static int nxsem_restoreholderprio(FAR struct semholder_s *pholder,
FAR sem_t *sem, FAR void *arg)
{
FAR struct tcb_s *htcb = pholder->htcb;
#if CONFIG_SEM_NNESTPRIO > 0
FAR struct tcb_s *stcb = (FAR struct tcb_s *)arg;
int rpriority;
int i;
int j;
#endif
int hpriority;
/* Release the holder if all counts have been given up
* before reprioritizing causes a context switch.
@ -489,117 +410,42 @@ static int nxsem_restoreholderprio(FAR struct semholder_s *pholder,
nxsem_freeholder(sem, pholder);
}
/* We attempt to restore thread priority to its base priority. If
* there is any thread with the higher priority waiting for the
* semaphore held by htcb then this value will be overwritten.
*/
hpriority = htcb->boost_priority > htcb->base_priority ?
htcb->boost_priority : htcb->base_priority;
/* Was the priority of the holder thread boosted? If so, then drop its
* priority back to the correct level. What is the correct level?
*/
if (htcb->sched_priority != htcb->base_priority)
if (htcb->sched_priority != hpriority)
{
#if CONFIG_SEM_NNESTPRIO > 0
/* Are there other, pending priority levels to revert to? */
if (htcb->npend_reprio < 1)
{
/* No... the holder thread has only been boosted once.
* npend_reprio should be 0 and the boosted priority should be the
* priority of the task that just got the semaphore
* (stcb->sched_priority)
*
* That latter assumption may not be true if the stcb's priority
* was also boosted so that it no longer matches the htcb's
* sched_priority. Or if CONFIG_SEM_NNESTPRIO is too small (so
* that we do not have a proper record of the reprioritizations).
/* Try to find the highest priority across all the threads that are
* waiting for any semaphore held by htcb.
*/
DEBUGASSERT(/* htcb->sched_priority == stcb->sched_priority && */
htcb->npend_reprio == 0);
for (pholder = htcb->holdsem; pholder != NULL;
pholder = pholder->tlink)
{
FAR struct tcb_s *stcb;
/* Reset the holder's priority back to the base priority. */
stcb = (FAR struct tcb_s *)dq_peek(SEM_WAITLIST(pholder->sem));
nxsched_reprioritize(htcb, htcb->base_priority);
if (stcb != NULL && stcb->sched_priority > hpriority)
{
hpriority = stcb->sched_priority;
}
}
/* There are multiple pending priority levels. The holder thread's
* "boosted" priority could greater than or equal to
* "stcb->sched_priority" (it could be greater if its priority was
* boosted because it also holds another semaphore).
/* Apply the selected priority to the thread (hopefully back to the
* threads base_priority).
*/
else if (htcb->sched_priority <= stcb->sched_priority)
{
/* The holder thread has been boosted to the same priority as the
* waiter thread that just received the count. We will simply
* reprioritize to the next highest priority that we have in
* rpriority.
*/
/* Find the highest pending priority and remove it from the list */
for (i = 1, j = 0; i < htcb->npend_reprio; i++)
{
if (htcb->pend_reprios[i] > htcb->pend_reprios[j])
{
j = i;
}
}
/* Remove the highest priority pending priority from the list */
rpriority = htcb->pend_reprios[j];
i = htcb->npend_reprio - 1;
if (i > 0)
{
htcb->pend_reprios[j] = htcb->pend_reprios[i];
}
htcb->npend_reprio = i;
/* And apply that priority to the thread (while retaining the
* base_priority)
*/
nxsched_set_priority(htcb, rpriority);
}
else
{
/* The holder thread has been boosted to a higher priority than the
* waiter task. The pending priority should be in the list (unless
* it was lost because of list overflow or because the holder
* was reprioritized again unbeknownst to the priority inheritance
* logic).
*
* Search the list for the matching priority.
*/
for (i = 0; i < htcb->npend_reprio; i++)
{
/* Does this pending priority match the priority of the thread
* that just received the count?
*/
if (htcb->pend_reprios[i] == stcb->sched_priority)
{
/* Yes, remove it from the list */
j = htcb->npend_reprio - 1;
if (j > 0)
{
htcb->pend_reprios[i] = htcb->pend_reprios[j];
}
htcb->npend_reprio = j;
break;
}
}
}
#else
/* There is no alternative restore priorities, drop the priority
* of the holder thread all the way back to the threads "base"
* priority.
*/
nxsched_reprioritize(htcb, htcb->base_priority);
#endif
nxsched_set_priority(htcb, hpriority);
}
return 0;
@ -952,7 +798,7 @@ void nxsem_release_holder(FAR sem_t *sem)
/* Find the container for this holder */
#if CONFIG_SEM_PREALLOCHOLDERS > 0
for (pholder = sem->hhead; pholder; pholder = pholder->flink)
for (pholder = sem->hhead; pholder != NULL; pholder = pholder->flink)
#else
int i;

4
sched/task/task_restart.c
View File

@ -165,9 +165,7 @@ int nxtask_restart(pid_t pid)
#ifdef CONFIG_PRIORITY_INHERITANCE
tcb->cmn.base_priority = tcb->cmn.init_priority;
# if CONFIG_SEM_NNESTPRIO > 0
tcb->cmn.npend_reprio = 0;
# endif
tcb->cmn.boost_priority = 0;
#endif
/* Re-initialize the processor-specific portion of the TCB. This will

192
sched/wqueue/kwork_inherit.c
View File

@ -66,7 +66,10 @@ static void lpwork_boostworker(pid_t wpid, uint8_t reqprio)
wtcb = nxsched_get_tcb(wpid);
DEBUGASSERT(wtcb);
#if CONFIG_SEM_NNESTPRIO > 0
/* REVISIT: Priority multi-boost is not supported */
DEBUGASSERT(wtcb->boost_priority == 0);
/* If the priority of the client thread that is greater than the base
* priority of the worker thread, then we may need to adjust the worker
* thread's priority now or later to that priority.
@ -74,70 +77,16 @@ static void lpwork_boostworker(pid_t wpid, uint8_t reqprio)
if (reqprio > wtcb->base_priority)
{
/* If the new priority is greater than the current, possibly already
* boosted priority of the worker thread, then we will have to raise
* the worker thread's priority now.
/* Save boost priority value as it might be needed in case of multiple
* re-prioritisations happen, then the priority of the thread can't go
* below the boost priority value until priority boost is canceled.
*/
if (reqprio > wtcb->sched_priority)
{
/* If the current priority of worker thread has already been
* boosted, then add the boost priority to the list of restoration
* priorities. When the higher priority waiter thread gets its
* count, then we need to revert the worker thread to this saved
* priority (not to its base priority).
*/
wtcb->boost_priority = reqprio;
if (wtcb->sched_priority > wtcb->base_priority)
{
/* Save the current, boosted priority of the worker thread. */
if (wtcb->npend_reprio < CONFIG_SEM_NNESTPRIO)
{
wtcb->pend_reprios[wtcb->npend_reprio] =
wtcb->sched_priority;
wtcb->npend_reprio++;
}
else
{
serr("ERROR: CONFIG_SEM_NNESTPRIO exceeded\n");
DEBUGASSERT(wtcb->npend_reprio < CONFIG_SEM_NNESTPRIO);
}
}
/* Raise the priority of the worker. This cannot cause a context
* switch because we have preemption disabled. The worker thread
* may be marked "pending" and the switch may occur during
* sched_unblock() processing.
*/
nxsched_set_priority(wtcb, reqprio);
}
else
{
/* The new priority is above the base priority of the worker,
* but not as high as its current working priority. Just put it
* in the list of pending restoration priorities so that when the
* higher priority thread gets its count, we can revert to this
* saved priority and not to the base priority.
*/
if (wtcb->npend_reprio < CONFIG_SEM_NNESTPRIO)
{
wtcb->pend_reprios[wtcb->npend_reprio] = reqprio;
wtcb->npend_reprio++;
}
else
{
serr("ERROR: CONFIG_SEM_NNESTPRIO exceeded\n");
DEBUGASSERT(wtcb->npend_reprio < CONFIG_SEM_NNESTPRIO);
}
}
}
#else
/* If the priority of the client thread that is less than of equal to the
* priority of the worker thread, then do nothing because the thread is
* already running at a sufficient priority.
/* If the priority of the client thread that is less than of equal to
* the priority of the worker thread, then do nothing because the
* thread is already running at a sufficient priority.
*/
if (reqprio > wtcb->sched_priority)
@ -150,7 +99,7 @@ static void lpwork_boostworker(pid_t wpid, uint8_t reqprio)
nxsched_set_priority(wtcb, reqprio);
}
#endif
}
}
/****************************************************************************
@ -174,128 +123,59 @@ static void lpwork_boostworker(pid_t wpid, uint8_t reqprio)
static void lpwork_restoreworker(pid_t wpid, uint8_t reqprio)
{
FAR struct tcb_s *wtcb;
#if CONFIG_SEM_NNESTPRIO > 0
uint8_t wpriority;
int index;
int selected;
#endif
/* Get the TCB of the low priority worker thread from the process ID. */
wtcb = nxsched_get_tcb(wpid);
DEBUGASSERT(wtcb);
/* REVISIT: Priority multi-boost is not supported. */
DEBUGASSERT(wtcb->boost_priority == reqprio);
/* Clear the threat boost priority. */
wtcb->boost_priority = 0;
/* Was the priority of the worker thread boosted? If so, then drop its
* priority back to the correct level. What is the correct level?
*/
if (wtcb->sched_priority != wtcb->base_priority)
{
#if CONFIG_SEM_NNESTPRIO > 0
/* Are there other, pending priority levels to revert to? */
FAR struct semholder_s *pholder;
uint8_t wpriority;
if (wtcb->npend_reprio < 1)
{
/* No... the worker thread has only been boosted once.
* npend_reprio should be 0 and the boosted priority should be the
* priority of the client task (reqprio)
*
* That latter assumption may not be true if the client's priority
* was also boosted so that it no longer matches the wtcb's
* sched_priority. Or if CONFIG_SEM_NNESTPRIO is too small (so
* that we do not have a proper record of the reprioritizations).
/* We attempt to restore task priority to its base priority. If there
* is any task with the higher priority waiting for the semaphore
* held by wtcb then this value will be overwritten.
*/
DEBUGASSERT(/* wtcb->sched_priority == reqprio && */
wtcb->npend_reprio == 0);
wpriority = wtcb->base_priority;
/* Reset the worker's priority back to the base priority. */
nxsched_reprioritize(wtcb, wtcb->base_priority);
}
/* There are multiple pending priority levels. The worker thread's
* "boosted" priority could greater than or equal to "reqprio" (it
* could be greater if its priority we boosted because it also holds
* some semaphore).
/* Try to find the highest priority across all the tasks that are
* waiting for any semaphore held by wtcb.
*/
else if (wtcb->sched_priority <= reqprio)
for (pholder = wtcb->holdsem; pholder != NULL;
pholder = pholder->tlink)
{
/* The worker thread has been boosted to the same priority as the
* waiter thread that just received the count. We will simply
* reprioritize to the next highest pending priority.
*/
FAR struct tcb_s *stcb;
/* Find the highest pending priority and remove it from the list */
stcb = (FAR struct tcb_s *)dq_peek(SEM_WAITLIST(pholder->sem));
for (index = 1, selected = 0; index < wtcb->npend_reprio; index++)
if (stcb != NULL && stcb->sched_priority > wpriority)
{
if (wtcb->pend_reprios[index] > wtcb->pend_reprios[selected])
{
selected = index;
wpriority = stcb->sched_priority;
}
}
/* Remove the highest priority pending priority from the list */
wpriority = wtcb->pend_reprios[selected];
index = wtcb->npend_reprio - 1;
if (index > 0)
{
wtcb->pend_reprios[selected] = wtcb->pend_reprios[index];
}
wtcb->npend_reprio = index;
/* And apply that priority to the thread (while retaining the
* base_priority)
/* Apply the selected priority to the worker thread (hopefully back
* to the threads base_priority).
*/
nxsched_set_priority(wtcb, wpriority);
}
else
{
/* The worker thread has been boosted to a higher priority than the
* waiter task. The pending priority should be in the list (unless
* it was lost because of list overflow or because the worker was
* reprioritized again unbeknownst to the priority inheritance
* logic).
*
* Search the list for the matching priority.
*/
for (index = 0; index < wtcb->npend_reprio; index++)
{
/* Does this pending priority match the priority of the thread
* that just received the count?
*/
if (wtcb->pend_reprios[index] == reqprio)
{
/* Yes, remove it from the list */
selected = wtcb->npend_reprio - 1;
if (selected > 0)
{
wtcb->pend_reprios[index] =
wtcb->pend_reprios[selected];
}
wtcb->npend_reprio = selected;
break;
}
}
}
#else
/* There is no alternative restore priorities, drop the priority
* of the worker thread all the way back to the threads "base"
* priority.
*/
nxsched_reprioritize(wtcb, wtcb->base_priority);
#endif
}
}
/****************************************************************************