Part 1 of userspace DP subset PR #10302 #10350
Conversation
lyakh
requested review from
abonislawski,
dabekjakub,
dbaluta,
iuliana-prodan,
jxstelter,
kv2019i,
lbetlej,
lgirdwood,
mmaka1,
plbossart and
ranj063
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR introduces new heap allocation APIs and refactors memory allocation in the module adapter to consolidate allocation/deallocation logic into helper functions.
- Adds
sof_heap_alloc()andsof_heap_free()functions for heap-specific memory allocation with flag support - Refactors module adapter memory allocation/deallocation into dedicated helper functions
- Extends module allocation API with
mod_alloc_ext()to support allocation flags (e.g., coherency control)
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| zephyr/lib/alloc.c | Adds sof_heap_alloc() and sof_heap_free() functions for managing heap allocations with flag support |
| zephyr/include/rtos/alloc.h | Declares new heap allocation functions |
| test/cmocka/src/common_mocks.c | Provides mock implementations of heap allocation functions for testing |
| src/platform/library/lib/alloc.c | Adds stub implementations using standard malloc/free |
| src/lib/alloc.c | Adds stub implementations using standard malloc/free |
| src/include/sof/audio/module_adapter/module/generic.h | Adds mod_alloc_ext() declaration and converts mod_alloc_align() to inline wrapper; fixes typo |
| src/audio/module_adapter/module_adapter.c | Extracts memory allocation/free logic into helper functions |
| src/audio/module_adapter/module/generic.c | Renames mod_alloc_align() to mod_alloc_ext() with flags parameter; fixes typos |
| posix/include/rtos/kernel.h | Adds stub k_heap struct definition |
| posix/include/rtos/alloc.h | Adds forward declarations and stub functions for heap allocation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| void *sof_heap_alloc(struct k_heap *heap, uint32_t flags, size_t bytes, | ||
| size_t alignment) | ||
| { | ||
| return malloc(bytes); |
There was a problem hiding this comment.
The alignment parameter is not used in this implementation. While this is a stub implementation, it should either align the allocated memory or document why alignment is not respected. Consider using aligned_alloc() or documenting this limitation.
| return malloc(bytes); | |
| /* Use aligned_alloc to respect the alignment parameter. | |
| * Note: aligned_alloc requires that bytes is a multiple of alignment. | |
| */ | |
| if (alignment && (bytes % alignment != 0)) { | |
| bytes = ((bytes + alignment - 1) / alignment) * alignment; | |
| } | |
| return aligned_alloc(alignment ? alignment : sizeof(void *), bytes); |
| void *sof_heap_alloc(struct k_heap *heap, uint32_t flags, size_t bytes, | ||
| size_t alignment) | ||
| { | ||
| return malloc(bytes); |
There was a problem hiding this comment.
The alignment parameter is not used in this implementation. While this is a stub implementation, it should either align the allocated memory or document why alignment is not respected. Consider using aligned_alloc() or documenting this limitation.
| return malloc(bytes); | |
| return aligned_alloc(alignment, bytes); |
| void WEAK *sof_heap_alloc(struct k_heap *heap, uint32_t flags, size_t bytes, | ||
| size_t alignment) | ||
| { | ||
| (void)heap; | ||
| (void)flags; | ||
| (void)alignment; | ||
|
|
||
| return malloc(bytes); | ||
| } |
There was a problem hiding this comment.
The alignment parameter is marked as unused but is not respected by the underlying malloc() call. This could lead to misaligned memory in tests. Consider using aligned_alloc() to honor the alignment requirement or documenting that alignment is not supported in test mocks.
lyakh
force-pushed
the
user-p1_1
branch
3 times, most recently
from
8592072 to
15c2b47
Compare
Ok, I'm good after chat. |
|
SOFCI TEST |
|
|
||
| if (!mod) { | ||
| comp_err(dev, "failed to allocate memory for module"); | ||
| goto err; |
There was a problem hiding this comment.
Would you consider moving the code from the 'err' label to this location?
There was a problem hiding this comment.
I'm usually all for "early returns," i.e. for
if (fail)
return;
instead of
if (fail)
goto err;
...
return 0;
err:
return -EINVAL;
but if there's some cleanup to be done in (all) error cases, I usually prefer a goto. When there's only one error case - well, that's a grey zone for me... But since we're likely to extend this function in the future, I'd rather keep it this way.
Add sof_heap_alloc() and sof_heap_free() to allocate and free memory on a private heap. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Memory, allocated using the module_driver_heap_*() API, should be freed using module_driver_heap_free(), not rfree(). Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Allocate and free memory for module and device objects and for input pins to separate functions to support varying allocation methods. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Add a definition of struct k_heap and SOF k_heap wrappers sof_heap_alloc() and sof_heap_free() for host native builds. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Add a new mod_alloc_ext() allocation function with support for allocator flags. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
s/allocatd/allocated/ Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
|
SOFCI TEST |
|
@lgirdwood @lyakh Hmm, you broke the doxygen CI run with this PR. Now we have this failing a required check in all PRs. |
4 participants
The simpler part of #10302 - no integration with the driver heap API