Skip to content

ipc: ipc-helper: use list_mutex to guard buffer list in userspace LL #10762

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kv2019i wants to merge 1 commit into
base: main
Choose a base branch
from
+13 −0
Open

ipc: ipc-helper: use list_mutex to guard buffer list in userspace LL #10762

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/ipc/ipc-helper.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,16 @@ __cold int ipc_comp_free(struct ipc *ipc, uint32_t comp_id)
return -EINVAL;
}

/* Lock buffer lists to prevent racing with the LL scheduler.
* In user-space builds, irq_local_disable() is a privileged
* operation, so use the per-component list_mutex instead
* (same pattern as PPL_LOCK in pipeline_disconnect()).
*/
#ifdef CONFIG_SOF_USERSPACE_LL
sys_mutex_lock(&icd->cd->list_mutex, K_FOREVER);
#else
Comment on lines +341 to +343
irq_local_disable(flags);
#endif
Comment on lines +341 to +345
Copy link
Copy Markdown
Member

@lgirdwood lgirdwood May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we can just replace irq_local_disable() regardless of user or not.

Copy link
Copy Markdown
Collaborator

@lyakh lyakh May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kv2019i how will buffer walking during actual pipeline processing in

sof/src/audio/pipeline/pipeline-stream.c

Line 135 in 58c5c5d

err = pipeline_for_each_comp(current, ctx, dir);
be protected after this change? Since we currently don't run cross-core in our tests (AFAIK), locking interrupts protected us against racing with the scheduler, so what will protect the scheduler after this change?

Copy link
Copy Markdown
Collaborator Author

@kv2019i kv2019i May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this is an oversight. The LL thread priority will protect once LL walk has started, but indeed this does not protect against the case where LL tick happens in middle of ipc_comp_free(). Let me work on improving this.

In principle @lgirdwood we could apply this for all builds, but this does increase regression risk and above case is a good example.

comp_dev_for_each_producer_safe(icd->cd, buffer, safe) {
comp_buffer_set_sink_component(buffer, NULL);
/* This breaks the list, but we anyway delete all buffers */
Expand All @@ -346,7 +355,11 @@ __cold int ipc_comp_free(struct ipc *ipc, uint32_t comp_id)
comp_buffer_reset_source_list(buffer);
}

#ifdef CONFIG_SOF_USERSPACE_LL
sys_mutex_unlock(&icd->cd->list_mutex);
#else
irq_local_enable(flags);
#endif

/* free component and remove from list */
comp_free(icd->cd);
Expand Down
Loading