Skip to content

module_adapter: dp: Improve module adapter creation function #10565

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
softwarecki wants to merge 5 commits into
base: main
Choose a base branch
from
+23 −44
Open

module_adapter: dp: Improve module adapter creation function #10565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
07d213a
module_adapter: Fill module fields before starting dp thread
softwarecki Feb 13, 2026
31573c9
module_adapter: dp: Add check for DP task creation error
softwarecki Feb 13, 2026
e32e702
module_adapter: Fix use after free in module_adapter_new_ext
softwarecki Feb 13, 2026
d099a1b
dp: thread: Remove user_memory_init_shared()
softwarecki Feb 23, 2026
f2a7c05
west.yml: update zephyr to d885cfebaa3
softwarecki Feb 23, 2026
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
22 changes: 11 additions & 11 deletions src/audio/module_adapter/module_adapter.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,22 +226,26 @@ struct comp_dev *module_adapter_new_ext(const struct comp_driver *drv,
if (!mod)
return NULL;

module_set_private_data(mod, mod_priv);
list_init(&mod->raw_data_buffers_list);
#if CONFIG_USERSPACE
mod->user_ctx = user_ctx;
#endif /* CONFIG_USERSPACE */

struct comp_dev *dev = mod->dev;

#if CONFIG_ZEPHYR_DP_SCHEDULER
/* create a task for DP processing */
if (config->proc_domain == COMP_PROCESSING_DOMAIN_DP) {
/* All data allocated, create a thread */
pipeline_comp_dp_task_init(dev);
ret = pipeline_comp_dp_task_init(dev);
if (ret) {
comp_cl_err(drv, "DP task creation failed with error %d.", ret);
goto err;
}
}
#endif /* CONFIG_ZEPHYR_DP_SCHEDULER */

module_set_private_data(mod, mod_priv);
list_init(&mod->raw_data_buffers_list);
#if CONFIG_USERSPACE
mod->user_ctx = user_ctx;
#endif /* CONFIG_USERSPACE */
Copy link
Collaborator

@lyakh lyakh Feb 23, 2026

Choose a reason for hiding this comment

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

I suppose this isn't breaking anything and in fact does seem logical, but for the understanding: is this use of uninitialised data really happening? This is called in IPC context. The DP thread suspends immediately when started - I suppose in the "thread" variant too? The thread shouldn't be woken up before this IPC processing completes?

Copy link
Collaborator Author

@softwarecki softwarecki Feb 23, 2026

Choose a reason for hiding this comment

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

Yes, the dp thread creation function uses user_ctx. If we don't assign this value first, a kernel dp thread will be created instead of a userspace thread.


dst = &mod->priv.cfg;
/*
* NOTE: dst->ext_data points to stack variable and contains
Expand Down Expand Up @@ -330,10 +334,6 @@ struct comp_dev *module_adapter_new_ext(const struct comp_driver *drv,
schedule_task_free(dev->task);
#endif
module_adapter_mem_free(mod);
#if CONFIG_IPC_MAJOR_4
dst->ext_data = NULL;
#endif

return NULL;
}

Expand Down
19 changes: 11 additions & 8 deletions src/schedule/zephyr_dp_schedule_thread.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <rtos/task.h>

#include <sof/audio/module_adapter/module/generic.h>
#include <sof/audio/module_adapter/library/userspace_proxy.h>
#include <sof/common.h>
#include <sof/list.h>
#include <sof/schedule/ll_schedule_domain.h>
Expand Down Expand Up @@ -262,12 +263,12 @@ int scheduler_dp_task_init(struct task **task,
/* success, fill the structures */
pdata->p_stack = p_stack;
pdata->mod = mod;
*task = &task_memory->task;

/* create a zephyr thread for the task */
pdata->thread_id = k_thread_create(pdata->thread, (__sparse_force void *)p_stack,
stack_size, dp_thread_fn, *task, NULL, NULL,
CONFIG_DP_THREAD_PRIORITY, (*task)->flags, K_FOREVER);
stack_size, dp_thread_fn, &task_memory->task, NULL, NULL,
CONFIG_DP_THREAD_PRIORITY, task_memory->task.flags,
K_FOREVER);

/* pin the thread to specific core */
ret = k_thread_cpu_pin(pdata->thread_id, core);
Expand All @@ -277,13 +278,13 @@ int scheduler_dp_task_init(struct task **task,
}

#ifdef CONFIG_USERSPACE
k_thread_access_grant(pdata->thread_id, pdata->event);
scheduler_dp_grant(pdata->thread_id, cpu_get_id());
if (options & K_USER) {
k_thread_access_grant(pdata->thread_id, pdata->event);
scheduler_dp_grant(pdata->thread_id, core);

if ((*task)->flags & K_USER) {
ret = user_memory_init_shared(pdata->thread_id, pdata->mod);
ret = k_mem_domain_add_thread(pdata->mod->user_ctx->comp_dom, pdata->thread_id);
if (ret < 0) {
tr_err(&dp_tr, "user_memory_init_shared() failed");
tr_err(&dp_tr, "k_mem_domain_add_thread() failed %d", ret);
goto e_thread;
}
}
Expand All @@ -293,6 +294,8 @@ int scheduler_dp_task_init(struct task **task,
k_event_init(pdata->event);
k_thread_start(pdata->thread_id);

/* success, fill output parameter */
*task = &task_memory->task;
return 0;

e_thread:
Expand Down
2 changes: 1 addition & 1 deletion west.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ manifest:

- name: zephyr
repo-path: zephyr
revision: b246d3f3f987f5ddfa6530bea2082c09ce1f00d8
revision: d885cfebaa3464bf8bbf731a8b7f94854a90ae15
remote: zephyrproject

# Import some projects listed in zephyr/west.yml@revision
Expand Down
12 changes: 0 additions & 12 deletions zephyr/include/rtos/userspace_helper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,18 +42,6 @@ struct userspace_context;
*/
struct k_heap *module_driver_heap_init(void);

/**
* Add DP scheduler created thread to module memory domain.
* @param thread_id - id of thread to be added to memory domain.
* @param module - processing module structure
*
* @return 0 for success, error otherwise.
*
* @note
* Function used only when CONFIG_USERSPACE is set.
*/
int user_memory_init_shared(k_tid_t thread_id, struct processing_module *mod);

/**
* Attach common userspace memory partition to a module memory domain.
* @param dom - memory domain to attach the common partition to.
Expand Down
12 changes: 0 additions & 12 deletions zephyr/lib/userspace_helper.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,18 +77,6 @@ int user_stack_free(void *p_stack)
return k_thread_stack_free(p_stack);
}

int user_memory_init_shared(k_tid_t thread_id, struct processing_module *mod)
{
struct k_mem_domain *comp_dom = mod->user_ctx->comp_dom;
int ret;

ret = k_mem_domain_add_partition(comp_dom, &common_partition);
if (ret < 0)
return ret;

return k_mem_domain_add_thread(comp_dom, thread_id);
}

int user_memory_attach_common_partition(struct k_mem_domain *dom)
{
return k_mem_domain_add_partition(dom, &common_partition);
Expand Down