fast-get: Fix shared data usage by multiple callers #10484
Conversation
fast_get() for the same data can be called by multiple clients from the same or other cores. Since sram_ptr points to cached memory, writeback is needed after memcpy() to also copy data to uncached memory. When fast_get() is called a second time for the same data, invalidate() is used to bring shared data into another core's cache. This fixes tests which use two SRC modules. Signed-off-by: Serhiy Katsyuba <serhiy.katsyuba@intel.com>
serhiy-katsyuba-intel
requested review from
abonislawski,
dbaluta,
iuliana-prodan,
kv2019i and
lyakh
as code owners
|
@lgirdwood, Zephyr / build-linux (zmain, imx8 imx8x imx8m imx8ulp) CI failes with |
There was a problem hiding this comment.
Pull request overview
This PR fixes a cache coherency issue in the fast_get() function when the same data is accessed by multiple callers across different cores. The changes ensure proper cache alignment and synchronization for shared memory access.
Changes:
- Modified memory allocation to use cache-line alignment instead of no alignment
- Added cache writeback operation after copying data to ensure visibility across cores
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| dcache_writeback_region((__sparse_force void __sparse_cache *)entry->sram_ptr, size); | ||
| entry->dram_ptr = dram_ptr; | ||
| entry->refcount = 1; |
There was a problem hiding this comment.
The cache writeback occurs after the refcount is initialized, but before the dram_ptr is set. If another core accesses this entry between lines 126-127, it could observe an incomplete state. Consider moving the dcache_writeback_region() call after line 127 to ensure all entry fields are fully initialized before the data becomes visible to other cores.
| dcache_writeback_region((__sparse_force void __sparse_cache *)entry->sram_ptr, size); | |
| entry->dram_ptr = dram_ptr; | |
| entry->refcount = 1; | |
| entry->dram_ptr = dram_ptr; | |
| entry->refcount = 1; | |
| dcache_writeback_region((__sparse_force void __sparse_cache *)entry->sram_ptr, size); |
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
Comments inline but looking at the modified code alone, seems good.
| entry->size = size; | ||
| entry->sram_ptr = ret; | ||
| memcpy_s(entry->sram_ptr, entry->size, dram_ptr, size); | ||
| dcache_writeback_region((__sparse_force void __sparse_cache *)entry->sram_ptr, size); |
There was a problem hiding this comment.
3 participants
fast_get() for the same data can be called by multiple clients from the same or other cores. Since sram_ptr points to cached memory, writeback is needed after memcpy() to also copy data to uncached memory. When fast_get() is called a second time for the same data, invalidate() is used to bring shared data into another core's cache.
This fixes tests which use two SRC modules.