Mod alloc ctx wrapper#10802
Conversation
Add a small header with inline wrapper functions that abstract the vregion vs heap allocation decision based on a mod_alloc_ctx context: - sof_ctx_alloc(): full allocation with flags and alignment, uses vregion_alloc_coherent_align() or vregion_alloc_align() when a vregion is available, falls back to sof_heap_alloc() otherwise. - sof_ctx_zalloc(): simplified zero-flag, zero-alignment shorthand. - sof_ctx_free(): matching free function. These replace the repeated if/else pattern found throughout the buffer and module allocator code. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
Replace the open-coded vregion vs heap allocation and free patterns in z_impl_mod_alloc_ext() and free_contents() with the new sof_ctx_alloc() and sof_ctx_free() wrappers. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
Replace open-coded vregion vs heap allocation and free patterns in buffer_alloc(), buffer_alloc_range(), buffer_alloc_struct(), buffer_free(), buffer_set_size(), and buffer_set_size_range() with the new sof_ctx_alloc() and sof_ctx_free() wrappers. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
Replace open-coded vregion vs heap allocation and free patterns in ring_buffer_free() and ring_buffer_create() with the new sof_ctx_alloc() and sof_ctx_free() wrappers. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a small allocation/free wrapper around struct mod_alloc_ctx to centralize the repeated “heap vs vregion” allocation pattern, and updates several call sites to use it.
Changes:
- Added
sof_ctx_alloc()/sof_ctx_free()helpers for allocating/freeing from amod_alloc_ctx(heap + optional vregion). - Replaced repeated
if (!alloc->vreg) ... else ...allocation/free logic in module adapter generic allocations, ring buffer, and comp buffer code. - Added a
sof_ctx_zalloc()helper (but it currently does not zero-initialize memory; see comments).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/include/sof/ctx_alloc.h | Adds the new mod_alloc_ctx allocation/free helper API. |
| src/audio/module_adapter/module/generic.c | Switches module allocation/free paths to the new ctx helpers. |
| src/audio/buffers/ring_buffer.c | Uses ctx helpers for ring buffer struct/data allocations and frees. |
| src/audio/buffers/comp_buffer.c | Uses ctx helpers for userspace-LL buffer stream allocations and frees. |
| /** | ||
| * Allocate zero-initialized memory from a mod_alloc_ctx context. | ||
| * @param ctx Allocation context. | ||
| * @param size Size in bytes. | ||
| * @return Pointer to allocated memory or NULL on failure. | ||
| */ | ||
| static inline void *sof_ctx_zalloc(struct mod_alloc_ctx *ctx, size_t size) | ||
| { | ||
| return sof_ctx_alloc(ctx, 0, size, 0); | ||
| } |
| if (flags & SOF_MEM_FLAG_COHERENT) | ||
| return vregion_alloc_coherent_align(ctx->vreg, VREGION_MEM_TYPE_INTERIM, | ||
| size, alignment); | ||
|
|
| */ | ||
| static inline void *sof_ctx_zalloc(struct mod_alloc_ctx *ctx, size_t size) | ||
| { | ||
| return sof_ctx_alloc(ctx, 0, size, 0); |
There was a problem hiding this comment.
this seems rather wrong and incomplete. If you do want such a wrapper, you'd want it with flags and alignment too. And memset() of course
| buffer = vregion_alloc_coherent(alloc->vreg, VREGION_MEM_TYPE_INTERIM, sizeof(*buffer)); | ||
| else | ||
| buffer = vregion_alloc(alloc->vreg, VREGION_MEM_TYPE_INTERIM, sizeof(*buffer)); | ||
| buffer = sof_ctx_alloc(alloc, flags, sizeof(*buffer), 0); |
There was a problem hiding this comment.
I think you wanted to remove lines 215-216 too
| * @param alignment Required alignment in bytes. | ||
| * @return Pointer to allocated memory or NULL on failure. | ||
| */ | ||
| static inline void *sof_ctx_alloc(struct mod_alloc_ctx *ctx, uint32_t flags, |
There was a problem hiding this comment.
@jsarha @lyakh @lgirdwood My head spins a bit with the naming. We have been adding layers to heap allocation recently and it's starts to be very hard to follow:
- rtos/alloc.h: rmalloc/rzalloc() and the new sof_heap_alloc()
- sof/lib/vregion.h: vregion_alloc()
- sof/lib/ctx_alloc.h: sof_ctx_alloc()
I wonder if we could come up with a better name for ctx_alloc.h? The main point seems to be capability to group heap allocs to region, so maybe:
- sof_region_alloc() ? this would use vregion.h if available and fallback to sof_heap_alloc() otherwise
It started to look like the "if (!res->alloc->vreg)" -pattern will fill up the universe, so thought we'd better do something about it.
I've tested this code on top of main, and I've also tested it as a base for our current ll-userspace feature thread. The both appear to work Ok.