{{ message }}
Fix PSRAM cache invalidation#9759
Merged
Merged
Conversation
This was referenced Oct 25, 2024
bill88t
approved these changes
Oct 25, 2024
bill88t
left a comment
There was a problem hiding this comment.
Passes every test I threw at it. The only thing I didn't test is for corruption.
Everything in the issue is fixed with this commit, thanks!
Member
tannewt
approved these changes
Oct 25, 2024
tannewt
left a comment
Member
There was a problem hiding this comment.
Thanks for the investigation and fix!
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.

The cache invalidation in #9752 needed to use
volatilepointers to point to the cache maintenance entries so they would be set correctly and in the right order.Note that the code from pico-sdk does use
volatile:https://github.com/earlephilhower/arduino-pico/pull/2551/files
It comes from the discussion here:
https://forums.raspberrypi.com/viewtopic.php?t=378249
That code also uses
_compiler_memory_barrier(), which may not be needed, but it helps guarantee ordering.Besides the example in #9755, this even simpler reproducer caused a hard fault on a Feather RP2350 with PSRAM before the fix:
After the fix, this works, which is more like the example in #9755 (#9755 doesn't have the first
gc.mem_free()):@tannewt Also, I don't think the extra 4 bytes is needed just below the PSRAM cache, so I took that out. I looked over the
tlsfcode, and I think it takes care not to write below the beginning of the pool. It does offset the first block by the size of a header. The first pool created is not passed a pointer one word up in free storage, so I think the second pool does not need to either. But you have spent a lot of time ontlsf, so maybe I am wrong.@bill88t Could you test if you have a chance?