Skip to content

[kernel][scheduler] fix: keep FIFO order for same-priority threads - #11907

Open
Yunlk wants to merge 2 commits into
RT-Thread:masterfrom
Yunlk:fix/8459-ready-list-order
Open

[kernel][scheduler] fix: keep FIFO order for same-priority threads#11907
Yunlk wants to merge 2 commits into
RT-Thread:masterfrom
Yunlk:fix/8459-ready-list-order

Conversation

@Yunlk

@Yunlk Yunlk commented Oct 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

拉取/合并请求描述：(PR description)

[

为什么提交这份PR (why to submit this PR)

关联 issue #8459（我已认领）。

rt_sched_insert_thread() 对任何没有 RT_THREAD_STAT_YIELD 标志的线程都插到 ready list 队首。而从 IPC 挂起队列恢复的线程不带这个标志——scheduler_up.c 在每次切走线程时就把 YIELD 位清掉了——于是同一批被唤醒的同优先级线程，执行顺序被反转：

  • A、B 依次阻塞到同一个信号量上，pending 队列是 A -> B
  • 先后两次 release 唤醒 A 和 B
  • ready list 变成 B -> A，结果 B 先执行

新启动（rt_thread_startup）的线程同样被插到队首，所以"先启动的先运行"也不成立。

QEMU vexpress-a9 实测（RT-Thread 5.4.0，同一 .config、同一 QEMU 参数）：

修复前：semaphore wakeup order = "BA"（期望 "AB"），startup order = "DC"（期望 "CD"）
修复后：semaphore wakeup order = "AB"，startup order = "CD"

你的解决方案是什么 (what is your solution)

在覆盖 stat 之前先把原值存下来，只有"仍在运行、且时间片未用尽被抢占"的线程保留队首插入——这正是原注释所描述的场景；其余情况（时间片用尽、从 IPC 挂起队列恢复、新启动）一律追加到队尾，使同优先级线程按就绪先后执行。

单核 scheduler_up.c 与多核 scheduler_mp.c 的两处插入分支做了一致修改。

新增 utest core.scheduler_ready_order，覆盖信号量唤醒路径与启动路径。SMP 下把工作线程绑定到测试线程所在核，否则它们会被空闲核立刻取走，插入顺序无法观测。

请提供验证的bsp和config (provide the config and bsp)

  • BSP: bsp/qemu-vexpress-a9
  • .config: CONFIG_RT_USING_UTEST=y, CONFIG_RT_USING_UTESTCASES=y, CONFIG_RT_UTEST_SCHEDULER=y, CONFIG_RT_USING_CI_ACTION=y, CONFIG_UTEST_THR_STACK_SIZE=8192
  • action: 本地 QEMU 运行（QEMU 11.0.0 + xpack arm-none-eabi-gcc 15.2.1），未使用 GitHub Actions

调度组用例全量对照：

用例 修复前 修复后
core.scheduler_ready_order FAILED（BA / DC） PASSED（AB / CD）
core.scheduler_mutex PASSED PASSED
core.scheduler_sem PASSED PASSED
core.scheduler_thread PASSED PASSED
core.scheduler_timed_mtx PASSED PASSED
core.scheduler_timed_sem PASSED PASSED
core.scheduler_timeout_race PASSED PASSED
汇总 6 passed / 1 failed 7 passed / 0 failed

关于 scheduler_timed_mtx / scheduler_timed_sem 打印的 Total failed times: N(in 1000)：这是用例内部的计时竞争计数，两次运行分别是 271 / 213 与 1 / 0，用例本身两次都判定 PASSED，属于运行间波动，不是本次改动引入的回归。

需要讨论的一点

这个改动会改变 IPC 唤醒之后的调度顺序，属于语义变更。issue #8459 里我提了两种取舍（把 RT_THREAD_STAT_YIELD 语义收窄为"仅时间片未用尽被抢占"，还是维持现状并只把同优先级 FIFO 的保证范围写进文档），目前还没有维护者结论。

所以在正文里把这一点明确写出来，希望能先拿到结论，而不是让这个改动悄悄合进去。如果认为只应保证"获取信号量的顺序"而不保证"执行顺序"，可以只合入 utest 部分，并把断言改为记录观测值（或在用例里直接断言当前行为），内核代码不动。

]

当前拉取/合并请求的状态 Intent for your PR

必须选择一项 Choose one (Mandatory):

  • 本拉取/合并请求是一个草稿版本 This PR is for a code-review and is intended to get feedback
  • 本拉取/合并请求是一个成熟版本 This PR is mature, and ready to be integrated into the repo

代码质量 Code Quality：

我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:

  • 已经仔细查看过代码改动的对比 Already check the difference between PR and old code
  • 代码风格正确，包括缩进空格，命名及其他风格 Style guide is adhered to, including spacing, naming and other styles
  • 没有垃圾代码，代码尽量精简，不包含#if 0代码，不包含已经被注释了的代码 All redundant code is removed and cleaned up
  • 所有变更均有原因及合理的，并且不会影响到其他软件组件代码或BSP All modifications are justified and not affect other components or BSP
  • 对难懂代码均提供对应的注释 I've commented appropriately where code is tricky
  • 代码是高质量的 Code in this PR is of high quality
  • 已经使用clang-format 源码格式化工具确保格式符合RT-Thread代码规范 This PR has been formatted with clang-format and complies with RT-Thread code specification
  • 如果是新增bsp, 已经添加ci检查到.github/ALL_BSP_COMPILE.json 详细请参考链接BSP自查

@Yunlk
[kernel][scheduler] fix: keep FIFO order for same-priority threads
b2f817e 
rt_sched_insert_thread() inserted every thread without
RT_THREAD_STAT_YIELD at the head of the ready list. A thread resumed from
an IPC pending list never carries that flag, because the scheduler clears
it when switching away from the thread (see scheduler_up.c). Waking several
same-priority threads in pending order therefore reversed their execution
order, and freshly started threads had the same problem.

Only a thread that was preempted while it was still running keeps the head
insertion now, which is the case the original code comment described. Every
other case (time slice consumed, resumed from an IPC pending list, freshly
started) is appended to the tail, so same-priority threads run in the order
in which they became ready.

The single core path and both multi-core insertion paths are updated
consistently.

Add utest core.scheduler_ready_order covering the semaphore wakeup path and
the startup path. On SMP the worker threads are bound to the CPU running the
test case so that the insertion order stays observable.

Refs RT-Thread#8459
@github-actions

github-actions Bot commented Oct 20, 2026

Copy link
Copy Markdown

👋 感谢您对 RT-Thread 的贡献！Thank you for your contribution to RT-Thread!

为确保代码符合 RT-Thread 的编码规范，请在你的仓库中执行以下步骤运行代码格式化工作流（如果格式化CI运行失败）。
To ensure your code complies with RT-Thread's coding style, please run the code formatting workflow by following the steps below (If the formatting of CI fails to run).

🛠 操作步骤 | Steps

  1. 前往 Actions 页面 | Go to the Actions page
    点击进入工作流 → | Click to open workflow →

  2. 点击 Run workflow | Click Run workflow

  • Use workflow from 保持默认分支（通常为 master）
    Keep the default branch (usually master) in Use workflow from
  • 在 branch 输入框填写 PR 分支 fix/8459-ready-list-order
    Enter PR branch fix/8459-ready-list-order in the branch field
  • 设置需排除的文件/目录（目录请以"/"结尾）
    Set files/directories to exclude (directories should end with "/")
  1. 等待工作流完成 | Wait for the workflow to complete
    格式化后的代码将作为独立提交推送至你的分支。
    The formatting changes will be pushed to your branch as a separate commit.

完成后，提交将自动更新至 fix/8459-ready-list-order 分支，关联的 Pull Request 也会同步更新。
Once completed, commits will be pushed to the fix/8459-ready-list-order branch automatically, and the related Pull Request will be updated.

如有问题欢迎联系我们，再次感谢您的贡献！💐
If you have any questions, feel free to reach out. Thanks again for your contribution!

@github-actions github-actions Bot added the Kernel PR has src relate code label Oct 20, 2026
@github-actions

github-actions Bot commented Oct 20, 2026

Copy link
Copy Markdown

📌 Code Review Assignment

🏷️ Tag: kernel

Reviewers: @GorrayLi @ReviewSun @hamburger-os @lianux-mm @wdfk-prog @xu18838022837

Changed Files (Click to expand)
  • src/scheduler_mp.c
  • src/scheduler_up.c
  • src/utest/SConscript
  • src/utest/sched_ready_order_tc.c

📊 Current Review Status (Last Updated: 2026-10-21 00:06 CST)

📝 Review Instructions

  1. 维护者可以通过单击此处来刷新审查状态: 🔄 刷新状态
    Maintainers can refresh the review status by clicking here: 🔄 Refresh Status

  2. 确认审核通过后评论 LGTM/lgtm
    Comment LGTM/lgtm after confirming approval

  3. PR合并前需至少一位维护者确认
    PR must be confirmed by at least one maintainer before merging

ℹ️ 刷新CI状态操作需要具备仓库写入权限。
ℹ️ Refresh CI status operation requires repository Write permission.

@CLAassistant

CLAassistant commented Oct 20, 2026 •
edited
Loading

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@Yunlk
Yunlk marked this pull request as ready for review October 20, 2026 17:32
@Yunlk

Yunlk commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

@Rbb666 这两个 workflow 仍处于 Waiting for approval（首次从 fork 贡献），需要维护者批准才会运行：

  • utest_auto_run —— 本 PR 新增的 core.scheduler_ready_order 就在这套用例里
  • RT-Thread BSP Static Build Check

本地已在 QEMU vexpress-a9 跑过完整调度组：修复前 6 passed / 1 failed，修复后 7 passed / 0 failed（对照数据在正文表格里）。批准后 CI 应该能印证同一结果。

另外，若对"同优先级线程是否必须保证执行顺序 FIFO"这个语义方向有不同意见，请直接说明，我可以只保留 utest 部分、内核代码不动。

@Yunlk

Yunlk commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

补充一项此前未覆盖的验证：时间片轮转是否被这次改动影响。

我原先只验证了"同优先级线程的唤醒顺序"，跑了既有 7 个调度用例，但没有专门测 tick 驱动的轮转——而这正是 rt_sched_insert_thread 的核心路径之一。

方法

QEMU vexpress-a9，3 个同优先级线程（priority 20），时间片均为 5 tick，线程体内是纯忙循环、不调用 rt_thread_yield()，运行 2 秒后统计各自循环计数。同一 .config、同一 QEMU 参数下，对 master 与本 PR 分支各跑一次。

结果

thread 0 thread 1 thread 2 max/min
本 PR（有修复） 34% 32% 32% 1.05x
master（基线） 32% 32% 35% 1.09x 
# 本 PR
[ROT] thread 0 : 132194612 loops (34%)
[ROT] thread 1 : 125453395 loops (32%)
[ROT] thread 2 : 125635222 loops (32%)
[ROT] total=383283229  min=125453395  max=132194612  max*100/min=105
[ROT] VERDICT: round-robin OK (all threads got a share)

# master
[ROT] thread 0 : 124247564 loops (32%)
[ROT] thread 1 : 126470877 loops (32%)
[ROT] thread 2 : 136556583 loops (35%)
[ROT] total=387275024  min=124247564  max=136556583  max*100/min=109
[ROT] VERDICT: round-robin OK (all threads got a share)

结论

三个线程各拿到约三分之一 CPU，修复前后差异在噪声范围内（1.05x vs 1.09x），轮转调度未被破坏。

机制上也对得上：时间片用尽时线程带 RT_THREAD_STAT_YIELD，本次改动仍让它进入队尾；只有"运行中且时间片未用尽被抢占"才保留队首插入。轮转走的正是 YIELD 分支，未被触及。

上面的验证脚本是临时写的，没有提交。如果需要把它固化成 utest（同优先级线程轮转公平性），我可以补。

@Yunlk Yunlk mentioned this pull request Oct 21, 2026
Open
8 of 9 tasks
@Yunlk

Yunlk commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

补一条旁证：#6645（2022-11 提交，标签 Status: waiting for test，至今 0 条评论）用的是同一个判别式。

c if ((thread->stat & RT_THREAD_STAT_MASK) == RT_THREAD_RUNNING) { if (thread->stat & RT_THREAD_STAT_YIELD_MASK) -> 队尾 else -> 队首 } else -> 队尾

也就是"只有运行中被抢占且时间片未用尽才插队首，其余（时间片用尽 / 挂起恢复 / 新启动）一律队尾"，与本 PR 完全一致。两边独立得出相同结论。

顺带说明本 PR 没有采纳它的另一部分改动：那个 patch 还把 rt_schedule() 里两处 stat &= ~RT_THREAD_STAT_YIELD_MASK 移到了 insert 末尾，并取消了 insert 开头的 stat 覆盖。本 PR 采用更小的改动（先存 stat_before 再判断，YIELD 清除时机不动），分支结果等价但影响面更小。若维护者认为 YIELD 生命周期本身就该调整，那属于另一个层面的改动，可以单独讨论。

详见 #6645 下的分析评论。

@Rbb666
Rbb666 requested a balanced review from Copilot October 21, 2026 09:30
Copilot AI reviewed Oct 21, 2026

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Kernel PR has src relate code

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Yunlk @CLAassistant