Skip to content

Conversation

@ranj063
Copy link
Collaborator

@ranj063 ranj063 commented Nov 6, 2022

Free the memory allocated for DMA channels when the ref count is 0 and the DMA block config during DAI reset when using natize zephyr drivers to prevent memory leaks seen during stress tests

Fixes #6535

Free the memory allocated for DMA channels when the ref count is 0 when
using native zephyr drivers.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063 ranj063 changed the title lib: dma: free dma channels when ref count is 0 Fix memory leaks with zephyr native drivers Nov 6, 2022
Return NULL when dma_init() fails.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
This prevents memory leaks during repeated PCM start/stop tests.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent!

if (--dma->sref == 0) {
rfree(dma->chan);
dma->chan = NULL;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this, we lose one "dma->chan = rzalloc(SOF_MEM_ZONE_RUNTIME_SHARED" worth of memory whenever sref goes to 1->0 and back.

Copy link

@juimonen juimonen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes tests work now, nice!

Copy link
Contributor

@aborisovich aborisovich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great find but is really using goto statement mandatory?
It is commonly known fact that this is not a clean code solution...
In fact, it is one of the classical examples of bad code layout. There are too many books written about it,

@kv2019i
Copy link
Collaborator

kv2019i commented Nov 7, 2022

@aborisovich wrote:

Great find but is really using goto statement mandatory? It is commonly known fact that this is not a clean code

Goto usage is pretty widespread in Linux kernel, but only for very specific usage/patterns. I think this falls to one of the well accepted patterns where goto is used to jump to error handler (kind of poor man's exception handler) to make it more explicit that locks are released correctly.

@kv2019i
Copy link
Collaborator

kv2019i commented Nov 7, 2022

Still 01.org trouble, checked the pr-device-test results offline and they are good (a few DUTs unavailable for the run, but we have test results with similar configurations that pass). Proceeding with merge.

@kv2019i kv2019i merged commit d259d1b into thesofproject:main Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] check-playback.sh sof-test fails on TGL in native Zephyr build with IPC4

6 participants