xiaoxiang781216 commented on code in PR #16673:
URL: https://github.com/apache/nuttx/pull/16673#discussion_r2250013681
##########
sched/sched/sched_removereadytorun.c:
##########
@@ -214,8 +184,7 @@ bool nxsched_remove_readytorun(FAR struct tcb_s *tcb)
tasklist = TLIST_HEAD(tcb, tcb->cpu);
/* The task is not running. Just remove its TCB from the task
- * list. In the SMP case this may be either the g_readytorun() or the
- * g_assignedtasks[cpu] list.
+ * list.
Review Comment:
merge to one line comment
##########
sched/sched/sched_removereadytorun.c:
##########
@@ -174,145 +175,44 @@ void nxsched_remove_running(FAR struct tcb_s *tcb)
*/
nxttcb = tcb->flink;
- DEBUGASSERT(nxttcb != NULL);
+ DEBUGASSERT(nxttcb != NULL && is_idle_task(nxttcb));
/* The task is running but the CPU that it was running on has been
* paused. We can now safely remove its TCB from the running
- * task list. In the SMP case this may be either the g_readytorun()
- * or the g_assignedtasks[cpu] list.
- */
-
- dq_rem_head((FAR dq_entry_t *)tcb, tasklist);
-
- /* Find the highest priority non-running tasks in the g_assignedtasks
- * list of other CPUs, and also non-idle tasks, place them in the
- * g_readytorun list. so as to find the task with the highest priority,
- * globally
- */
-
- for (int i = 0; i < CONFIG_SMP_NCPUS; i++)
- {
- if (i == cpu)
- {
- /* The highest priority task of the current
- * CPU has been found, which is nxttcb.
- */
-
- continue;
- }
-
- for (rtrtcb = (FAR struct tcb_s *)g_assignedtasks[i].head;
- !is_idle_task(rtrtcb); rtrtcb = rtrtcb->flink)
- {
- if (rtrtcb->task_state != TSTATE_TASK_RUNNING &&
- CPU_ISSET(cpu, &rtrtcb->affinity))
- {
- /* We have found the task with the highest priority whose
- * CPU index is i. Since this task must be between the two
- * tasks, we can use the dq_rem_mid macro to delete it.
- */
-
- dq_rem_mid(rtrtcb);
Review Comment:
let's create a new patch remove dq_rem_head, dq_rem_mid and
dq_addfirst_nonempty
##########
sched/sched/sched.h:
##########
@@ -132,6 +134,17 @@ struct tasklist_s
uint8_t attr; /* List attribute flags */
};
+/* This enumeration defines smp schedule task switch rule */
+
+enum task_deliver_e
+{
+ NOT_ASSIGNED = 0, /* No schedule switch pending */
Review Comment:
SWITCH_NONE
##########
sched/sched/sched.h:
##########
@@ -518,23 +532,53 @@ static inline_function bool nxsched_add_prioritized(FAR
struct tcb_s *tcb,
}
# ifdef CONFIG_SMP
+
+/* Try to switch the head of the ready-to-run list to active on "target_cpu".
+ * "cpu" is "this_cpu()", and passed only for optimization.
+ */
+
+static inline_function bool nxsched_deliver_task(
Review Comment:
```
static inline_function bool
nxsched_deliver_task(int cpu, int target_cpu,
enum task_deliver_e priority)
```
##########
sched/sched/sched_unlock.c:
##########
@@ -81,18 +83,22 @@ void sched_unlock(void)
sched_note_preemption(rtcb, false);
/* Release any ready-to-run tasks that have collected in
- * g_pendingtasks.
+ * g_pendingtasks (or in g_readytorun for SMP)
*
* NOTE: This operation has a very high likelihood of causing
* this task to be switched out!
*/
- if (list_pendingtasks()->head != NULL)
+#ifdef CONFIG_SMP
+ ptcb = (FAR struct tcb_s *)dq_peek(list_readytorun());
+ if (ptcb && ptcb->sched_priority > rtcb->sched_priority &&
+ nxsched_deliver_task(rtcb->cpu, rtcb->cpu, SWITCH_HIGHER))
Review Comment:
should we change the 2nd argument to ptcb->cpu
##########
sched/sched/sched.h:
##########
@@ -518,23 +532,53 @@ static inline_function bool nxsched_add_prioritized(FAR
struct tcb_s *tcb,
}
# ifdef CONFIG_SMP
+
+/* Try to switch the head of the ready-to-run list to active on "target_cpu".
+ * "cpu" is "this_cpu()", and passed only for optimization.
+ */
+
+static inline_function bool nxsched_deliver_task(
+ int cpu, int target_cpu,
+ enum task_deliver_e priority)
+{
+ bool ret = false;
+
+ /* If there is already a schedule interrupt pending, there is
+ * no need to do anything now.
+ */
+
+ if (g_delivertasks[target_cpu] != priority)
+ {
+ if (cpu == target_cpu)
+ {
+ ret = nxsched_switch_running(target_cpu, priority == SWITCH_EQUAL);
Review Comment:
target_cpu to cpu
##########
sched/sched/sched_removereadytorun.c:
##########
@@ -149,40 +148,10 @@ void nxsched_remove_running(FAR struct tcb_s *tcb)
tcb->task_state == TSTATE_TASK_RUNNING);
cpu = tcb->cpu;
- tasklist = &g_assignedtasks[cpu];
-
- /* Check if the TCB to be removed is at the head of a running list.
- * For the case of SMP, there are two lists involved: (1) the
- * g_readytorun list that holds non-running tasks that have not been
- * assigned to a CPU, and (2) and the g_assignedtasks[] lists which hold
- * tasks assigned a CPU, including the task that is currently running on
- * that CPU. Only this latter list contains the currently active task
- * only removing the head of that list can result in a context switch.
- *
- * tcb->blink == NULL will tell us if the TCB is at the head of the
- * running list and, hence, a candidate for the new running task.
- *
- * If so, then the tasklist RUNNABLE attribute will inform us if the list
- * holds the currently executing task and, hence, if a context switch
- * should occur.
- */
- DEBUGASSERT(tcb->blink == NULL);
- DEBUGASSERT(TLIST_ISRUNNABLE(tcb->task_state));
-
- /* There must always be at least one task in the list (the IDLE task)
- * after the TCB being removed.
- */
-
- nxttcb = tcb->flink;
- DEBUGASSERT(nxttcb != NULL && is_idle_task(nxttcb));
-
- /* The task is running but the CPU that it was running on has been
- * paused. We can now safely remove its TCB from the running
- * task list.
- */
+ /* Next task will be the idle task */
- dq_remfirst(tasklist);
+ nxttcb = &g_idletcb[cpu];
Review Comment:
why the next running task is always idle?
##########
sched/sched/sched_addreadytorun.c:
##########
@@ -114,170 +114,167 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
return ret;
}
-#endif /* !CONFIG_SMP */
+
+#else /* !CONFIG_SMP */
/****************************************************************************
- * Name: nxsched_add_readytorun
+ * Name: nxsched_switch_running
*
* Description:
- * This function adds a TCB to one of the ready to run lists. That might
- * be:
- *
- * 1. The g_readytorun list if the task is ready-to-run but not running
- * and not assigned to a CPU.
- * 2. The g_assignedtask[cpu] list if the task is running or if has been
- * assigned to a CPU.
- *
- * If the currently active task has preemption disabled and the new TCB
- * would cause this task to be preempted, the new task is added to the
- * g_pendingtasks list instead. The pending tasks will be made
- * ready-to-run when preemption isunlocked.
+ * This function switches the head of the current CPU's assigned tasks
+ * list to the TCB given as parameter. The idle task is not switched out.
+ * If the running task can't be swapped out, the btcb is pushed to
+ * the ready-to-run list.
*
* Input Parameters:
- * btcb - Points to the blocked TCB that is ready-to-run
+ * cpu - Always this_cpu(). Given as argument only for
+ * optimization
+ * switch_equal - When true, switch away a task of equal priority compared
+ * to the pending one
*
* Returned Value:
- * true if the currently active task (the head of the ready-to-run list)
- * has changed.
+ * true if the currently active task is switched to the btcb
*
* Assumptions:
- * - The caller has established a critical section before calling this
- * function (calling sched_lock() first is NOT a good idea -- use
- * enter_critical_section()).
+ * - The caller has established a critical section
* - The caller has already removed the input rtcb from whatever list it
* was in.
* - The caller handles the condition that occurs if the head of the
- * ready-to-run list has changed.
+ * assigned tasks list has changed.
*
****************************************************************************/
-#ifdef CONFIG_SMP
-bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
+bool nxsched_switch_running(int cpu, bool switch_equal)
{
- FAR struct tcb_s *rtcb;
- FAR struct tcb_s *headtcb;
- FAR dq_queue_t *tasklist;
- bool doswitch;
- int task_state;
- int cpu;
- int me;
-
- cpu = nxsched_select_cpu(btcb->affinity);
-
- /* Get the task currently running on the CPU (may be the IDLE task) */
+ FAR struct tcb_s *rtcb = current_task(cpu);
+ int sched_priority = rtcb->sched_priority;
+ FAR struct tcb_s *btcb;
+ bool ret = false;
- rtcb = current_task(cpu);
+ DEBUGASSERT(cpu == this_cpu());
- /* 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.
- */
-
- if (rtcb->sched_priority < btcb->sched_priority)
+ if (nxsched_islocked_tcb(rtcb) ||
+ (!is_idle_task(rtcb) && (rtcb->flags & TCB_FLAG_CPU_LOCKED) != 0))
{
- task_state = TSTATE_TASK_RUNNING;
+ return false;
}
- else
+
+ if (switch_equal)
{
- task_state = TSTATE_TASK_READYTORUN;
+ sched_priority--;
}
- /* If the selected state is TSTATE_TASK_RUNNING, then we would like to
- * start running the task. Be we cannot do that if pre-emption is
- * disabled. If the selected state is TSTATE_TASK_READYTORUN, then it
- * should also go to the pending task list so that it will have a chance
- * to be restarted when the scheduler is unlocked.
- *
- * There is an interaction here with IRQ locking. Even if the pre-
- * emption is enabled, tasks will be forced to pend if the IRQ lock
- * is also set UNLESS the CPU starting the thread is also the holder of
- * the IRQ lock. irq_cpu_locked() performs an atomic check for that
- * situation.
+ /* If there is a task in readytorun list, which is eglible to run on this
+ * CPU, and has higher priority than the current task,
+ * switch the current task to that one.
*/
- if (nxsched_islocked_tcb(this_task()))
+ for (btcb = (FAR struct tcb_s *)dq_peek(list_readytorun());
+ btcb && btcb->sched_priority > sched_priority;
+ btcb = btcb->flink)
{
- /* Add the new ready-to-run task to the g_pendingtasks task list for
- * now.
+ /* Check if the task found in ready-to-run list is allowed to run on
+ * this CPU
*/
- nxsched_add_prioritized(btcb, list_pendingtasks());
- btcb->task_state = TSTATE_TASK_PENDING;
- doswitch = false;
- }
- else if (task_state == TSTATE_TASK_READYTORUN)
- {
- /* The new btcb was added either (1) in the middle of the assigned
- * task list (the btcb->cpu field is already valid) or (2) was
- * added to the ready-to-run list (the btcb->cpu field does not
- * matter). Either way, it won't be running.
- *
- * Add the task to the ready-to-run (but not running) task list
- */
+ if (CPU_ISSET(cpu, &btcb->affinity))
+ {
+ FAR dq_queue_t *tasklist = list_assignedtasks(cpu);
- nxsched_add_prioritized(btcb, list_readytorun());
+ /* Found a task, remove it from ready-to-run list */
- btcb->task_state = TSTATE_TASK_READYTORUN;
- doswitch = false;
- }
- else /* (task_state == TSTATE_TASK_RUNNING) */
- {
- /* If we are modifying some assigned task list other than our own, we
- * will need to switch that CPU.
- */
+ dq_rem((FAR struct dq_entry_s *)btcb, list_readytorun());
- me = this_cpu();
- if (cpu != me)
- {
- if (g_delivertasks[cpu] == NULL)
+ /* Remove the current task from assigned tasks list and put it
+ * to the ready-to-run. But leave idle task.
+ */
+
+ if (!is_idle_task(rtcb))
{
- g_delivertasks[cpu] = btcb;
- btcb->cpu = cpu;
- btcb->task_state = TSTATE_TASK_ASSIGNED;
- up_send_smp_sched(cpu);
+ dq_remfirst(tasklist);
+ rtcb->task_state = TSTATE_TASK_READYTORUN;
+ nxsched_add_prioritized(rtcb, list_readytorun());
+
+ /* We should now have only the idle task assigned */
+
+ DEBUGASSERT(
+ is_idle_task((FAR struct tcb_s *)dq_peek(tasklist)));
}
else
{
- rtcb = g_delivertasks[cpu];
- if (rtcb->sched_priority < btcb->sched_priority)
- {
- g_delivertasks[cpu] = btcb;
- btcb->cpu = cpu;
- btcb->task_state = TSTATE_TASK_ASSIGNED;
- nxsched_add_prioritized(rtcb, &g_readytorun);
- rtcb->task_state = TSTATE_TASK_READYTORUN;
- }
- else
- {
- nxsched_add_prioritized(btcb, &g_readytorun);
- btcb->task_state = TSTATE_TASK_READYTORUN;
- }
+ rtcb->task_state = TSTATE_TASK_ASSIGNED;
}
- return false;
+ dq_addfirst((FAR dq_entry_t *)btcb, tasklist);
+ up_update_task(btcb);
+
+ btcb->cpu = cpu;
+ btcb->task_state = TSTATE_TASK_RUNNING;
+ ret = true;
+ break;
}
+ }
+
+ return ret;
+}
- tasklist = &g_assignedtasks[cpu];
+/****************************************************************************
+ * Name: nxsched_add_readytorun
+ *
+ * Description:
+ * This function adds a TCB to one of the ready to run lists. The list
+ * will be:
+ *
+ * 1. The g_readytorun list if the task is ready-to-run but not running
+ * and not assigned to a CPU.
+ * 2. The g_assignedtask[cpu] list if the task is running or if has been
+ * assigned to a CPU.
+ *
+ * If the currently active task has preemption disabled and the new TCB
+ * would cause this task to be preempted, the new task is added to the
+ * g_pendingtasks list instead. The pending tasks will be made
+ * ready-to-run when preemption isunlocked.
+ *
+ * Input Parameters:
+ * btcb - Points to the blocked TCB that is ready-to-run
+ *
+ * Returned Value:
+ * true if the currently active task (the head of the ready-to-run list)
+ * has changed.
+ *
+ * Assumptions:
+ * - The caller has established a critical section before calling this
+ * function (calling sched_lock() first is NOT a good idea -- use
+ * enter_critical_section()).
+ * - The caller has already removed the input rtcb from whatever list it
+ * was in.
+ * - The caller handles the condition that occurs if the head of the
+ * ready-to-run list has changed.
+ *
+ ****************************************************************************/
- /* Change "head" from TSTATE_TASK_RUNNING to TSTATE_TASK_ASSIGNED */
+bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
+{
+ bool doswitch = false;
- headtcb = (FAR struct tcb_s *)tasklist->head;
- DEBUGASSERT(headtcb->task_state == TSTATE_TASK_RUNNING);
- headtcb->task_state = TSTATE_TASK_ASSIGNED;
+ /* Add the btcb to the ready to run list, and
+ * try to run it on some CPU if it was added to the head of the list
+ */
- /* Add btcb to the head of the g_assignedtasks
- * task list and mark it as running
- */
+ btcb->task_state = TSTATE_TASK_READYTORUN;
- dq_addfirst_nonempty((FAR dq_entry_t *)btcb, tasklist);
- up_update_task(btcb);
+ nxsched_add_prioritized(btcb, list_readytorun());
- DEBUGASSERT(task_state == TSTATE_TASK_RUNNING);
- btcb->cpu = cpu;
- btcb->task_state = TSTATE_TASK_RUNNING;
+ int target_cpu = nxsched_select_cpu(btcb->affinity);
+ if (target_cpu < CONFIG_SMP_NCPUS)
+ {
+ FAR struct tcb_s *tcb = current_task(target_cpu);
- doswitch = true;
+ if (tcb->sched_priority < btcb->sched_priority)
+ {
+ doswitch = nxsched_deliver_task(this_cpu(), target_cpu,
+ SWITCH_HIGHER);
Review Comment:
align
##########
sched/sched/sched_process_delivered.c:
##########
@@ -62,83 +62,46 @@
void nxsched_process_delivered(int cpu)
{
- FAR dq_queue_t *tasklist;
- FAR struct tcb_s *next;
- FAR struct tcb_s *prev;
- struct tcb_s *btcb = NULL;
- struct tcb_s *tcb;
-
DEBUGASSERT(g_cpu_nestcount[cpu] == 0);
DEBUGASSERT(up_interrupt_context());
+ enum task_deliver_e priority;
Review Comment:
move before line 65
##########
sched/sched/sched_addreadytorun.c:
##########
@@ -114,170 +114,167 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
return ret;
}
-#endif /* !CONFIG_SMP */
+
+#else /* !CONFIG_SMP */
/****************************************************************************
- * Name: nxsched_add_readytorun
+ * Name: nxsched_switch_running
*
* Description:
- * This function adds a TCB to one of the ready to run lists. That might
- * be:
- *
- * 1. The g_readytorun list if the task is ready-to-run but not running
- * and not assigned to a CPU.
- * 2. The g_assignedtask[cpu] list if the task is running or if has been
- * assigned to a CPU.
- *
- * If the currently active task has preemption disabled and the new TCB
- * would cause this task to be preempted, the new task is added to the
- * g_pendingtasks list instead. The pending tasks will be made
- * ready-to-run when preemption isunlocked.
+ * This function switches the head of the current CPU's assigned tasks
+ * list to the TCB given as parameter. The idle task is not switched out.
+ * If the running task can't be swapped out, the btcb is pushed to
+ * the ready-to-run list.
*
* Input Parameters:
- * btcb - Points to the blocked TCB that is ready-to-run
+ * cpu - Always this_cpu(). Given as argument only for
+ * optimization
+ * switch_equal - When true, switch away a task of equal priority compared
+ * to the pending one
*
* Returned Value:
- * true if the currently active task (the head of the ready-to-run list)
- * has changed.
+ * true if the currently active task is switched to the btcb
Review Comment:
but no btcb anymore
--
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-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
