lib: vregion: lazy interim heap creation from remaining lifetime space#10863
lib: vregion: lazy interim heap creation from remaining lifetime space#10863jsarha wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the Zephyr vregion allocator to use a single contiguous backing buffer where allocations begin in “lifetime” mode and later switch to a lazily-created “interim” k_heap carved out of the remaining space. This simplifies partitioning and removes the enum vregion_mem_type parameter from vregion_alloc*() APIs by making allocation mode internal state.
Changes:
- Replace fixed lifetime/interim partitions with a single vregion buffer and a lazy interim heap initialized when switching modes.
- Update allocation APIs and call sites to remove the explicit
vregion_mem_typeparameter; addvregion_set_interim(). - Switch DP components to interim allocations at pipeline completion; update Zephyr vregion tests accordingly.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
zephyr/test/vregion.c |
Updates ztest to new vregion_create() signature and mode-switching behavior. |
zephyr/lib/vregion.c |
Implements new contiguous layout, internal allocation mode, and lazy interim heap initialization; updates allocation/free APIs. |
zephyr/lib/fast-get.c |
Updates vregion allocation call to new vregion_alloc_align() signature. |
zephyr/include/rtos/alloc.h |
Updates sof_ctx_alloc() to use new vregion allocation signatures. |
src/lib/objpool.c |
Updates vregion allocation calls to new signatures. |
src/include/sof/lib/vregion.h |
Updates public API signatures; adds vregion_set_interim() and VREGION_MEM_TYPE_INVALID. |
src/audio/pipeline/pipeline-graph.c |
Switches DP modules’ vregions to interim mode at pipeline completion. |
src/audio/module_adapter/module_adapter.c |
Updates vregion creation and allocation calls to new API. |
Comments suppressed due to low confidence (2)
zephyr/lib/vregion.c:284
- interim_alloc() takes a struct vregion *vr parameter but does not use it, which can trigger unused-parameter warnings in some builds. Either remove the parameter (and adjust callers) or explicitly mark it unused.
static void *interim_alloc(struct vregion *vr, struct interim_heap *heap,
size_t size, size_t align)
{
void *ptr;
ptr = k_heap_aligned_alloc(&heap->heap, align, size, K_NO_WAIT);
if (!ptr)
zephyr/lib/vregion.c:320
- lifetime_alloc() takes a struct vregion *vr parameter but does not use it, which can trigger unused-parameter warnings in some builds. Either remove the parameter (and adjust callers) or explicitly mark it unused.
static void *lifetime_alloc(struct vregion *vr, struct vlinear_heap *heap,
size_t size, size_t align)
{
void *ptr;
uint8_t *aligned_ptr;
size_t heap_obj_size;
| } else { | ||
| struct processing_module *mod = comp_mod(current); | ||
|
|
||
| if (mod->priv.resources.alloc) | ||
| vregion_set_interim(mod->priv.resources.alloc->vreg); |
There was a problem hiding this comment.
Not sure if we can anymore have non module components in a pipeline, but I'll fix that.
4e56898 to
22382c0
Compare
Refactor the vregion memory layout to use a single contiguous buffer instead of two separately page-aligned partitions. The vregion lifetime allocations start at the base and growing upward. The interim k_heap is created lazily when vregion_set_interim() is called for whatever space remains after lifetime allocations. This eliminates the rigid partition boundary that previously wasted memory when lifetime usage was smaller or larger than pre-configured. The interim heap creation is deferred until actually needed, at which point the lifetime region is sealed and any further allocation requests are redirected to the interim. The vregion tracks internal allocation mode (lifetime or interim). All allocations start in lifetime mode. The vregion_set_interim() function switches to interim mode. The enum vregion_mem_type parameter has been removed from the vregion_alloc*() API since the mode is now internal state. vregion_set_interim() is called in pipeline_comp_complete() in case the component's processing domain is Data Processing. The DP components are so far the only place where vregions are used at the moment. Add a guard to skip k_heap_init() in interim_heap_init() if the remaining interim size is too small (< 1024 bytes), which would otherwise trigger an assert failure in sys_heap_init(). Mark the vregion type to VREGION_MEM_TYPE_INVALID if that happens. Key changes: - vregion_create(): Takes single memsize argument instead of separate lifetime_size and interim_size - New vregion_set_interim(): Switches allocation mode from lifetime to interim, warns on repeated calls - vregion_alloc*(): No longer take enum vregion_mem_type parameter, use internal state instead - New interim_heap_init(): Called lazily, page-aligns interim start, logs lifetime used and interim available at INFO level - vregion_free(): Guards interim pointer range - pipeline_comp_completee(): Calls vregion_set_interim() to switch mode Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
22382c0 to
2c01641
Compare
lyakh
left a comment
There was a problem hiding this comment.
this changes the concept of course. E.g. with the original approach an audio module could during initialisation explicitly allocate a (large) "interim" buffer if it knows, that it can be reallocated later, e.g. during reconfiguration. Now this is impossible, so that buffer will be allocated in the "lifetime" part and will be wasted there when the module re-allocates it. This can also lead to out-of-memory situations if the module claims its memory needs based on just one of those buffers, whereas now it will memory for at least 2 of them... Maybe in such cases the module can switch into the interim mode earlier - when allocating that buffer? Maybe we need a mod_alloc_switch_dynamic(mod) and EXPORT_SYMBOL() it then? This PR makes the API simpler and cleaner too.
| switch (vr->type) { | ||
| case VREGION_MEM_TYPE_INTERIM: | ||
| p = interim_alloc(&vr->interim, size, alignment); | ||
| p = interim_alloc(vr, &vr->interim, size, alignment); |
There was a problem hiding this comment.
if you're already passing vr, you don't need to pass &vr->interim additionally
| } | ||
|
|
||
| /* interim heap starts right after current lifetime pointer, page-aligned */ | ||
| interim_base = UINT_TO_POINTER(ALIGN_UP(POINTER_TO_UINT(vr->lifetime.ptr), |
There was a problem hiding this comment.
just an idea: Is it possible to allocate the remaining space out of the lifetime heap and use if for the interim heap? That would also "cap" the lifetime (prevent further allocations).
Refactor the vregion memory layout to use a single contiguous buffer instead of two separately page-aligned partitions. The vregion lifetime allocations start at the base and growing upward. The interim k_heap is created lazily when vregion_set_interim() is called for whatever space remains after lifetime allocations.
This eliminates the rigid partition boundary that previously wasted memory when lifetime usage was smaller or larger than pre-configured. The interim heap creation is deferred until actually needed, at which point the lifetime region is sealed and any further allocation requests are redirected to the interim.
The vregion tracks internal allocation mode (lifetime or interim). All allocations start in lifetime mode. The vregion_set_interim() function switches to interim mode. The enum vregion_mem_type parameter has been removed from the vregion_alloc*() API since the mode is now internal state.
vregion_set_interim() is called in pipeline_comp_complete() in case the component's processing domain is Data Processing. The DP components are so far the only place where vregions are used at the moment.
Add a guard to skip k_heap_init() in interim_heap_init() if the remaining interim size is too small (< 1024 bytes), which would otherwise trigger an assert failure in sys_heap_init(). Mark the vregion type to VREGION_MEM_TYPE_INVALID if that happens.
Key changes: