ipc: move delayed IPC sending to the primary core #9764
Conversation
src/ipc/ipc-common.c
Outdated
| @@ -170,6 +170,11 @@ void ipc_send_queued_msg(void) | |||
| k_spin_unlock(&ipc->lock, key); | |||
| } | |||
|
|
|||
| #ifdef __ZEPHYR__ | |||
| static struct k_work_q ipc_send_wq; | |||
| static K_THREAD_STACK_DEFINE(ipc_send_wq_stack, 2048); | |||
There was a problem hiding this comment.
CONFIG_IPC_SEND_STACK_SIZE - lets make this a tunable.
There was a problem hiding this comment.
@lgirdwood is it really worth it? This only occurs at one place. Maybe just put it in a header together with an additional one for
Line 17 in c7b7c91
There was a problem hiding this comment.
Kconfig is cheap - we should have this option for all system threads.
There was a problem hiding this comment.
@lgirdwood thinking a bit more - maybe we can have a single SOF work queue, run on core 0, serving both these (and potentially other) needs? EDF is currently used by IPC reception, KPB, mtrace logging, we could move the work queue out of it to make it global and then also use it for IPC sending?
There was a problem hiding this comment.
We can look at this as a secondary phase, yes this will save memory but we need to look at what will/wont work here.
There was a problem hiding this comment.
@lgirdwood if that approach is acceptable, I'd rather do that than adding significant infrastructure to only remove it again soon. I'll push that version for a test.
src/ipc/ipc-common.c
Outdated
| k_work_queue_start(&ipc_send_wq, | ||
| ipc_send_wq_stack, | ||
| K_THREAD_STACK_SIZEOF(ipc_send_wq_stack), | ||
| 1, NULL); | ||
|
|
||
| k_thread_suspend(thread); | ||
|
|
||
| k_thread_cpu_mask_clear(thread); | ||
| k_thread_cpu_mask_enable(thread, PLATFORM_PRIMARY_CORE_ID); | ||
| k_thread_name_set(thread, "ipc_send_wq"); | ||
|
|
||
| k_thread_resume(thread); |
There was a problem hiding this comment.
@lyakh probably worth a comment describing the aim here since its more than a couple of lines of code.
@peter-mitsis @andyross is there a convenience API for the above "pin work to a single core" ?
There was a problem hiding this comment.
a slight improvement would be to use k_thread_cpu_pin()
lyakh
requested review from
bardliao,
marcinszkudlinski and
pblaszko
as code owners
lyakh
requested review from
kv2019i,
iuliana-prodan and
dbaluta
as code owners
|
Interesting, the first attempt to run both IPC processing and sending on the same work queue resulted in apparent races, the new attempt produced IO errors on LNL https://sof-ci.01.org/sofpr/PR9764/build10135/devicetest/index.html?model=LNLM_SDW_AIOC&testcase=multiple-pause-resume-50 which don't look relevant but I cannot find any evidence of them appearing before. The QB failure is OTOH unrelated |
Instead of hard-coding EDF scheduler work queue thread stack size, add a Kconfig option for it. Signed-off-by: Guennadi Liakhovetski <[email protected]>
Low level IPC processing should be confined to the primary core. Move delayed IPC sending to a dedicated work queue with core 0 affinity. Fixes: thesofproject#8165 Signed-off-by: Guennadi Liakhovetski <[email protected]>
|
jenkins seems to be hanging, try to relaunch |
|
SOFCI TEST |
Seeing its completed, but results not uploaded yet... lets check later. |
|
LNL test came back empty https://sof-ci.01.org/sofpr/PR9764/build10171/devicetest/index.html . The other two were clean. Need to re-run yet again. |
|
SOFCI TEST |
2 similar comments
|
SOFCI TEST |
|
SOFCI TEST |
|
@wszypelt please run multicore tests with this PR (or just full scope to compare easily) |
Low level IPC processing should be confined to the primary core. Move delayed IPC sending to a dedicated work queue with core 0 affinity.
Fixes: #8165