audio: module_adapter: remove unnecessary IRQ lock/unlock#10974
Conversation
In the past, the module adapter prepare and free methods called buffer_attach() and buffer_detach() directly. To call these methods safely, it was required to disable IRQs. See commit 3e3d0cd ("buffer: prevent cache corruption") for history and rationale. However, in commit ecc55f8 ("buf: split sink_list connector to 2 fields") the direct calls to buffer.h were removed. The locks were still kept. Reviewing the code today, the protected operations work on buffer list maintained in "mod->raw_data_buffers_list". This list is used only by the module adapter itself. External connections go through component "dev->bsink_list" and "dev->bsource_list", which are separate objects and only used in prepare() by the module adapter. As the pipeline code will ensure module's copy() is not called concurrently with prepare() and free(), it is no longer required to use locks to protect against list operations on the internal list. Removing direct calls to disable interrupts allows to run this code in user-space. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
|
For context, related to #10558 |
There was a problem hiding this comment.
Pull request overview
This PR removes local IRQ disable/enable around internal raw_data_buffers_list list updates in the module adapter’s prepare()/free() paths. This aligns the implementation with the current design where these buffers are owned and managed solely by the module adapter, and helps enable running this code in user-space.
Changes:
- Removed
irq_local_disable()/irq_local_enable()guardingraw_data_buffers_listprepend/delete operations inmodule_adapter_prepare(). - Removed the same IRQ guard around
raw_data_buffers_listdeletions inmodule_adapter_free(). - Cleaned up now-unneeded
flagslocals associated with the removed IRQ locking.
| buff_size, memory_flags, | ||
| PLATFORM_DCACHE_ALIGN, | ||
| BUFFER_USAGE_NOT_SHARED); | ||
| uint32_t flags; |
There was a problem hiding this comment.
I tried to follow the flow along the lines of:
ipc4_user_process_glb_message() ->
ipc4_set_pipeline_state() ->
ipc4_pipeline_prepare() ->
switch (status) {
case COMP_STATE_READY:
ipc4_pcm_params() ->
pipeline_prepare() ->
pipeline_comp_prepare() ->
comp_prepare() ->
module_adapter_prepare()...
so, yes, looks like this won't run if the pipeline is already running
In the past, the module adapter prepare and free methods called buffer_attach() and buffer_detach() directly. To call these methods safely, it was required to disable IRQs. See commit 3e3d0cd ("buffer: prevent cache corruption") for history and rationale.
However, in commit ecc55f8 ("buf: split sink_list connector to 2 fields") the direct calls to buffer.h were removed. The locks were still kept.
Reviewing the code today, the protected operations work on buffer list maintained in "mod->raw_data_buffers_list". This list is used only by the module adapter itself. External connections go through component "dev->bsink_list" and "dev->bsource_list", which are separate objects and only used in prepare() by the module adapter.
As the pipeline code will ensure module's copy() is not called concurrently with prepare() and free(), it is no longer required to use locks to protect against list operations on the internal list. Removing direct calls to disable interrupts allows to run this code in user-space.