-
Notifications
You must be signed in to change notification settings - Fork 349
Module heap api #10141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Module heap api #10141
Changes from all commits
065e8ee
9ce9785
90c9ffe
59b7fe7
df29ba2
9d894fb
bf99df9
e6962ff
dd1f009
7b2e881
5832ea5
cb0f95b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -110,22 +110,31 @@ int module_init(struct processing_module *mod) | |||
| return 0; | ||||
| } | ||||
|
|
||||
| void *module_allocate_memory(struct processing_module *mod, uint32_t size, uint32_t alignment) | ||||
| /** | ||||
| * Allocates aligned memory block for module. | ||||
| * @param mod Pointer to the module this memory block is allocatd for. | ||||
| * @param bytes Size in bytes. | ||||
| * @param alignment Alignment in bytes. | ||||
| * @return Pointer to the allocated memory or NULL if failed. | ||||
| * | ||||
| * The allocated memory is automatically freed when the module is unloaded. | ||||
| */ | ||||
| void *mod_alloc_align(struct processing_module *mod, uint32_t size, uint32_t alignment) | ||||
| { | ||||
| struct comp_dev *dev = mod->dev; | ||||
| struct module_memory *container; | ||||
| void *ptr; | ||||
|
|
||||
| if (!size) { | ||||
| comp_err(dev, "module_allocate_memory: requested allocation of 0 bytes."); | ||||
| comp_err(dev, "mod_alloc: requested allocation of 0 bytes."); | ||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we really need to include the function name in the error message? Zephyr automatically prefixes log entries with the function name.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. actually, now that we are Zephyr only, we should use
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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). |
||||
| return NULL; | ||||
| } | ||||
|
|
||||
| /* Allocate memory container */ | ||||
| container = rzalloc(SOF_MEM_FLAG_USER, | ||||
| sizeof(struct module_memory)); | ||||
| if (!container) { | ||||
| comp_err(dev, "module_allocate_memory: failed to allocate memory container."); | ||||
| comp_err(dev, "mod_alloc: failed to allocate memory container."); | ||||
| return NULL; | ||||
| } | ||||
|
|
||||
|
|
@@ -136,18 +145,60 @@ void *module_allocate_memory(struct processing_module *mod, uint32_t size, uint3 | |||
| ptr = rballoc(SOF_MEM_FLAG_USER, size); | ||||
|
|
||||
| if (!ptr) { | ||||
| comp_err(dev, "module_allocate_memory: failed to allocate memory for comp %x.", | ||||
| comp_err(dev, "mod_alloc: failed to allocate memory for comp %x.", | ||||
| dev_comp_id(dev)); | ||||
jsarha marked this conversation as resolved.
Show resolved
Hide resolved
|
||||
| rfree(container); | ||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. Reserving only couple of words long containers separately from heap is very inefficient. I'll do that as a follow up |
||||
| return NULL; | ||||
| } | ||||
| /* Store reference to allocated memory */ | ||||
| container->ptr = ptr; | ||||
| container->size = size; | ||||
| list_item_prepend(&container->mem_list, &mod->priv.memory.mem_list); | ||||
|
|
||||
| return ptr; | ||||
| } | ||||
| EXPORT_SYMBOL(mod_alloc_align); | ||||
|
|
||||
| /** | ||||
| * Allocates memory block for module. | ||||
| * @param mod Pointer to module this memory block is allocated for. | ||||
| * @param bytes Size in bytes. | ||||
| * @return Pointer to the allocated memory or NULL if failed. | ||||
| * | ||||
| * Like mod_alloc_align() but the alignment can not be specified. However, | ||||
| * rballoc() will always aligns the memory to PLATFORM_DCACHE_ALIGN. | ||||
| */ | ||||
| void *mod_alloc(struct processing_module *mod, uint32_t size) | ||||
| { | ||||
| return mod_alloc_align(mod, size, 0); | ||||
| } | ||||
| EXPORT_SYMBOL(mod_alloc); | ||||
|
|
||||
| int module_free_memory(struct processing_module *mod, void *ptr) | ||||
| /** | ||||
| * Allocates memory block for module and initializes it to zero. | ||||
| * @param mod Pointer to module this memory block is allocated for. | ||||
| * @param bytes Size in bytes. | ||||
| * @return Pointer to the allocated memory or NULL if failed. | ||||
| * | ||||
| * Like mod_alloc() but the allocated memory is initialized to zero. | ||||
| */ | ||||
| void *mod_zalloc(struct processing_module *mod, uint32_t size) | ||||
| { | ||||
| void *ret = mod_alloc(mod, size); | ||||
|
|
||||
| if (ret) | ||||
| memset(ret, 0, size); | ||||
|
|
||||
| return ret; | ||||
| } | ||||
| EXPORT_SYMBOL(mod_zalloc); | ||||
|
|
||||
| /** | ||||
| * 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) | ||||
| { | ||||
| struct module_memory *mem; | ||||
| struct list_item *mem_list; | ||||
|
|
@@ -167,11 +218,12 @@ int module_free_memory(struct processing_module *mod, void *ptr) | |||
| } | ||||
| } | ||||
|
|
||||
| comp_err(mod->dev, "module_free_memory: error: could not find memory pointed by %p", | ||||
| comp_err(mod->dev, "mod_free: error: could not find memory pointed by %p", | ||||
| ptr); | ||||
|
|
||||
| return -EINVAL; | ||||
| } | ||||
| EXPORT_SYMBOL(mod_free); | ||||
|
|
||||
| int module_prepare(struct processing_module *mod, | ||||
| struct sof_source **sources, int num_of_sources, | ||||
|
|
@@ -343,7 +395,13 @@ int module_reset(struct processing_module *mod) | |||
| return 0; | ||||
| } | ||||
|
|
||||
| void module_free_all_memory(struct processing_module *mod) | ||||
| /** | ||||
| * Frees all the memory allocated for this module | ||||
| * @param mod Pointer to module this memory block was allocated for. | ||||
| * | ||||
| * This function is called automatically when the module is unloaded. | ||||
| */ | ||||
| void mod_free_all(struct processing_module *mod) | ||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in fact I'm wondering if
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@jsarha this is interesting... So module and component structures aren't allocated in module-managed memory? If they were and you freed them in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||||
| { | ||||
| struct module_memory *mem; | ||||
| struct list_item *mem_list; | ||||
|
|
@@ -357,6 +415,7 @@ void module_free_all_memory(struct processing_module *mod) | |||
| rfree(mem); | ||||
| } | ||||
| } | ||||
| EXPORT_SYMBOL(mod_free_all); | ||||
|
|
||||
| int module_free(struct processing_module *mod) | ||||
| { | ||||
|
|
||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commit doesn't say why these are renamed. Just to make them shorter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.