thesofproject / sof

Sound Open Firmware
Other
548 stars 314 forks source link

[BUG][Zephyr] Inconsistent IPC context priority when running on different cores #6676

Open lyakh opened 1 year ago

lyakh commented 1 year ago

This should be verified, but by double-checking in the code I came up with the following analysis: IPCs can be handled by the primary or a secondary core. When handled by the primary core, the IPC runs in the IPC task context, which is an EDF task, and under Zephyr the EDF scheduler runs with the lowest Zephyr thread priority. If an IPC has to be handled by a secondary core, an IDC is sent to that core. IDCs under Zephyr are processed in the P4WQ work context, and those are processed by a Zephyr thread with the highest thread priority. IPC processing priority on the primary and secondary cores should be the same.

lgirdwood commented 1 year ago

I think @mwasko has this information in some sof-docs PRs ? @marcinszkudlinski fyi. My expectation is that IPC thread priority would be the same regardless of core as this would provide a consistent IPC handling experience across all cores.

mwasko commented 1 year ago

@lyakh there is PR on sof-docs that describe target design for IPC/IDC handling https://github.com/thesofproject/sof-docs/pull/440, should be published on SOF Wiki soon. Generally both on primary and secondary cores the IPC/IDC messages should be processed in a task with budget (high priority preemptible thread) which has higher priority then typical EDF task but can be preempted by LL tasks.

lyakh commented 1 year ago

@lyakh there is PR on sof-docs that describe target design for IPC/IDC handling thesofproject/sof-docs#440, should be published on SOF Wiki soon. Generally both on primary and secondary cores the IPC/IDC messages should be processed in a task with budget (high priority preemptible thread) which has higher priority then typical EDF task but can be preempted by LL tasks.

The IPC handler on the primary core is itself an EDF task, so it has the EDF task priority. But yes, the IPC task priority has to be made consistent

lgirdwood commented 1 year ago

@mwasko @lyakh - I guess this is still open. We need an intermediate option to set the P4WQ to the same thread priority as core 0 IPC priority unless TWB is imminent ?

mwasko commented 1 year ago

@lyakh does the current implementation is blocking any scenario? @pblaszko has did some improvements in IPC/IDC area to enable multi-core processing, the question is whether we need some more adjustments till we implement TWB?

lyakh commented 1 year ago

@lgirdwood @mwasko I'm not aware of any failures caused by this. Yes, we could set the global P4WQ priority to match that of the IPC thread. That sounds a bit brutal, since P4WQ is a global service, but in practice it's so far only used by our IDC.

lgirdwood commented 1 year ago

yeah - we just need some consistency across cores so that behavior is the same until TwB. Dont want to spend time debugging "works on core 0 but not on core 1" type issues.