module: memory: partition out memory APIs from generic.c #10426

lgirdwood · 2025-12-10T17:29:11Z

Move all the module memory APIs into common and heap specific files with no other changes.

This is to enable easier abstraction and partitioning around memory for modules and userspace going forward.

Copilot

Pull request overview

This PR refactors module memory management by extracting memory-related APIs from generic.c into two new dedicated files: memory-common.c and memory-heap.c. This reorganization improves code modularity and facilitates future abstractions for module memory management and userspace integration.

Key changes:

Extracted heap-related memory operations into memory-heap.c
Moved common memory initialization and wrapper functions to memory-common.c
Updated build configurations to include the new source files

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`src/audio/module_adapter/module/memory-heap.c`	New file containing heap allocation/deallocation functions previously in `generic.c`
`src/audio/module_adapter/module/memory-common.c`	New file containing module initialization and memory API wrappers
`src/audio/module_adapter/module/generic.c`	Removed memory management code that was moved to new files
`src/audio/module_adapter/module_adapter.c`	Removed `module_adapter_mem_alloc` and `module_adapter_mem_free` functions (moved to `memory-heap.c`)
`src/include/sof/audio/module_adapter/module/generic.h`	Added declarations for newly exposed functions
`src/audio/module_adapter/CMakeLists.txt`	Updated to compile new memory source files
`test/cmocka/src/audio/*/CMakeLists.txt`	Updated test build configurations to link new memory source files

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-10T17:29:57Z

src/audio/module_adapter/module/memory-heap.c

+// Author: Marcin Rajwa <[email protected]>
+
+/*
+ * \file generic.c


The file header comment incorrectly references 'generic.c'. This should be updated to 'memory-heap.c' to accurately reflect the current filename.

Suggested change

* \file generic.c

* \file memory-heap.c

Copilot · 2025-12-10T17:29:57Z

src/audio/module_adapter/module/memory-common.c

+// Author: Marcin Rajwa <[email protected]>
+
+/*
+ * \file generic.c


The file header comment incorrectly references 'generic.c'. This should be updated to 'memory-common.c' to accurately reflect the current filename.

Suggested change

* \file generic.c

* \file memory-common.c

This seems valid.

Copilot · 2025-12-10T17:29:58Z

src/audio/module_adapter/module/memory-heap.c

+	void *mod_heap_buf = mod_heap_mem + heap_prefix_size;
+
+	k_heap_init(mod_heap, mod_heap_buf, heap_size - heap_prefix_size);
+


The module_adapter_dp_heap_new function is missing the Zephyr-specific initialization code that was present in the original implementation (lines setting mod_heap->heap.init_mem and mod_heap->heap.init_bytes under #ifdef __ZEPHYR__). This code should be restored after line 405 to maintain compatibility with Zephyr builds.

Suggested change

#ifdef __ZEPHYR__

mod_heap->heap.init_mem = mod_heap_buf;

mod_heap->heap.init_bytes = heap_size - heap_prefix_size;

#endif

This seems valid.

it is - fixed.

kv2019i · 2025-12-11T08:54:21Z

src/audio/module_adapter/module/memory-common.c

+// Author: Marcin Rajwa <[email protected]>
+
+/*
+ * \file generic.c


This seems valid.

softwarecki · 2025-12-11T09:44:25Z

I can understand extracting functions related to memory allocation (mod_alloc, mod_free, etc.) into a separate file. However, the rest of the changes do not make sense in my opinion. Moving functions like module_init, module_free, module_adapter_mem_alloc, and module_adapter_mem_free seems unnecessary and introduces more confusion. Can you provide a clear justification for these relocations? Could you share what the next steps are in this refactoring? It would help clarify the overall direction.

softwarecki · 2025-12-11T09:13:49Z

src/audio/module_adapter/module/memory-common.c

+
+LOG_MODULE_DECLARE(module_adapter, CONFIG_SOF_LOG_LEVEL);
+
+int module_init(struct processing_module *mod)


This function does not appear to be related to memory. Can you explain why it was moved?

This one doesn't do any memory allocations directly, they are done via other calls so its quite generic.

@lgirdwood I agree with @softwarecki on this - even though this function does call mod_resource_init() and sets mod->priv.resources.rsrc_mngr its main function is to call interface->init() so it should stay together with other functions, calling other methods in struct module_interface like module_process_sink_src(), module_reset() etc. - in generic.c

This is to avoid duplication of this function, i.e. the same code can be used with with both heap and virtual region

softwarecki · 2025-12-11T09:20:08Z

src/audio/module_adapter/module/memory-heap.c

+ * Like comp_data_blob_handler_new() but the handler is automatically freed.
+ */
+#if CONFIG_COMP_BLOB
+struct comp_data_blob_handler *mod_data_blob_handler_new(struct processing_module *mod)


It seems this function has nothing to do with heap and fits better under the memory module. I assume the reason for placing it here is that it uses the static container_get and therefore must stay in this file.

It does - virtual regions wont need to track and will allocate differently.

softwarecki · 2025-12-11T09:22:20Z

src/audio/module_adapter/module/memory-heap.c

+}
+EXPORT_SYMBOL(mod_free_all);
+
+int module_free(struct processing_module *mod)


This function does not appear to be related to memory. Can you explain why it was moved?

It does some memory freeing, not quite the opposite of init() which is common.

softwarecki · 2025-12-11T09:23:12Z

src/audio/module_adapter/module/memory-heap.c

+	void *mod_heap_buf = mod_heap_mem + heap_prefix_size;
+
+	k_heap_init(mod_heap, mod_heap_buf, heap_size - heap_prefix_size);
+


This seems valid.

softwarecki · 2025-12-11T09:23:44Z

src/audio/module_adapter/module/memory-heap.c

+	return mod_heap;
+}
+
+struct processing_module *module_adapter_mem_alloc(const struct comp_driver *drv,


It seems this function has nothing to do with heap

It does many allocations and creates a heap.

The heap is created by function module_adapter_dp_heap_new. The file name suggests that functions operating on the heap should be placed here. This function only uses the allocator and does not create it.

and none of this is needed for regions, it is heap only based logic.

softwarecki · 2025-12-11T09:25:05Z

src/audio/module_adapter/module/memory-heap.c

+	return NULL;
+}
+
+void module_adapter_mem_free(struct processing_module *mod)


see above, we are freeing memory here.

This function only calls the memory release routine. It does not operate on the heap directly but simply use sof_heap_free. Based on this logic, we could move here any function that invokes any function related to memory allocation.

we wont always be using a heap.

Move all the module memory APIs into common and heap specific files with no other changes. This is to enable easier abstraction and partitioning around memory for modules and userspace going forward. Signed-off-by: Liam Girdwood <[email protected]>

softwarecki

I see that these two functions are the only places in module_adapter where sof_heap_alloc and sof_heap_free are called. I assume this was the criterion for moving them. Does the plan include forking these new files later to use the new allocator you are working on? If so, feel free to admit it, I will not mind at all.

softwarecki · 2025-12-12T09:15:43Z

src/audio/module_adapter/module/memory-heap.c

+	return NULL;
+}
+
+void module_adapter_mem_free(struct processing_module *mod)


This function only calls the memory release routine. It does not operate on the heap directly but simply use sof_heap_free. Based on this logic, we could move here any function that invokes any function related to memory allocation.

softwarecki · 2025-12-12T09:27:01Z

src/audio/module_adapter/module/memory-heap.c

+	return mod_heap;
+}
+
+struct processing_module *module_adapter_mem_alloc(const struct comp_driver *drv,


The heap is created by function module_adapter_dp_heap_new. The file name suggests that functions operating on the heap should be placed here. This function only uses the allocator and does not create it.

lgirdwood · 2025-12-12T14:44:14Z

I see that these two functions are the only places in module_adapter where sof_heap_alloc and sof_heap_free are called. I assume this was the criterion for moving them. Does the plan include forking these new files later to use the new allocator you are working on? If so, feel free to admit it, I will not mind at all.

Yes your right, so anywhere we use the sof_heap_alloc()/free() is where I'm splitting today. However, the vregion is partitioned to include zephyr heaps for interim usage so it may well be as we integrate more code from memory-heap.c to memory-common.c as we go, its just we can easily do this today without breaking too much (and we depend on kernel patches too). Hence the need for a memory-region.c in a future PR which we can then incrementally move common code in memory-common.c and keep the heap/region specific code apart where needed.

Copilot AI review requested due to automatic review settings December 10, 2025 17:29

lgirdwood requested review from dbaluta, jxstelter, kv2019i, lbetlej, mmaka1, plbossart and ranj063 as code owners December 10, 2025 17:29

Copilot AI reviewed Dec 10, 2025

View reviewed changes

kv2019i approved these changes Dec 11, 2025

View reviewed changes

softwarecki reviewed Dec 11, 2025

View reviewed changes

lgirdwood force-pushed the topic/mod-mem-split branch from 8ebc3c4 to e5ca01c Compare December 11, 2025 17:21

softwarecki reviewed Dec 12, 2025

View reviewed changes

softwarecki requested a review from abonislawski December 12, 2025 09:28

		void *mod_heap_buf = mod_heap_mem + heap_prefix_size;

		k_heap_init(mod_heap, mod_heap_buf, heap_size - heap_prefix_size);

+#ifdef __ZEPHYR__
+	mod_heap->heap.init_mem = mod_heap_buf;
+	mod_heap->heap.init_bytes = heap_size - heap_prefix_size;
+#endif


		LOG_MODULE_DECLARE(module_adapter, CONFIG_SOF_LOG_LEVEL);

		int module_init(struct processing_module *mod)

module: memory: partition out memory APIs from generic.c #10426

Are you sure you want to change the base?

module: memory: partition out memory APIs from generic.c #10426

Uh oh!

Conversation

lgirdwood commented Dec 10, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

softwarecki commented Dec 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

softwarecki left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

lgirdwood commented Dec 12, 2025 •

edited

Loading