You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/09/29 17:56:24 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6318: sched/semaphore: fix priority boost restoration for priority inheritance
xiaoxiang781216 commented on code in PR #6318:
URL: https://github.com/apache/incubator-nuttx/pull/6318#discussion_r983864632
##########
sched/semaphore/sem_holder.c:
##########
@@ -489,117 +410,55 @@ 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).
- */
-
- DEBUGASSERT(/* htcb->sched_priority == stcb->sched_priority && */
- htcb->npend_reprio == 0);
-
- /* Reset the holder's priority back to the base priority. */
-
- nxsched_reprioritize(htcb, htcb->base_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).
+ /* Try to find the highest priority across all the threads that are
+ * waiting for any semaphore held by htcb. Skip search if htcb does
+ * not hold any semaphore.
*/
- else if (htcb->sched_priority <= stcb->sched_priority)
+ if (htcb->holdsem != NULL)
{
- /* 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.
- */
+ FAR struct tcb_s *stcb;
- /* Find the highest pending priority and remove it from the list */
-
- for (i = 1, j = 0; i < htcb->npend_reprio; i++)
+ for (stcb = (FAR struct tcb_s *)g_waitingforsemaphore.head;
Review Comment:
let's rebase to the master
##########
sched/wqueue/kwork_inherit.c:
##########
@@ -174,127 +123,72 @@ 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? */
-
- 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).
- */
-
- DEBUGASSERT(/* wtcb->sched_priority == reqprio && */
- wtcb->npend_reprio == 0);
-
- /* Reset the worker's priority back to the base priority. */
+ uint8_t wpriority;
- 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).
+ /* 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.
*/
- else if (wtcb->sched_priority <= reqprio)
- {
- /* 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.
- */
+ wpriority = wtcb->base_priority;
- /* Find the highest pending priority and remove it from the list */
-
- for (index = 1, selected = 0; index < wtcb->npend_reprio; index++)
- {
- if (wtcb->pend_reprios[index] > wtcb->pend_reprios[selected])
- {
- selected = index;
- }
- }
-
- /* 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)
- */
+ /* Try to find the highest priority across all the tasks that are
+ * waiting for any semaphore held by wtcb. Skip search if htcb does
+ * not hold any semaphore.
+ */
- nxsched_set_priority(wtcb, wpriority);
- }
- else
+ if (wtcb->holdsem != NULL)
{
- /* 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.
- */
+ FAR struct tcb_s *stcb;
- for (index = 0; index < wtcb->npend_reprio; index++)
+ for (stcb = (FAR struct tcb_s *)g_waitingforsemaphore.head;
Review Comment:
ditto
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org