GTEST/UCP/FT: test FT in sequence of PUT/AM/FLUSH#11352
Open
evgeny-leksikov wants to merge 4 commits intoopenucx:masterfrom
Open
GTEST/UCP/FT: test FT in sequence of PUT/AM/FLUSH#11352evgeny-leksikov wants to merge 4 commits intoopenucx:masterfrom
evgeny-leksikov wants to merge 4 commits intoopenucx:masterfrom
Conversation
evgeny-leksikov
force-pushed
the
gtest_ft_put_am_flush_seq
branch
from
4a25cdd to
2dff170
Compare
evgeny-leksikov
force-pushed
the
gtest_ft_put_am_flush_seq
branch
2 times, most recently
from
bf0b4c8 to
4b99e51
Compare
The lane-change adjustment block in ucp_ep_flush_progress, introduced when num_lanes was replaced with the all_lanes bitmap, had two issues: 1. req->send.flush.all_lanes was updated only via OR with new lanes, so destroyed-lane bits stayed in all_lanes. On a subsequent EP reconfiguration (e.g. wireup CM swap rdmacm->tcp) the same destroyed lane was detected again and uct_comp.count was decremented a second time, eventually tripping the count >= 0 assertion when remaining lane flushes completed. 2. ep_destroyed_lanes was not masked with ~started_lanes, so a started lane that subsequently disappeared would also be subtracted from uct_comp.count even though it was already accounted for - either by the synchronous-OK decrement, by ucp_ep_flush_error, or by the pending uct completion delivered via the discard flow. Mask ep_destroyed_lanes with ~started_lanes and resync all_lanes to ep_live_lanes after each adjustment, restoring the invariant the previous num_lanes-based code maintained. Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit 975e15b)
… superset The previous loop condition `started_lanes != ep_live_lanes` is true whenever the two bitmaps differ, including the case where started_lanes is a strict superset of ep_live_lanes (a lane was started and then destroyed). In that case `ep_live_lanes & ~started_lanes` is 0, and the ucs_ffs64(0) call inside the loop body invokes UB on x86_64 (bsfq with a zero input), producing a garbage lane index that subsequently feeds ucp_ep_get_lane() out of bounds. Cache the unstarted-live-lane bitmap into next_lanes and use `(next_lanes != 0)` as the loop condition. This safely handles the superset case (loop exits cleanly) and also avoids re-computing `ep_live_lanes & ~started_lanes` twice per iteration. Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit cf91dde)
evgeny-leksikov
force-pushed
the
gtest_ft_put_am_flush_seq
branch
from
d6b2106 to
51b9b60
Compare
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
1 participant
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.
What?
test UCP/EP/FT in sequence of PUT/AM/FLUSH
Why?
testing coverage
Note:
Depends on #11351