amo: fix LR/SC atomicity and forward progress - #29
Open
Aquaticfuller wants to merge 1 commit into
Open
Conversation
Three defects in spatz_cache_amo, which sits per L1 cache controller and
is therefore shared by every core of every tile.
1. Any core's LR overwrote a live reservation, so symmetric LR/SC retry
loops could clobber each other in a ring and starve. Keep the
incumbent reservation, and age it out (ResvTimeoutCycles) so the
LR-without-SC case that made this guard unusable stays bounded.
2. core_id is 2 bits and only unique within a tile, but the reservation
owner, the foreign-write invalidation and the SC response match all
compared it alone: one tile's core could satisfy or take delivery of
another tile's SC. Key on the full hart id {tile_id, core_id}.
3. The SC outcome was looked up in single registers holding "the"
outstanding SC. A response that missed that entry fell through with
the raw memory word as its data, and sc.w reads rd == 0 as success,
so any zero word there told a hart its store-conditional had
succeeded when it never wrote. Carry the outcome with the
transaction instead: tcdm_user_t gains is_sc/sc_fail, stamped at
issue and echoed back by the memory system, so a response decodes
its own status. Removes the single-entry tracking entirely.
New test lrsc-forward-progress: 16 cores, symmetric CAS-increment retry
loop on one word. Before: 22/32, and 8 iterations/core did not finish in
900 s. After: 256/256 with 16 iterations/core in 25,778 cycles.
RLC M1_N1350_K100 unchanged at 241,339 cycles, 32/32 scoreboards; spin-lock,
byte-enable, cache-coverage-min, v12-race pass.
Reported by Johannes Pfau (defect 1).
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.
Three defects in spatz_cache_amo, which sits per L1 cache controller and is therefore shared by every core of every tile.
Any core's LR overwrote a live reservation, so symmetric LR/SC retry loops could clobber each other in a ring and starve. Keep the incumbent reservation, and age it out (ResvTimeoutCycles) so the LR-without-SC case that made this guard unusable stays bounded.
core_id is 2 bits and only unique within a tile, but the reservation owner, the foreign-write invalidation and the SC response match all compared it alone: one tile's core could satisfy or take delivery of another tile's SC. Key on the full hart id {tile_id, core_id}.
The SC outcome was looked up in single registers holding "the" outstanding SC. A response that missed that entry fell through with the raw memory word as its data, and sc.w reads rd == 0 as success, so any zero word there told a hart its store-conditional had succeeded when it never wrote. Carry the outcome with the transaction instead: tcdm_user_t gains is_sc/sc_fail, stamped at issue and echoed back by the memory system, so a response decodes its own status. Removes the single-entry tracking entirely.
New test lrsc-forward-progress: 16 cores, symmetric CAS-increment retry
loop on one word. Before: 22/32, and 8 iterations/core did not finish in
900 s. After: 256/256 with 16 iterations/core in 25,778 cycles.
RLC M1_N1350_K100 unchanged at 241,339 cycles, 32/32 scoreboards; spin-lock, byte-enable, cache-coverage-min, v12-race pass.