buffer: extend ability to allocate on specific heap to all functions by kv2019i · Pull Request #10719 · thesofproject/sof

kv2019i · 2026-04-23T13:13:49Z

Complete making buffer.h compatible with user-space work.

Tested as part of the bigger #10558

Copilot

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_buffer allocation/free paths to use heap-aware sof_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.

Copilot · 2026-04-23T13:20:52Z

@@ -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;



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().

Suggested change

if (!alignment)

alignment = PLATFORM_DCACHE_ALIGN;

The same semantics are now implemented by sof_heap_alloc() so 0 can be passed forward to it.

Copilot · 2026-04-23T13:20:52Z

+	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;
 	}


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).

Fixed in V2.

Copilot · 2026-04-23T13:20:52Z

+		new_ptr = sof_heap_alloc(buffer->audio_buffer.heap, buffer->flags, new_size, alignment);
+		if (new_ptr)


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.

sof_heap_alloc() implements these semantics for alignment==0, so no need to add a special case for PLATFORM_DCACHE_ALIGN here.

wjablon1 · 2026-04-24T10:10:28Z

-	if (new_ptr)
+	if (new_ptr) {
+		sof_heap_free(buffer->audio_buffer.heap, audio_stream_get_addr(&buffer->stream));
 		buffer->stream.addr = new_ptr;


Shouldn't we use audio_stream_set_addr here for setting add ptr? Or just get rid of those helper functions if they are not used consistently.

Ack @wjablon1 , you are right, this would be a step into wrong direction, we should keep using the audio_stream abstraction. Will fix in V2.

wjablon1 · 2026-04-24T10:40:20Z

+				minimum_size, buffer->flags);
+			return -ENOMEM;
+		}
+		new_size = minimum_size;


Why is this step needed? Is it for the extra alignment as the comment in 380 line says?
/* Align preferred size to a multiple of the minimum size */

Right, I was struggling with this one as well. As far as I understand the old code, this was a bug in the old code. If the allocation fails (so new_ptr==NULL), but the old allocation is still bigger than the last allocation attempted, the old implementation would set the buffer size to a value that is smaller than what the user asked, but also smaller than what is actually allocated.

This seems wrong, so I'm setting new size to the minimum user asked.

Other alternative would be to set the size of the actual size (allocated).

Not sure which is best (and/or least confusing).

but for the alternative you probably would need to adjust the actual size to meet the mentioned "extra alignment"... and still this is only fallback for the initial allocation failure so I would't bother... better option would be to have a proper realloc function.

kv2019i

Thanks @wjablon1 for the review, some comments inline.

kv2019i · 2026-04-24T11:27:24Z

-	if (new_ptr)
+	if (new_ptr) {
+		sof_heap_free(buffer->audio_buffer.heap, audio_stream_get_addr(&buffer->stream));
 		buffer->stream.addr = new_ptr;


Ack @wjablon1 , you are right, this would be a step into wrong direction, we should keep using the audio_stream abstraction. Will fix in V2.

kv2019i · 2026-04-24T11:34:04Z

+				minimum_size, buffer->flags);
+			return -ENOMEM;
+		}
+		new_size = minimum_size;


Right, I was struggling with this one as well. As far as I understand the old code, this was a bug in the old code. If the allocation fails (so new_ptr==NULL), but the old allocation is still bigger than the last allocation attempted, the old implementation would set the buffer size to a value that is smaller than what the user asked, but also smaller than what is actually allocated.

This seems wrong, so I'm setting new size to the minimum user asked.

Other alternative would be to set the size of the actual size (allocated).

Not sure which is best (and/or least confusing).

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>

kv2019i · 2026-04-24T11:59:10Z

V2 pushed:

addressed correct use of audio_stream_set_addr() raised by @wjablon1

Copilot AI review requested due to automatic review settings April 23, 2026 13:13

kv2019i requested review from abonislawski, dabekjakub, dbaluta, iuliana-prodan, lbetlej, lgirdwood, lyakh, mmaka1 and plbossart as code owners April 23, 2026 13:13

Copilot started reviewing on behalf of kv2019i April 23, 2026 13:15 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

kv2019i force-pushed the 202604-rbrealloc branch from 41c0708 to ed502d7 Compare April 23, 2026 14:40

kv2019i requested a review from jsarha April 23, 2026 14:41

wjablon1 reviewed Apr 24, 2026

View reviewed changes

kv2019i commented Apr 24, 2026

View reviewed changes

kv2019i added 2 commits April 24, 2026 14:58

buffer: extend ability to allocate on specific heap to all functions

03c5bb9

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>

alloc.h: remove rbrealloc() and rbrealloc_align()

f8e79c2

These interfaces are no longer used anywhere, so they can be safely removed. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>

kv2019i force-pushed the 202604-rbrealloc branch from ed502d7 to f8e79c2 Compare April 24, 2026 11:58

		@@ -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;

		new_ptr = sof_heap_alloc(buffer->audio_buffer.heap, buffer->flags, new_size, alignment);
		if (new_ptr)

Conversation

kv2019i commented Apr 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wjablon1 Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kv2019i commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wjablon1 Apr 24, 2026 •

edited

Loading