Per pipeline vregions/mod_alloc_ctx by jsarha · Pull Request #11164 · thesofproject/sof

jsarha · 2026-09-03T13:16:40Z

This is the last PR from my vregions proto PR: #10783

This move all low latency module allocations within a pipeline to a pipeline specific vregion. This should help with memory fragmentation as the whole vregion can freed when the pipeline is closed and freed. The later commit will put the vregion behind struct mod_alloc_ctx.

While reviewing this my self, the pipeline->heep looks now a bit redundant. There maybe room for one more PR, that would get rid of pipeline->heap and replacing its usage with mod_alloc_ctx.

Copilot

🟡 Changes recommended

There are confirmed allocator/ownership bugs in module_adapter_mem_free() that can free memory with the wrong allocator and potentially free a pipeline-owned alloc context on early error paths.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR introduces a pipeline-scoped allocation context (mod_alloc_ctx) so that low-latency (LL) modules within the same pipeline can allocate from a pipeline-specific vregion (when available), reducing fragmentation and enabling bulk release when the pipeline is torn down.

Changes:

Add a shared per-pipeline allocation context pointer to struct pipeline.
Create and manage an optional pipeline vregion during pipeline lifecycle (create/free) and switch allocations to interim mode on pipeline_complete().
Update module adapter allocation to use the pipeline’s alloc context for LL modules (falling back to heap when no vregion is present).

File summaries

File	Description
src/include/sof/audio/pipeline.h	Adds a pipeline-level `mod_alloc_ctx *alloc` pointer for shared module allocations.
src/audio/pipeline/pipeline-graph.c	Creates/frees the pipeline alloc context and optional vregion; switches vregion to interim on pipeline completion.
src/audio/module_adapter/module_adapter.c	Routes LL module allocations/frees through the pipeline alloc context when available.

Review details

Suppressed comments (1)

src/audio/module_adapter/module_adapter.c:227

alloc is allocated via sof_heap_alloc(mod_heap, ...) (when not using the pipeline alloc context), but here it is freed with rfree(). Mixing allocators can corrupt heap state; free it with sof_heap_free(alloc->heap, alloc) instead.

	} else if (alloc->vreg) {
		if (!vregion_put(alloc->vreg))
			rfree(alloc);
	} else {
		rfree(alloc);

Files reviewed: 3/3 changed files
Comments generated: 1
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

lyakh

not a complete review yet, but I think at least this should be rearranged into 3 commits: (1) add mod_alloc_ctx to pipeline and move heap to it (part of your current second commit), (2) allocate vregion when needed (your current first commit), (3) move module allocations to pipeline vregion (the rest of your second commit)

lyakh · 2026-09-03T14:08:07Z

 		vregion_free(mod_vreg, mod);
-		if (!vregion_put(mod_vreg))
+		if (!vregion_put(mod_vreg) || proc_domain == COMP_PROCESSING_DOMAIN_LL)
 			sof_heap_free(alloc->heap, alloc);


are you sure this is symmetric? IIUC you only allocate LL pipeline memory on vregion if certain particular conditions are satisfied (certain extended init parameters are set). But here you free it unconditionally for all LL pipelines

Yes, I am sure. If we have no memory data for the pipeline, we do not create the vregion at all, and everything works through a regular heap. And we can call vregion_get(NULL) and vregion_put(NULL) as much as we want.

intel-sofci · 2026-09-03T14:41:20Z

PR 11164: test results

Run date: 2026-09-03 21:39 UTC

Tested commit: 6407e81546647d9dddf6310425669e6d07212e3c

jsarha · 2026-09-03T15:41:57Z

This at least fails in ll-userspace setup. I'll fix those issues first. I'll then see about rearranging the commits in completely new order.

Replace the bare k_heap pointer in struct pipeline with a mod_alloc_ctx object. The context is created in pipeline_new() and freed symmetrically in pipeline_free(). No vregion is associated with this context yet, so allocation behavior is unchanged: sof_ctx_alloc()/sof_ctx_zalloc()/sof_ctx_free() fall back to the plain heap when the context has no vregion. Update all remaining pipeline->heap consumers, including the pipeline and trigger task allocations in pipeline-schedule.c, to go through the alloc context instead of the heap pointer directly. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

Create a per-pipeline vregion in pipeline_new() when the IPC4 pipeline extension payload specifies the required heap size, and attach it to the pipeline's alloc context. LL modules on a pipeline with a vregion use it as their allocation backend via vregion_get(), instead of the driver's default heap, and share the pipeline's mod_alloc_ctx. A use_ppl_alloc flag gates the sharing to LL modules only, so DP modules continue to create their own vregion and alloc context as before. Also the behaviour in the case the where ppl_alloc is not available remains unchanged. module_adapter_mem_free() detects whether a module's alloc belongs to its pipeline and either just releases the vregion reference (ppl_alloc case) or tears down the module's own alloc. Setting of dev->pipeline is moved earlier in module_adapter_new_ext() so that we can still use module_adapter_mem_free() in its error handling. Call vregion_set_interim() for the pipeline vregion in pipeline_complete() to switch the allocator to interim mode after all lifetime allocations are done, and release it in pipeline_free(), warning if the refcount does not reach zero. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

jsarha · 2026-09-03T21:03:51Z

All suggestios from @lyakh applied. The new first commit now adds struct mod_alloc_ctx *alloc to struct pipeline and removes the old struct k_heap heap. After this commit I felt that adding extraneous vreg object to struct pipeline is kind of backwards and dropped that intermediate step and I added the vreg directly to the pipeline->alloc .

lyakh · 2026-09-04T06:53:30Z

 	/* show heap status */
 	heap_trace_all(0);

+	alloc = sof_heap_alloc(heap, SOF_MEM_FLAG_USER, sizeof(*alloc), 0);


I think in the original version alloc was allocated using rmalloc(), i.e. only accessible to the kernel. I think that this version is correct and the previous one would have problems with userspace LL. Could you confirm?

Yes, that is why I changed it.

lyakh · 2026-09-04T07:06:57Z

+	ipc_pipe = ipc_get_comp_by_ppl_id(ipc, COMP_TYPE_PIPELINE, config->pipeline_id,
+					  IPC_COMP_IGNORE_REMOTE);
+	if (ipc_pipe && ipc_pipe->pipeline)
+		ppl_alloc = ipc_pipe->pipeline->alloc;


is it actually valid to have ipc_pipe == NULL? And why do we ignore remote pipelines? I suppose remote pipelines are only possible with DP, is that the intention here? But DP on the same core we accept. I think you want IPC_COMP_ALL here. And ipc_pipe shouldn't be NULL even for chain DMA, right? So if it's NULL, it's an error? And ipc_pipe->pipeline should be non-NULL too? Also the allocation context must be there too. So in practice ppl_alloc is always non-NULL?

That is now indeed true.

lyakh · 2026-09-04T07:09:50Z

 	uint32_t flags = config->proc_domain == COMP_PROCESSING_DOMAIN_DP ?
 		SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT : SOF_MEM_FLAG_USER;
+	bool use_ppl_alloc = ppl_alloc &&
+			     config->proc_domain == COMP_PROCESSING_DOMAIN_LL;


if my comment below is correct, ppl_alloc != NULL always, so this is true for all LL components.

Yes, indeed. There should always be a mod_alloc_ctx. The both the heap and vreg may be NULL, but that does not matter.

lyakh · 2026-09-04T07:13:02Z

+
+		if (IS_ENABLED(CONFIG_SOF_VREGIONS) && IS_ENABLED(CONFIG_USERSPACE) &&
+		    !IS_ENABLED(CONFIG_SOF_USERSPACE_USE_DRIVER_HEAP) &&
+		    config->proc_domain == COMP_PROCESSING_DOMAIN_DP) {


if my other comments are correct, then you only enter this else branch for DP components, so this condition is always true. If that's right let's remove that check here and add comments that the if branch is for all LL components, and here it's only DP. And let's rename the use_ppl_alloc variable then into something like ll_domain or ll_sched_dom or similar.

In fact you can do even better: keep the original if in old lines 96-97. Since it checks for DP, you're guaranteed, that your current use_ppl_alloc is false in that case. Then replace else in old line 108 with else if (use_ppl_alloc) or just else if (config->proc_domain == COMP_PROCESSING_DOMAIN_LL) and then use the last else branch - an equivalent to the present one. That way the diff will be much smaller and easier to review.

lyakh · 2026-09-04T07:40:54Z

+		alloc = sof_heap_alloc(mod_heap, flags, sizeof(*alloc), 0);
+		if (!alloc) {
+			vregion_put(mod_vreg);
+			return NULL;


use goto edev instead, optionally add an error message

mod_vreg is not defined outside the else branch, but I'll add an error message.

lyakh · 2026-09-04T07:53:38Z

-		sof_heap_free(mod_heap, mod->dev);
-		sof_heap_free(mod_heap, mod);
-		sof_heap_free(mod_heap, alloc);
+		rfree(alloc);


this is unclear now. Is this possible? If it is - you entered this function with valid non-NULL mod and mod->dev, what happens to them now? If this is invalid, then this should be made clear.

Oh, the rfree() is obviously wrong and should be sof_heap_free(alloc->heap, alloc), but what else are you suggesting?

I am not sure I am following your question. Are you talking about the final else-branch? That is needed in case alloc->vreg == NULL, like in non userspace dp case. Then there is no reason to delay the freeing of alloc ctx.

lyakh · 2026-09-04T08:20:25Z

@jsarha looks much better now! Much easier to understand now.

Copilot AI lite review requested due to automatic review settings September 3, 2026 13:16

jsarha requested review from dbaluta, kv2019i, lbetlej, lgirdwood, mmaka1, plbossart and ranj063 as code owners September 3, 2026 13:16

Copilot started reviewing on behalf of jsarha September 3, 2026 13:21 View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

Comment thread src/audio/module_adapter/module_adapter.c

lyakh requested changes Sep 3, 2026

View reviewed changes

Jyri Sarha added 2 commits September 3, 2026 20:35

jsarha force-pushed the per_pipeline_vregions branch from 843591d to 6407e81 Compare September 3, 2026 20:57

jsarha requested review from abonislawski and iuliana-prodan as code owners September 3, 2026 20:57

jsarha mentioned this pull request Sep 3, 2026

app: overlays: ptl: increased LL user-space heap #11143

Merged

lyakh reviewed Sep 4, 2026

View reviewed changes

kv2019i mentioned this pull request Sep 4, 2026

[DRAFT/TESTING] userspace LL/audio test PR (latest ver: V43) #10558

Open

Uh oh!

Conversation

jsarha commented Sep 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

lyakh 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!

Uh oh!

Uh oh!

Uh oh!

intel-sofci commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR 11164: test results

Uh oh!

jsarha commented Sep 3, 2026

Uh oh!

jsarha commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

lyakh commented Sep 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

intel-sofci commented Sep 3, 2026 •

edited

Loading

jsarha commented Sep 3, 2026 •

edited

Loading