diff mbox series

ALSA: memalloc: Add fallback SG-buffer free if fallback is used for noncontig

Message ID 20220425122814.751-1-peter.ujfalusi@linux.intel.com
State New
Headers show
Series ALSA: memalloc: Add fallback SG-buffer free if fallback is used for noncontig | expand

Commit Message

Péter Ujfalusi April 25, 2022, 12:28 p.m. UTC
From: Peter Ujfalusi <peter.ujfalusi@gmail.com>

If the dma_alloc_noncontiguous() fails in snd_dma_noncontig_alloc() we are
taking a fallback path which should be taken into account on the free path
since the way to free the two type of allocations are not the same.

Fixes: 925ca893b4a6 ("ALSA: memalloc: Add fallback SG-buffer allocations for x86")
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@gmail.com>
---
Hi Takashi,

I'm not sure about thisa as I can not get my systems to use the fallback, but in
theory this shiuld be done, no?

Since you have introduced the fallback, I believe there are cases when it is
taken and it might be related to some strange memory allocation errors happening
in SOF during firmware loading, like:
https://github.com/thesofproject/linux/issues/3609
https://github.com/thesofproject/linux/issues/3584
https://github.com/thesofproject/linux/issues/3530

Regards,
Peter

 sound/core/memalloc.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Péter Ujfalusi April 25, 2022, 12:49 p.m. UTC | #1
On 25/04/2022 15:28, Peter Ujfalusi wrote:
> From: Peter Ujfalusi <peter.ujfalusi@gmail.com>
> 
> If the dma_alloc_noncontiguous() fails in snd_dma_noncontig_alloc() we are
> taking a fallback path which should be taken into account on the free path
> since the way to free the two type of allocations are not the same.
> 
> Fixes: 925ca893b4a6 ("ALSA: memalloc: Add fallback SG-buffer allocations for x86")
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@gmail.com>
> ---
> Hi Takashi,
> 
> I'm not sure about thisa as I can not get my systems to use the fallback, but in
> theory this shiuld be done, no?

RIght, this is not needed as on the free path the callback is picked
based on the dmab->dev.type, so it should be picking the correct free
after all.

Please this patch.

> Since you have introduced the fallback, I believe there are cases when it is
> taken and it might be related to some strange memory allocation errors happening
> in SOF during firmware loading, like:
> https://github.com/thesofproject/linux/issues/3609
> https://github.com/thesofproject/linux/issues/3584
> https://github.com/thesofproject/linux/issues/3530

Still these reports are real and somehow they are pointing to dma
allocation issues.

Hrm, the fallback got backported to 5.17.4, so it might have been fixed
already?

> Regards,
> Peter
> 
>  sound/core/memalloc.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c
> index 15dc7160ba34..475fd38a4a48 100644
> --- a/sound/core/memalloc.c
> +++ b/sound/core/memalloc.c
> @@ -537,6 +537,13 @@ static void *snd_dma_noncontig_alloc(struct snd_dma_buffer *dmab, size_t size)
>  
>  static void snd_dma_noncontig_free(struct snd_dma_buffer *dmab)
>  {
> +	if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC_SG_FALLBACK ||
> +	    dmab->dev.type == SNDRV_DMA_TYPE_DEV_SG_FALLBACK) {
> +		/* The allocation is done with a fallback, use the matching free */
> +		snd_dma_sg_fallback_free(dmab);
> +		return;
> +	}
> +
>  	dma_vunmap_noncontiguous(dmab->dev.dev, dmab->area);
>  	dma_free_noncontiguous(dmab->dev.dev, dmab->bytes, dmab->private_data,
>  			       dmab->dev.dir);
diff mbox series

Patch

diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c
index 15dc7160ba34..475fd38a4a48 100644
--- a/sound/core/memalloc.c
+++ b/sound/core/memalloc.c
@@ -537,6 +537,13 @@  static void *snd_dma_noncontig_alloc(struct snd_dma_buffer *dmab, size_t size)
 
 static void snd_dma_noncontig_free(struct snd_dma_buffer *dmab)
 {
+	if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC_SG_FALLBACK ||
+	    dmab->dev.type == SNDRV_DMA_TYPE_DEV_SG_FALLBACK) {
+		/* The allocation is done with a fallback, use the matching free */
+		snd_dma_sg_fallback_free(dmab);
+		return;
+	}
+
 	dma_vunmap_noncontiguous(dmab->dev.dev, dmab->area);
 	dma_free_noncontiguous(dmab->dev.dev, dmab->bytes, dmab->private_data,
 			       dmab->dev.dir);