scheduler: dp: fix memory leak / implicit freeing by lyakh · Pull Request #10515 · thesofproject/sof

lyakh · 2026-02-03T08:21:29Z

Task memory was freed implicitly by one DP implementation and was leaked by another one. Free it explicitly for both.

An alternative implementation of #10512

Copilot

Pull request overview

This PR centralizes and fixes DP task teardown so that task memory and associated kernel objects are explicitly freed in both DP implementations, eliminating a leak in one path and removing reliance on implicit module-resource cleanup in the other.

Changes:

Introduces scheduler_dp_internal_free(struct task *task) as the unified DP task teardown hook, invoked from scheduler_dp_task_free() in zephyr_dp_schedule.c.
For the kernel-thread DP implementation (zephyr_dp_schedule_thread.c), documents the task/pdata allocation layout and uses scheduler_dp_internal_free() to free any dynamically allocated k_objects and the single heap block containing struct task + struct task_dp_pdata.
For the userspace-application DP implementation (zephyr_dp_schedule_application.c), factors out scheduler_dp_domain_free() as a file-local helper and ensures scheduler_dp_internal_free() frees the DP task allocation (via mod_free), its kernel objects, and its memory domain; the header is updated accordingly.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
`src/schedule/zephyr_dp_schedule_thread.c`	Adds a clarified helper allocation layout comment and a `scheduler_dp_internal_free()` implementation that correctly frees per-task `k_object`s (when present) and the contiguous DP task allocation from `user_heap`.
`src/schedule/zephyr_dp_schedule_application.c`	Makes `scheduler_dp_domain_free()` file-local, introduces a shared `scheduler_dp_task_memory` struct, and implements `scheduler_dp_internal_free()` to release semaphore, thread object, DP task memory (via `mod_free`), and the associated memory domain, with matching error-path cleanup in `scheduler_dp_task_init()`.
`src/schedule/zephyr_dp_schedule.h`	Replaces the public `scheduler_dp_domain_free()` declaration (and its non-application stub) with a declaration for the new `scheduler_dp_internal_free()` teardown helper.
`src/schedule/zephyr_dp_schedule.c`	Switches `scheduler_dp_task_free()` from inlined conditional cleanup (k_objects + domain) to a single call to `scheduler_dp_internal_free()`, which now handles the full per-task cleanup including previously leaked allocations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

checkupup · 2026-02-03T08:59:29Z

src/schedule/zephyr_dp_schedule.h

-#else
-static inline void scheduler_dp_domain_free(struct processing_module *pmod) {}
-#endif
+void scheduler_dp_internal_free(struct task *task);


Could we just change void scheduler_dp_internal_free(struct task *task); as void scheduler_dp_domain_free(struct processing_module *pmod); to force each dp schedule implements their own scheduler_dp_domain_free and avoid introducing a new interface name scheduler_dp_internal_free?

@checkupup it isn't about just freeing the domain any more, so I preferred to use a new name. It isn't that much of an interface, it's internal to the DP scheduler, so don't think there's an issue with that

checkupup · 2026-02-03T09:02:03Z

src/schedule/zephyr_dp_schedule_application.c

+	k_object_free(pdata->thread);
+	mod_free(pdata->mod, container_of(task, struct scheduler_dp_task_memory, task));
+
+	scheduler_dp_domain_free(pdata->mod);


The scheduler_dp_domain_free function may still access the task pointer, so this call should be moved before the task memory is released.

@checkupup ah, correct, let me fix that, thanks!

softwarecki

Its a step in the right direction. The LL task cleanup still needs to be addressed.

checkupup · 2026-02-03T09:25:19Z

src/schedule/zephyr_dp_schedule_application.c

+	struct task_dp_pdata *pdata = task->priv_data;
+
+	k_object_free(pdata->sem);
+	k_object_free(pdata->thread);


Is if (pdata->thread != &pdata->thread_struct) still needed here?

No. The "application" variant always creates a thread in userspace and allocates these objects dynamically.

@checkupup no, it's always the case for the "application" version, it was needed before because the code was covering both versions

checkupup · 2026-02-03T09:54:50Z

src/schedule/zephyr_dp_schedule_application.c

+	k_object_free(pdata->thread);
+	mod_free(pdata->mod, container_of(task, struct scheduler_dp_task_memory, task));
+
+	scheduler_dp_domain_free(pdata);


should be:

scheduler_dp_domain_free(pdata); mod_free(pdata->mod, container_of(task, struct scheduler_dp_task_memory, task));

The memory where pdata resides has been deallocated after calling mod_free.

@checkupup arrrgh... sure, you're certainly right. Thanks!

Task memory was freed implicitly by one DP implementation and was leaked by another one. Free it explicitly for both. Reported-by: Jun Lai <jun.lai@dolby.com> Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

lgirdwood

I think all comments have now been addressed. @checkupup is this working for you ?

checkupup · 2026-02-04T02:16:38Z

I think all comments have now been addressed. @checkupup is this working for you ?

Looks good for me now.

lyakh requested a review from pblaszko as a code owner February 3, 2026 08:21

Copilot AI review requested due to automatic review settings February 3, 2026 08:21

lyakh requested review from LaurentiuM1234, dbaluta and marcinszkudlinski as code owners February 3, 2026 08:21

Copilot started reviewing on behalf of lyakh February 3, 2026 08:21 View session

lyakh mentioned this pull request Feb 3, 2026

module-adapter: fix memory leaks caused by DP dask #10512

Closed

Copilot AI reviewed Feb 3, 2026

View reviewed changes

checkupup reviewed Feb 3, 2026

View reviewed changes

softwarecki approved these changes Feb 3, 2026

View reviewed changes

lyakh force-pushed the leak branch from 2cd44b0 to ac38cea Compare February 3, 2026 09:23

checkupup reviewed Feb 3, 2026

View reviewed changes

scheduler: dp: fix memory leak / implicit freeing

28e458e

Task memory was freed implicitly by one DP implementation and was leaked by another one. Free it explicitly for both. Reported-by: Jun Lai <jun.lai@dolby.com> Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

lyakh force-pushed the leak branch from ac38cea to 28e458e Compare February 3, 2026 11:04

lgirdwood reviewed Feb 3, 2026

View reviewed changes

tmleman approved these changes Feb 4, 2026

View reviewed changes

kv2019i merged commit c523494 into thesofproject:main Feb 4, 2026
47 of 52 checks passed

lyakh deleted the leak branch February 4, 2026 13:08

Conversation

lyakh commented Feb 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

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!

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

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!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

checkupup commented Feb 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants