Skip to content

Commit c523494

Browse files
lyakhkv2019i
authored andcommitted
scheduler: dp: fix memory leak / implicit freeing
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>
1 parent 74e4191 commit c523494

File tree

4 files changed

+41
-30
lines changed

4 files changed

+41
-30
lines changed

src/schedule/zephyr_dp_schedule.c

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -295,24 +295,12 @@ static int scheduler_dp_task_free(void *data, struct task *task)
295295
pdata->thread_id = NULL;
296296
}
297297

298-
#ifdef CONFIG_USERSPACE
299-
#if CONFIG_SOF_USERSPACE_PROXY
300-
if (pdata->event != &pdata->event_struct)
301-
k_object_free(pdata->event);
302-
#else
303-
k_object_free(pdata->sem);
304-
#endif
305-
if (pdata->thread != &pdata->thread_struct)
306-
k_object_free(pdata->thread);
307-
#endif
308-
309298
/* free task stack */
310299
ret = user_stack_free(pdata->p_stack);
311300
pdata->p_stack = NULL;
312301

313-
scheduler_dp_domain_free(pdata->mod);
302+
scheduler_dp_internal_free(task);
314303

315-
/* all other memory has been allocated as a single malloc, will be freed later by caller */
316304
return ret;
317305
}
318306

src/schedule/zephyr_dp_schedule.h

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,4 @@ void scheduler_dp_grant(k_tid_t thread_id, uint16_t core);
5858
int scheduler_dp_task_init(struct task **task, const struct sof_uuid_entry *uid,
5959
const struct task_ops *ops, struct processing_module *mod,
6060
uint16_t core, size_t stack_size, uint32_t options);
61-
#if CONFIG_SOF_USERSPACE_APPLICATION
62-
void scheduler_dp_domain_free(struct processing_module *pmod);
63-
#else
64-
static inline void scheduler_dp_domain_free(struct processing_module *pmod) {}
65-
#endif
61+
void scheduler_dp_internal_free(struct task *task);

src/schedule/zephyr_dp_schedule_application.c

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -383,34 +383,46 @@ void dp_thread_fn(void *p1, void *p2, void *p3)
383383
* Safe to call with partial successful initialisation,
384384
* k_mem_domain_remove_partition() then just returns -ENOENT
385385
*/
386-
void scheduler_dp_domain_free(struct processing_module *pmod)
386+
static void scheduler_dp_domain_free(struct task_dp_pdata *pdata)
387387
{
388+
struct processing_module *pmod = pdata->mod;
388389
struct k_mem_domain *mdom = pmod->mdom;
389390

390391
llext_manager_rm_domain(pmod->dev->ipc_config.id, mdom);
391392

392-
struct task_dp_pdata *pdata = pmod->dev->task->priv_data;
393-
394393
k_mem_domain_remove_partition(mdom, pdata->mpart + SOF_DP_PART_HEAP);
395394
k_mem_domain_remove_partition(mdom, pdata->mpart + SOF_DP_PART_CFG);
396395

397396
pmod->mdom = NULL;
398397
objpool_free(&dp_mdom_head, mdom);
399398
}
400399

400+
/* memory allocation helper structure */
401+
struct scheduler_dp_task_memory {
402+
struct task task;
403+
struct task_dp_pdata pdata;
404+
struct comp_driver drv;
405+
struct ipc4_flat flat;
406+
};
407+
408+
void scheduler_dp_internal_free(struct task *task)
409+
{
410+
struct task_dp_pdata *pdata = task->priv_data;
411+
412+
k_object_free(pdata->sem);
413+
k_object_free(pdata->thread);
414+
scheduler_dp_domain_free(pdata);
415+
416+
mod_free(pdata->mod, container_of(task, struct scheduler_dp_task_memory, task));
417+
}
418+
401419
/* Called only in IPC context */
402420
int scheduler_dp_task_init(struct task **task, const struct sof_uuid_entry *uid,
403421
const struct task_ops *ops, struct processing_module *mod,
404422
uint16_t core, size_t stack_size, uint32_t options)
405423
{
406424
k_thread_stack_t *p_stack;
407-
/* memory allocation helper structure */
408-
struct {
409-
struct task task;
410-
struct task_dp_pdata pdata;
411-
struct comp_driver drv;
412-
struct ipc4_flat flat;
413-
} *task_memory;
425+
struct scheduler_dp_task_memory *task_memory;
414426

415427
int ret;
416428

@@ -558,7 +570,7 @@ int scheduler_dp_task_init(struct task **task, const struct sof_uuid_entry *uid,
558570
return 0;
559571

560572
e_dom:
561-
scheduler_dp_domain_free(mod);
573+
scheduler_dp_domain_free(pdata);
562574
e_thread:
563575
k_thread_abort(pdata->thread_id);
564576
e_kobj:

src/schedule/zephyr_dp_schedule_thread.c

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ int scheduler_dp_task_init(struct task **task,
187187

188188
/* memory allocation helper structure */
189189
struct {
190-
struct task task;
190+
struct task task; /* keep first, used for freeing below */
191191
struct task_dp_pdata pdata;
192192
} *task_memory;
193193

@@ -308,3 +308,18 @@ int scheduler_dp_task_init(struct task **task,
308308
sof_heap_free(user_heap, task_memory);
309309
return ret;
310310
}
311+
312+
void scheduler_dp_internal_free(struct task *task)
313+
{
314+
struct task_dp_pdata *pdata = task->priv_data;
315+
316+
#ifdef CONFIG_USERSPACE
317+
if (pdata->event != &pdata->event_struct)
318+
k_object_free(pdata->event);
319+
if (pdata->thread != &pdata->thread_struct)
320+
k_object_free(pdata->thread);
321+
#endif
322+
323+
/* task is the first member in task_memory above */
324+
sof_heap_free(pdata->mod->dev->drv->user_heap, task);
325+
}

0 commit comments

Comments
 (0)