thesofproject / sof

Sound Open Firmware
Other
560 stars 318 forks source link

[FEATURE][Zephyr] Add stack checks to component info #6764

Open cujomalainey opened 1 year ago

cujomalainey commented 1 year ago

Is your feature request related to a problem? Please describe. Currently some components require stacks larger than the default, if you forget to modify the stack you are in for a bad time trying to figure out what you messed up

Describe the solution you'd like Zephyr provides us a better opportunity given its rich APIs. In theory we can actually check our current stack size to make sure we are compatible before we even run the component if the information is packaged with the component.

Describe alternatives you've considered N/A, Stack is based on FW currently but core assignment is in the topology, makes it very hard to validate outside of runtime

Additional context https://github.com/thesofproject/sof/pull/6746#pullrequestreview-1211882309

lgirdwood commented 1 year ago

@marcinszkudlinski @juimonen I assume we can add stack sizes to rimage toml to build into the manifest ?

cujomalainey commented 1 year ago

What about comp_driver_info?

lgirdwood commented 1 year ago

What about comp_driver_info?

I think we have a similar use case for Windows and trying to align the flow around Zephyr/IPC. @mwasko and @marcinszkudlinski working on configuration flow now.

cujomalainey commented 1 year ago

Understood :)

mwasko commented 1 year ago

What about comp_driver_info?

I think we have a similar use case for Windows and trying to align the flow around Zephyr/IPC. @mwasko and @marcinszkudlinski working on configuration flow now.

@cujomalainey with the loadable libraries introduction we have a stack_size field defined and a plan is to use it for DP/EDF module Thread initialization.

lgirdwood commented 1 year ago

Pencil this in for v2.5 at end of Q1/23

kv2019i commented 1 year ago

V2.5 branching this week and nobody assigned, so moving to v2.6.

lgirdwood commented 1 year ago

Moved to v2.7 - Loadable modules in progress.

kv2019i commented 1 year ago

I've verified Zephyr CONFIG_STACK_SENTINEL=y with SOF and it's work as advertized. I left a mention of this to sof/app/debug_overlay.conf , so it's easier for SOF developer to find. This doesn't address the enhancement ticket as such, but this is one concrete tool to investigate suspected stack sizing issues.

kv2019i commented 1 year ago

FYI to @jxstelter and @pjdobrowolski working on loadable modules support.

kv2019i commented 1 year ago

@cujomalainey @lgirdwood @alex-cri I propose we move to v2.8. There is incremental work done that helps to address issues like https://github.com/thesofproject/sof/pull/6746/commits/104ed3e906ac1105060a5bc5487752cde80e61ba , including:

But e.g. for the more common low-latency modules, the stack size continues to be a global build time setting and we do not have infra in SOF for modules to declare the stack sizes, and to discover the maximum at build time and use this to size the LL-thread size. I believe this is at the root of the original ask @cujomalainey , right. The loadable modules further complicate this (a loaded LL-module will have to run in the same thread, so we cannot re-dimension the stack size of existing LL-threads).

alex-cri commented 1 year ago

@kv2019i , I agree with your takeaways. Will give a day for any further comments to come in then plan to move to v2.8.

kv2019i commented 11 months ago

Bumping to v2.9. If anyone has fresh examples and possible experiences using the Zephyr stack debug tools (and any shortcomings), please add here.

kv2019i commented 5 months ago

The Zephyr shell support added in v2.10 provides more tools to debug stack usage, but the original ask of providing infra to describe component stack needs, and checking this at build time, remains open.

@cujomalainey with the new (static) tools in Zephyr, has this still been a problem? I'll clear the milestone now, we can discuss the priority and potentially bring back to v2.10. FYI @lgirdwood

cujomalainey commented 5 months ago

@andyross who can test this out and see if zephyr catches the issues

lgirdwood commented 4 months ago

@andyross @cujomalainey Thread analyzer will help here too that @jsarha is working on. I now think all the Zephyr parts for support on Intel HW are upstream now, just now pending SOF application parts for reporting.

kv2019i commented 2 months ago

Please add feedback on whether the existing tools in Zephyr are sufficient, or should we keep this open and target for better tooling at the audio component/module level. I'll move this to v2.12 for now as no new activity is planned this month. The Zephyr thread analyzer was merged this week, along with SOF debug stream to get this out to host, so this is tool is now available as well.