From 8669183852845f527d7761f6582bcc4d997eb812 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Sat, 1 Oct 2016 11:38:22 -0600 Subject: [PATCH] sched/pthread and task: When a pthread is started, there is a small bit of logic that will run on the thread of execution of the new pthread. In the case where the new pthread has a lower priority than the parent thread, then this could cause both the parent thread and the new pthread to be blocked at the priority of the lower priority pthread (assuming that CONFIG_PRIORITY_INHERITANCE is not selected). This change temporarily boosts the priority of the new pthread to at least the priority of the new pthread to at least the priority of the parent thread. When that bit of logic has executed on the thread of execution of the new pthread, it will then drop to the correct priority (if necessary) before calling into the new pthread's entry point. --- include/nuttx/sched.h | 4 ++-- sched/pthread/pthread_create.c | 32 +++++++++++++++++++++++++++++++- sched/task/task_restart.c | 4 ++-- sched/task/task_setup.c | 15 +++------------ 4 files changed, 38 insertions(+), 17 deletions(-) diff --git a/include/nuttx/sched.h b/include/nuttx/sched.h index 4d7bb88f11..ec438727f8 100644 --- a/include/nuttx/sched.h +++ b/include/nuttx/sched.h @@ -551,6 +551,7 @@ struct tcb_s start_t start; /* Thread start function */ entry_t entry; /* Entry Point into the thread */ uint8_t sched_priority; /* Current priority of the thread */ + uint8_t init_priority; /* Initial priority of the thread */ #ifdef CONFIG_PRIORITY_INHERITANCE #if CONFIG_SEM_NNESTPRIO > 0 @@ -654,9 +655,8 @@ struct task_tcb_s FAR void *starthookarg; /* The argument passed to the function */ #endif - /* Values needed to restart a task ********************************************/ + /* [Re-]start name + start-up parameters **************************************/ - uint8_t init_priority; /* Initial priority of the task */ FAR char **argv; /* Name+start-up parameters */ }; diff --git a/sched/pthread/pthread_create.c b/sched/pthread/pthread_create.c index 487827fbcb..be81a5f0ff 100644 --- a/sched/pthread/pthread_create.c +++ b/sched/pthread/pthread_create.c @@ -183,6 +183,16 @@ static void pthread_start(void) pjoin->started = true; (void)pthread_givesemaphore(&pjoin->data_sem); + /* The priority of this thread may have been boosted to avoid priority + * inversion problems. If that is the case, then drop to the correct + * execution priority. + */ + + if (ptcb->cmn.sched_priority > ptcb->cmn.init_priority) + { + DEBUGVERIFY(sched_setpriority(&ptcb->cmn, ptcb->cmn.init_priority)); + } + /* Pass control to the thread entry point. In the kernel build this has to * be handled differently if we are starting a user-space pthread; we have * to switch to user-mode before calling into the pthread. @@ -488,7 +498,27 @@ int pthread_create(FAR pthread_t *thread, FAR const pthread_attr_t *attr, ret = sem_init(&pjoin->exit_sem, 0, 0); } - /* Activate the task */ + /* If the priority of the new pthread is lower than the priority of the + * parent thread, then starting the pthread could result in both the + * parent and the pthread to be blocked. This is a recipe for priority + * inversion issues. + * + * We avoid this here by boosting the priority of the (inactive) pthread + * so it has the same priority as the parent thread. + */ + + if (ret == OK) + { + FAR struct tcb_s *parent = this_task(); + DEBUGASSERT(parent != NULL); + + if (ptcb->cmn.sched_priority < parent->sched_priority) + { + ret = sched_setpriority(&ptcb->cmn, parent->sched_priority); + } + } + + /* Then activate the task */ sched_lock(); if (ret == OK) diff --git a/sched/task/task_restart.c b/sched/task/task_restart.c index 5f3b76958f..70da24df57 100644 --- a/sched/task/task_restart.c +++ b/sched/task/task_restart.c @@ -169,12 +169,12 @@ int task_restart(pid_t pid) /* Reset the current task priority */ - tcb->cmn.sched_priority = tcb->init_priority; + tcb->cmn.sched_priority = tcb->cmn.init_priority; /* Reset the base task priority and the number of pending reprioritizations */ #ifdef CONFIG_PRIORITY_INHERITANCE - tcb->cmn.base_priority = tcb->init_priority; + tcb->cmn.base_priority = tcb->cmn.init_priority; # if CONFIG_SEM_NNESTPRIO > 0 tcb->cmn.npend_reprio = 0; # endif diff --git a/sched/task/task_setup.c b/sched/task/task_setup.c index 0819049785..7b8b086434 100644 --- a/sched/task/task_setup.c +++ b/sched/task/task_setup.c @@ -363,6 +363,7 @@ static int thread_schedsetup(FAR struct tcb_s *tcb, int priority, /* Save task priority and entry point in the TCB */ tcb->sched_priority = (uint8_t)priority; + tcb->init_priority = (uint8_t)priority; #ifdef CONFIG_PRIORITY_INHERITANCE tcb->base_priority = (uint8_t)priority; #endif @@ -633,20 +634,10 @@ static inline int task_stackargsetup(FAR struct task_tcb_s *tcb, int task_schedsetup(FAR struct task_tcb_s *tcb, int priority, start_t start, main_t main, uint8_t ttype) { - int ret; - /* Perform common thread setup */ - ret = thread_schedsetup((FAR struct tcb_s *)tcb, priority, start, - (CODE void *)main, ttype); - if (ret == OK) - { - /* Save task restart priority */ - - tcb->init_priority = (uint8_t)priority; - } - - return ret; + return thread_schedsetup((FAR struct tcb_s *)tcb, priority, start, + (CODE void *)main, ttype); } /****************************************************************************