Skip to content

Commit 4b670bd

Browse files
Andy Rossnashif
authored andcommitted
tests/kernel/smp: current CPU is not atomic
This test was whiteboxing the _current_cpu pointer to extract the CPU ID. That's actually racy: the thread can be preempted and migrated to another CPU between the _current_cpu expression and the read of the ID field. Do it right. Signed-off-by: Andy Ross <[email protected]>
1 parent 0e32f4d commit 4b670bd

File tree

1 file changed

+13
-3
lines changed

1 file changed

+13
-3
lines changed

tests/kernel/smp/src/main.c

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,16 @@ static struct k_thread tthread[THREADS_NUM];
4545
static K_THREAD_STACK_ARRAY_DEFINE(tstack, THREADS_NUM, STACK_SIZE);
4646

4747
static volatile int thread_started[THREADS_NUM - 1];
48+
49+
static int curr_cpu(void)
50+
{
51+
unsigned int k = arch_irq_lock();
52+
int ret = arch_curr_cpu()->id;
53+
54+
arch_irq_unlock(k);
55+
return ret;
56+
}
57+
4858
/**
4959
* @brief Tests for SMP
5060
* @defgroup kernel_smp_tests SMP Tests
@@ -120,7 +130,7 @@ static void child_fn(void *p1, void *p2, void *p3)
120130
ARG_UNUSED(p3);
121131
int parent_cpu_id = POINTER_TO_INT(p1);
122132

123-
zassert_true(parent_cpu_id != arch_curr_cpu()->id,
133+
zassert_true(parent_cpu_id != curr_cpu(),
124134
"Parent isn't on other core");
125135

126136
sync_count++;
@@ -140,7 +150,7 @@ void test_cpu_id_threads(void)
140150
/* Make sure idle thread runs on each core */
141151
k_sleep(K_MSEC(1000));
142152

143-
int parent_cpu_id = arch_curr_cpu()->id;
153+
int parent_cpu_id = curr_cpu();
144154

145155
k_tid_t tid = k_thread_create(&t2, t2_stack, T2_STACK_SIZE, child_fn,
146156
INT_TO_POINTER(parent_cpu_id), NULL,
@@ -161,7 +171,7 @@ static void thread_entry(void *p1, void *p2, void *p3)
161171
int count = 0;
162172

163173
tinfo[thread_num].executed = 1;
164-
tinfo[thread_num].cpu_id = arch_curr_cpu()->id;
174+
tinfo[thread_num].cpu_id = curr_cpu();
165175

166176
while (count++ < 5) {
167177
k_busy_wait(DELAY_US);

0 commit comments

Comments
 (0)