mirror of
https://github.com/eclipse-threadx/threadx.git
synced 2026-10-06 06:59:08 +08:00
Stopped the thread priority change test leaving core 0 to a finished thread (#647)
The SMP suite has been failing on threadx_thread_priority_change since 30 June.
The test reports SUCCESS and the process then never exits, so ctest kills it at
the thousand second timeout, twice, and the log carries nothing past the result
line. Instrumenting the harness teardown produced the state at the hang:
last stage reached: test_control_return: control thread resume returned
_tx_thread_preempt_disable: 0
core 0: current=thread 0 execute=thread 0
thread test control thread state=0 priority=0 threshold=0 core_control=1
thread thread 0 state=1 priority=0 threshold=0 inherit=0
Core 0 is held by a thread in state 1, TX_COMPLETED, while the control thread
sits in state 0, TX_READY, at priority 0. The Linux SMP scheduler fills a core
only when _tx_thread_current_ptr for it is null, and clears that pointer only
for a thread carrying a deferred preemption, which thread 0 is not. So core 0
can never be handed on, and with TX_THREAD_SMP_ONLY_CORE_0_DEFAULT and
TX_SMP_NOT_POSSIBLE the control thread has nowhere else to go. The scheduler
re-reads the same state every two milliseconds for as long as it is allowed to.
What put thread 0 at priority 0 is the last thing this test does:
thread_0.tx_thread_inherit_priority = 0;
_tx_thread_smp_simple_priority_change(&thread_0, 16);
with the stated intent of reaching the branch where the new priority is below
the inheritance priority. Zero cannot reach that branch, because 16 is not less
than 0. The other branch runs instead, and that branch assigns the inheritance
priority as the thread's priority while the code after it links the thread into
the list for the new priority regardless. Thread 0 therefore came away claiming
priority 0 while living in the priority 16 list.
Both halves of that hurt. Priority 0 ties with the control thread, so resuming
the control thread raised no preemption and left the execute pointer alone. The
mismatch between the recorded priority and the list the thread is linked into
then means that completing thread 0 removes it from a list it was never in,
leaving core 0 pointing at it for good.
Give the inheritance priority a value above the new one, which is what the
branch the comment names actually requires, and put it back to
TX_MAX_PRIORITIES afterwards so nothing downstream reasons about an
inheritance that is not there. Hold protection across the call as well: this is
an internal routine that expects it, and it was being called in the open.
Measured before and after by printing the thread's state at the point the test
reports success. With the inheritance priority at 0 it is priority 0 threshold
0, matching the hang above, on every run. With it above the new priority it is
priority 16 threshold 16, which agrees with the list the thread is linked into,
and resuming the control thread preempts core 0 the ordinary way.
All five SMP configurations pass 110 of 110 at the parallelism CI uses.
The comparison in _tx_thread_smp_simple_priority_change deserves a second look
on its own account. When its else branch runs, the thread's recorded priority
and the list it is linked into disagree by construction. Only this test is
known to reach that branch, by supplying an inheritance priority that cannot
arise in ordinary operation, so nothing here claims a defect in shipped paths.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
83dfbc3534
commit
fe40079353
@@ -465,11 +465,28 @@ TX_THREAD *temp_thread;
|
||||
/* Restore interrupts. */
|
||||
TX_RESTORE
|
||||
|
||||
/* Hit branch in _tx_thread_smp_simple_priority_change such that the
|
||||
new priority is less than the inheritance priority. */
|
||||
thread_0.tx_thread_inherit_priority = 0;
|
||||
/* Hit the branch in _tx_thread_smp_simple_priority_change where the new
|
||||
priority is below the inheritance priority. That needs an inheritance
|
||||
priority above the new one, so 0 does not reach it: with 0 the other
|
||||
branch runs, and that branch records the inheritance priority as the
|
||||
thread's priority while still linking the thread at the new priority.
|
||||
Thread 0 came out of here claiming priority 0 while living in the
|
||||
priority 16 list, which is what has been hanging this test in CI.
|
||||
Priority 0 ties with the control thread, so resuming the control thread
|
||||
raised no preemption, and once thread 0 completed, core 0 was left owned
|
||||
by a completed thread with the control thread ready and unscheduled.
|
||||
|
||||
This routine is internal and expects protection to be in force, so hold
|
||||
it here rather than calling in the open. */
|
||||
TX_DISABLE
|
||||
thread_0.tx_thread_inherit_priority = 20;
|
||||
_tx_thread_smp_simple_priority_change(&thread_0, 16);
|
||||
|
||||
/* Put the inheritance priority back to the value that means this thread is
|
||||
not inheriting one, so nothing downstream reasons about a phantom. */
|
||||
thread_0.tx_thread_inherit_priority = TX_MAX_PRIORITIES;
|
||||
TX_RESTORE
|
||||
|
||||
/* Successful test. */
|
||||
printf("SUCCESS!\n");
|
||||
test_control_return(0);
|
||||
|
||||
Reference in New Issue
Block a user