Uh oh!
There was an error while loading. Please reload this page.
zephyr/wrapper: Disable check for CONFIG_SYS_HEAP_ALIGNED_ALLOC - #3544
zephyr/wrapper: Disable check for CONFIG_SYS_HEAP_ALIGNED_ALLOC#3544andyross wants to merge 1 commit into
Conversation
This kconfig variable no longer exists in upstream Zephyr (the feature it used to control is now zero-overhead and always enabled). Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
We removed a kconfig that upstream SOF is checking for. Hack this in until the fix gets merged there. This patch can be removed when this pull request is merged in upstream SOF: thesofproject/sof#3544 Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
lgirdwood
commented
Oct 23, 2020
Looks like CI has a reboot. Rerun. |
lgirdwood
commented
Oct 23, 2020
SOFCI TEST |
lyakh
commented
Oct 23, 2020
@lgirdwood in fact we need |
lgirdwood
commented
Oct 23, 2020
Please align with @andyross for solution |
andyross
commented
Oct 23, 2020
When it was introduced, the sys_heap_aligned_alloc() API required a particular heap configuration (the 8-byte "big" chunk header). This had overhead, so it was made into an optional feature under the control of that kconfig option. Later, changes from @npitre fixed that, allowing the call to be used with the small headers. That eliminated the justification for making it optional, so the kconfig was removed too. Basically: it always works now. You don't need to set that. And because it's not part of the kconfig schema any more you can't, which is why the #error needs to be removed in SOF. |
lgirdwood
commented
Oct 29, 2020
@andyross thanks - can you do the same patch for master. I think CI has an issue with a test failing as the tested feature is not in the stable branch, We can then cherry-pick into stable (since it has passed CI) |
lyakh
left a comment
There was a problem hiding this comment.
is working with the latest rebase, thanks!
lgirdwood
commented
Nov 3, 2020
@lyakh can you create a PR for this to merge in master. We can then merge this into stable after it in master. Thanks |
lgirdwood
commented
Nov 11, 2020
@lyakh ping |
lyakh
commented
Nov 11, 2020
@lgirdwood I commented in an email to you, @andyross and others, that Zephyr still doesn't work correctly with cAVS 1.8+ in my tests without aligned allocation code. So, I don't think we can merge this until that is fixed? |
andyross
commented
Nov 11, 2020
Close, rolled into #3601 and sent to master |
lyakh
commented
Nov 11, 2020
@andyross I just retested with the new version of the aligned-allocation fix from @npitrezephyrproject-rtos/zephyr#29460 and your branch https://github.com/andyross/zephyr/tree/sof-rebased-22Oct20 i.e. I took that your branch, added my cAVS 1.8 extensions and fixes, reverted your patch zephyrproject-rtos/zephyr@a84a8a0 from that branch and applied zephyrproject-rtos/zephyr#29460 - and confirmed that that still doesn't work. |
npitre
commented
Nov 11, 2020
via email
Is there a simple test case I could have a look at? |
andyross
commented
Nov 11, 2020
@npitre Unfortunately this requires a development x86 board with a CAVS 1.8 audio DSP (Whiskey Lake et. al.), so it's unlikely you can work it directly. What happened here is that we had an side tree running for SOF integration that ran into a snag a while back that bisected to your big optimization pass on the heap. To keep things moving (because I was sure this was going to turn out to have been a buffer overflow in SOF) I just pushed a big revert of that work into this temporary tree. It's all been merged, finally, and the work needed to figure out what was going on with the heap led to the subtle alignment bug fixed in #2911 So @lyakh is reporting that he's got another runtime failure on slightly different hardware (that I don't have either) which is resolved by reverting the heap optimization series (your commits ending with commit 4fc1444823145b5d). I remain relatively sure that this is going to turn out to be an upstream SOF buffer overflow too, honestly, but we'll see. I was wrong before too. |
andyross
commented
Nov 11, 2020
Let's close this stale PR and move discussion to zephyrproject-rtos/zephyr#29954 |
[Not sure if submitting to stable-v1.6 is correct or not? Should I be submitting to master and then backporting instead?]
This kconfig variable no longer exists in upstream Zephyr (the feature
it used to control is now zero-overhead and always enabled).
Signed-off-by: Andy Ross andrew.j.ross@intel.com