From 9281f900d0c46229928043194c1837fd9b7a5165 Mon Sep 17 00:00:00 2001 From: Jyri Sarha Date: Mon, 11 Aug 2025 01:30:13 +0300 Subject: [PATCH 1/9] modules: Allocate memory containers in chunks Do not allocate module memory containers one by one, but allocate them in chunks. The bookkeeping of allocated resources is done using containers that are allocated from heap. This effectively doubles the amount of heap allocations. This is not very efficient especially since the containers are only 20 bytes in size. This commit changes the allocation of containers so that they are always allocated in chunks of 16 containers, or what is selected with MODULE_MEMORY_API_CONTAINER_CHUNK_SIZE Kconfig option. The unused containers are not freed when the associated resource is freed. Instead the unused containers are kept in free containers list. All the containers are freed when mod_free_all() is called, for instance when the module unloads. Signed-off-by: Jyri Sarha --- src/audio/module_adapter/Kconfig | 10 +++ src/audio/module_adapter/module/generic.c | 89 ++++++++++++++----- src/audio/module_adapter/module_adapter.c | 3 +- src/include/module/module/base.h | 2 +- .../sof/audio/module_adapter/module/generic.h | 14 ++- 5 files changed, 92 insertions(+), 26 deletions(-) diff --git a/src/audio/module_adapter/Kconfig b/src/audio/module_adapter/Kconfig index bb9081812271..ad7f879d7049 100644 --- a/src/audio/module_adapter/Kconfig +++ b/src/audio/module_adapter/Kconfig @@ -3,6 +3,16 @@ menu "Processing modules" visible if COMP_MODULE_ADAPTER + config MODULE_MEMORY_API_CONTAINER_CHUNK_SIZE + int "Number of memory containers to allocate at once" + default 16 + help + The per module resource containers are allocated in + chunks. The unused containers are kept in free + list. When the free list is empty the amount of + containers to allocate at once is selected by this + config option. + config CADENCE_CODEC bool "Cadence codec" default n diff --git a/src/audio/module_adapter/module/generic.c b/src/audio/module_adapter/module/generic.c index 03a345552f71..b78f27059e13 100644 --- a/src/audio/module_adapter/module/generic.c +++ b/src/audio/module_adapter/module/generic.c @@ -92,7 +92,9 @@ int module_init(struct processing_module *mod) } /* Init memory list */ - list_init(&md->memory.mem_list); + list_init(&md->resources.mem_list); + list_init(&md->resources.free_cont_list); + list_init(&md->resources.cont_chunk_list); /* Now we can proceed with module specific initialization */ ret = interface->init(mod); @@ -110,6 +112,42 @@ int module_init(struct processing_module *mod) return 0; } +struct container_chunk { + struct list_item chunk_list; + struct module_memory containers[CONFIG_MODULE_MEMORY_API_CONTAINER_CHUNK_SIZE]; +}; + +static struct module_memory *container_get(struct processing_module *mod) +{ + struct module_resources *res = &mod->priv.resources; + struct module_memory *container; + + if (list_is_empty(&res->free_cont_list)) { + struct container_chunk *chunk = rzalloc(SOF_MEM_FLAG_USER, sizeof(*chunk)); + int i; + + if (!chunk) { + comp_err(mod->dev, "allocating more containers failed"); + return NULL; + } + + list_item_append(&chunk->chunk_list, &res->cont_chunk_list); + for (i = 0; i < ARRAY_SIZE(chunk->containers); i++) + list_item_append(&chunk->containers[i].mem_list, &res->free_cont_list); + } + + container = list_first_item(&res->free_cont_list, struct module_memory, mem_list); + list_item_del(&container->mem_list); + return container; +} + +static void container_put(struct processing_module *mod, struct module_memory *container) +{ + struct module_resources *res = &mod->priv.resources; + + list_item_append(&container->mem_list, &res->free_cont_list); +} + /** * Allocates aligned memory block for module. * @param mod Pointer to the module this memory block is allocatd for. @@ -121,20 +159,16 @@ int module_init(struct processing_module *mod) */ void *mod_alloc_align(struct processing_module *mod, uint32_t size, uint32_t alignment) { - struct comp_dev *dev = mod->dev; - struct module_memory *container; + struct module_memory *container = container_get(mod); + struct module_resources *res = &mod->priv.resources; void *ptr; - if (!size) { - comp_err(dev, "mod_alloc: requested allocation of 0 bytes."); + if (!container) return NULL; - } - /* Allocate memory container */ - container = rzalloc(SOF_MEM_FLAG_USER, - sizeof(struct module_memory)); - if (!container) { - comp_err(dev, "mod_alloc: failed to allocate memory container."); + if (!size) { + comp_err(mod->dev, "mod_alloc: requested allocation of 0 bytes."); + container_put(mod, container); return NULL; } @@ -145,15 +179,15 @@ void *mod_alloc_align(struct processing_module *mod, uint32_t size, uint32_t ali ptr = rballoc(SOF_MEM_FLAG_USER, size); if (!ptr) { - comp_err(dev, "mod_alloc: failed to allocate memory for comp %x.", - dev_comp_id(dev)); - rfree(container); + comp_err(mod->dev, "mod_alloc: failed to allocate memory for comp %x.", + dev_comp_id(mod->dev)); + container_put(mod, container); return NULL; } /* Store reference to allocated memory */ container->ptr = ptr; container->size = size; - list_item_prepend(&container->mem_list, &mod->priv.memory.mem_list); + list_item_prepend(&container->mem_list, &res->mem_list); return ptr; } @@ -200,6 +234,7 @@ EXPORT_SYMBOL(mod_zalloc); */ int mod_free(struct processing_module *mod, void *ptr) { + struct module_resources *res = &mod->priv.resources; struct module_memory *mem; struct list_item *mem_list; struct list_item *_mem_list; @@ -208,12 +243,12 @@ int mod_free(struct processing_module *mod, void *ptr) return 0; /* Find which container keeps this memory */ - list_for_item_safe(mem_list, _mem_list, &mod->priv.memory.mem_list) { + list_for_item_safe(mem_list, _mem_list, &res->mem_list) { mem = container_of(mem_list, struct module_memory, mem_list); if (mem->ptr == ptr) { rfree(mem->ptr); list_item_del(&mem->mem_list); - rfree(mem); + container_put(mod, mem); return 0; } } @@ -403,16 +438,24 @@ int module_reset(struct processing_module *mod) */ void mod_free_all(struct processing_module *mod) { - struct module_memory *mem; - struct list_item *mem_list; - struct list_item *_mem_list; + struct module_resources *res = &mod->priv.resources; + struct list_item *list; + struct list_item *_list; /* Find which container keeps this memory */ - list_for_item_safe(mem_list, _mem_list, &mod->priv.memory.mem_list) { - mem = container_of(mem_list, struct module_memory, mem_list); + list_for_item_safe(list, _list, &res->mem_list) { + struct module_memory *mem = container_of(list, struct module_memory, mem_list); + rfree(mem->ptr); list_item_del(&mem->mem_list); - rfree(mem); + } + + list_for_item_safe(list, _list, &res->cont_chunk_list) { + struct container_chunk *chunk = + container_of(list, struct container_chunk, chunk_list); + + list_item_del(&chunk->chunk_list); + rfree(chunk); } } EXPORT_SYMBOL(mod_free_all); diff --git a/src/audio/module_adapter/module_adapter.c b/src/audio/module_adapter/module_adapter.c index da0353d892ab..5fb8dc990218 100644 --- a/src/audio/module_adapter/module_adapter.c +++ b/src/audio/module_adapter/module_adapter.c @@ -1291,10 +1291,11 @@ EXPORT_SYMBOL(module_adapter_free); size_t module_adapter_heap_usage(struct processing_module *mod) { + struct module_resources *res = &mod->priv.resources; struct list_item *mem_list, *_mem_list; size_t size = 0; - list_for_item_safe(mem_list, _mem_list, &mod->priv.memory.mem_list) { + list_for_item_safe(mem_list, _mem_list, &res->mem_list) { struct module_memory *mem = container_of(mem_list, struct module_memory, mem_list); size += mem->size; diff --git a/src/include/module/module/base.h b/src/include/module/module/base.h index 2371d77124f5..881e74a2d108 100644 --- a/src/include/module/module/base.h +++ b/src/include/module/module/base.h @@ -59,7 +59,7 @@ struct module_data { enum module_state state; size_t new_cfg_size; /**< size of new module config data */ void *runtime_params; - struct module_memory memory; /**< memory allocated by module */ + struct module_resources resources; /**< resources allocated by module */ struct module_processing_data mpd; /**< shared data comp <-> module */ #endif /* SOF_MODULE_PRIVATE */ }; diff --git a/src/include/sof/audio/module_adapter/module/generic.h b/src/include/sof/audio/module_adapter/module/generic.h index 1001c4ba450c..c63f98dbd9c2 100644 --- a/src/include/sof/audio/module_adapter/module/generic.h +++ b/src/include/sof/audio/module_adapter/module/generic.h @@ -117,7 +117,19 @@ struct module_param { /** * \struct module_memory - * \brief module memory block - used for every memory allocated by module + * \brief module resources block - used for module allocation records + * The allocations are recorded so that they can be automatically freed + * when the module unloads. + */ +struct module_resources { + struct list_item mem_list; /**< Allocad memory containers */ + struct list_item free_cont_list; /**< Unused memory containers */ + struct list_item cont_chunk_list; /**< Memory container chunks */ +}; + +/** + * \struct module_memory + * \brief module memory container - used for every memory allocated by module */ struct module_memory { void *ptr; /**< A pointr to particular memory block */ From 6576498b4b4029aa7d71753977368066edb5a0b5 Mon Sep 17 00:00:00 2001 From: Jyri Sarha Date: Tue, 12 Aug 2025 00:16:13 +0300 Subject: [PATCH 2/9] modules: Add high water mark to module_adapter_heap_usage() Add heap usage high water mark to module_adapter_heap_usage() and shell's "sof module_heap_usage" command. Signed-off-by: Jyri Sarha --- src/audio/module_adapter/module/generic.c | 7 +++++++ src/audio/module_adapter/module_adapter.c | 13 ++++--------- .../sof/audio/module_adapter/module/generic.h | 4 +++- zephyr/sof_shell.c | 8 +++++--- 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/audio/module_adapter/module/generic.c b/src/audio/module_adapter/module/generic.c index b78f27059e13..27892af025b1 100644 --- a/src/audio/module_adapter/module/generic.c +++ b/src/audio/module_adapter/module/generic.c @@ -95,6 +95,8 @@ int module_init(struct processing_module *mod) list_init(&md->resources.mem_list); list_init(&md->resources.free_cont_list); list_init(&md->resources.cont_chunk_list); + md->resources.heap_usage = 0; + md->resources.heap_high_water_mark = 0; /* Now we can proceed with module specific initialization */ ret = interface->init(mod); @@ -189,6 +191,10 @@ void *mod_alloc_align(struct processing_module *mod, uint32_t size, uint32_t ali container->size = size; list_item_prepend(&container->mem_list, &res->mem_list); + res->heap_usage += size; + if (res->heap_usage > res->heap_high_water_mark) + res->heap_high_water_mark = res->heap_usage; + return ptr; } EXPORT_SYMBOL(mod_alloc_align); @@ -247,6 +253,7 @@ int mod_free(struct processing_module *mod, void *ptr) mem = container_of(mem_list, struct module_memory, mem_list); if (mem->ptr == ptr) { rfree(mem->ptr); + res->heap_usage -= mem->size; list_item_del(&mem->mem_list); container_put(mod, mem); return 0; diff --git a/src/audio/module_adapter/module_adapter.c b/src/audio/module_adapter/module_adapter.c index 5fb8dc990218..10f722fd9bbd 100644 --- a/src/audio/module_adapter/module_adapter.c +++ b/src/audio/module_adapter/module_adapter.c @@ -1289,19 +1289,14 @@ void module_adapter_free(struct comp_dev *dev) } EXPORT_SYMBOL(module_adapter_free); -size_t module_adapter_heap_usage(struct processing_module *mod) +size_t module_adapter_heap_usage(struct processing_module *mod, size_t *hwm) { struct module_resources *res = &mod->priv.resources; - struct list_item *mem_list, *_mem_list; - size_t size = 0; - list_for_item_safe(mem_list, _mem_list, &res->mem_list) { - struct module_memory *mem = container_of(mem_list, struct module_memory, mem_list); + if (hwm) + *hwm = res->heap_high_water_mark; - size += mem->size; - } - - return size; + return res->heap_usage; } EXPORT_SYMBOL(module_adapter_heap_usage); diff --git a/src/include/sof/audio/module_adapter/module/generic.h b/src/include/sof/audio/module_adapter/module/generic.h index c63f98dbd9c2..eeac83cc438d 100644 --- a/src/include/sof/audio/module_adapter/module/generic.h +++ b/src/include/sof/audio/module_adapter/module/generic.h @@ -125,6 +125,8 @@ struct module_resources { struct list_item mem_list; /**< Allocad memory containers */ struct list_item free_cont_list; /**< Unused memory containers */ struct list_item cont_chunk_list; /**< Memory container chunks */ + size_t heap_usage; + size_t heap_high_water_mark; }; /** @@ -235,7 +237,7 @@ int module_adapter_trigger(struct comp_dev *dev, int cmd); void module_adapter_free(struct comp_dev *dev); int module_adapter_reset(struct comp_dev *dev); -size_t module_adapter_heap_usage(struct processing_module *mod); +size_t module_adapter_heap_usage(struct processing_module *mod, size_t *hwm); #if CONFIG_IPC_MAJOR_3 static inline diff --git a/zephyr/sof_shell.c b/zephyr/sof_shell.c index 60785c4eb5c4..f10a2c9275b5 100644 --- a/zephyr/sof_shell.c +++ b/zephyr/sof_shell.c @@ -55,13 +55,15 @@ static int cmd_sof_module_heap_usage(const struct shell *sh, } list_for_item_safe(clist, _clist, &ipc->comp_list) { + size_t usage, hwm; + icd = container_of(clist, struct ipc_comp_dev, list); if (icd->type != COMP_TYPE_COMPONENT) continue; - shell_print(sh, "comp id 0x%08x\t%8zu bytes\t(%zu max)", icd->id, - module_adapter_heap_usage(comp_mod(icd->cd)), - comp_mod(icd->cd)->priv.cfg.heap_bytes); + usage = module_adapter_heap_usage(comp_mod(icd->cd), &hwm); + shell_print(sh, "comp id 0x%08x%9zu usage%9zu hwm %9zu max\tbytes", + icd->id, usage, hwm, comp_mod(icd->cd)->priv.cfg.heap_bytes); } return 0; } From 781301c112dcc45a0e3dd0a483035eff37a643f4 Mon Sep 17 00:00:00 2001 From: Jyri Sarha Date: Thu, 14 Aug 2025 15:23:08 +0300 Subject: [PATCH 3/9] cmocka: Fix mixer test before it breaks due to blob handler dependency Add src/audio/data_blob.c to mixer cmocka test sources to fix the dependency problem from adding comp_data_blob_handler_new_ext() to audio/module_adapter/module/generic.c. Signed-off-by: Jyri Sarha --- test/cmocka/src/audio/mixer/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/test/cmocka/src/audio/mixer/CMakeLists.txt b/test/cmocka/src/audio/mixer/CMakeLists.txt index 723cb091ff61..ea8cad0bd79e 100644 --- a/test/cmocka/src/audio/mixer/CMakeLists.txt +++ b/test/cmocka/src/audio/mixer/CMakeLists.txt @@ -25,6 +25,7 @@ cmocka_test(mixer ${PROJECT_SOURCE_DIR}/src/audio/pipeline/pipeline-schedule.c ${PROJECT_SOURCE_DIR}/src/audio/pipeline/pipeline-stream.c ${PROJECT_SOURCE_DIR}/src/audio/pipeline/pipeline-xrun.c + ${PROJECT_SOURCE_DIR}/src/audio/data_blob.c ${PROJECT_SOURCE_DIR}/src/module/audio/source_api.c ${PROJECT_SOURCE_DIR}/src/module/audio/sink_api.c ${PROJECT_SOURCE_DIR}/src/math/numbers.c From 41ff293255d065a453ead9b230d8deb8bff30b44 Mon Sep 17 00:00:00 2001 From: Jyri Sarha Date: Tue, 12 Aug 2025 15:02:40 +0300 Subject: [PATCH 4/9] modules: Add mod_data_blob_handler_new() to module API Add mod_data_blob_handler_new() to module API. The function is otherwise the same as comp_data_blob_handler_new(), but it takes a module pointer as the first argument, and the blob handler is automatically freed when the module unloads. The handler allocated with mod_data_blob_handler_new() should not be freed with comp_data_blob_handler_free(), mod_data_blob_handler_free() should be used. Signed-off-by: Jyri Sarha --- src/audio/module_adapter/module/generic.c | 123 +++++++++++++----- .../sof/audio/module_adapter/module/generic.h | 30 +++-- 2 files changed, 116 insertions(+), 37 deletions(-) diff --git a/src/audio/module_adapter/module/generic.c b/src/audio/module_adapter/module/generic.c index 27892af025b1..4fcf2606f109 100644 --- a/src/audio/module_adapter/module/generic.c +++ b/src/audio/module_adapter/module/generic.c @@ -14,6 +14,7 @@ #include #include +#include LOG_MODULE_DECLARE(module_adapter, CONFIG_SOF_LOG_LEVEL); @@ -92,7 +93,7 @@ int module_init(struct processing_module *mod) } /* Init memory list */ - list_init(&md->resources.mem_list); + list_init(&md->resources.res_list); list_init(&md->resources.free_cont_list); list_init(&md->resources.cont_chunk_list); md->resources.heap_usage = 0; @@ -116,13 +117,13 @@ int module_init(struct processing_module *mod) struct container_chunk { struct list_item chunk_list; - struct module_memory containers[CONFIG_MODULE_MEMORY_API_CONTAINER_CHUNK_SIZE]; + struct module_resource containers[CONFIG_MODULE_MEMORY_API_CONTAINER_CHUNK_SIZE]; }; -static struct module_memory *container_get(struct processing_module *mod) +static struct module_resource *container_get(struct processing_module *mod) { struct module_resources *res = &mod->priv.resources; - struct module_memory *container; + struct module_resource *container; if (list_is_empty(&res->free_cont_list)) { struct container_chunk *chunk = rzalloc(SOF_MEM_FLAG_USER, sizeof(*chunk)); @@ -135,19 +136,19 @@ static struct module_memory *container_get(struct processing_module *mod) list_item_append(&chunk->chunk_list, &res->cont_chunk_list); for (i = 0; i < ARRAY_SIZE(chunk->containers); i++) - list_item_append(&chunk->containers[i].mem_list, &res->free_cont_list); + list_item_append(&chunk->containers[i].list, &res->free_cont_list); } - container = list_first_item(&res->free_cont_list, struct module_memory, mem_list); - list_item_del(&container->mem_list); + container = list_first_item(&res->free_cont_list, struct module_resource, list); + list_item_del(&container->list); return container; } -static void container_put(struct processing_module *mod, struct module_memory *container) +static void container_put(struct processing_module *mod, struct module_resource *container) { struct module_resources *res = &mod->priv.resources; - list_item_append(&container->mem_list, &res->free_cont_list); + list_item_append(&container->list, &res->free_cont_list); } /** @@ -161,7 +162,7 @@ static void container_put(struct processing_module *mod, struct module_memory *c */ void *mod_alloc_align(struct processing_module *mod, uint32_t size, uint32_t alignment) { - struct module_memory *container = container_get(mod); + struct module_resource *container = container_get(mod); struct module_resources *res = &mod->priv.resources; void *ptr; @@ -189,7 +190,8 @@ void *mod_alloc_align(struct processing_module *mod, uint32_t size, uint32_t ali /* Store reference to allocated memory */ container->ptr = ptr; container->size = size; - list_item_prepend(&container->mem_list, &res->mem_list); + container->type = MOD_RES_HEAP; + list_item_prepend(&container->list, &res->res_list); res->heap_usage += size; if (res->heap_usage > res->heap_high_water_mark) @@ -233,30 +235,84 @@ void *mod_zalloc(struct processing_module *mod, uint32_t size) } EXPORT_SYMBOL(mod_zalloc); +/** + * Creates a blob handler and releases it when the module is unloaded + * @param mod Pointer to module this memory block is allocated for. + * @return Pointer to the created data blob handler + * + * 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) +{ + struct module_resources *res = &mod->priv.resources; + struct module_resource *container = container_get(mod); + struct comp_data_blob_handler *bhp; + + if (!container) + return NULL; + + bhp = comp_data_blob_handler_new_ext(mod->dev, false, NULL, NULL); + if (!bhp) { + container_put(mod, container); + return NULL; + } + + container->bhp = bhp; + container->size = 0; + container->type = MOD_RES_BLOB_HANDLER; + list_item_prepend(&container->list, &res->res_list); + + return bhp; +} +EXPORT_SYMBOL(mod_data_blob_handler_new); +#endif + +static int free_contents(struct processing_module *mod, struct module_resource *container) +{ + struct module_resources *res = &mod->priv.resources; + + switch (container->type) { + case MOD_RES_HEAP: + rfree(container->ptr); + res->heap_usage -= container->size; + return 0; +#if CONFIG_COMP_BLOB + case MOD_RES_BLOB_HANDLER: + comp_data_blob_handler_free(container->bhp); + return 0; +#endif + default: + comp_err(mod->dev, "Unknown resource type: %d", container->type); + } + return -EINVAL; +} + /** * Frees the memory block removes it from module's book keeping. * @param mod Pointer to module this memory block was allocated for. * @param ptr Pointer to the memory block. */ -int mod_free(struct processing_module *mod, void *ptr) +int mod_free(struct processing_module *mod, const void *ptr) { struct module_resources *res = &mod->priv.resources; - struct module_memory *mem; - struct list_item *mem_list; - struct list_item *_mem_list; + struct module_resource *container; + struct list_item *res_list; + struct list_item *_res_list; if (!ptr) return 0; /* Find which container keeps this memory */ - list_for_item_safe(mem_list, _mem_list, &res->mem_list) { - mem = container_of(mem_list, struct module_memory, mem_list); - if (mem->ptr == ptr) { - rfree(mem->ptr); - res->heap_usage -= mem->size; - list_item_del(&mem->mem_list); - container_put(mod, mem); - return 0; + list_for_item_safe(res_list, _res_list, &res->res_list) { + container = container_of(res_list, struct module_resource, list); + if (container->ptr == ptr) { + int ret = free_contents(mod, container); + + list_item_del(&container->list); + container_put(mod, container); + return ret; } } @@ -267,6 +323,14 @@ int mod_free(struct processing_module *mod, void *ptr) } EXPORT_SYMBOL(mod_free); +#if CONFIG_COMP_BLOB +void mod_data_blob_handler_free(struct processing_module *mod, struct comp_data_blob_handler *dbh) +{ + mod_free(mod, (void *)dbh); +} +EXPORT_SYMBOL(mod_data_blob_handler_free); +#endif + int module_prepare(struct processing_module *mod, struct sof_source **sources, int num_of_sources, struct sof_sink **sinks, int num_of_sinks) @@ -438,8 +502,8 @@ int module_reset(struct processing_module *mod) } /** - * Frees all the memory allocated for this module - * @param mod Pointer to module this memory block was allocated for. + * Frees all the resources registered for this module + * @param mod Pointer to module that should have its resource freed. * * This function is called automatically when the module is unloaded. */ @@ -450,11 +514,12 @@ void mod_free_all(struct processing_module *mod) struct list_item *_list; /* Find which container keeps this memory */ - list_for_item_safe(list, _list, &res->mem_list) { - struct module_memory *mem = container_of(list, struct module_memory, mem_list); + list_for_item_safe(list, _list, &res->res_list) { + struct module_resource *container = + container_of(list, struct module_resource, list); - rfree(mem->ptr); - list_item_del(&mem->mem_list); + free_contents(mod, container); + list_item_del(&container->list); } list_for_item_safe(list, _list, &res->cont_chunk_list) { diff --git a/src/include/sof/audio/module_adapter/module/generic.h b/src/include/sof/audio/module_adapter/module/generic.h index eeac83cc438d..735bdc6c89f9 100644 --- a/src/include/sof/audio/module_adapter/module/generic.h +++ b/src/include/sof/audio/module_adapter/module/generic.h @@ -116,27 +116,37 @@ struct module_param { }; /** - * \struct module_memory + * \struct module_resources * \brief module resources block - used for module allocation records * The allocations are recorded so that they can be automatically freed * when the module unloads. */ struct module_resources { - struct list_item mem_list; /**< Allocad memory containers */ + struct list_item res_list; /**< Allocad resource containers */ struct list_item free_cont_list; /**< Unused memory containers */ struct list_item cont_chunk_list; /**< Memory container chunks */ size_t heap_usage; size_t heap_high_water_mark; }; +enum mod_resource_type { + MOD_RES_UNINITIALIZED = 0, + MOD_RES_HEAP, + MOD_RES_BLOB_HANDLER, +}; + /** - * \struct module_memory + * \struct module_resource * \brief module memory container - used for every memory allocated by module */ -struct module_memory { - void *ptr; /**< A pointr to particular memory block */ - struct list_item mem_list; /**< list of memory allocated by module */ - size_t size; +struct module_resource { + union { + void *ptr; /**< Pointer to heap allocated memory */ + struct comp_data_blob_handler *bhp; /**< Blob handler ptr */ + }; + struct list_item list; /**< list element */ + size_t size; /**< Size of allocated heap memory, 0 if not from heap */ + enum mod_resource_type type; /**< Resource type */ }; /** @@ -171,7 +181,11 @@ int module_init(struct processing_module *mod); void *mod_alloc_align(struct processing_module *mod, uint32_t size, uint32_t alignment); void *mod_alloc(struct processing_module *mod, uint32_t size); void *mod_zalloc(struct processing_module *mod, uint32_t size); -int mod_free(struct processing_module *mod, void *ptr); +int mod_free(struct processing_module *mod, const void *ptr); +#if CONFIG_COMP_BLOB +struct comp_data_blob_handler *mod_data_blob_handler_new(struct processing_module *mod); +void mod_data_blob_handler_free(struct processing_module *mod, struct comp_data_blob_handler *dbh); +#endif void mod_free_all(struct processing_module *mod); int module_prepare(struct processing_module *mod, struct sof_source **sources, int num_of_sources, From 60fe4145d0d5c2e3e13ab5f4a8fda47fa565ece0 Mon Sep 17 00:00:00 2001 From: Jyri Sarha Date: Tue, 26 Aug 2025 00:06:30 +0300 Subject: [PATCH 5/9] modules: Add mod_fast_get() and mod_fast_put() Add module API versions of fast_get() and fast_put(). The SRAM copies reserved with mod_fast_get() are released automatically when the module unloads, and those SRAM copies should not be freed with the regular fast_put(). The sram-copy allocated with mod_fast_get() should not be freed with regular fast_put(), but mod_fast_put() should be used. Signed-off-by: Jyri Sarha --- src/audio/module_adapter/module/generic.c | 47 +++++++++++++++++++ .../sof/audio/module_adapter/module/generic.h | 6 +++ 2 files changed, 53 insertions(+) diff --git a/src/audio/module_adapter/module/generic.c b/src/audio/module_adapter/module/generic.c index 4fcf2606f109..6e78747601c6 100644 --- a/src/audio/module_adapter/module/generic.c +++ b/src/audio/module_adapter/module/generic.c @@ -15,6 +15,7 @@ #include #include +#include LOG_MODULE_DECLARE(module_adapter, CONFIG_SOF_LOG_LEVEL); @@ -269,6 +270,39 @@ mod_data_blob_handler_new(struct processing_module *mod) EXPORT_SYMBOL(mod_data_blob_handler_new); #endif +/** + * Make a module associated shared SRAM copy of DRAM read-only data. + * @param mod Pointer to module this copy is allocated for. + * @return Pointer to the SRAM copy. + * + * Like fast_get() but the handler is automatically freed. + */ +#if CONFIG_FAST_GET +const void *mod_fast_get(struct processing_module *mod, const void * const dram_ptr, size_t size) +{ + struct module_resources *res = &mod->priv.resources; + struct module_resource *container = container_get(mod); + const void *ptr; + + if (!container) + return NULL; + + ptr = fast_get(dram_ptr, size); + if (!ptr) { + container_put(mod, container); + return NULL; + } + + container->sram_ptr = ptr; + container->size = 0; + container->type = MOD_RES_FAST_GET; + list_item_prepend(&container->list, &res->res_list); + + return ptr; +} +EXPORT_SYMBOL(mod_fast_get); +#endif + static int free_contents(struct processing_module *mod, struct module_resource *container) { struct module_resources *res = &mod->priv.resources; @@ -282,6 +316,11 @@ static int free_contents(struct processing_module *mod, struct module_resource * case MOD_RES_BLOB_HANDLER: comp_data_blob_handler_free(container->bhp); return 0; +#endif +#if CONFIG_FAST_GET + case MOD_RES_FAST_GET: + fast_put(container->sram_ptr); + return 0; #endif default: comp_err(mod->dev, "Unknown resource type: %d", container->type); @@ -331,6 +370,14 @@ void mod_data_blob_handler_free(struct processing_module *mod, struct comp_data_ EXPORT_SYMBOL(mod_data_blob_handler_free); #endif +#if CONFIG_FAST_GET +void mod_fast_put(struct processing_module *mod, const void *sram_ptr) +{ + mod_free(mod, sram_ptr); +} +EXPORT_SYMBOL(mod_fast_put); +#endif + int module_prepare(struct processing_module *mod, struct sof_source **sources, int num_of_sources, struct sof_sink **sinks, int num_of_sinks) diff --git a/src/include/sof/audio/module_adapter/module/generic.h b/src/include/sof/audio/module_adapter/module/generic.h index 735bdc6c89f9..f0cee2370290 100644 --- a/src/include/sof/audio/module_adapter/module/generic.h +++ b/src/include/sof/audio/module_adapter/module/generic.h @@ -133,6 +133,7 @@ enum mod_resource_type { MOD_RES_UNINITIALIZED = 0, MOD_RES_HEAP, MOD_RES_BLOB_HANDLER, + MOD_RES_FAST_GET, }; /** @@ -143,6 +144,7 @@ struct module_resource { union { void *ptr; /**< Pointer to heap allocated memory */ struct comp_data_blob_handler *bhp; /**< Blob handler ptr */ + const void *sram_ptr; /**< SRAM ptr from fast_get() */ }; struct list_item list; /**< list element */ size_t size; /**< Size of allocated heap memory, 0 if not from heap */ @@ -186,6 +188,10 @@ int mod_free(struct processing_module *mod, const void *ptr); struct comp_data_blob_handler *mod_data_blob_handler_new(struct processing_module *mod); void mod_data_blob_handler_free(struct processing_module *mod, struct comp_data_blob_handler *dbh); #endif +#if CONFIG_FAST_GET +const void *mod_fast_get(struct processing_module *mod, const void * const dram_ptr, size_t size); +void mod_fast_put(struct processing_module *mod, const void *sram_ptr); +#endif void mod_free_all(struct processing_module *mod); int module_prepare(struct processing_module *mod, struct sof_source **sources, int num_of_sources, From a5cafd84b9f88c7c3fe8c1c31e27450496e74f04 Mon Sep 17 00:00:00 2001 From: Jyri Sarha Date: Fri, 15 Aug 2025 12:18:44 +0300 Subject: [PATCH 6/9] modules: Add safeguard to mod_alloc() and friends Add safeguard to mod_alloc() and friends that checks that they are always called from the same thread (e.g. no locking needed). The checking code has to be also behind defined(__ZEPHYR__) to keep cmocka tests working. Signed-off-by: Jyri Sarha --- src/audio/module_adapter/Kconfig | 11 +++++++++++ src/audio/module_adapter/module/generic.c | 17 ++++++++++++++++- .../sof/audio/module_adapter/module/generic.h | 8 ++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/audio/module_adapter/Kconfig b/src/audio/module_adapter/Kconfig index ad7f879d7049..3c762c73f7f8 100644 --- a/src/audio/module_adapter/Kconfig +++ b/src/audio/module_adapter/Kconfig @@ -13,6 +13,17 @@ menu "Processing modules" containers to allocate at once is selected by this config option. + config MODULE_MEMORY_API_DEBUG + bool "Turn on memory API thread safety checks" + default y if DEBUG + help + The Module Memory API structures are not protected + by locks. This is because the initialization, + allocation, and freeing of resources should always + be done in the same thread. This option adds an + assert to make sure no other thread makes such + operations. + config CADENCE_CODEC bool "Cadence codec" default n diff --git a/src/audio/module_adapter/module/generic.c b/src/audio/module_adapter/module/generic.c index 6e78747601c6..b822f4ed3606 100644 --- a/src/audio/module_adapter/module/generic.c +++ b/src/audio/module_adapter/module/generic.c @@ -17,6 +17,14 @@ #include #include +/* The __ZEPHYR__ condition is to keep cmocka tests working */ +#if CONFIG_MODULE_MEMORY_API_DEBUG && defined(__ZEPHYR__) +#define MEM_API_CHECK_THREAD(res) __ASSERT((res)->rsrc_mngr == k_current_get(), \ + "Module memory API operation from wrong thread") +#else +#define MEM_API_CHECK_THREAD(res) +#endif + LOG_MODULE_DECLARE(module_adapter, CONFIG_SOF_LOG_LEVEL); int module_load_config(struct comp_dev *dev, const void *cfg, size_t size) @@ -99,7 +107,9 @@ int module_init(struct processing_module *mod) list_init(&md->resources.cont_chunk_list); md->resources.heap_usage = 0; md->resources.heap_high_water_mark = 0; - +#if CONFIG_MODULE_MEMORY_API_DEBUG && defined(__ZEPHYR__) + md->resources.rsrc_mngr = k_current_get(); +#endif /* Now we can proceed with module specific initialization */ ret = interface->init(mod); if (ret) { @@ -167,6 +177,7 @@ void *mod_alloc_align(struct processing_module *mod, uint32_t size, uint32_t ali struct module_resources *res = &mod->priv.resources; void *ptr; + MEM_API_CHECK_THREAD(res); if (!container) return NULL; @@ -251,6 +262,7 @@ mod_data_blob_handler_new(struct processing_module *mod) struct module_resource *container = container_get(mod); struct comp_data_blob_handler *bhp; + MEM_API_CHECK_THREAD(res); if (!container) return NULL; @@ -284,6 +296,7 @@ const void *mod_fast_get(struct processing_module *mod, const void * const dram_ struct module_resource *container = container_get(mod); const void *ptr; + MEM_API_CHECK_THREAD(res); if (!container) return NULL; @@ -340,6 +353,7 @@ int mod_free(struct processing_module *mod, const void *ptr) struct list_item *res_list; struct list_item *_res_list; + MEM_API_CHECK_THREAD(res); if (!ptr) return 0; @@ -560,6 +574,7 @@ void mod_free_all(struct processing_module *mod) struct list_item *list; struct list_item *_list; + MEM_API_CHECK_THREAD(res); /* Find which container keeps this memory */ list_for_item_safe(list, _list, &res->res_list) { struct module_resource *container = diff --git a/src/include/sof/audio/module_adapter/module/generic.h b/src/include/sof/audio/module_adapter/module/generic.h index f0cee2370290..ced9fc710b11 100644 --- a/src/include/sof/audio/module_adapter/module/generic.h +++ b/src/include/sof/audio/module_adapter/module/generic.h @@ -19,6 +19,11 @@ #include #include "module_interface.h" +/* The __ZEPHYR__ condition is to keep cmocka tests working */ +#if CONFIG_MODULE_MEMORY_API_DEBUG && defined(__ZEPHYR__) +#include +#endif + /* * helpers to determine processing type * Needed till all the modules use PROCESSING_MODE_SINK_SOURCE @@ -127,6 +132,9 @@ struct module_resources { struct list_item cont_chunk_list; /**< Memory container chunks */ size_t heap_usage; size_t heap_high_water_mark; +#if CONFIG_MODULE_MEMORY_API_DEBUG && defined(__ZEPHYR__) + k_tid_t rsrc_mngr; +#endif }; enum mod_resource_type { From 70b3d56b5b33daa9a18a41678c33a258a5cd591a Mon Sep 17 00:00:00 2001 From: Jyri Sarha Date: Mon, 25 Aug 2025 19:14:25 +0300 Subject: [PATCH 7/9] modules: Use list_for_item() instead of list_for_item_safe() in mod_free() Use list_for_item() instead of list_for_item_safe() in mod_free(). There is no need for *_safe() version when loop is not continued after an element is removed from the list. Signed-off-by: Jyri Sarha --- src/audio/module_adapter/module/generic.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/audio/module_adapter/module/generic.c b/src/audio/module_adapter/module/generic.c index b822f4ed3606..cb4ea2e3d5ae 100644 --- a/src/audio/module_adapter/module/generic.c +++ b/src/audio/module_adapter/module/generic.c @@ -351,14 +351,13 @@ int mod_free(struct processing_module *mod, const void *ptr) struct module_resources *res = &mod->priv.resources; struct module_resource *container; struct list_item *res_list; - struct list_item *_res_list; MEM_API_CHECK_THREAD(res); if (!ptr) return 0; /* Find which container keeps this memory */ - list_for_item_safe(res_list, _res_list, &res->res_list) { + list_for_item(res_list, &res->res_list) { container = container_of(res_list, struct module_resource, list); if (container->ptr == ptr) { int ret = free_contents(mod, container); From 289efb13b581b06bed7b6cd4a88e530314128548 Mon Sep 17 00:00:00 2001 From: Jyri Sarha Date: Wed, 13 Aug 2025 00:37:08 +0300 Subject: [PATCH 8/9] Audio: SRC: Take mod_fast_get() and mod_fast_put() into use Take mod_fast_get() and mod_fast_put() into use. Signed-off-by: Jyri Sarha --- src/audio/src/src_common.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/audio/src/src_common.c b/src/audio/src/src_common.c index f96b3bc6e358..6331a6b9180e 100644 --- a/src/audio/src/src_common.c +++ b/src/audio/src/src_common.c @@ -627,12 +627,11 @@ int src_allocate_copy_stages(struct processing_module *mod, struct src_param *pr return -EINVAL; } - stage_dst[0].coefs = fast_get(stage_src1->coefs, coef_size[0]); - stage_dst[1].coefs = fast_get(stage_src2->coefs, coef_size[1]); + stage_dst[0].coefs = mod_fast_get(mod, stage_src1->coefs, coef_size[0]); + stage_dst[1].coefs = mod_fast_get(mod, stage_src2->coefs, coef_size[1]); if (!stage_dst[0].coefs || !stage_dst[1].coefs) { comp_err(mod->dev, "failed to allocate coefficients"); - fast_put(stage_dst[0].coefs); return -ENOMEM; } @@ -708,12 +707,5 @@ __cold int src_free(struct processing_module *mod) comp_info(mod->dev, "src_free()"); -#if CONFIG_FAST_GET - struct comp_data *cd = module_get_private_data(mod); - if (cd->param.stage1) { - fast_put(cd->param.stage1->coefs); - fast_put(cd->param.stage2->coefs); - } -#endif return 0; } From 2fc31183309899dafd931df354d5224506ebab39 Mon Sep 17 00:00:00 2001 From: Jyri Sarha Date: Sun, 31 Aug 2025 23:44:09 +0300 Subject: [PATCH 9/9] modules: Component id should be printed as hex Change all comp id prints from %d to %#x for better readability. Signed-off-by: Jyri Sarha --- src/audio/module_adapter/module/generic.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/audio/module_adapter/module/generic.c b/src/audio/module_adapter/module/generic.c index cb4ea2e3d5ae..929098d9903e 100644 --- a/src/audio/module_adapter/module/generic.c +++ b/src/audio/module_adapter/module/generic.c @@ -87,7 +87,7 @@ int module_init(struct processing_module *mod) return -EPERM; #endif if (!interface) { - comp_err(dev, "module interface not defined for comp id %d", + comp_err(dev, "module interface not defined for comp id %#x", dev_comp_id(dev)); return -EIO; } @@ -96,7 +96,7 @@ int module_init(struct processing_module *mod) if (!interface->init || (!!interface->process + !!interface->process_audio_stream + !!interface->process_raw_data < 1)) { - comp_err(dev, "comp %d is missing mandatory interfaces", + comp_err(dev, "comp %#x is missing mandatory interfaces", dev_comp_id(dev)); return -EIO; } @@ -113,7 +113,7 @@ int module_init(struct processing_module *mod) /* Now we can proceed with module specific initialization */ ret = interface->init(mod); if (ret) { - comp_err(dev, "module_init() error %d: module specific init failed, comp id %d", + comp_err(dev, "module_init() error %d: module specific init failed, comp id %#x", ret, dev_comp_id(dev)); return ret; } @@ -467,7 +467,7 @@ int module_process_legacy(struct processing_module *mod, ret = -EOPNOTSUPP; if (ret && ret != -ENOSPC && ret != -ENODATA) { - comp_err(dev, "module_process() error %d: for comp %d", + comp_err(dev, "module_process() error %d: for comp %#x", ret, dev_comp_id(dev)); return ret; } @@ -507,7 +507,7 @@ int module_process_sink_src(struct processing_module *mod, ret = ops->process(mod, sources, num_of_sources, sinks, num_of_sinks); if (ret && ret != -ENOSPC && ret != -ENODATA) { - comp_err(dev, "module_process() error %d: for comp %d", + comp_err(dev, "module_process() error %d: for comp %#x", ret, dev_comp_id(dev)); return ret; } @@ -540,7 +540,7 @@ int module_reset(struct processing_module *mod) if (ret) { if (ret != PPL_STATUS_PATH_STOP) comp_err(mod->dev, - "module_reset() error %d: module specific reset() failed for comp %d", + "module_reset() error %d: module specific reset() failed for comp %#xd", ret, dev_comp_id(mod->dev)); return ret; }