Skip to content

Modify the logic of utest-smp_thread_preemptions and add comments #11012

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Tm-C-mT wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Modify the logic of utest-smp_thread_preemptions and add comments #11012

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
05846c6
[utest]:Modify the execution logic of smp_thread_preemptions
Tm-C-mT Dec 2, 2025
317bd5a
[docs][utest]:Add comments for smp_thread_preemption
Tm-C-mT Dec 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions src/utest/smp/smp_thread_preemption_tc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,37 @@
* @note Create multiple threads, low-priority threads run first,
* high-priority threads preempt low-priority threads, and
* print the current status of each core in the thread's entry function.
*
* 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.
Comment on lines +27 to +42
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

[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.
Suggested change
* - 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.

Copilot uses AI. Check for mistakes.
*/
Comment on lines +20 to 43
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

🟢 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

Copilot generated this review using guidance from repository custom instructions.

#define THREAD_PRIORITY_HIGH 21
#define THREAD_PRIORITY_LOW 30
#define THREAD_STACK_SIZE UTEST_THR_STACK_SIZE

static rt_thread_t threads[2];
static rt_uint8_t finish_flag = 0;
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

🔴 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:

  1. Race condition: On SMP systems, the write from one core may not be visible to another core reading the flag
  2. Compiler optimization: The compiler may optimize away the loop at line 74 since finish_flag is not declared as volatile
  3. Potential deadlock: If the low-priority thread doesn't see the update, it will loop forever

Recommended Solution:

  • Declare finish_flag as volatile rt_uint8_t to prevent compiler optimizations
  • OR use RT-Thread's atomic operations if available
  • OR use an event/semaphore for proper synchronization

中文：finish_flag 变量被多个线程访问但没有适当的同步机制。高优先级线程写入它（第62行），低优先级线程在忙等待循环中读取它（第74行），但没有任何内存屏障或原子操作。这可能导致：

  1. 竞态条件：在 SMP 系统上，一个核心的写入可能对另一个核心读取该标志不可见
  2. 编译器优化：编译器可能会优化掉第74行的循环，因为 finish_flag 未声明为 volatile
  3. 潜在死锁：如果低优先级线程看不到更新，它将永远循环

建议的解决方案：

  • 将 finish_flag 声明为 volatile rt_uint8_t 以防止编译器优化
  • 或使用 RT-Thread 的原子操作（如果可用）
  • 或使用事件/信号量进行适当的同步

Example / 示例:

static volatile rt_uint8_t finish_flag = 0;
Suggested change
static rt_uint8_t finish_flag = 0;
static volatile rt_uint8_t finish_flag = 0;

Copilot uses AI. Check for mistakes.
static struct rt_spinlock lock;

/* High Priority Thread */
Expand All @@ -35,6 +59,7 @@ static void thread_high_entry(void *parameter)
extern long list_thread(void);
list_thread();
rt_spin_unlock(&lock);
finish_flag = 1;
}

/* Low Priority Thread */
Expand All @@ -46,6 +71,7 @@ static void thread_low_entry(void *parameter)
extern long list_thread(void);
list_thread();
rt_spin_unlock(&lock);
while(!finish_flag);
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

🟠 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_t or rt_event_t for 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);

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

🟡 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);

Copilot generated this review using guidance from repository custom instructions.
}

static void thread_preemptions_tc(void)
Expand All @@ -72,6 +98,7 @@ static void thread_preemptions_tc(void)
static rt_err_t utest_tc_init(void)
{
rt_spin_lock_init(&lock);
finish_flag = 0;
return RT_EOK;
}

Expand Down