Covered the SMP suspension teardown and the long byte pool search - #677
Merged
fdesbiens merged 1 commit intoAug 28, 2026
Merged
Conversation
Against the merged SMP coverage report -- every build configuration instrumented and unioned -- sixty-four lines of common_smp/src were uncovered, 5114 of 5178. Fifty-three of them are closed here and the report reads 5167 of 5178. The SMP coverage floor goes from 98 to 99 with it. Thirty-six of the sixty-four were one loop repeated four times: the walk in tx_block_pool_delete, tx_byte_pool_delete, tx_event_flags_delete and tx_queue_delete that releases every thread suspended on the object with TX_DELETED. The suite deletes all four object types after every single test, and that is exactly why the loop never ran. test_control_cleanup in the ThreadX suite deletes the application's objects first and its threads last, so a test that ends with a thread parked on a queue has that thread walked out of it by tx_queue_delete. The SMP suite's cleanup deletes the threads first, deliberately -- it was changed so that no application-owned object is still referenced when the object loops run, which is what stopped a class of teardown hang. The side effect is that all four deletes now run against an empty suspension list. tx_semaphore_delete is the one member of the family that was already covered, because threadx_semaphore_delete_test deletes a busy semaphore on purpose. threadx_object_delete_suspension_test is the same idea for the other four. Two threads suspend on each of a block pool, a byte pool, an event flags group and a queue; the control thread waits on the object's own suspended count through tx_*_info_get rather than on an ordering it cannot guarantee across four cores, deletes the object, and checks both waiters came out with TX_DELETED. Two waiters rather than one so the loop takes its back edge as well as its body, and every wait is bounded in ticks so a suspension that never arrives fails the test instead of hanging it. threadx_trace_entry_update_test and threadx_thread_misaligned_stack_test are ports of the two tests that closed the equivalent gaps in common/src, and close fourteen more lines here: tx_block_allocate 123, 175, 182, 319 and 326, tx_byte_allocate 130, 210, 217, 359 and 366, tx_thread_system_suspend 504 and 560, tx_trace_object_register 221, and tx_thread_create 133. The one substantive change is core confinement. The trace test needs thread 0 to suspend and thread 1 to then release what it waits for; on four cores thread 1 gives the block back before thread 0 has suspended and the update block behind the suspension is never reached, so both threads are excluded from cores 1 to 3. The misaligned stack test needed no such change. threadx_byte_memory_long_search_test closes three of the eleven in tx_byte_pool_search. Lines 264, 267 and 270 are the TX_BYTE_POOL_MULTIPLE_BLOCK_SEARCH limit -- twenty on this port -- where a long search drops and retakes protection so that it cannot lock the other cores out for the whole walk. No byte pool in the suite ever had twenty fragments. This one is filled with small chunks until it refuses another and then has every second chunk released, so the free fragments are never adjacent and cannot be merged, and the request is larger than any of them but smaller than the pool's theoretical total, which is what makes _tx_byte_pool_search walk rather than refuse at the door. The layout is asserted rather than assumed: the test checks the fragment count and checks the probe request really does fail before the workers start, because either would otherwise turn it into a silent no-op. Eleven lines remain and they are not a to-do list. Eight are the delay loop in tx_byte_pool_search that fires when another thread claims the pool inside the window the search opens. The Linux SMP port serialises all four cores on one pthread mutex, so that window is an unlock immediately followed by a lock on that mutex, and glibc hands an uncontended mutex straight back to the thread that just released it: measured over 180,003 windows across three cores, with zero handovers. The shipped test therefore does 250 searches per worker rather than the sixty thousand that probe used, because the twenty-block threshold is crossed by the first search. The other three are in tx_thread_smp_utilities. Line 149 is a range guard placed after the shift it is meant to guard, so reaching it needs a shift by the width of the type; the fix is to move the check above the shift, matching the TX_MAX_PRIORITIES > 32 variant of the same function, and that belongs in its own change. Lines 1073 and 1074 need a mutex owner that is genuinely executing on another core when a waiter suspends, and three shapes were tried without producing one on this port. Measured twice before and twice after, every gcda deleted between runs and 570 of 570 tests passing each time: 5114 of 5178 both times before, 5167 of 5178 both times after. Branch coverage goes from 2768 to 2821 and 2823 of 3548. A floor of 99 needs 5127, so the ratchet lands with forty lines of headroom against a numerator that has been seen moving by two between runs. Assisted-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Against the merged SMP coverage report — every build configuration instrumented and unioned —
sixty-four lines of
common_smp/srcwere uncovered, 5114 of 5178, 98.76%. Fifty-three ofthem are closed here and the report reads 5167 of 5178, 99.79%. The SMP coverage floor
goes from 98 to 99 with it, in the same change so that this PR's own run is the proof the
floor clears.
#666did this forcommon/src; this is the SMP half, which it never touched.The largest gap was one loop, four times, and the reason it was uncovered is not obvious
Thirty-six of the sixty-four are the same nine-line loop in
tx_block_pool_delete,tx_byte_pool_delete,tx_event_flags_deleteandtx_queue_delete— the walk that releasesevery thread suspended on the object with
TX_DELETED.The suite deletes all four object types after every single test, and that is exactly why the
loop never ran.
test_control_cleanup()in the ThreadX suite deletes the application'sobjects first and its threads last, so a test that ends with a thread parked on a queue has
that thread walked out of the queue by
tx_queue_delete. The SMP suite's cleanup deletes thethreads first, on purpose — it was changed so that no application-owned object is still
referenced when the object loops run, which is what stopped a class of teardown hang. Correct,
and the side effect is that every one of those four deletes now runs against an empty
suspension list.
tx_semaphore_delete.cis the one member of the family that was alreadycovered, because
threadx_semaphore_delete_testdeletes a busy semaphore deliberately.Please do not "fix" this by putting the SMP cleanup back the way the ThreadX one is.
threadx_object_delete_suspension_testis the same idea as the semaphore test, for the otherfour. Two threads suspend on each object; the control thread waits on the object's own
suspended count through
tx_*_info_getrather than on an ordering it cannot guarantee acrossfour cores, deletes it, and checks both waiters came out with
TX_DELETED. Two waiters ratherthan one so the loop takes its back edge as well as its body, and every wait is bounded in
ticks so that a suspension which never arrives fails the test instead of hanging it.
Two ports of #666's tests — 14 lines
threadx_trace_entry_update_testtx_block_allocate123/175/182/319/326,tx_byte_allocate130/210/217/359/366,tx_thread_system_suspend504/560,tx_trace_object_register221threadx_thread_misaligned_stack_testtx_thread_create133The one substantive change is core confinement. The trace test depends on thread 0 suspending
and thread 1 then releasing what it waits for; on four cores thread 1 gives the block back
before thread 0 has suspended, and the update block behind the suspension is never reached. Both
threads are excluded from cores 1–3. The misaligned stack test needed no such change.
The long byte pool search — 3 of 11
Lines 264/267/270 of
tx_byte_pool_search.care theTX_BYTE_POOL_MULTIPLE_BLOCK_SEARCHlimit — twenty on this port — where a long search drops and retakes protection so it cannot
lock the other cores out for the whole walk. No byte pool in the suite ever had twenty
fragments.
threadx_byte_memory_long_search_testfills one with small chunks until itrefuses another and releases every second one, so no free fragment has a free neighbour and the
search cannot merge its way out; the request is larger than any single fragment but smaller
than the pool's theoretical total, which is what makes
_tx_byte_pool_searchwalk rather thanrefuse at the door. The layout is asserted rather than assumed — the test checks the fragment
count and checks the probe request really does fail before the workers start, because either
would otherwise turn it into a silent no-op.
The eleven that remain are explained, not skipped
Eight are the delay loop at
tx_byte_pool_search.c:283–301, which fires when anotherthread claims the pool inside the window the search opens. The Linux SMP port serialises all
four cores on one
pthread_mutex, so that window is an unlock immediately followed by a lockon that mutex, and glibc hands an uncontended mutex straight back to the thread that just
released it. Measured: 180,003 windows across three cores, zero handovers. The shipped test
does 250 searches per worker rather than the sixty thousand that probe used, because the
twenty-block threshold is crossed by the first search and the rest buys nothing. Reaching those
eight needs a port whose cores really are concurrent.
Three are in
tx_thread_smp_utilities.c. Line 149 is a range guard placed after theshift it is meant to guard —
ULONG >> prioritywithpriority >= TX_MAX_PRIORITIES— soreaching it needs a shift by the width of the type. The
TX_MAX_PRIORITIES > 32variant of thesame function, sixty lines above, tests the range first and is written correctly; the fix is
to reorder the check, and that belongs in its own change because it edits the SMP scheduler
and this is a test-only PR apart from the threshold. Lines 1073/1074 are the priority-inheritance
arm of
_tx_thread_smp_simple_priority_change, reachable only from_tx_mutex_priority_changeon a raise, and only when the mutex owner is genuinely executing on another core as the waiter
suspends. Three shapes were tried and none produced that on this port.
Measurements
Two runs before and two after, every
.gcdadeleted between runs, 570 of 570 tests passingeach time:
The suite gains four tests, 110 to 114 per configuration, and about a second per configuration
of wall clock.
A floor of 99 needs 5127, so the ratchet lands with forty lines of headroom against a
numerator that has been seen moving by two between runs on a CI machine.