From 2b572c7d3bd41f7ee8eaa292f34b80533bdef821 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 21 Feb 2023 15:15:07 +0100 Subject: [PATCH] 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 ea361ef98..a2e471063 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,