Repository navigation
[FEATURE] RFC New SOF_SCHEDULE_COMM scheduler - IPC & IDC scheduling refactor #7238
Description
Is your feature request related to a problem? Please describe.
Feature is related to the IPC4 refactoring. Currently IDC for zephyr platforms uses zephyr p4wq abstraction to schedule IDC messages, while IPC4 for zephyr platforms uses SOF list_item primitive supported by k__work__delayable zephyr abstraction.
Pointing out further issues:
- IPC synchronization is scattered among zephyr ipc driver and sof code (we use a
ipc.task_listfor messaging in sof code but there is alsodevdata->tx_ack_pendingvariable fixing up synchronization in thezephyr\soc\xtensa\intel_adsp\common\ipc.cfile. - IPC operates on
sof_taskabstraction, while IDC uses only k_p4wq_work as single unit of work (single message handling). - IPC operates on EDF scheduler that had never been truly finished (since it lacks any deadlines calculation and does not provide any preemption).
Moreover, IPC4 code for Zephyr platforms is very scattered so understanding the implementation takes ages.
Motivation to use SOF Scheduler abstraction
- IPC code is using SOF tasks now that are strongly composed into common SOF IPC code (ipc3/ipc4 commons).
It would be very hard to cut out ipc task abstraction away from common code (for IPC4/zephyr and replace with something else). SOF tasks are managed by SOF schedulers. - AMS uses LL scheduler (
src\lib\ams.c) for its task realization, what is technically wrong. At the time of implementation it was the only one scheduler working on secondary cores. Using new comm_scheduler for AMS tasks will provide them with
clear priority placement (highest task priority group - AMS tasks will be always placed closest to the p4wq queue HEAD). - Clear prioritization of all communication tasks (IPC, IDC, AMS). Right now it is not clear how those tasks are prioritized.
While we can claim that IPC responses have higher priority than IPC notifications (by the implementation of how elements are added toipc.msg_list), we can not claim that AMS messages that use IDC have higher priority than IPC that also use IDC.
Also we also can not claim that IDC has higher priority than IPC since they both use the same thread priority of
#define EDF_ZEPHYR_PRIORITY 1but completely separate queue implementations. - IPC synchronization code in Zephyr Intel ADSP IPC driver added in soc: intel_adsp: ipc: Do not send message until previous one is acked zephyrproject-rtos/zephyr#51894 , can be removed and placed into the new comm_scheduler (that will make use of the
k_p4wq_work.done_semprimitive). This way we will handle IPC messaging synchronization in one place (not scattered among sof & zephyr code). To use mentioned semaphore we need a wrapper code (that conceptually fits the scheduler abstraction). - Easy expandability of new "system middle priority" tasks. When all communication tasks are placed into single scheduler with clean priorities and synchronization rules, we can easily add another sof tasks of middle system priority and decide their priority in comparison to other tasks.
- New scheduler provides an interface for tasks deadlines. Each tasks type will have own deadline that will be passed to
k_p4wq_workitem.
Describe the solution you'd like
Introduce new type of SOF scheduler - "comm_scheduler".
It would be responsible for scheduling IPC and IDC messages using zephyr treads, priorities, preemption and deadlines.
It would be a wrapper for p4wq abstraction.
Each core would run its own "comm_scheduler" instance.
EDIT: In IPC scheduling and execution refactor rev1.1 model we decided to use single comm_scheduler instance for all cores.
It will have inside CORE_COUNT * p4wq queues. Each queue managed by 1 thread pinned to each core.
comm_scheduler would operate on sof_task abstraction while mapping those to underlying k_p4wq_work items managed by the k_p4wq_work.
New scheduler would work on the single Zephyr preemptive thread of the priority index = 0.
This would place it in the whole system as "middle priority tasks" scheduler (since the LL schedulers work on the Zephyr cooperative threads that have priority index < 0 and the new DP scheduler has priority index > 0.
Also this change would cause IDC messages to be assigned to sof_task abstraction so it may be managed by the generic scheduler interface.
Model prepared
Prepared a model of the proposed changes.
Model consists of the "class diagram like" static description of data structures along with comments on their runtime uses/behavior.
It describes the existing IPC scheduling mechanism and the new scheduler model.
There are also two "sequence diagram like" graphs describing a flows on the comm_scheduler instances.
First sequence diagram shows scenario when the IPC message is received from Host by interrupt, IPC target is Core0 and the response send.
Second sequence diagram shows a scenario when IPC message is received from Host by interrupt, IPC target is Core1, so the message is redirected to Core1 using IDC messaging. It also shows new improved synchronization mechanisms (synchronization of IPC messages with each other, and synchronization of IPC messages with IDC completion).
Activity diagram describes a scenario "Received IPC host interrupt to create a pipeline on Core1". It describes scheduler
implementation in detail presenting threads, synchronization points and correlation of task with p_k4wq_work object.
Model revisions
IPC & IDC scheduling and execution refactor rev1.0.pdf
IPC & IDC scheduling and execution refactor rev1.1.pdf
IPC & IDC scheduling and execution refactor rev1.2.pdf
Change list between model rev1.0 and rev.1.1:
- Addressed issues indicated by @mmaka1 to make the diagram more readable and consistent:
- Added information on what is a thread in the communication model and some legend with symbols explanation.
- In the communication diagrams removed "interrupt context" as object representation and replaced it with message as it should be from the start.
- Added information on budget to communication model and the legend.
Change list between model rev1.1 and 1.2:
- Added more elements to class diagram to describe static correlation of
taskobject withk_p4wq_workobject:- turned out that there is new requirement to the task1 from the roadmap - "Expand functionality of P4WQ so it may add tasks to the queue in the specified order":
k_p4wq_workstruct needs to have additionalvoid *priv_datafield to provide a correlation withtaskstruct items. - added new struct
comm_scheduler_task_datathat maps structtasktok_p4wq_workitems. - added new function
static void comm_task_handler()that is a callback executed byk_p4wq_workitems and contains completion, rescheduling and synchronization logic for tasks executed at given time.
- turned out that there is new requirement to the task1 from the roadmap - "Expand functionality of P4WQ so it may add tasks to the queue in the specified order":
- Added new activity diagram that describes in detail how implementation of the new scheduler would work:
- diagram describes interaction between tasks, work items and
static void comm_task_handler()function. - diagram shows what actions are executed on what thread and synchronization points (colors of arrows).
- diagram describes interaction between tasks, work items and
Please share your feedback on the implementation idea. In this issue I would like to discuss all possible matters and doubts before starting works on this feature. Moreover, I would like to create a reasonable roadmap of works to be done (how to split this on PRs).
Tasks roadmap to implement the feature
- Expand functionality of P4WQ so it may add tasks to the queue in the specified order (by group prepend and append).
- Implement new
comm_scheduleralong with unit tests - Refactor SOF IPC4 for Zephyr platforms and Zephyr Intel ADSP IPC driver to use new
comm_scheduler
(src\ipc\ipc-zephyr.cand many ipc-common source files). - Refactor IDC for Zephyr platforms so it uses new
comm_scheduler(src\idc\zephyr_idc.c) and Async Messages Service - Remove EDF scheduler if no dependencies to non Zephyr platforms
Activity
marcinszkudlinski commentedon Mar 10, 2023
existing EDF scheduler may be removed when COMM is introduced,
mmaka1 commentedon Mar 10, 2023
If both, the scheduling and the execution of tasks is run within the same execution context, "comm_thread" name of a participant might be better than "comm_scheduler".
In the SOF case, (1) the "medium" group does not run till completion, the time slice is limited (it is called a "budget"). When the tasks from the "medium" group run out of budget, the "low" group starts. The budgets are reset every system tick (1 ms typically).
(2) The "medium" group may also run opportunistically when the "low" group is idle and there is still time available until the next system tick when the "high" group starts running.
kv2019i commentedon Mar 14, 2023
Looks great @aborisovich ! One thing I was still looking (might be there, just didn't caught my eye) is what is the primary motivation to use the SOF scheduler abstraction for this? Was the primary reason to make it easier to facilitate IPC3/non-Zephyr usage, or do we have more reasons? This would be good to write down as it's sure to come up in code reviews.
aborisovich commentedon Mar 14, 2023
I've provided new section in the issue description for the motivation on using SOF scheduler.
I will also soon add another revision of the model that will include more information on how AMS tasks are managed and how SOF tasks are mapped to k_p4wq_work items.
lgirdwood commentedon Mar 14, 2023
@dbaluta I assume also good for NXP IPC on Zephyr ?
@aborisovich pls also looks for areas where we can align sof scheduler APIs to be more like Zephyr APIs i.e. any low hanging fruit. The general direction should be to align the scheduler API to be thin and map nicely to Zephyr.
lgirdwood commentedon Mar 14, 2023
@aborisovich one more thing, do you have an ETA ? Shall we say v2.6 (end of June) ?
lyakh commentedon Mar 16, 2023
Can we have an even higher level goal / design description? I.e. what design are we targeting? Like all IPCs should be processed in Zephyr thread context, with a certain (or varied?) priority,... All IDCs should also run in Zephyr threads. Which ones should preempt which other ones, what priorities should be, what should preempt what, what we need deadlines for etc.?
dbaluta commentedon Mar 16, 2023
@lgirdwood I'm trying to wrap my head around this. I still need to read this several times. Anyhow, we are happy to test any change.
From what I understand this has something to do with changing the way we schedule IPC messages comming from Host processor?
aborisovich commentedon Mar 16, 2023
Unfortunately I think just as @dbaluta indicated:
Is unfortunately the best way to get the answers for the all questions. I can try to provide some answers to some questions here, but if further questions arise please try to get answers from the diagrams I uploaded. I'll do this in the form of Q&A list.
Q: How does the comm_scheduler looks like statically?
A: comm_scheduler is a one instance object for the whole system. Inside this instance we have a "set of mechanics" used by each Core. A single "set of mechanics" is:
comm_scheduler.sof_tasksoftaskstructs (that are IPC or IDC or AMS, have their UUIDs and custom callbacks forrun(),complete()andget_deadline()fptrs).comm_scheduler.sof_taskslist has elements added by the order of occurrence (the underlying list ofcomm_scheduler.message_queueis not - elements of that listk_p4wq_workare added to specific group (prepended or appended). Groups are regions of thecomm_scheduler.message_queuewhich define the execution priority for items).struct task* executed_taskpointing to currently executedtask(k_p4wq_worktask is actually executed but it maps back totaskand calls itstask.run()andtask.complete()callbacks over thestatic void comm_task_handler()function).thread_priority=0that picks up elements fromcomm_scheduler.message_queuelist head.comm_scheduler.message_queue(this happens inside the staticvoid comm_task_handler()body. There are 3 groups - Group0 (highest priority) AMStaskobjects, Group1 fortaskobjects of "ipc_task" type which is ipc response to host interrupt ipc message, and Group2: same as Group1 but those are ipc notifications (we will distinguish those by ipc header)).taskscheduled for execution may have CoreCount - 1 child task, that it waits for to complete on other cores.Q: What is preemptible?
A: If the
taskwill exceed deadline its thread priority will be lowered and it may be preempted until tasks from other schedulers are completed (eg. DP scheduler).Q: Can
taskbe rescheduled?A: Yes, it can be rescheduled eg. when during sending IPC response to host, host register state does not allow incoming IPC messages. This will cause a
taskto be rescheduled (while its parent task may complete execution eg. IDC on other core).Rescheduled task will be put again into the
comm_scheduler.message_queueto his matching group, but this time it will be prepended to the group not appended as new tasks created for scheduling (and will not schedule child IDC tasks which had already been completed).Q: How are deadlines calculated?
A: This is not implemented now but we are going to have predefined deadlines for the
tasktype. Please correct me If I am wrong @marcinszkudlinski .So... yeah. There is a lot to take.
aborisovich commentedon Mar 16, 2023
Unfortunately I did not see any low hanging fruits here. Well maybe except one that we will put existing zephyr_idc
that already uses k_p4wq into the new comm_scheduler that will use the same abstraction.
As I see it that different schedulers perform different tasks and are allowed to use different zephyr tools for underlying implementation. We can probably look for some common parts to make code smaller but doing it too forcefully will cause spaghetti code again.
aborisovich commentedon Mar 16, 2023
No ETAs for now. Job this size I would expect to be done by the Autumn to be honest.
However, we need to proceed somewhere with the code refactorings of communication layer to improve stability and performance so it's not like this topic is not relevant.
lyakh commentedon Mar 17, 2023
@aborisovich it still seems to me that all this information isn't answering my question. My question isn't about how this scheduler should work. It's about why we think that IPC and other messaging protocols need it? All the documentation I see here so far is built like: "here's a new scheduler, this is how it will work. And we want to use it for X." Whereas it seems to me, that a good approach would be: "we have services A, B, C, we're unhappy about how they're implemented now, and we think a good implementation should have features 1, 2, 3."
As a side note, not related to the main question above, but just as a reminder: we also had an idea of using zbus for them.
aborisovich commentedon Mar 19, 2023
@lyakh I've tried my best to describe those things in the issue description, in those two sections:
They contain:
If you have other concrete questions that are not asked or answered there, please go ahead and ask. I've tried to present the system picture as precisely as possible but it is hard to get a general picture with all the details.
Thanks for the remainder, I need to get more intel on this.
lyakh commentedon Mar 20, 2023
@aborisovich ok, right, thanks. A couple of thoughts:
Then yes, that answers my question of why we're unhappy with the current situation, but I still haven't quite found an answer to "what our goal is." Which I think should be a step before deciding on an implementation.
I.e. can we list all our messaging APIs and define how we want them to work - what all the contexts are (IPC request from the host, IPC response to the host, IPC notification to the host, notifications, AMS,...) in which Zephyr context they all should be executed (presumably all Zephyr threads), with which priorities, preemption, any deadlines or budgets etc.
abonislawski commentedon Mar 20, 2023
Example set of changes in Zephyr P4Q to support SOF_SCHEDULE_COMM:
This will allow to simplify P4Q changes and solve all @aborisovich requirements with few assumptions: group prepend/append done via group_id extending (like subgroups), FIFO order will be maintained because of SOF_SCHEDULE_COMM design (constant deadlines)
lyakh commentedon Mar 21, 2023
@aborisovich I'm actually not sure that connection is very strong. The IPC subsystem only uses the IPC task to offload the IPC processing from the interrupt context to a Zephyr thread. IPC sending is handled in Zephyr work queue...
andyross commentedon May 20, 2023
Just so folks are aware, I NAK'd the specific set of changes to p4wq, they really don't match the metaphor of the data structure. There are some suggestions there for how to get the same features in an app-managed compound data structure.
That said... upleveling this to SOF and not Zephyr: this protocol design seems to be getting extremely thick. Having been in the IPC layer(s) recently, it seems like a better starting point might be some cleanup at the protocol layer? Lots of the top level complexity seems poorly justified to me, and having to do surgery to the bottom-most bits to support the requirements of the knots at the top of the stack is likely to be counterproductive.
aborisovich commentedon Jun 21, 2023
Sure, please submit RFC with idea on how to refactor the IPC protocol in SOF with some concrete design.
It would probably include strong refactor of IPC3 which nxp guys are using, I'm not sure how welcome they would be to this idea. In my opinion it would be best to avoid any refactor now in SOF just because we would like everyone to migrate to IPC4 and remove IPC3 completely. Then it would be much easier to clean up lost of facade code in commons.
However @andyross you are keen on changes to IPC3 and IPC4 then please do submit your RFC.
kv2019i commentedon Aug 17, 2023
@lgirdwood @mwasko Development of this has stopped, assigned this to Michal you and clearing the milestone for now until we have clarity on how this will be taken forward.
kv2019i commentedon Dec 17, 2025
Work on this has stopped, closing this.