Add lock to mod resources tracking#10960
Open
jsarha wants to merge 3 commits into
Open
Conversation
added 3 commits
June 25, 2026 22:14
Convert mod_data_blob_handler_new() to a Zephyr syscall following the same pattern as mod_alloc_ext(), mod_free(), and mod_fast_get(). This allows modules running in user mode to call the function through the syscall interface. The z_vrfy_ handler validates the processing_module pointer and its heap region before dispatching to the implementation. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
Add a k_spinlock to struct module_resources and take it around every objpool access in z_impl_mod_alloc_ext(), z_impl_mod_data_blob_handler_new(), z_impl_mod_fast_get(), z_impl_mod_free(), and mod_free_all(). This prevents concurrent access to the resource tracking pool when a module's IPC thread and processing thread operate in parallel (e.g. set_large_config arriving while the module is running). Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
Remove the MEM_API_CHECK_THREAD() macro, CONFIG_MODULE_MEMORY_API_DEBUG Kconfig option, and the rsrc_mngr field from struct module_resources. This single-thread-owner check is now redundant: the previous commit added a spinlock that properly serializes all accesses to the resource pool, making the debug-only thread identity warning unnecessary. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
jsarha
requested review from
dbaluta,
kv2019i,
lbetlej,
lgirdwood,
mmaka1,
plbossart and
ranj063
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds locking around module resource tracking in the module adapter to allow safe resource bookkeeping when allocations/frees can occur from multiple threads (e.g., IPC thread vs DP thread). It also removes the now-obsolete thread-check Kconfig option and adjusts the blob handler creation API to follow the Zephyr syscall pattern in full Zephyr application builds.
Changes:
- Add a
k_spinlocktostruct module_resourcesand use it to protect tracked resource allocation/free paths. - Convert
mod_data_blob_handler_new()to a__syscallAPI for full Zephyr application builds (withz_impl_/z_vrfy_and mrsh glue). - Remove
MODULE_MEMORY_API_DEBUGKconfig and the associated thread-check logic.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/include/sof/audio/module_adapter/module/generic.h |
Adds spinlock to module resource struct; updates blob handler API to syscall vs z_impl_ mapping depending on build. |
src/audio/module_adapter/module/generic.c |
Initializes and applies the new spinlock around most resource tracking operations; adds syscall verification/marshalling for blob handler creation. |
src/audio/module_adapter/Kconfig |
Removes the legacy thread-safety debug-check option. |
Comment on lines
179
to
183
| struct module_resources *res = &mod->priv.resources; | ||
| struct module_resource *container; | ||
|
|
||
| MEM_API_CHECK_THREAD(res); | ||
|
|
||
| container = container_get(mod); | ||
| if (!container) |
lyakh
requested changes
Jun 26, 2026
| k_spin_unlock(&res->lock, key); | ||
|
|
||
| /* Make sure resource lists and accounting are reset */ | ||
| mod_resource_init(mod); |
| struct module_resources *res = &mod->priv.resources; | ||
|
|
||
| /* Init memory list */ | ||
| k_spinlock_init(&res->lock); |
Collaborator
There was a problem hiding this comment.
why a spinlock? Spinlocks disable interrupts. I'd really prefer a mutex if possible
Contributor
Author
There was a problem hiding this comment.
Well, the memory operations should be fast, but sure I can easily make it mutex.
|
|
||
| MEM_API_CHECK_THREAD(res); | ||
|
|
||
| key = k_spin_lock(&res->lock); |
Collaborator
There was a problem hiding this comment.
so, I suppose we should add locking everywhere where we currently call MEM_API_CHECK_THREAD(), so I've also missed mod_balloc_align(), right?
Contributor
Author
There was a problem hiding this comment.
Yep, Copilot pointed that out too, I am already doing that.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is a result from the discussion in this PR: #10862