Skip to content

Conversation

@lyakh
Copy link
Collaborator

@lyakh lyakh commented Feb 24, 2022

Extensive testing showed, that making memory allocations from the SOF_MEM_ZONE_RUNTIME zone uncached fixes bug #4414 on APL with Zephyr. We plan to introduce a clean separation between cached and uncached memory access modes, but this is a significant change, planned for the next SOF release. Until then we disable cached runtime allocations in Zephyr.

@paulstelian97
Copy link
Collaborator

@dbaluta Does this affect our pipeline performance, especially for decoders (which might have state in the RUNTIME zone)? Just a FYI, I guess we can allow this to go and locally revert if it is a problem...

@lyakh
Copy link
Collaborator Author

lyakh commented Feb 24, 2022

A dedicated manual CI run with this PR triggered the bug again. Making this a draft for further investigation.

@lyakh lyakh changed the title zephyr: move SOF_MEM_ZONE_RUNTIME to uncached [Bad Fix][Do Not Merge] zephyr: move SOF_MEM_ZONE_RUNTIME to uncached Feb 24, 2022
@marcinszkudlinski
Copy link
Contributor

marcinszkudlinski commented Feb 24, 2022

anyway, isn't too drastic sollution? Turning off the cache may

  1. (obvious) make significant performance drop
  2. (much worse and much more dangerous) lead to many cache related defect stay undetected
    Stabilizing the firmware after turning the cache back on may take weeks

@lyakh
Copy link
Collaborator Author

lyakh commented Feb 24, 2022

anyway, isn't too drastic sollution? Turning off the cache may

1. (obvious) make significant performance drop

2. (much worse and much more dangerous) lead to many cache related defect stay undetected
   Stabilizing the firmware after turning the cache back on may take weeks

@marcinszkudlinski (1) performance - we will compare once we have a working solution. Zephyr reports load statistics, so should be easy to check. (2) regressions - as the commit message says, we will enforce correct cache usage by adding compile-time checks, using sparse and maybe the compiler itself. That should really clean up most errors in that area

@lyakh lyakh force-pushed the zone-runtime branch 2 times, most recently from b2b0278 to 5f4f27c Compare February 24, 2022 15:39
@lyakh lyakh requested a review from mrajwa as a code owner February 24, 2022 15:39
zephyr/wrapper.c Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would't say "bad fix" as the title says. Cached or uncached - in fact it is the very same heap and aligment always should be kept to the cacheline.

@lyakh lyakh changed the title [Bad Fix][Do Not Merge] zephyr: move SOF_MEM_ZONE_RUNTIME to uncached [Do Not Merge] zephyr: move SOF_MEM_ZONE_RUNTIME to uncached Feb 25, 2022
@lyakh lyakh force-pushed the zone-runtime branch 5 times, most recently from 294cc24 to 73e3683 Compare March 3, 2022 15:11
@lyakh lyakh changed the title [Do Not Merge] zephyr: move SOF_MEM_ZONE_RUNTIME to uncached zephyr: move SOF_MEM_ZONE_RUNTIME to uncached Mar 3, 2022
@lyakh lyakh changed the title zephyr: move SOF_MEM_ZONE_RUNTIME to uncached zephyr: make DMA LLI allocations always uncached, change voloume to use rmalloc() Mar 3, 2022
@lyakh
Copy link
Collaborator Author

lyakh commented Mar 7, 2022

SOFCI TEST

@lgirdwood
Copy link
Member

SOFCI TEST

@lgirdwood
Copy link
Member

Re test with the single commit.

@lyakh lyakh changed the title zephyr: make DMA LLI allocations always uncached, change voloume to use rmalloc() zephyr: make DMA LLI allocations always uncached Mar 18, 2022
Multi-core configurations already use uncached memory for LLI. This
also makes sense to avoid having to manually synchronise cache. Force
LLI objects uncached.

Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
@lyakh
Copy link
Collaborator Author

lyakh commented Mar 18, 2022

CI: three Zephyr platforms failed individual cases with "sof-logger dead" and a single non-Zephyr TGLU platform failed 2 suspend-resume tests with audio

@lgirdwood lgirdwood merged commit e1ca6f6 into thesofproject:main Mar 21, 2022
@lyakh lyakh deleted the zone-runtime branch April 4, 2022 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants