From b490e89b8c26404d25de8d65b2aa8a0c093cf3ce Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 21 Feb 2023 15:15:07 +0100 Subject: [PATCH 1/2] ll-schedule: fix tasks removed during execution Zephyr LL-scheduler is supposed to be able to handle tasks, removed or added while the scheduler is executing. However, there is a bug in that implementation. If the task, that is currently executing, is removed, its list head, that links it into the list of tasks, is initialised. So when trying to get a pointer to the next task we obtain a pointer to the same task. BugLink: https://github.com/thesofproject/sof/issues/7084 Signed-off-by: Guennadi Liakhovetski --- src/schedule/zephyr_ll.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/schedule/zephyr_ll.c b/src/schedule/zephyr_ll.c index d3947e522868..058548695861 100644 --- a/src/schedule/zephyr_ll.c +++ b/src/schedule/zephyr_ll.c @@ -162,19 +162,20 @@ static void zephyr_ll_run(void *data) { struct zephyr_ll *sch = data; struct task *task; - struct list_item *list; + struct list_item *list, *tmp, task_head = LIST_INIT(task_head); uint32_t flags; zephyr_ll_lock(sch, &flags); /* - * We have to traverse the list manually, because we drop the lock while - * executing tasks, at that time tasks can be removed from or added to - * the list. + * We drop the lock while executing tasks, at that time tasks can be + * removed from or added to the list, including the task that was + * executed. Use a temporary list to make sure that the main list is + * always consistent and contains the tasks, that we haven't run in this + * cycle yet. */ - list = sch->tasks.next; - while (list != &sch->tasks) { + for (list = sch->tasks.next; !list_is_empty(&sch->tasks); list = sch->tasks.next) { enum task_state state; struct zephyr_ll_pdata *pdata; @@ -190,6 +191,10 @@ static void zephyr_ll_run(void *data) pdata->run = true; task->state = SOF_TASK_STATE_RUNNING; + /* Move the task to a temporary list */ + list_item_del(list); + list_item_append(list, &task_head); + zephyr_ll_unlock(sch, &flags); /* @@ -207,12 +212,6 @@ static void zephyr_ll_run(void *data) zephyr_ll_lock(sch, &flags); - /* - * The .next pointer could've been changed while the lock wasn't - * held - */ - list = list->next; - if (pdata->freeing) { /* * zephyr_ll_task_free() is trying to free this task. @@ -238,6 +237,12 @@ static void zephyr_ll_run(void *data) } } + /* Move tasks back */ + list_for_item_safe(list, tmp, &task_head) { + list_item_del(list); + list_item_append(list, &sch->tasks); + } + zephyr_ll_unlock(sch, &flags); notifier_event(sch, NOTIFIER_ID_LL_POST_RUN, From b3a808afa8c767d8ff0a925eddd902e371ca5d6e Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 24 Feb 2023 16:38:28 +0100 Subject: [PATCH 2/2] chain-dma: fix scheduling exception Task state shouldn't be modified by client code, it is fully managed by the scheduler. Setting task status to INIT after scheduling a task is wrong and for chain DMA it leads to a scheduler exception. Simply remove the offending line. BugLink: https://github.com/thesofproject/sof/issues/7084 Signed-off-by: Guennadi Liakhovetski --- src/audio/chain_dma.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/audio/chain_dma.c b/src/audio/chain_dma.c index 6786065656ad..ea536f0472aa 100644 --- a/src/audio/chain_dma.c +++ b/src/audio/chain_dma.c @@ -303,7 +303,6 @@ static int chain_task_start(struct comp_dev *dev) } pm_policy_state_lock_get(PM_STATE_RUNTIME_IDLE, PM_ALL_SUBSTATES); - cd->chain_task.state = SOF_TASK_STATE_INIT; k_spin_unlock(&drivers->lock, key); return 0;