[PATCH] ALSA: memalloc: Add fallback SG-buffer free if fallback is used for noncontig
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(+)
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);
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);
On Mon, 25 Apr 2022 14:49:35 +0200, P9ter Ujfalusi wrote:
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.
Yes, the type got overwritten at switching to the fallback, so your change is superfluous.
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?
Yes, it should be. Let me know if the problem still persists even after the fallback support is backported to stable kernels.
thanks,
Takashi
Hi Peter,
I love your patch! Yet something to improve:
[auto build test ERROR on tiwai-sound/for-next] [also build test ERROR on v5.18-rc4 next-20220422] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Peter-Ujfalusi/ALSA-memalloc-... base: https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next config: riscv-randconfig-r035-20220425 (https://download.01.org/0day-ci/archive/20220426/202204260255.k5hl4BYG-lkp@i...) compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 1cddcfdc3c683b393df1a5c9063252eb60e52818) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install riscv cross compiling tool for clang build # apt-get install binutils-riscv64-linux-gnu # https://github.com/intel-lab-lkp/linux/commit/bf91cb1cd103c5f1e78fa154c30f14... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Peter-Ujfalusi/ALSA-memalloc-Add-fallback-SG-buffer-free-if-fallback-is-used-for-noncontig/20220425-203012 git checkout bf91cb1cd103c5f1e78fa154c30f1436be2723ac # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash sound/
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All errors (new ones prefixed by >>):
sound/core/memalloc.c:540:24: error: use of undeclared identifier 'SNDRV_DMA_TYPE_DEV_WC_SG_FALLBACK'
if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC_SG_FALLBACK || ^
sound/core/memalloc.c:541:24: error: use of undeclared identifier 'SNDRV_DMA_TYPE_DEV_SG_FALLBACK'
dmab->dev.type == SNDRV_DMA_TYPE_DEV_SG_FALLBACK) { ^
sound/core/memalloc.c:543:3: error: call to undeclared function 'snd_dma_sg_fallback_free'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
snd_dma_sg_fallback_free(dmab); ^ sound/core/memalloc.c:543:3: note: did you mean 'snd_dma_vmalloc_free'? sound/core/memalloc.c:312:13: note: 'snd_dma_vmalloc_free' declared here static void snd_dma_vmalloc_free(struct snd_dma_buffer *dmab) ^ 3 errors generated.
vim +/SNDRV_DMA_TYPE_DEV_WC_SG_FALLBACK +540 sound/core/memalloc.c
537 538 static void snd_dma_noncontig_free(struct snd_dma_buffer *dmab) 539 {
540 if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC_SG_FALLBACK || 541 dmab->dev.type == SNDRV_DMA_TYPE_DEV_SG_FALLBACK) {
542 /* The allocation is done with a fallback, use the matching free */
543 snd_dma_sg_fallback_free(dmab);
544 return; 545 } 546 547 dma_vunmap_noncontiguous(dmab->dev.dev, dmab->area); 548 dma_free_noncontiguous(dmab->dev.dev, dmab->bytes, dmab->private_data, 549 dmab->dev.dir); 550 } 551
Hi Peter,
I love your patch! Yet something to improve:
[auto build test ERROR on tiwai-sound/for-next] [also build test ERROR on v5.18-rc4 next-20220422] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Peter-Ujfalusi/ALSA-memalloc-... base: https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next config: arm-randconfig-r033-20220425 (https://download.01.org/0day-ci/archive/20220426/202204260301.TAt89QGn-lkp@i...) compiler: arm-linux-gnueabi-gcc (GCC) 11.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/bf91cb1cd103c5f1e78fa154c30f14... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Peter-Ujfalusi/ALSA-memalloc-Add-fallback-SG-buffer-free-if-fallback-is-used-for-noncontig/20220425-203012 git checkout bf91cb1cd103c5f1e78fa154c30f1436be2723ac # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash sound/core/
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All errors (new ones prefixed by >>):
sound/core/memalloc.c: In function 'snd_dma_noncontig_free':
sound/core/memalloc.c:540:31: error: 'SNDRV_DMA_TYPE_DEV_WC_SG_FALLBACK' undeclared (first use in this function); did you mean 'SNDRV_DMA_TYPE_DEV_WC_SG'?
540 | if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC_SG_FALLBACK || | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | SNDRV_DMA_TYPE_DEV_WC_SG sound/core/memalloc.c:540:31: note: each undeclared identifier is reported only once for each function it appears in
sound/core/memalloc.c:541:31: error: 'SNDRV_DMA_TYPE_DEV_SG_FALLBACK' undeclared (first use in this function); did you mean 'SNDRV_DMA_TYPE_DEV_SG'?
541 | dmab->dev.type == SNDRV_DMA_TYPE_DEV_SG_FALLBACK) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | SNDRV_DMA_TYPE_DEV_SG
sound/core/memalloc.c:543:17: error: implicit declaration of function 'snd_dma_sg_fallback_free'; did you mean 'snd_dma_vmalloc_free'? [-Werror=implicit-function-declaration]
543 | snd_dma_sg_fallback_free(dmab); | ^~~~~~~~~~~~~~~~~~~~~~~~ | snd_dma_vmalloc_free cc1: some warnings being treated as errors
vim +540 sound/core/memalloc.c
537 538 static void snd_dma_noncontig_free(struct snd_dma_buffer *dmab) 539 {
540 if (dmab->dev.type == SNDRV_DMA_TYPE_DEV_WC_SG_FALLBACK || 541 dmab->dev.type == SNDRV_DMA_TYPE_DEV_SG_FALLBACK) {
542 /* The allocation is done with a fallback, use the matching free */
543 snd_dma_sg_fallback_free(dmab);
544 return; 545 } 546 547 dma_vunmap_noncontiguous(dmab->dev.dev, dmab->area); 548 dma_free_noncontiguous(dmab->dev.dev, dmab->bytes, dmab->private_data, 549 dmab->dev.dir); 550 } 551
Hi Peter,
I love your patch! Perhaps something to improve:
[auto build test WARNING on tiwai-sound/for-next] [also build test WARNING on v5.18-rc4 next-20220422] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Peter-Ujfalusi/ALSA-memalloc-... base: https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next config: i386-randconfig-a001 (https://download.01.org/0day-ci/archive/20220426/202204261147.hFS8RozR-lkp@i...) compiler: gcc-11 (Debian 11.2.0-20) 11.2.0 reproduce (this is a W=1 build): # https://github.com/intel-lab-lkp/linux/commit/bf91cb1cd103c5f1e78fa154c30f14... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Peter-Ujfalusi/ALSA-memalloc-Add-fallback-SG-buffer-free-if-fallback-is-used-for-noncontig/20220425-203012 git checkout bf91cb1cd103c5f1e78fa154c30f1436be2723ac # save the config file mkdir build_dir && cp config build_dir/.config make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash sound/core/
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All warnings (new ones prefixed by >>):
sound/core/memalloc.c: In function 'snd_dma_noncontig_free': sound/core/memalloc.c:543:17: error: implicit declaration of function 'snd_dma_sg_fallback_free'; did you mean 'snd_dma_sg_fallback_alloc'? [-Werror=implicit-function-declaration] 543 | snd_dma_sg_fallback_free(dmab); | ^~~~~~~~~~~~~~~~~~~~~~~~ | snd_dma_sg_fallback_alloc sound/core/memalloc.c: At top level:
sound/core/memalloc.c:755:13: warning: conflicting types for 'snd_dma_sg_fallback_free'; have 'void(struct snd_dma_buffer *)'
755 | static void snd_dma_sg_fallback_free(struct snd_dma_buffer *dmab) | ^~~~~~~~~~~~~~~~~~~~~~~~ sound/core/memalloc.c:755:13: error: static declaration of 'snd_dma_sg_fallback_free' follows non-static declaration sound/core/memalloc.c:543:17: note: previous implicit declaration of 'snd_dma_sg_fallback_free' with type 'void(struct snd_dma_buffer *)' 543 | snd_dma_sg_fallback_free(dmab); | ^~~~~~~~~~~~~~~~~~~~~~~~ cc1: some warnings being treated as errors
vim +755 sound/core/memalloc.c
925ca893b4a651 Takashi Iwai 2022-04-13 754 925ca893b4a651 Takashi Iwai 2022-04-13 @755 static void snd_dma_sg_fallback_free(struct snd_dma_buffer *dmab) 925ca893b4a651 Takashi Iwai 2022-04-13 756 { 925ca893b4a651 Takashi Iwai 2022-04-13 757 vunmap(dmab->area); 925ca893b4a651 Takashi Iwai 2022-04-13 758 __snd_dma_sg_fallback_free(dmab, dmab->private_data); 925ca893b4a651 Takashi Iwai 2022-04-13 759 } 925ca893b4a651 Takashi Iwai 2022-04-13 760
participants (3)
-
kernel test robot -
Peter Ujfalusi -
Takashi Iwai