buffer: extend ability to allocate on specific heap to all functions #10719
buffer: extend ability to allocate on specific heap to all functions #10719kv2019i wants to merge 2 commits intothesofproject:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Extends heap-specific allocation support for audio buffers to improve Zephyr userspace compatibility by removing rbrealloc*() usage and switching buffer allocations/frees to sof_heap_alloc() / sof_heap_free().
Changes:
- Removed
rbrealloc_align()API and its platform/test implementations. - Updated
comp_bufferallocation/free paths to use heap-awaresof_heap_alloc()/sof_heap_free(). - Simplified buffer resize logic to allocate/free explicitly instead of reallocating.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
zephyr/lib/alloc.c |
Removes Zephyr-side rbrealloc_align() implementation (API cleanup). |
zephyr/include/rtos/alloc.h |
Removes rbrealloc*() declarations from Zephyr allocation API. |
posix/include/rtos/alloc.h |
Removes rbrealloc*() declarations from POSIX allocation API. |
src/audio/buffers/comp_buffer.c |
Switches buffer data allocation/free + resize to heap-aware APIs. |
src/lib/alloc.c |
Removes non-Zephyr rbrealloc_align() implementation. |
src/platform/library/lib/alloc.c |
Removes platform library rbrealloc_align() implementation. |
test/cmocka/src/common_mocks.c |
Removes mock rbrealloc_align() implementation to match API removal. |
| @@ -341,14 +341,8 @@ int buffer_set_size(struct comp_buffer *buffer, uint32_t size, uint32_t alignmen | |||
| if (size == audio_stream_get_size(&buffer->stream)) | |||
| return 0; | |||
|
|
|||
There was a problem hiding this comment.
In buffer_set_size(), when callers pass alignment==0 (e.g. module_adapter.c and copier_ipcgtw.c do), this now forwards 0 to sof_heap_alloc(). Previously the alignment==0 path used rbrealloc() which implicitly aligned to PLATFORM_DCACHE_ALIGN. To preserve behavior and avoid potentially invalid/insufficient alignment from the underlying heap, map alignment==0 to PLATFORM_DCACHE_ALIGN (or to audio_stream_get_align()/existing stream alignment) before calling sof_heap_alloc().
| if (!alignment) | |
| alignment = PLATFORM_DCACHE_ALIGN; |
There was a problem hiding this comment.
The same semantics are now implemented by sof_heap_alloc() so 0 can be passed forward to it.
| for (new_size = preferred_size; new_size >= minimum_size; | ||
| new_size -= minimum_size) { | ||
| new_ptr = sof_heap_alloc(buffer->audio_buffer.heap, buffer->flags, new_size, alignment); | ||
| if (new_ptr) | ||
| break; | ||
| } |
There was a problem hiding this comment.
buffer_set_size_range(): the resize loop decrements new_size until it drops below minimum_size. If all allocations fail, new_ptr stays NULL and new_size will end up < minimum_size (often 0), but the function later calls buffer_init_stream(buffer, new_size). That can set an invalid size despite the earlier validation. Consider handling the “no allocation succeeded” case explicitly: for grow requests return -ENOMEM without changing the stream; for shrink requests skip allocating a new block and just set the stream size to the chosen target (>= minimum_size).
| new_ptr = sof_heap_alloc(buffer->audio_buffer.heap, buffer->flags, new_size, alignment); | ||
| if (new_ptr) |
There was a problem hiding this comment.
In buffer_set_size_range(), alignment is forwarded directly to sof_heap_alloc(). When alignment==0, earlier code used rbrealloc() (i.e., PLATFORM_DCACHE_ALIGN via the rbrealloc wrapper). To avoid potentially allocating buffers with weaker-than-expected alignment, consider normalizing alignment==0 to PLATFORM_DCACHE_ALIGN (or the stream’s current alignment) before the allocation loop.
There was a problem hiding this comment.
sof_heap_alloc() implements these semantics for alignment==0, so no need to add a special case for PLATFORM_DCACHE_ALIGN here.
Continue the work in commit 9567234 ("buffer: allocate on specific heap") and add ability to specify the heap to all buffer interface functions. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
These interfaces are no longer used anywhere, so they can be safely removed. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
41c0708 to
ed502d7
Compare
Complete making buffer.h compatible with user-space work.
Tested as part of the bigger #10558