-
Notifications
You must be signed in to change notification settings - Fork 349
Module api: various fixes and developments #10223
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
Changes from all commits
ae351ee
7cc4a37
66ce358
e766d25
fda4ef7
7fffe2c
49f5d6e
efd4119
5b2b540
a719c1b
dbe5520
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 |
|---|---|---|
|
|
@@ -160,21 +160,25 @@ static void container_put(struct processing_module *mod, struct module_resource | |
| } | ||
|
|
||
| /** | ||
| * Allocates aligned memory block for module. | ||
| * Allocates aligned buffer 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. | ||
| * The allocated memory is automatically freed when the module is | ||
| * unloaded. The back-end, rballoc(), always aligns the memory to | ||
| * PLATFORM_DCACHE_ALIGN at the minimum. | ||
| */ | ||
| void *mod_alloc_align(struct processing_module *mod, uint32_t size, uint32_t alignment) | ||
| void *mod_balloc_align(struct processing_module *mod, size_t size, size_t alignment) | ||
| { | ||
| struct module_resource *container = container_get(mod); | ||
| struct module_resources *res = &mod->priv.resources; | ||
| struct module_resource *container; | ||
| void *ptr; | ||
|
|
||
| MEM_API_CHECK_THREAD(res); | ||
|
|
||
| container = container_get(mod); | ||
| if (!container) | ||
| return NULL; | ||
|
|
||
|
|
@@ -184,14 +188,12 @@ void *mod_alloc_align(struct processing_module *mod, uint32_t size, uint32_t ali | |
| return NULL; | ||
| } | ||
|
|
||
| /* Allocate memory for module */ | ||
| if (alignment) | ||
| ptr = rballoc_align(SOF_MEM_FLAG_USER, size, alignment); | ||
| else | ||
| ptr = rballoc(SOF_MEM_FLAG_USER, size); | ||
| /* Allocate buffer memory for module */ | ||
| ptr = rballoc_align(SOF_MEM_FLAG_USER, size, alignment); | ||
|
|
||
| if (!ptr) { | ||
| comp_err(mod->dev, "failed to allocate memory."); | ||
| comp_err(mod->dev, "Failed to alloc %zu bytes %zu alignment for comp %#x.", | ||
| size, alignment, dev_comp_id(mod->dev)); | ||
| container_put(mod, container); | ||
| return NULL; | ||
| } | ||
|
|
@@ -207,41 +209,57 @@ void *mod_alloc_align(struct processing_module *mod, uint32_t size, uint32_t ali | |
|
|
||
| return ptr; | ||
| } | ||
| EXPORT_SYMBOL(mod_alloc_align); | ||
| EXPORT_SYMBOL(mod_balloc_align); | ||
|
|
||
| /** | ||
| * Allocates memory block for module. | ||
| * @param mod Pointer to module this memory block is allocated for. | ||
| * @param bytes Size in bytes. | ||
| * 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. | ||
| * | ||
| * Like mod_alloc_align() but the alignment can not be specified. However, | ||
| * rballoc() will always aligns the memory to PLATFORM_DCACHE_ALIGN. | ||
| * The allocated memory is automatically freed when the module is unloaded. | ||
| */ | ||
| void *mod_alloc(struct processing_module *mod, uint32_t size) | ||
| void *mod_alloc_align(struct processing_module *mod, size_t size, size_t alignment) | ||
| { | ||
| return mod_alloc_align(mod, size, 0); | ||
| } | ||
| EXPORT_SYMBOL(mod_alloc); | ||
| struct module_resources *res = &mod->priv.resources; | ||
| struct module_resource *container; | ||
| 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); | ||
| MEM_API_CHECK_THREAD(res); | ||
|
|
||
| if (ret) | ||
| memset(ret, 0, size); | ||
| container = container_get(mod); | ||
| if (!container) | ||
| return NULL; | ||
|
|
||
| return ret; | ||
| if (!size) { | ||
| comp_err(mod->dev, "requested allocation of 0 bytes."); | ||
| container_put(mod, container); | ||
| return NULL; | ||
| } | ||
|
|
||
| /* Allocate memory for module */ | ||
| ptr = rmalloc_align(SOF_MEM_FLAG_USER, size, alignment); | ||
|
|
||
| if (!ptr) { | ||
| comp_err(mod->dev, "Failed to alloc %zu bytes %zu alignment for comp %#x.", | ||
| size, alignment, dev_comp_id(mod->dev)); | ||
|
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. unrelated to this commit: can we change |
||
| container_put(mod, container); | ||
| return NULL; | ||
| } | ||
| /* Store reference to allocated memory */ | ||
jsarha marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| container->ptr = ptr; | ||
| container->size = size; | ||
| 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) | ||
| res->heap_high_water_mark = res->heap_usage; | ||
|
|
||
| return ptr; | ||
| } | ||
| EXPORT_SYMBOL(mod_zalloc); | ||
| EXPORT_SYMBOL(mod_alloc_align); | ||
|
|
||
| /** | ||
| * Creates a blob handler and releases it when the module is unloaded | ||
|
|
@@ -255,10 +273,12 @@ 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; | ||
| struct module_resource *container; | ||
|
|
||
| MEM_API_CHECK_THREAD(res); | ||
|
|
||
| container = container_get(mod); | ||
| if (!container) | ||
| return NULL; | ||
|
|
||
|
|
@@ -289,10 +309,12 @@ EXPORT_SYMBOL(mod_data_blob_handler_new); | |
| 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); | ||
| struct module_resource *container; | ||
| const void *ptr; | ||
|
|
||
| MEM_API_CHECK_THREAD(res); | ||
|
|
||
| container = container_get(mod); | ||
| if (!container) | ||
| return NULL; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,11 @@ | |
|
|
||
| /* testbench mem alloc definition */ | ||
|
|
||
| void *rmalloc_align(uint32_t flags, size_t bytes, uint32_t alignment) | ||
| { | ||
| return malloc(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. Hmm. I wonder if we should clarify rmalloc() and rballoc() semantics first. Originally in SOF, rballoc() was used to allocate 1k aligned buffers, but it was changed by this commit 50f7b0e from @lgirdwood . The documentation was never clarified explicitly, both calls are used to allocate buffers of memory. Now in practise, the audio components have continued to use rballoc() while OS and driver code has used rmalloc() UNLESS alignment has been needed. As you note, there's really no difference with the alignment anymore, and most recently, since completing transition to Zephyr and addition of virtual heaps in commit 3932884 , only differences betwen rmalloc() and rballoc() are:
I don't think this serves much purpose anymore and we might just as well clarify that heap interfaces we offer to apps. Any reason to keep both rmalloc() and rballoc()? If the virtual heap is the only difference and we want to keep this option, should the interface be renamed? Or atleast this needs to be documented better. @softwarecki can you comment on the rationale why virtual heap is only plugged to rballoc()?
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. @kv2019i I didn't want to switch the entire SOF to virtual heap usage all at once - the idea was to do it gradually, starting with components that benefit most from it. That's why it's currently only used in rballoc(). The second reason is that the virtual heap has some implementation limitations. Buffer sizes must be powers of two, and each group of buffers must be aligned to the page size (4KB). These constraints make it unsuitable for general-purpose allocations, which is why rmalloc() still uses the traditional heap. We need the virtual heap mainly to support deep buffering. That mechanism relies on allocating the largest possible buffer from a predefined set of sizes. Without slot-based allocation, a single request could consume all available memory, blocking further allocations. Additionally, virtual heap helps reduce fragmentation and allows power savings by mapping only the used memory pages - unused memory banks can be powered down.
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. Excellent, thank you @softwarecki , that helps a lot. I think we should merge rmalloc and rballoc and have some flag to hint the allocator that virtual heap should/may be used. Current calls to rballoc() are good places to use this new flag as these have been prove to work with the virtual heap. Sounds good? @jsarha and @lgirdwood ?
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.
@softwarecki then maybe
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. @lyakh I think we should aim to have as many users of the virtual heap as possible to fully benefit from its advantages:
Deep buffering just happens to leverage these properties, but it's not the reason virtual heap was introduced.
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. @softwarecki sure, and we should weigh those against VMH drawbacks:
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. @jsarha @softwarecki @lyakh both VMH and Zephyr heap have benefits and drawbacks today, I've asked @jsarha to add a Kconfig to select the heap implementation at build time. i.e. all the alloc() calls will either go to VMH or Zephyr heap via a Kconfig selection. This way we keep both heaps as options and can continually fix/improve heaps and pick the correct heap per target/use case.
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 Isn't this what CONFIG_VIRTUAL_HEAP=y does. Yes "y", rballoc() uses virtual heap, if 'n', it's not used anywhere.
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. Even if we set CONFIG_VIRTUAL_HEAP=y vheap is not used for anything but rballoc. Now there is an extra option whether or not mod_alloc should use rmalloc (default) or rballoc, effectively directing all memory allocation done by processing modules (this does not include pipleline and other audio routing code), to virtual heap. |
||
| } | ||
|
|
||
| void *rmalloc(uint32_t flags, size_t bytes) | ||
| { | ||
| return malloc(bytes); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,10 +63,26 @@ void WEAK *rbrealloc_align(void *ptr, uint32_t flags, | |
| { | ||
| (void)flags; | ||
| (void)old_bytes; | ||
| (void)alignment; | ||
|
|
||
| return realloc(ptr, bytes); | ||
| } | ||
|
|
||
| void WEAK *rmalloc_align(uint32_t flags, size_t bytes, uint32_t alignment) | ||
| { | ||
| (void)flags; | ||
| (void)alignment; | ||
|
|
||
| return malloc(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. this works, right? So no cmocka test actually cares about alignment
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, with my full PR #10195 that includes cmocka memory leak fixes, all the cmocka tests pass without error from valgrind. |
||
| } | ||
|
|
||
| void WEAK *rmalloc(uint32_t flags, size_t bytes) | ||
| { | ||
| (void)flags; | ||
|
|
||
| return malloc(bytes); | ||
| } | ||
|
|
||
| void WEAK rfree(void *ptr) | ||
| { | ||
| free(ptr); | ||
|
|
||
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.
This effectively cuts virtual heap from any use and we lose the user intent whether a piece of code wanted to allocate with potentially virtual heap in used, versus doing allocations in a way where virtual heap is ensured never to be used (see my comment to commit 1).
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.
Processing module allocations in the system are not all allocations. In fact they of the audio buffer allocations they hardly play a role. The audio buffers are mostly allocated by the pipeline code which does not use module API.