Modify the logic of utest-smp_thread_preemptions and add comments #11012
Conversation
Currently, the print information of this test case fails to demonstrate that the high-priority thread has preempted the low-priority thread. This is because when the high-priority thread prints the thread list, there is no information about the low-priority thread (tlow), as tlow has already completed execution and been destroyed. Therefore, the current execution logic cannot confirm the successful completion of the preemption operation. Solution: After the low-priority thread (tlow) releases the lock, add a busy-wait loop while(!finish_flag);. At this point, when the high-priority thread (thigh) prints the thread list information, tlow can be observed in the ready state, indicating that it has been preempted by thigh. Signed-off-by: Mengchen Teng <teng_mengchen@163.com>
Add comments for smp_thread_preemption. Signed-off-by: Mengchen Teng <teng_mengchen@163.com>
|
👋 感谢您对 RT-Thread 的贡献！Thank you for your contribution to RT-Thread! 为确保代码符合 RT-Thread 的编码规范，请在你的仓库中执行以下步骤运行代码格式化工作流（如果格式化CI运行失败）。 🛠 操作步骤 | Steps
完成后，提交将自动更新至 如有问题欢迎联系我们，再次感谢您的贡献！💐 |
📌 Code Review Assignment🏷️ Tag: kernelReviewers: @GorrayLi @ReviewSun @hamburger-os @lianux-mm @wdfk-prog @xu18838022837 Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2025-12-02 18:49 CST)
📝 Review Instructions
|
There was a problem hiding this comment.
Pull request overview
This PR modifies the SMP thread preemption unit test by adding a synchronization flag mechanism and comprehensive documentation to better demonstrate thread preemption behavior.
Key Changes
- Introduced a
finish_flagvariable to coordinate thread execution and prevent premature test completion - Added extensive documentation block describing test objectives, scenarios, verification metrics, dependencies, and expected results
- Modified thread behavior: low-priority thread now waits for high-priority thread to complete before exiting
| #define THREAD_STACK_SIZE UTEST_THR_STACK_SIZE | ||
|
|
||
| static rt_thread_t threads[2]; | ||
| static rt_uint8_t finish_flag = 0; |
There was a problem hiding this comment.
🔴 Critical Concurrency Issue / 严重的并发问题
English: The finish_flag variable is accessed by multiple threads without proper synchronization. The high-priority thread writes to it (line 62) and the low-priority thread reads it in a busy-wait loop (line 74) without any memory barriers or atomic operations. This can lead to:
- Race condition: On SMP systems, the write from one core may not be visible to another core reading the flag
- Compiler optimization: The compiler may optimize away the loop at line 74 since
finish_flagis not declared asvolatile - Potential deadlock: If the low-priority thread doesn't see the update, it will loop forever
Recommended Solution:
- Declare
finish_flagasvolatile rt_uint8_tto prevent compiler optimizations - OR use RT-Thread's atomic operations if available
- OR use an event/semaphore for proper synchronization
中文：finish_flag 变量被多个线程访问但没有适当的同步机制。高优先级线程写入它（第62行），低优先级线程在忙等待循环中读取它（第74行），但没有任何内存屏障或原子操作。这可能导致：
- 竞态条件：在 SMP 系统上，一个核心的写入可能对另一个核心读取该标志不可见
- 编译器优化：编译器可能会优化掉第74行的循环，因为
finish_flag未声明为volatile - 潜在死锁：如果低优先级线程看不到更新，它将永远循环
建议的解决方案：
- 将
finish_flag声明为volatile rt_uint8_t以防止编译器优化 - 或使用 RT-Thread 的原子操作（如果可用）
- 或使用事件/信号量进行适当的同步
Example / 示例:
static volatile rt_uint8_t finish_flag = 0;| static rt_uint8_t finish_flag = 0; | |
| static volatile rt_uint8_t finish_flag = 0; |
| extern long list_thread(void); | ||
| list_thread(); | ||
| rt_spin_unlock(&lock); | ||
| while(!finish_flag); |
There was a problem hiding this comment.
🟠 Performance Issue: Busy-Wait Loop / 性能问题：忙等待循环
English: The busy-wait loop while(!finish_flag); at line 74 continuously consumes CPU cycles without yielding, which is inefficient and can interfere with test execution on SMP systems. This is particularly problematic in an embedded RTOS context where CPU resources are precious.
Recommended Solution:
Use RT-Thread synchronization primitives instead of busy-waiting. For example:
- Use
rt_sem_torrt_event_tfor proper thread synchronization - This will allow the low-priority thread to block properly and yield CPU time
中文：第74行的忙等待循环 while(!finish_flag); 持续消耗 CPU 周期而不让出控制权，这是低效的，并且可能干扰 SMP 系统上的测试执行。在嵌入式 RTOS 环境中，CPU 资源宝贵，这尤其成问题。
建议的解决方案：
使用 RT-Thread 同步原语而不是忙等待。例如：
- 使用
rt_sem_t或rt_event_t进行正确的线程同步 - 这将允许低优先级线程正确阻塞并让出 CPU 时间
Example / 示例:
// Use semaphore instead
static rt_sem_t finish_sem;
// In init:
rt_sem_init(&finish_sem, "finish", 0, RT_IPC_FLAG_FIFO);
// In thread_high_entry:
rt_sem_release(&finish_sem);
// In thread_low_entry:
rt_sem_take(&finish_sem, RT_WAITING_FOREVER);| extern long list_thread(void); | ||
| list_thread(); | ||
| rt_spin_unlock(&lock); | ||
| while(!finish_flag); |
There was a problem hiding this comment.
🟡 Code Style Issue: Missing Space / 代码风格问题：缺少空格
English: According to RT-Thread coding standards, there should be a space after while keyword and before the opening parenthesis. The code has while(!finish_flag) but should be while (!finish_flag).
中文：根据 RT-Thread 编码规范，while 关键字后面和开括号前应该有一个空格。代码中是 while(!finish_flag)，应该是 while (!finish_flag)。
Example / 示例:
while (!finish_flag);| * - A high-priority thread thigh and a low-priority thread tlow are created. Since tlow is created | ||
| * - prior to thigh, it prints the thread list information first and then waits for the finish_flag | ||
| * - bit. Once thigh becomes ready, it preempts tlow, prints the thread list, and then sets the | ||
| * - finish_flag bit. After that, tlow exits the busy-wait loop. | ||
| * | ||
| * Verification Metrics: | ||
| * - First, the message "Low priority thread is running" will be displayed, followed immediately by | ||
| * - a thread_list information list. Subsequently, the message "High priority thread is running" will | ||
| * - appear, and in the thread_list that follows, the tlow thread can be observed in the ready state. | ||
| * | ||
| * Dependencies: | ||
| * - Enable RT_USING_SMP, set RT_THREAD_PRIORITY_MAX = 256. | ||
| * | ||
| * Expected Results: | ||
| * - You will see the message "[ PASSED ] [ result ] testcase (core.smp_thread_preemptions)", and observe | ||
| * - the tlow thread in the ready state in the thread_list printed by the thigh thread. |
There was a problem hiding this comment.
[nitpick] 🟡 Documentation Issue: Comment Formatting / 文档问题：注释格式
English: The multi-line comments starting with "- " on lines 27-30 and 33-35 are inconsistent with the rest of the documentation block. Standard documentation style would not have the leading "- " on continuation lines, or should consistently use bullet points throughout.
中文：第27-30行和第33-35行以 "- " 开头的多行注释与文档块的其余部分不一致。标准文档风格不应在续行上有前导 "- "，或应始终使用项目符号。
Suggested Format / 建议格式:
* Test Scenarios:
* A high-priority thread thigh and a low-priority thread tlow are created. Since tlow is created
* prior to thigh, it prints the thread list information first and then waits for the finish_flag
* bit. Once thigh becomes ready, it preempts tlow, prints the thread list, and then sets the
* finish_flag bit. After that, tlow exits the busy-wait loop.| * - A high-priority thread thigh and a low-priority thread tlow are created. Since tlow is created | |
| * - prior to thigh, it prints the thread list information first and then waits for the finish_flag | |
| * - bit. Once thigh becomes ready, it preempts tlow, prints the thread list, and then sets the | |
| * - finish_flag bit. After that, tlow exits the busy-wait loop. | |
| * | |
| * Verification Metrics: | |
| * - First, the message "Low priority thread is running" will be displayed, followed immediately by | |
| * - a thread_list information list. Subsequently, the message "High priority thread is running" will | |
| * - appear, and in the thread_list that follows, the tlow thread can be observed in the ready state. | |
| * | |
| * Dependencies: | |
| * - Enable RT_USING_SMP, set RT_THREAD_PRIORITY_MAX = 256. | |
| * | |
| * Expected Results: | |
| * - You will see the message "[ PASSED ] [ result ] testcase (core.smp_thread_preemptions)", and observe | |
| * - the tlow thread in the ready state in the thread_list printed by the thigh thread. | |
| * A high-priority thread thigh and a low-priority thread tlow are created. Since tlow is created | |
| * prior to thigh, it prints the thread list information first and then waits for the finish_flag | |
| * bit. Once thigh becomes ready, it preempts tlow, prints the thread list, and then sets the | |
| * finish_flag bit. After that, tlow exits the busy-wait loop. | |
| * | |
| * Verification Metrics: | |
| * First, the message "Low priority thread is running" will be displayed, followed immediately by | |
| * a thread_list information list. Subsequently, the message "High priority thread is running" will | |
| * appear, and in the thread_list that follows, the tlow thread can be observed in the ready state. | |
| * | |
| * Dependencies: | |
| * Enable RT_USING_SMP, set RT_THREAD_PRIORITY_MAX = 256. | |
| * | |
| * Expected Results: | |
| * You will see the message "[ PASSED ] [ result ] testcase (core.smp_thread_preemptions)", and observe | |
| * the tlow thread in the ready state in the thread_list printed by the thigh thread. |
| * | ||
| * Test Case Name: [smp_thread_preemptions] | ||
| * | ||
| * Test Objectives: | ||
| * - Test the thread preemption logic under the SMP architecture | ||
| * | ||
| * Test Scenarios: | ||
| * - A high-priority thread thigh and a low-priority thread tlow are created. Since tlow is created | ||
| * - prior to thigh, it prints the thread list information first and then waits for the finish_flag | ||
| * - bit. Once thigh becomes ready, it preempts tlow, prints the thread list, and then sets the | ||
| * - finish_flag bit. After that, tlow exits the busy-wait loop. | ||
| * | ||
| * Verification Metrics: | ||
| * - First, the message "Low priority thread is running" will be displayed, followed immediately by | ||
| * - a thread_list information list. Subsequently, the message "High priority thread is running" will | ||
| * - appear, and in the thread_list that follows, the tlow thread can be observed in the ready state. | ||
| * | ||
| * Dependencies: | ||
| * - Enable RT_USING_SMP, set RT_THREAD_PRIORITY_MAX = 256. | ||
| * | ||
| * Expected Results: | ||
| * - You will see the message "[ PASSED ] [ result ] testcase (core.smp_thread_preemptions)", and observe | ||
| * - the tlow thread in the ready state in the thread_list printed by the thigh thread. | ||
| */ |
There was a problem hiding this comment.
🟢 PR Title Format / PR 标题格式
English: The PR title should follow RT-Thread format: [module][subsystem] Description in lowercase.
Current title: "Modify the logic of utest-smp_thread_preemptions and add comments"
Based on the modified file src/utest/smp/smp_thread_preemption_tc.c, the suggested title format should be:
Suggested title: [utest][smp] Add synchronization flag to thread preemption test
or
[utest][smp] Improve thread preemption test with finish_flag and documentation
中文：PR 标题应遵循 RT-Thread 格式：小写的 [模块][子系统] 描述。
当前标题："Modify the logic of utest-smp_thread_preemptions and add comments"
基于修改的文件 src/utest/smp/smp_thread_preemption_tc.c，建议的标题格式应为：
建议标题：[utest][smp] Add synchronization flag to thread preemption test
或
[utest][smp] Improve thread preemption test with finish_flag and documentation
1 participant
拉取/合并请求描述：(PR description)
[
为什么提交这份PR (why to submit this PR)
Modify the logic of utest-smp_thread_preemptions and add comments
你的解决方案是什么 (what is your solution)
请提供验证的bsp和config (provide the config and bsp)
]
当前拉取/合并请求的状态 Intent for your PR
必须选择一项 Choose one (Mandatory):
代码质量 Code Quality：
我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:
#if 0代码，不包含已经被注释了的代码 All redundant code is removed and cleaned up