Module heap api #10141

jsarha · 2025-07-28T20:50:42Z

Module memory allocation API functions face lift. Once this is approved I go through the rest of the modules after SRC.

Copilot

Pull Request Overview

This PR introduces a module-level memory allocation API to replace direct memory allocation calls, providing better memory management tracking for audio modules. The changes rename and consolidate memory allocation functions to use a standardized module-based approach.

Replaced direct rzalloc, rballoc, and rfree calls with new module-based allocation functions
Added module-level memory tracking with automatic cleanup
Updated function signatures to pass processing modules instead of component devices

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
`src/include/sof/audio/module_adapter/module/generic.h`	Updates API declarations, replacing old allocation functions with new `mod_alloc`, `mod_zalloc`, and `mod_free`
`src/audio/module_adapter/module/generic.c`	Implements the new module allocation API with memory tracking and adds `mod_zalloc` function
`src/audio/module_adapter/module_adapter.c`	Adds automatic cleanup of tracked module memory allocations during component free
`src/audio/src/*.c`	Updates SRC audio modules to use new allocation API instead of direct memory calls
`src/audio/module_adapter/module/waves/waves.c`	Migrates Waves audio processing module to use new allocation functions
`src/audio/module_adapter/module/cadence.c`	Updates Cadence codec module to use new memory allocation API
`src/audio/codec/dts/dts.c`	Converts DTS codec to use module-based memory allocation

Comments suppressed due to low confidence (2)

src/audio/module_adapter/module_adapter.c:1232

The variable name err_count is misleading as it's counting memory chunks that were properly freed, not errors. Consider renaming to freed_count or chunk_count.

		err_count++;

src/audio/module_adapter/module_adapter.c:1235

The variable name err_count is confusing in this context as it's used to warn about successfully freed memory chunks, not errors.

	if (err_count)

src/audio/module_adapter/module/generic.c

lgirdwood

LGTM, just one open.

src/audio/codec/dts/dts.c

lyakh · 2025-07-30T13:33:40Z

src/audio/module_adapter/module_adapter.c

+		rfree(mem->ptr);
+		list_item_del(&mem->mem_list);
+		rfree(mem);
+		err_count++;


is this a bug or is this a feature? I thought that the purpose of this managed allocation was the same as in the Linux kernel - you allocate what you need per "module" and in the end it's all freed automatically when the module is freed?

I guess that is upto discussion. Its easy enough to drop the warning message.

Warning removed, and redundant - init failure branch or src_free() calls to mod_free() - removed.

src/audio/src/src_common.c

lgirdwood

@jsarha changes LGTM, but many CI errors. I think we can merge once passing CI and then follow up and convert remaining modules to use high level API.

@softwarecki the intention here is this "high level" module alloc API will map the allocation request to the correct user heap and memory domain. i.e. the module context will have the domain ID which will be mapped to the heap by the infra.

Rename module_allocate_memory and module_free_memory to mod_alloc and mod_free. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

Rename module_free_all_memory() to mod_free_all(). Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

Container memory should be freed if the requested memory allocation fails. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

Add mod_zalloc() function that is otherwise the same as mod_alloc(), but allocated memory is initialized to zero. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

lyakh · 2025-08-04T07:03:23Z

src/audio/module_adapter/module/generic.c

 	if (!ptr) {
 		comp_err(dev, "mod_alloc: failed to allocate memory for comp %x.",
 			 dev_comp_id(dev));
+		rfree(container);


a proposal for a future improvement: I think we'll be doing lots of module-managed allocating and freeing, so maybe a pool of container objects would make sense?

Yes. Reserving only couple of words long containers separately from heap is very inefficient. I'll do that as a follow up

lyakh · 2025-08-04T07:05:01Z

src/audio/module_adapter/module/cadence.c

 		return ret;
 	}

-	cd->mem_tabs = mod_alloc(mod, mem_tabs_size, 4);


are all rmalloc() allocations guaranteed to be 4-bytes aligned?

And rballoc, which is the backend used, is aligned to cache line lenght. The shortest cache line I could find from SOF supported platforms was 32 bytes. But still I left the 16 byte alignment in place, but certainly any heap implementation will retun a point at 32-bit word boundary, won't it 😅?

lyakh · 2025-08-04T07:09:03Z

src/audio/module_adapter/module/generic.c

+ *
+ * This function is called automatically when the module is unloaded.
+ */
 void mod_free_all(struct processing_module *mod)


in fact I'm wondering if mod_free_all needs to be exported at all? Modules themselves shouldn't call it, it should only be called by the framework before freeing the actual module

Probably yes, but that change can not part of this commit, but that should be done as a separate commmit after "module_adapter: Free all module associated memory module_adapter_free()".

... but no, that is not as easy as one might think. Cadence codec API module uses mod_free_all() in reset() [1] call back. So I'll leave this as it is for the moent.

[1]

sof/src/audio/module_adapter/module/cadence.c

Line 850 in bd08ae0

module_free_all_memory(mod);

... but no, that is not as easy as one might think. Cadence codec API module uses mod_free_all() in reset() [1] call back. So I'll leave this as it is for the moent.

@jsarha this is interesting... So module and component structures aren't allocated in module-managed memory? If they were and you freed them in .reset() that would seem like a typical self-inflicted foot injury case to me

Yes, that is indeed the case. It uses normal heap allocations for that and mod_alloc() for prepare etc. allocations, so it can just drop them with one call at reset. Need to sort that out at some point, but I leave it for later.

lyakh · 2025-08-04T07:18:33Z

src/audio/module_adapter/module_adapter.c

 }
 EXPORT_SYMBOL(module_adapter_free);

+size_t module_adapter_heap_usage(struct comp_dev *dev)


does the argument have to be struct comp_dev? This is module-specific, isn't it? And mod_alloc() uses struct processing_module too

Guess not. But it saves some code at the only place that is actually using the function ATM. But in the end ist not big deal, one way or the other.

lyakh · 2025-08-04T07:20:10Z

src/include/sof/audio/module_adapter/module/generic.h

 int module_adapter_cmd(struct comp_dev *dev, int cmd, void *data, int max_data_size);
 int module_adapter_trigger(struct comp_dev *dev, int cmd);
 void module_adapter_free(struct comp_dev *dev);
+size_t module_adapter_heap_usage(struct comp_dev *dev);


is this a good place for this prototype? Prototypes around it belong to the component API, which this function doesn't belong to

Let's see if I can find a better place for it.

lyakh · 2025-08-04T07:27:41Z

zephyr/sof_shell.c


+	SHELL_CMD(module_heap_usage, NULL,
+		  "Print heap memory usage of each module\n",
+		  cmd_sof_module_heap_usage),


would it be good to make it similar to free (1)? Call it e.g. mod_free or modfree and maybe format output a bit similar (at least inspired by) to that of free? Can we print how much heap the modules claim to need?

Can you be a little more specific on what you'd like to see here? free provides system wide information, but here I was specifically asked to provide per module usage, and nothing more. System wide information does not fall into scope of this PR.

But if you are talking about some per module statistics, then ATM there is no such thing. Once memory is freed its gone and there is not trace or statistics about it. I can add some statistics collection like that (memory high watermark per module or something), but is the wasted cycles and memory really worth it? Most of the modules will anyway only reserve the memory they need and then its all reserved until the module unloads.

But the last request - the topology max heap usage number - should be easy to add. Wonder if I should also add some warning print if any module exceeeds its max heap usage number.

yes, basically I meant adding maximum size from the topology

lyakh · 2025-08-04T07:37:13Z

src/audio/src/src_common.c

 {
+#if CONFIG_FAST_GET
 	struct comp_data *cd = module_get_private_data(mod);
+#endif


could just move it after line 715/718 below

To do that I may have to use some antihistamine to overcome my allergic reations to mixing code and variable declarations.

softwarecki · 2025-08-04T18:11:12Z

src/audio/module_adapter/module/generic.c


 	if (!size) {
-		comp_err(dev, "module_allocate_memory: requested allocation of 0 bytes.");
+		comp_err(dev, "mod_alloc: requested allocation of 0 bytes.");


Do we really need to include the function name in the error message? Zephyr automatically prefixes log entries with the function name.

No, but I do not want to mess with this commit that is strictly replace-string operation on the source tree. Maybe I should go through all our modules in another commit, in the next PR.

actually, now that we are Zephyr only, we should use LOG(), but this can be another PR.

@lgirdwood didn't we already conclude that no, we want to keep audio specific macros for audio codes that add audio topology information (component and pipe ids) to the log messages. For generic code outside modules, using plain Zephyr LOG is then ok (when not in module context).

Free all module associated memory module_adapter_free() by calling mod_free_all(). Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

Drop alignment argument from mod_alloc() and introduce mod_alloc_align() and apply all the necessary changes to mod_alloc() calls. Also adds Doxygen documentation to mod_alloc_align, mod_alloc, mod_zalloc, mod_free, and mod_free_all. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

Remove redundant mod_free() and mod_free_all() calls. The calls became redundant after mod_free_all() was added to module_adapter_free(). This change concerns only the modules that are already exclusively using mod_alloc() and friends from module API. Namely, Cadence Codec API (sof/audio/module_adapter/module/cadence.c) was left out even thou it uses mod_alloc() and friends, since it still also uses direct rballoc(), rzalloc(), rmalloc(), and rfree() calls. Cadence Codec API will be dealt with the rest of the modules that do not yet use module heap API. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

Adds exported module_adapter_heap_usage() function that returns current heap usage of the module in question. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

Fix implicit declaration of function 'strtol' by including stdlib.h. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

Declare struct comp_dev to avoid warnings if ll_schedule_domain.h is included before struct comp_dev is defined. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

Adds new command to sof sub-command set. The new command, module_heap_usage, prints out all active component ids, their heap memory usage allocated through module API, and configured maximum heap size. NOTE: For the moment there is nothing preventing a module from allocating more memory than the configured maximum. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

Change all memory allocations to go through module API mod_alloc(), mod_zalloc(), and mod_free(). The mod_free() calls are dropped form error handling branches and from src_free() as the memory is anyway freed as the module should unload shortly. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

kv2019i

I guess ok, but some of the renames are not explained in commit messages and leave me wondering why the commit is done.

Oh, and this changes the module generic.h interface. We are 100% sure there are no external users for this interface? @lgirdwood @abonislawski ?

kv2019i · 2025-08-05T14:11:29Z

src/include/sof/audio/module_adapter/module/generic.h

-void *module_allocate_memory(struct processing_module *mod, uint32_t size, uint32_t alignment);
-int module_free_memory(struct processing_module *mod, void *ptr);
+void *mod_alloc(struct processing_module *mod, uint32_t size, uint32_t alignment);
+int mod_free(struct processing_module *mod, void *ptr);


Are we sure we don't have out-of-tree modules that use current generic.h naming? Should we add compatibility macros for the old names?

out of tree need to adapt.

kv2019i · 2025-08-05T14:11:54Z

src/audio/codec/dts/dts.c

 	struct processing_module *mod = mod_void;
 	struct comp_dev *dev = mod->dev;
 	void *pMem;



commit doesn't say why these are renamed. Just to make them shorter?

Yes, simply to make the names shorter and not to trigger line wrapping storm all around the place when all modules are changed to use module API for memory allocations.

kv2019i · 2025-08-05T14:14:19Z

zephyr/sof_shell.c

 * Author: Kai Vehmanen <kai.vehmanen@linux.intel.com>
 */

 #include <rtos/sof.h> /* sof_get() */


This could have been a separate PR.

I agree (refreshing to have request to drop something out ot this PR instead of including more and more). But still hope this could be merged as one PR.

lgirdwood · 2025-08-05T14:54:58Z

I guess ok, but some of the renames are not explained in commit messages and leave me wondering why the commit is done.

Oh, and this changes the module generic.h interface. We are 100% sure there are no external users for this interface? @lgirdwood @abonislawski ?

Any external non upstream usage will have to do the update themselves.

jsarha force-pushed the module_heap_api branch from c622f16 to e1f2d1e Compare July 28, 2025 21:01

jsarha marked this pull request as ready for review July 29, 2025 12:16

Copilot AI review requested due to automatic review settings July 29, 2025 12:16

jsarha requested review from abonislawski, dbaluta, jxstelter, kv2019i, lbetlej, lgirdwood, mmaka1, plbossart, ranj063 and singalsu as code owners July 29, 2025 12:16

Copilot AI reviewed Jul 29, 2025

View reviewed changes

src/audio/module_adapter/module/generic.c Show resolved Hide resolved

jsarha force-pushed the module_heap_api branch from e1f2d1e to 4944442 Compare July 29, 2025 13:02

lgirdwood reviewed Jul 30, 2025

View reviewed changes

src/audio/codec/dts/dts.c Outdated Show resolved Hide resolved

lyakh reviewed Jul 30, 2025

View reviewed changes

jsarha force-pushed the module_heap_api branch from 4944442 to ebcb2c4 Compare August 1, 2025 15:01

jsarha requested a review from iuliana-prodan as a code owner August 1, 2025 15:01

jsarha force-pushed the module_heap_api branch from ebcb2c4 to b6cf243 Compare August 1, 2025 15:13

lgirdwood approved these changes Aug 1, 2025

View reviewed changes

jsarha force-pushed the module_heap_api branch from b6cf243 to 7fb3a9c Compare August 2, 2025 09:13

Jyri Sarha added 4 commits August 3, 2025 23:30

modules: Rename module_allocate_memory and module_free_memory

065e8ee

Rename module_allocate_memory and module_free_memory to mod_alloc and mod_free. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

modules: Rename module_free_all_memory() to mod_free_all()

9ce9785

Rename module_free_all_memory() to mod_free_all(). Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

modules: Fix mod_alloc() error branch memory leak

90c9ffe

Container memory should be freed if the requested memory allocation fails. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

modules: Add mod_zalloc() function

59b7fe7

Add mod_zalloc() function that is otherwise the same as mod_alloc(), but allocated memory is initialized to zero. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

jsarha force-pushed the module_heap_api branch from 7fb3a9c to 374e369 Compare August 3, 2025 20:34

lyakh reviewed Aug 4, 2025

View reviewed changes

softwarecki reviewed Aug 4, 2025

View reviewed changes

module_adapter: Free all module associated memory module_adapter_free()

df29ba2

Free all module associated memory module_adapter_free() by calling mod_free_all(). Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

Jyri Sarha added 3 commits August 4, 2025 21:12

module_adapter: Add module_adapter_heap_usage()

e6962ff

Adds exported module_adapter_heap_usage() function that returns current heap usage of the module in question. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

jsarha force-pushed the module_heap_api branch from 374e369 to d81f7cc Compare August 4, 2025 20:31

zephyr: shell: Fix implicit declaration of function 'strtol'

dd1f009

Fix implicit declaration of function 'strtol' by including stdlib.h. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

jsarha force-pushed the module_heap_api branch 2 times, most recently from 534df07 to fae3bfe Compare August 4, 2025 21:40

Jyri Sarha added 2 commits August 5, 2025 00:58

include: ll_schedule_domain: Fix struct comp_dev related warnings

7b2e881

Declare struct comp_dev to avoid warnings if ll_schedule_domain.h is included before struct comp_dev is defined. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

jsarha force-pushed the module_heap_api branch from fae3bfe to 513635c Compare August 4, 2025 21:59

jsarha force-pushed the module_heap_api branch from 513635c to cb0f95b Compare August 5, 2025 08:21

lyakh approved these changes Aug 5, 2025

View reviewed changes

kv2019i reviewed Aug 5, 2025

View reviewed changes

kv2019i approved these changes Aug 6, 2025

View reviewed changes

kv2019i merged commit 9fcc269 into thesofproject:main Aug 6, 2025
36 of 45 checks passed

jsarha mentioned this pull request Aug 13, 2025

Module API additions and conversion of some modules to use them #10164

Merged

Module heap api #10141

Module heap api #10141

Uh oh!

Conversation

jsarha commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

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!

Uh oh!

lgirdwood 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!

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!

jsarha Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

kv2019i 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!

jsarha commented Jul 28, 2025 •

edited

Loading

jsarha Aug 8, 2025 •

edited

Loading