From a442245479d2d387ecdc7ac7608c03bf7efff39a Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Tue, 16 Feb 2016 10:14:33 -0600 Subject: [PATCH] SMP: Fix some non-SMP errors that crept in; fix a recursion problem; re-partition some functionality to improve design and readability --- arch | 2 +- sched/sched/Make.defs | 4 + sched/sched/sched.h | 3 + sched/sched/sched_addreadytorun.c | 45 +---- sched/sched/sched_cpuselect.c | 110 ++++++++++++ sched/sched/sched_reprioritize.c | 24 --- sched/sched/sched_setpriority.c | 289 +++++++++++++++++++++--------- sched/sched/sched_unlock.c | 4 +- sched/semaphore/spinlock.c | 2 + 9 files changed, 328 insertions(+), 155 deletions(-) create mode 100644 sched/sched/sched_cpuselect.c diff --git a/arch b/arch index 8012d549ae5..790c6be472d 160000 --- a/arch +++ b/arch @@ -1 +1 @@ -Subproject commit 8012d549ae583abf8e6d902a42fe6e1a534bb2a5 +Subproject commit 790c6be472dd5c386723e5b54bef495300ad6d7d diff --git a/sched/sched/Make.defs b/sched/sched/Make.defs index 65710efdd26..3c521a25d77 100644 --- a/sched/sched/Make.defs +++ b/sched/sched/Make.defs @@ -47,6 +47,10 @@ ifeq ($(CONFIG_PRIORITY_INHERITANCE),y) CSRCS += sched_reprioritize.c endif +ifeq ($(CONFIG_SMP),y) +CSRCS += sched_cpuselect.c +endif + ifeq ($(CONFIG_SCHED_WAITPID),y) CSRCS += sched_waitpid.c ifeq ($(CONFIG_SCHED_HAVE_PARENT),y) diff --git a/sched/sched/sched.h b/sched/sched/sched.h index ecdc74a4ecd..61ff5fd64b6 100644 --- a/sched/sched/sched.h +++ b/sched/sched/sched.h @@ -422,9 +422,12 @@ void sched_sporadic_lowpriority(FAR struct tcb_s *tcb); #endif #ifdef CONFIG_SMP +int sched_cpu_select(void); # define sched_islocked(tcb) spin_islocked(g_cpu_schedlock) #else # define sched_islocked(tcb) ((tcb)->lockcount > 0) +# define sched_cpu_select (0) + #endif /* CPU load measurement support */ diff --git a/sched/sched/sched_addreadytorun.c b/sched/sched/sched_addreadytorun.c index d80ab456cbd..2157e01ea08 100644 --- a/sched/sched/sched_addreadytorun.c +++ b/sched/sched/sched_addreadytorun.c @@ -90,8 +90,7 @@ bool sched_addreadytorun(FAR struct tcb_s *btcb) * also disabled. */ - if ((rtcb->lockcount > 0 || rtcb->irqcount > 0) && - rtcb->sched_priority < btcb->sched_priority) + if (rtcb->lockcount > 0 && rtcb->sched_priority < btcb->sched_priority) { /* Yes. Preemption would occur! Add the new ready-to-run task to the * g_pendingtasks task list for now. @@ -184,52 +183,20 @@ bool sched_addreadytorun(FAR struct tcb_s *btcb) /* Yes.. that that is the CPU we must use */ cpu = btcb->cpu; - rtcb = (FAR struct tcb_s *)g_assignedtasks[cpu].head; } else { - uint8_t minprio; - int i; - /* Otherwise, find the CPU that is executing the lowest priority task * (possibly its IDLE task). */ - rtcb = NULL; - minprio = SCHED_PRIORITY_MAX; - cpu = 0; - - for (i = 0; i < CONFIG_SMP_NCPUS; i++) - { - FAR struct tcb_s *candidate = - (FAR struct tcb_s *)g_assignedtasks[i].head; - - /* If this thread is executing its IDLE task, the use it. The - * IDLE task is always the last task in the assigned task list. - */ - - if (candidate->flink == NULL) - { - /* The IDLE task should always be assigned to this CPU and - * have a priority of zero. - */ - - DEBUGASSERT(candidate->sched_priority == 0); - - rtcb = candidate; - cpu = i; - break; - } - else if (candidate->sched_priority < minprio) - { - DEBUGASSERT(candidate->sched_priority > 0); - - rtcb = candidate; - cpu = i; - } - } + cpu = sched_cpu_select(); } + /* Get the task currently running on the CPU (maybe the IDLE task) */ + + rtcb = (FAR struct tcb_s *)g_assignedtasks[cpu].head; + /* Determine the desired new task state. First, if the new task priority * is higher then the priority of the lowest priority, running task, then * the new task will be running and a context switch switch will be required. diff --git a/sched/sched/sched_cpuselect.c b/sched/sched/sched_cpuselect.c new file mode 100644 index 00000000000..2ca906ff389 --- /dev/null +++ b/sched/sched/sched_cpuselect.c @@ -0,0 +1,110 @@ +/**************************************************************************** + * sched/sched/sched_cpuselect.c + * + * Copyright (C) 2016 Gregory Nutt. All rights reserved. + * Author: Gregory Nutt + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * 3. Neither the name NuttX nor the names of its contributors may be + * used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE + * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, + * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS + * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED + * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN + * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include + +#include +#include + +#include + +#include "sched/sched.h" + +#ifdef CONFIG_SMP + +/**************************************************************************** + * Public Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: sched_cpu_select + * + * Description: + * Return the index to the CPU with the lowest priority running task, + * possbily its IDLE task. + * + * Inputs: + * None + * + * Return Value: + * Index of the CPU with the lowest priority running task + * + ****************************************************************************/ + +static int sched_cpu_select(void) +{ + uint8_t minprio; + int cpu; + int i; + + /* Otherwise, find the CPU that is executing the lowest priority task + * (possibly its IDLE task). + */ + + minprio = SCHED_PRIORITY_MAX; + cpu = 0; + + for (i = 0; i < CONFIG_SMP_NCPUS; i++) + { + FAR struct tcb_s *rtcb = (FAR struct tcb_s *)g_assignedtasks[i].head; + + /* If this thread is executing its IDLE task, the use it. The IDLE + * task is always the last task in the assigned task list. + */ + + if (rtcb->flink == NULL) + { + /* The IDLE task should always be assigned to this CPU and have + * a priority of zero. + */ + + DEBUGASSERT(rtcb->sched_priority == 0); + return i; + } + else if (rtcb->sched_priority < minprio) + { + DEBUGASSERT(rtcb->sched_priority > 0); + cpu = i; + } + } + + return cpu; +} + +#endif /* CONFIG_SMP */ diff --git a/sched/sched/sched_reprioritize.c b/sched/sched/sched_reprioritize.c index b9bff17f36a..d9431cf165e 100644 --- a/sched/sched/sched_reprioritize.c +++ b/sched/sched/sched_reprioritize.c @@ -47,30 +47,6 @@ #ifdef CONFIG_PRIORITY_INHERITANCE -/**************************************************************************** - * Pre-processor Definitions - ****************************************************************************/ - -/**************************************************************************** - * Private Type Declarations - ****************************************************************************/ - -/**************************************************************************** - * Public Data - ****************************************************************************/ - -/**************************************************************************** - * Private Variables - ****************************************************************************/ - -/**************************************************************************** - * Private Function Prototypes - ****************************************************************************/ - -/**************************************************************************** - * Private Functions - ****************************************************************************/ - /**************************************************************************** * Public Functions ****************************************************************************/ diff --git a/sched/sched/sched_setpriority.c b/sched/sched/sched_setpriority.c index 275de2426ef..2661edcf710 100644 --- a/sched/sched/sched_setpriority.c +++ b/sched/sched/sched_setpriority.c @@ -48,6 +48,197 @@ #include "sched/sched.h" +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: sched_running_setpriority + * + * Description: + * This function sets the priority of a running task. This does nothing + * if we are increasing the priority of a running task. If we are dropping + * the priority of a running task, then this could cause then next lower + * priority task to run, + * + * NOTE: Setting a task's priority to the same value has a similar effect + * to sched_yield() -- The task will be moved to after all other tasks + * with the same priority. + * + * Inputs: + * tcb - the TCB of task to reprioritize. + * sched_priority - The new task priority + * + * Return Value: + * None + * + ****************************************************************************/ + +static inline void sched_running_setpriority(FAR struct tcb_s *tcb, + int sched_priority) +{ + /* A context switch will occur if the new priority of the running + * task becomes less than OR EQUAL TO the next highest priority + * ready to run task. + */ + + if (sched_priority <= tcb->flink->sched_priority) + { + /* A context switch will occur. */ + + up_reprioritize_rtr(tcb, (uint8_t)sched_priority); + } + + /* Otherwise, we can just change priority since it has no effect */ + + else + { + /* Change the task priority */ + + tcb->sched_priority = (uint8_t)sched_priority; + } +} + +/**************************************************************************** + * Name: sched_readytorun_setpriority + * + * Description: + * This function sets the priority of a ready-to-run task. This may alter + * the position of the task in the ready-to-run list and if the priority + * is increased, may cause the task to become running. + * + * Inputs: + * tcb - the TCB of task to reprioritize. + * sched_priority - The new task priority + * + * Return Value: + * None + * + ****************************************************************************/ + +static void sched_readytorun_setpriority(FAR struct tcb_s *tcb, + int sched_priority) +{ + FAR struct tcb_s *rtcb; + +#ifdef CONFIG_SMP + int cpu; + + /* CASE 2a. The task is ready-to-run (but not running) but not assigned to + * a CPU. An increase in priority could cause a context switch may be caused + * by the re-prioritization. The task is not assigned and may run on any CPU. + */ + + if (tcb->task_state == TSTATE_TASK_READYTORUN) + { + cpu = sched_cpu_select(); + } + + /* CASE 2b. The task is ready to run, and assigned to a CPU. An increase + * in priority could cause this task to become running but the task can + * only run on its assigned CPU. + */ + + else + { + cpu = tcb->cpu; + } + + /* The running task is the the task at the head of the g_assignedtasks[] + * associated with the selected CPU. + */ + + rtcb = current_task(cpu); + +#else + /* CASE 2. The task is ready-to-run (but not running) and a context switch + * may be caused by the re-prioritization. + */ + + rtcb = this_task(); +#endif + + /* A context switch will occur if the new priority of the ready-to-run + * task is (strictly) greater than the current running task + */ + + if (sched_priority > rtcb->sched_priority) + { + /* A context switch will occur. */ + + up_reprioritize_rtr(tcb, (uint8_t)sched_priority); + } + + /* Otherwise, we can just change priority and re-schedule (since it have + * no other effect). + */ + + else + { + /* Remove the TCB from the ready-to-run task list that it resides in */ + + ASSERT(!sched_removereadytorun(tcb)); + + /* Change the task priority */ + + tcb->sched_priority = (uint8_t)sched_priority; + + /* Put it back into the correct ready-to-run task list */ + + DEBUGASSERT(!sched_addreadytorun(tcb)); + } +} + +/**************************************************************************** + * Name: sched_blocked_setpriority + * + * Description: + * Change the priority of a blocked tasks. The only issue here is that + * the task may like in a prioritized or an non-prioritized queue. + * + * Inputs: + * tcb - the TCB of task to reprioritize. + * sched_priority - The new task priority + * + * Return Value: + * None + * + ****************************************************************************/ + +static inline void sched_blocked_setpriority(FAR struct tcb_s *tcb, + int sched_priority) +{ + FAR dq_queue_t *tasklist; + tstate_t task_state = tcb->task_state; + + /* CASE 3a. The task resides in a prioritized list. */ + + tasklist = TLIST_BLOCKED(task_state); + if (TLIST_ISPRIORITIZED(task_state)) + { + /* Remove the TCB from the prioritized task list */ + + dq_rem((FAR dq_entry_t *)tcb, tasklist); + + /* Change the task priority */ + + tcb->sched_priority = (uint8_t)sched_priority; + + /* Put it back into the prioritized list at the correct position. */ + + sched_addprioritized(tcb, tasklist); + } + + /* CASE 3b. The task resides in a non-prioritized list. */ + + else + { + /* Just change the task's priority */ + + tcb->sched_priority = (uint8_t)sched_priority; + } +} + /**************************************************************************** * Public Functions ****************************************************************************/ @@ -59,7 +250,7 @@ * This function sets the priority of a specified task. * * NOTE: Setting a task's priority to the same value has a similar effect - * to sched_yield() -- The task will be moved to after all other tasks + * to sched_yield() -- The task will be moved to after all other tasks * with the same priority. * * Inputs: @@ -81,9 +272,6 @@ int sched_setpriority(FAR struct tcb_s *tcb, int sched_priority) { - FAR struct tcb_s *rtcb = this_task(); - FAR dq_queue_t *tasklist; - tstate_t task_state; irqstate_t flags; /* Verify that the requested priority is in the valid range */ @@ -101,37 +289,16 @@ int sched_setpriority(FAR struct tcb_s *tcb, int sched_priority) flags = enter_critical_section(); - /* There are four cases that must be considered: */ + /* There are three major cases (and two sub-cases) that must be considered: */ - task_state = tcb->task_state; - switch (task_state) + switch (tcb->task_state) { - /* CASE 1. The task is and a context switch may be caused by the - * re-prioritization + /* CASE 1. The task is running and a context switch may be caused by + * the re-prioritization */ case TSTATE_TASK_RUNNING: - - /* A context switch will occur if the new priority of the running - * task becomes less than OR EQUAL TO the next highest priority - * ready to run task. - */ - - if (sched_priority <= tcb->flink->sched_priority) - { - /* A context switch will occur. */ - - up_reprioritize_rtr(tcb, (uint8_t)sched_priority); - } - - /* Otherwise, we can just change priority since it has no effect */ - - else - { - /* Change the task priority */ - - tcb->sched_priority = (uint8_t)sched_priority; - } + sched_running_setpriority(tcb, sched_priority); break; /* CASE 2. The task is ready-to-run (but not running) and a context @@ -142,72 +309,16 @@ int sched_setpriority(FAR struct tcb_s *tcb, int sched_priority) #ifdef CONFIG_SMP case TSTATE_TASK_ASSIGNED: #endif - - /* A context switch will occur if the new priority of the ready-to - * run task is (strictly) greater than the current running task - */ - - if (sched_priority > rtcb->sched_priority) - { - /* A context switch will occur. */ - - up_reprioritize_rtr(tcb, (uint8_t)sched_priority); - } - - /* Otherwise, we can just change priority and re-schedule (since it - * have no other effect). - */ - - else - { - /* Remove the TCB from the ready-to-run task list */ - - ASSERT(!sched_removereadytorun(tcb)); - - /* Change the task priority */ - - tcb->sched_priority = (uint8_t)sched_priority; - - /* Put it back into the ready-to-run task list */ - - DEBUGASSERT(!sched_addreadytorun(tcb)); - } + sched_readytorun_setpriority(tcb, sched_priority); break; + /* CASE 3. The task is not in the ready to run list. Changing its * Priority cannot effect the currently executing task. */ default: - - /* CASE 3a. The task resides in a prioritized list. */ - - tasklist = TLIST_BLOCKED(task_state); - if (TLIST_ISPRIORITIZED(task_state)) - { - /* Remove the TCB from the prioritized task list */ - - dq_rem((FAR dq_entry_t *)tcb, tasklist); - - /* Change the task priority */ - - tcb->sched_priority = (uint8_t)sched_priority; - - /* Put it back into the prioritized list at the correct - * position - */ - - sched_addprioritized(tcb, tasklist); - } - - /* CASE 3b. The task resides in a non-prioritized list. */ - - else - { - /* Just change the task's priority */ - - tcb->sched_priority = (uint8_t)sched_priority; - } + sched_blocked_setpriority(tcb, sched_priority); break; } diff --git a/sched/sched/sched_unlock.c b/sched/sched/sched_unlock.c index f8d7f79f715..2c12f98f4db 100644 --- a/sched/sched/sched_unlock.c +++ b/sched/sched/sched_unlock.c @@ -116,9 +116,9 @@ int sched_unlock(void) */ #ifdef CONFIG_SMP - if (g_pendingtasks.head != NULL) -#else if (g_pendingtasks.head != NULL && rtcb->irqcount <= 0) +#else + if (g_pendingtasks.head != NULL) #endif { up_release_pending(); diff --git a/sched/semaphore/spinlock.c b/sched/semaphore/spinlock.c index 2b6c56f052c..e97f4a012f7 100644 --- a/sched/semaphore/spinlock.c +++ b/sched/semaphore/spinlock.c @@ -120,7 +120,9 @@ void spin_lock(FAR volatile spinlock_t *lock) { while (up_testset(lock) == SP_LOCKED) { +#if 0 /* Would recurse */ sched_yield(); +#endif } }