Skip to content

fix(scheduler): do not preempt a running task of equal priority - #725

Open
ytakano wants to merge 4 commits into
tier4:mainfrom
ytakano:fix_equal_priority_preemption
Open

ytakano wants to merge 4 commits into
tier4:mainfrom
ytakano:fix_equal_priority_preemption

Conversation

@ytakano

@ytakano ytakano commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Task::cmp breaks priority ties by task ID (larger ID is greater) so that
the preemption-pending heaps have a total order. The preemption decisions
reused that ordering via task > target_task, so a task woken with the
same priority as a running task preempted it whenever its ID was larger.
This violates the FIFO order within a priority level, bypasses the RR
time quantum, and costs a context switch with no scheduling benefit. It
is most visible with small CPU sets, where "all CPUs in the set are busy"
is the common case.

Add Task::preempts(), which requires a strictly higher priority, and
use it in every preemption decision: invoke_preemption*() of
PrioritizedFIFO, PrioritizedRR, GEDF and ClusteredEDF, and the check in
do_preemption() that re-queues a pending task when the running task on
the target CPU has changed. Task::cmp is left as is for the heaps.

The one exception is the PrioritizedRR time quantum:
invoke_preemption_tick() pushes the next runnable task to the
preemption-pending heap without a priority check, and do_preemption()
lets such an equal-priority pending task preempt a running PrioritizedRR
task so that the quantum keeps rotating tasks of the same priority. Every
wake path requires a strictly higher priority, so this is the only case
where equal priority preempts.

Related links

#726 will be resolved after this PR.

How was this PR tested?

  • scripts/ci.sh passes on my machine.
  • QEMU x86_64, q35 with 16 vCPUs (the make qemu-x86_64 machine), built with
    make x86_64 RELEASE=1 OPT="--release --features userland/<test>", serial console
    captured to a file for 120 s.
    • test_prioritized_rr: 48 tasks (24 at priority 1, 24 at priority 0) on 15 worker
      cores. In the last 30 s of the run, every one of the 24 priority-1 tasks logs
      291-297 times (one line per 100 ms), and no priority-0 task logs. With the first
      revision of this PR (before the RR exception in do_preemption()), all 24
      priority-1 tasks still appear, but 13 of them (no=23..47) log 299-300 times, i.e.
      they are never displaced, while no=1..21 get only 58-247: the time quantum did not
      rotate tasks, and the only rotation came from the buffered-logger task preempting
      the lowest-ID task.
    • test_prioritized_fifo: 15 low-priority (0) tasks start at t=4745-4758 ms and each
      spawns a high-priority (31) task 100 ms later. All 15 high-priority tasks start at
      4857-4859 ms and finish at 4957-4960 ms, and every low-priority task finishes after
      that (4962-5064 ms). High priority preempts low priority immediately; no
      equal-priority preemption is observed.

Notes for reviewers

ytakano and others added 2 commits October 6, 2026 18:59
`Task::cmp` breaks priority ties by task ID (larger ID is greater) so that
the preemption-pending heaps have a total order. The preemption decisions
reused that ordering via `task > target_task`, so a task woken with the
same priority as a running task preempted it whenever its ID was larger.
This violates the FIFO order within a priority level, bypasses the RR
time quantum, and costs a context switch with no scheduling benefit. It
is most visible with small CPU sets, where "all CPUs in the set are busy"
is the common case.

Add `Task::preempts()`, which requires a strictly higher priority, and
use it in every preemption decision: `invoke_preemption*()` of
PrioritizedFIFO, PrioritizedRR, GEDF and ClusteredEDF, and the check in
`do_preemption()` that re-queues a pending task when the running task on
the target CPU has changed. `Task::cmp` is left as is for the heaps.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@ytakano ytakano changed the title fix(preemption): Fix equal priority preemption fix(scheduler): do not preempt a running task of equal priority Oct 6, 2026
@ytakano
ytakano marked this pull request as ready for review October 7, 2026 09:23
@ytakano
ytakano requested a review from atsushi421 October 7, 2026 09:23

@atsushi421 atsushi421 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scripts/ci.sh cannot observe this change. The std feature enables no_preempt, which sets PREEMPTION_ENABLED to false and compiles out the IPI handler in preempt.rs. Please run test_prioritized_rr and test_prioritized_fifo on QEMU and note the result in the PR description. test_prioritized_rr expects every high-priority task to keep running, so it should show the quantum issue in the inline comment.

Comment thread awkernel_async_lib/src/task/preempt.rs Outdated
`PrioritizedRRScheduler::invoke_preemption_tick()` pushes the next
runnable task to the preemption-pending heap without a priority check,
and in RR that task normally has the same priority as the running one.
With the strict comparison introduced in 82f9b6a, `do_preemption()`
re-queued it every time, so the RR time quantum never switched tasks and
the tick re-sent the IPI on every primary loop iteration.

Let an equal-priority pending task preempt the running task when that
task is PrioritizedRR. Every wake path requires a strictly higher
priority, so this is the only case where equal priority preempts.
Document the exception in `Task::preempts()` and the mdbook.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@ytakano

ytakano commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

Please run test_prioritized_rr and test_prioritized_fifo on QEMU and note the result in the PR description.

Updated the description.
The log files are big (2MB >), I don't attach these files.

@ytakano
ytakano requested a review from atsushi421 October 16, 2026 01:59

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants