* [alsa-devel] [PATCH 0/2] ASoC: fixup topology dai_link remove issue
@ 2019-11-18 1:49 Kuninori Morimoto
2019-11-18 1:50 ` [alsa-devel] [PATCH 1/2] ASoC: soc-component: tidyup snd_soc_pcm_component_new/free() parameter Kuninori Morimoto
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Kuninori Morimoto @ 2019-11-18 1:49 UTC (permalink / raw)
To: Mark Brown; +Cc: Takashi Iwai, Linux-ALSA, Pierre-Louis Bossart
Hi Mark
Cc Pierre-Louis, Takashi-san
Currently, I'm focusing to ASoC cleanup / balance-up.
But, it found more unbalance issue, and Intel noticed about it.
These patches fix dai_link remove issue on topology.
I want to get Acked-by or Reviewed-by from Takashi-san
for 2) patch if possible.
These are already tested by Intel CI, and all issues were solved.
(https://github.com/thesofproject/linux/pull/1504)
Extra Tested-by / Reviewed-by are very welcome from Intel
Kuninori Morimoto (2):
1) ASoC: soc-component: tidyup snd_soc_pcm_component_new/free() parameter
2) ASoC: soc-pcm: remove soc_pcm_private_free()
include/sound/soc-component.h | 4 ++--
sound/soc/soc-component.c | 8 +++-----
sound/soc/soc-core.c | 19 +++++++++++--------
sound/soc/soc-pcm.c | 12 +-----------
4 files changed, 17 insertions(+), 26 deletions(-)
--
2.7.4
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* [alsa-devel] [PATCH 1/2] ASoC: soc-component: tidyup snd_soc_pcm_component_new/free() parameter 2019-11-18 1:49 [alsa-devel] [PATCH 0/2] ASoC: fixup topology dai_link remove issue Kuninori Morimoto @ 2019-11-18 1:50 ` Kuninori Morimoto 2019-11-20 17:18 ` [alsa-devel] Applied "ASoC: soc-component: tidyup snd_soc_pcm_component_new/free() parameter" to the asoc tree Mark Brown 2019-11-18 1:51 ` [alsa-devel] [PATCH 2/2] ASoC: soc-pcm: remove soc_pcm_private_free() Kuninori Morimoto 2019-11-18 15:25 ` [alsa-devel] [PATCH 0/2] ASoC: fixup topology dai_link remove issue Pierre-Louis Bossart 2 siblings, 1 reply; 12+ messages in thread From: Kuninori Morimoto @ 2019-11-18 1:50 UTC (permalink / raw) To: Mark Brown; +Cc: Takashi Iwai, Linux-ALSA, Pierre-Louis Bossart From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> This patch uses rtd instead of pcm at snd_soc_pcm_component_new/free() parameter. This is prepare for dai_link remove bug fix on topology. Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> --- include/sound/soc-component.h | 4 ++-- sound/soc/soc-component.c | 8 +++----- sound/soc/soc-pcm.c | 4 ++-- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h index 6aa3ecb..f8fadf0 100644 --- a/include/sound/soc-component.h +++ b/include/sound/soc-component.h @@ -412,7 +412,7 @@ struct page *snd_soc_pcm_component_page(struct snd_pcm_substream *substream, unsigned long offset); int snd_soc_pcm_component_mmap(struct snd_pcm_substream *substream, struct vm_area_struct *vma); -int snd_soc_pcm_component_new(struct snd_pcm *pcm); -void snd_soc_pcm_component_free(struct snd_pcm *pcm); +int snd_soc_pcm_component_new(struct snd_soc_pcm_runtime *rtd); +void snd_soc_pcm_component_free(struct snd_soc_pcm_runtime *rtd); #endif /* __SOC_COMPONENT_H */ diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c index 98ef066..1590e80 100644 --- a/sound/soc/soc-component.c +++ b/sound/soc/soc-component.c @@ -498,9 +498,8 @@ int snd_soc_pcm_component_mmap(struct snd_pcm_substream *substream, return -EINVAL; } -int snd_soc_pcm_component_new(struct snd_pcm *pcm) +int snd_soc_pcm_component_new(struct snd_soc_pcm_runtime *rtd) { - struct snd_soc_pcm_runtime *rtd = pcm->private_data; struct snd_soc_rtdcom_list *rtdcom; struct snd_soc_component *component; int ret; @@ -516,13 +515,12 @@ int snd_soc_pcm_component_new(struct snd_pcm *pcm) return 0; } -void snd_soc_pcm_component_free(struct snd_pcm *pcm) +void snd_soc_pcm_component_free(struct snd_soc_pcm_runtime *rtd) { - struct snd_soc_pcm_runtime *rtd = pcm->private_data; struct snd_soc_rtdcom_list *rtdcom; struct snd_soc_component *component; for_each_rtd_components(rtd, rtdcom, component) if (component->driver->pcm_destruct) - component->driver->pcm_destruct(component, pcm); + component->driver->pcm_destruct(component, rtd->pcm); } diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 4bf71e3..c624d30 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2898,7 +2898,7 @@ static void soc_pcm_private_free(struct snd_pcm *pcm) /* need to sync the delayed work before releasing resources */ flush_delayed_work(&rtd->delayed_work); - snd_soc_pcm_component_free(pcm); + snd_soc_pcm_component_free(rtd); } /* create a new pcm */ @@ -3036,7 +3036,7 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) if (capture) snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &rtd->ops); - ret = snd_soc_pcm_component_new(pcm); + ret = snd_soc_pcm_component_new(rtd); if (ret < 0) { dev_err(rtd->dev, "ASoC: pcm constructor failed: %d\n", ret); return ret; -- 2.7.4 _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [alsa-devel] Applied "ASoC: soc-component: tidyup snd_soc_pcm_component_new/free() parameter" to the asoc tree 2019-11-18 1:50 ` [alsa-devel] [PATCH 1/2] ASoC: soc-component: tidyup snd_soc_pcm_component_new/free() parameter Kuninori Morimoto @ 2019-11-20 17:18 ` Mark Brown 0 siblings, 0 replies; 12+ messages in thread From: Mark Brown @ 2019-11-20 17:18 UTC (permalink / raw) To: Kuninori Morimoto Cc: Takashi Iwai, Linux-ALSA, Mark Brown, Pierre-Louis Bossart The patch ASoC: soc-component: tidyup snd_soc_pcm_component_new/free() parameter has been applied to the asoc tree at https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.5 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark From b2b2afbb48eac7215f951a8a462aa6837e0d495f Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Date: Mon, 18 Nov 2019 10:50:32 +0900 Subject: [PATCH] ASoC: soc-component: tidyup snd_soc_pcm_component_new/free() parameter This patch uses rtd instead of pcm at snd_soc_pcm_component_new/free() parameter. This is prepare for dai_link remove bug fix on topology. Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Link: https://lore.kernel.org/r/87pnhqx89j.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown <broonie@kernel.org> --- include/sound/soc-component.h | 4 ++-- sound/soc/soc-component.c | 8 +++----- sound/soc/soc-pcm.c | 4 ++-- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h index 6aa3ecb7b6d3..f8fadf0e696a 100644 --- a/include/sound/soc-component.h +++ b/include/sound/soc-component.h @@ -412,7 +412,7 @@ struct page *snd_soc_pcm_component_page(struct snd_pcm_substream *substream, unsigned long offset); int snd_soc_pcm_component_mmap(struct snd_pcm_substream *substream, struct vm_area_struct *vma); -int snd_soc_pcm_component_new(struct snd_pcm *pcm); -void snd_soc_pcm_component_free(struct snd_pcm *pcm); +int snd_soc_pcm_component_new(struct snd_soc_pcm_runtime *rtd); +void snd_soc_pcm_component_free(struct snd_soc_pcm_runtime *rtd); #endif /* __SOC_COMPONENT_H */ diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c index 98ef0666add2..1590e805d016 100644 --- a/sound/soc/soc-component.c +++ b/sound/soc/soc-component.c @@ -498,9 +498,8 @@ int snd_soc_pcm_component_mmap(struct snd_pcm_substream *substream, return -EINVAL; } -int snd_soc_pcm_component_new(struct snd_pcm *pcm) +int snd_soc_pcm_component_new(struct snd_soc_pcm_runtime *rtd) { - struct snd_soc_pcm_runtime *rtd = pcm->private_data; struct snd_soc_rtdcom_list *rtdcom; struct snd_soc_component *component; int ret; @@ -516,13 +515,12 @@ int snd_soc_pcm_component_new(struct snd_pcm *pcm) return 0; } -void snd_soc_pcm_component_free(struct snd_pcm *pcm) +void snd_soc_pcm_component_free(struct snd_soc_pcm_runtime *rtd) { - struct snd_soc_pcm_runtime *rtd = pcm->private_data; struct snd_soc_rtdcom_list *rtdcom; struct snd_soc_component *component; for_each_rtd_components(rtd, rtdcom, component) if (component->driver->pcm_destruct) - component->driver->pcm_destruct(component, pcm); + component->driver->pcm_destruct(component, rtd->pcm); } diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 4bf71e3211d8..c624d30bfa3c 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2898,7 +2898,7 @@ static void soc_pcm_private_free(struct snd_pcm *pcm) /* need to sync the delayed work before releasing resources */ flush_delayed_work(&rtd->delayed_work); - snd_soc_pcm_component_free(pcm); + snd_soc_pcm_component_free(rtd); } /* create a new pcm */ @@ -3036,7 +3036,7 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) if (capture) snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &rtd->ops); - ret = snd_soc_pcm_component_new(pcm); + ret = snd_soc_pcm_component_new(rtd); if (ret < 0) { dev_err(rtd->dev, "ASoC: pcm constructor failed: %d\n", ret); return ret; -- 2.20.1 _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [alsa-devel] [PATCH 2/2] ASoC: soc-pcm: remove soc_pcm_private_free() 2019-11-18 1:49 [alsa-devel] [PATCH 0/2] ASoC: fixup topology dai_link remove issue Kuninori Morimoto 2019-11-18 1:50 ` [alsa-devel] [PATCH 1/2] ASoC: soc-component: tidyup snd_soc_pcm_component_new/free() parameter Kuninori Morimoto @ 2019-11-18 1:51 ` Kuninori Morimoto 2019-11-20 17:18 ` [alsa-devel] Applied "ASoC: soc-pcm: remove soc_pcm_private_free()" to the asoc tree Mark Brown 2019-12-05 12:16 ` [alsa-devel] [PATCH 2/2] ASoC: soc-pcm: remove soc_pcm_private_free() Enric Balletbo Serra 2019-11-18 15:25 ` [alsa-devel] [PATCH 0/2] ASoC: fixup topology dai_link remove issue Pierre-Louis Bossart 2 siblings, 2 replies; 12+ messages in thread From: Kuninori Morimoto @ 2019-11-18 1:51 UTC (permalink / raw) To: Mark Brown; +Cc: Takashi Iwai, Linux-ALSA, Pierre-Louis Bossart From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> soc-topology adds extra dai_link by using snd_soc_add_dai_link(), and removes it by snd_soc_romove_dai_link(). This snd_soc_add/remove_dai_link() and/or its related functions are unbalanced before, and now, these are balance-uped. But, it finds the random operation issue, and it is reported by Pierre-Louis. When card was released, topology will call snd_soc_remove_dai_link() via (A). static void soc_cleanup_card_resources(struct snd_soc_card *card) { struct snd_soc_dai_link *link, *_link; /* This should be called before snd_card_free() */ (A) soc_remove_link_components(card); /* free the ALSA card at first; this syncs with pending operations */ if (card->snd_card) { (B) snd_card_free(card->snd_card); card->snd_card = NULL; } /* remove and free each DAI */ (X) soc_remove_link_dais(card); for_each_card_links_safe(card, link, _link) (C) snd_soc_remove_dai_link(card, link); ... } At (A), topology calls snd_soc_remove_dai_link(). Then topology rtd, and its related all data are freed. Next, (B) is called, and then, pcm->private_free = soc_pcm_private_free() is called. static void soc_pcm_private_free(struct snd_pcm *pcm) { struct snd_soc_pcm_runtime *rtd = pcm->private_data; /* need to sync the delayed work before releasing resources */ flush_delayed_work(&rtd->delayed_work); snd_soc_pcm_component_free(rtd); } Here, it gets rtd via pcm->private_data. But, topology related rtd are already freed at (A). Normal sound card has no damage, becase it frees rtd at (C). These are finalizing rtd related data. Thus, these should be called when rtd was freed, not sound card was freed. It is very natural and understandable. In other words, pcm->private_free = soc_pcm_private_free() is no longer needed. Extra issue is that there is zero chance to call soc_remove_dai() for topology related dai at (X). Because (A) removes rtd connection from card too, and, (X) is based on card connected rtd. This means, (X) need to be called before (C) (= for normal sound) and (A) (= for topology). Now, I want to focus this patch which is the reason why snd_card_free() = (B) is located there. commit 4efda5f2130da033aeedc5b3205569893b910de2 ("ASoC: Fix use-after-free at card unregistration") Original snd_card_free() was called last of this function. But moved to top to avoid use-after-free issue. The issue was happen at soc_pcm_free() which was pcm->private_free, today it is updated/renamed to soc_pcm_private_free(). In other words, (B) need to be called before (C) (= for normal sound) and (A) (= for topology), because it needs (not yet freed) rtd. But, (A) need to be called before (B), because it needs card->snd_card pointer. If we call flush_delayed_work() and snd_soc_pcm_component_free() (= same as soc_pcm_private_free()) when rtd was freed (= (C), (A)), there is no reason to call snd_card_free() at top of this function. It can be called end of this function, again. But, in such case, it will likely break unbind again, as Takashi-san reported. When unbind is performed in a busy state, the code may release still-in-use resources. At least we need to call snd_card_disconnect_sync() at the first place. The final code will be... static void soc_cleanup_card_resources(struct snd_soc_card *card) { struct snd_soc_dai_link *link, *_link; if (card->snd_card) (Z) snd_card_disconnect_sync(card->snd_card); (X) soc_remove_link_dais(card); (A) soc_remove_link_components(card); for_each_card_links_safe(card, link, _link) (C) snd_soc_remove_dai_link(card, link); ... if (card->snd_card) { (B) snd_card_free(card->snd_card); card->snd_card = NULL; } } To avoid release still-in-use resources, call snd_card_disconnect_sync() at (Z). (X) is needed for both non-topology and topology. topology removes rtd via (A), and non topology removes rtd via (C). snd_card_free() is no longer related to use-after-free issue. Thus, locating (B) is no problem. Fixes: df95a16d2a9626 ("ASoC: soc-core: fix RIP warning on card removal") Fixes: bc7a9091e5b927 ("ASoC: soc-core: add soc_unbind_dai_link()") Reported-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Tested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> --- sound/soc/soc-core.c | 19 +++++++++++-------- sound/soc/soc-pcm.c | 10 ---------- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 977a7bf..e3a53ef 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -419,6 +419,9 @@ static void soc_free_pcm_runtime(struct snd_soc_pcm_runtime *rtd) list_del(&rtd->list); + flush_delayed_work(&rtd->delayed_work); + snd_soc_pcm_component_free(rtd); + /* * we don't need to call kfree() for rtd->dev * see @@ -1945,19 +1948,14 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card, { struct snd_soc_dai_link *link, *_link; - /* This should be called before snd_card_free() */ - soc_remove_link_components(card); - - /* free the ALSA card at first; this syncs with pending operations */ - if (card->snd_card) { - snd_card_free(card->snd_card); - card->snd_card = NULL; - } + if (card->snd_card) + snd_card_disconnect_sync(card->snd_card); snd_soc_dapm_shutdown(card); /* remove and free each DAI */ soc_remove_link_dais(card); + soc_remove_link_components(card); for_each_card_links_safe(card, link, _link) snd_soc_remove_dai_link(card, link); @@ -1972,6 +1970,11 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card, /* remove the card */ if (card_probed && card->remove) card->remove(card); + + if (card->snd_card) { + snd_card_free(card->snd_card); + card->snd_card = NULL; + } } static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index c624d30..2c4f50c 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2892,15 +2892,6 @@ static int dpcm_fe_dai_close(struct snd_pcm_substream *fe_substream) return ret; } -static void soc_pcm_private_free(struct snd_pcm *pcm) -{ - struct snd_soc_pcm_runtime *rtd = pcm->private_data; - - /* need to sync the delayed work before releasing resources */ - flush_delayed_work(&rtd->delayed_work); - snd_soc_pcm_component_free(rtd); -} - /* create a new pcm */ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) { @@ -3042,7 +3033,6 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) return ret; } - pcm->private_free = soc_pcm_private_free; pcm->no_device_suspend = true; out: dev_info(rtd->card->dev, "%s <-> %s mapping ok\n", -- 2.7.4 _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [alsa-devel] Applied "ASoC: soc-pcm: remove soc_pcm_private_free()" to the asoc tree 2019-11-18 1:51 ` [alsa-devel] [PATCH 2/2] ASoC: soc-pcm: remove soc_pcm_private_free() Kuninori Morimoto @ 2019-11-20 17:18 ` Mark Brown 2019-12-05 12:16 ` [alsa-devel] [PATCH 2/2] ASoC: soc-pcm: remove soc_pcm_private_free() Enric Balletbo Serra 1 sibling, 0 replies; 12+ messages in thread From: Mark Brown @ 2019-11-20 17:18 UTC (permalink / raw) To: Kuninori Morimoto Cc: Takashi Iwai, Linux-ALSA, Mark Brown, Pierre-Louis Bossart The patch ASoC: soc-pcm: remove soc_pcm_private_free() has been applied to the asoc tree at https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.5 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark From 0ced7b050224b18ca73e38e7068f36be8e708c06 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Date: Mon, 18 Nov 2019 10:51:11 +0900 Subject: [PATCH] ASoC: soc-pcm: remove soc_pcm_private_free() soc-topology adds extra dai_link by using snd_soc_add_dai_link(), and removes it by snd_soc_romove_dai_link(). This snd_soc_add/remove_dai_link() and/or its related functions are unbalanced before, and now, these are balance-uped. But, it finds the random operation issue, and it is reported by Pierre-Louis. When card was released, topology will call snd_soc_remove_dai_link() via (A). static void soc_cleanup_card_resources(struct snd_soc_card *card) { struct snd_soc_dai_link *link, *_link; /* This should be called before snd_card_free() */ (A) soc_remove_link_components(card); /* free the ALSA card at first; this syncs with pending operations */ if (card->snd_card) { (B) snd_card_free(card->snd_card); card->snd_card = NULL; } /* remove and free each DAI */ (X) soc_remove_link_dais(card); for_each_card_links_safe(card, link, _link) (C) snd_soc_remove_dai_link(card, link); ... } At (A), topology calls snd_soc_remove_dai_link(). Then topology rtd, and its related all data are freed. Next, (B) is called, and then, pcm->private_free = soc_pcm_private_free() is called. static void soc_pcm_private_free(struct snd_pcm *pcm) { struct snd_soc_pcm_runtime *rtd = pcm->private_data; /* need to sync the delayed work before releasing resources */ flush_delayed_work(&rtd->delayed_work); snd_soc_pcm_component_free(rtd); } Here, it gets rtd via pcm->private_data. But, topology related rtd are already freed at (A). Normal sound card has no damage, becase it frees rtd at (C). These are finalizing rtd related data. Thus, these should be called when rtd was freed, not sound card was freed. It is very natural and understandable. In other words, pcm->private_free = soc_pcm_private_free() is no longer needed. Extra issue is that there is zero chance to call soc_remove_dai() for topology related dai at (X). Because (A) removes rtd connection from card too, and, (X) is based on card connected rtd. This means, (X) need to be called before (C) (= for normal sound) and (A) (= for topology). Now, I want to focus this patch which is the reason why snd_card_free() = (B) is located there. commit 4efda5f2130da033aeedc5b3205569893b910de2 ("ASoC: Fix use-after-free at card unregistration") Original snd_card_free() was called last of this function. But moved to top to avoid use-after-free issue. The issue was happen at soc_pcm_free() which was pcm->private_free, today it is updated/renamed to soc_pcm_private_free(). In other words, (B) need to be called before (C) (= for normal sound) and (A) (= for topology), because it needs (not yet freed) rtd. But, (A) need to be called before (B), because it needs card->snd_card pointer. If we call flush_delayed_work() and snd_soc_pcm_component_free() (= same as soc_pcm_private_free()) when rtd was freed (= (C), (A)), there is no reason to call snd_card_free() at top of this function. It can be called end of this function, again. But, in such case, it will likely break unbind again, as Takashi-san reported. When unbind is performed in a busy state, the code may release still-in-use resources. At least we need to call snd_card_disconnect_sync() at the first place. The final code will be... static void soc_cleanup_card_resources(struct snd_soc_card *card) { struct snd_soc_dai_link *link, *_link; if (card->snd_card) (Z) snd_card_disconnect_sync(card->snd_card); (X) soc_remove_link_dais(card); (A) soc_remove_link_components(card); for_each_card_links_safe(card, link, _link) (C) snd_soc_remove_dai_link(card, link); ... if (card->snd_card) { (B) snd_card_free(card->snd_card); card->snd_card = NULL; } } To avoid release still-in-use resources, call snd_card_disconnect_sync() at (Z). (X) is needed for both non-topology and topology. topology removes rtd via (A), and non topology removes rtd via (C). snd_card_free() is no longer related to use-after-free issue. Thus, locating (B) is no problem. Fixes: df95a16d2a9626 ("ASoC: soc-core: fix RIP warning on card removal") Fixes: bc7a9091e5b927 ("ASoC: soc-core: add soc_unbind_dai_link()") Reported-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Tested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Link: https://lore.kernel.org/r/87o8xax88g.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown <broonie@kernel.org> --- sound/soc/soc-core.c | 19 +++++++++++-------- sound/soc/soc-pcm.c | 10 ---------- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 977a7bfad519..e3a53ef1db04 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -419,6 +419,9 @@ static void soc_free_pcm_runtime(struct snd_soc_pcm_runtime *rtd) list_del(&rtd->list); + flush_delayed_work(&rtd->delayed_work); + snd_soc_pcm_component_free(rtd); + /* * we don't need to call kfree() for rtd->dev * see @@ -1945,19 +1948,14 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card, { struct snd_soc_dai_link *link, *_link; - /* This should be called before snd_card_free() */ - soc_remove_link_components(card); - - /* free the ALSA card at first; this syncs with pending operations */ - if (card->snd_card) { - snd_card_free(card->snd_card); - card->snd_card = NULL; - } + if (card->snd_card) + snd_card_disconnect_sync(card->snd_card); snd_soc_dapm_shutdown(card); /* remove and free each DAI */ soc_remove_link_dais(card); + soc_remove_link_components(card); for_each_card_links_safe(card, link, _link) snd_soc_remove_dai_link(card, link); @@ -1972,6 +1970,11 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card, /* remove the card */ if (card_probed && card->remove) card->remove(card); + + if (card->snd_card) { + snd_card_free(card->snd_card); + card->snd_card = NULL; + } } static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index c624d30bfa3c..2c4f50c44591 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2892,15 +2892,6 @@ static int dpcm_fe_dai_close(struct snd_pcm_substream *fe_substream) return ret; } -static void soc_pcm_private_free(struct snd_pcm *pcm) -{ - struct snd_soc_pcm_runtime *rtd = pcm->private_data; - - /* need to sync the delayed work before releasing resources */ - flush_delayed_work(&rtd->delayed_work); - snd_soc_pcm_component_free(rtd); -} - /* create a new pcm */ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) { @@ -3042,7 +3033,6 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) return ret; } - pcm->private_free = soc_pcm_private_free; pcm->no_device_suspend = true; out: dev_info(rtd->card->dev, "%s <-> %s mapping ok\n", -- 2.20.1 _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [alsa-devel] [PATCH 2/2] ASoC: soc-pcm: remove soc_pcm_private_free() 2019-11-18 1:51 ` [alsa-devel] [PATCH 2/2] ASoC: soc-pcm: remove soc_pcm_private_free() Kuninori Morimoto 2019-11-20 17:18 ` [alsa-devel] Applied "ASoC: soc-pcm: remove soc_pcm_private_free()" to the asoc tree Mark Brown @ 2019-12-05 12:16 ` Enric Balletbo Serra 2019-12-05 12:54 ` Daniel Baluta 1 sibling, 1 reply; 12+ messages in thread From: Enric Balletbo Serra @ 2019-12-05 12:16 UTC (permalink / raw) To: Kuninori Morimoto Cc: Takashi Iwai, Linux-ALSA, Mark Brown, Pierre-Louis Bossart Dear all, Missatge de Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> del dia dl., 18 de nov. 2019 a les 2:52: > > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > soc-topology adds extra dai_link by using snd_soc_add_dai_link(), > and removes it by snd_soc_romove_dai_link(). > > This snd_soc_add/remove_dai_link() and/or its related > functions are unbalanced before, and now, these are balance-uped. > But, it finds the random operation issue, and it is reported by > Pierre-Louis. > > When card was released, topology will call snd_soc_remove_dai_link() > via (A). > > static void soc_cleanup_card_resources(struct snd_soc_card *card) > { > struct snd_soc_dai_link *link, *_link; > > /* This should be called before snd_card_free() */ > (A) soc_remove_link_components(card); > > /* free the ALSA card at first; this syncs with pending operations */ > if (card->snd_card) { > (B) snd_card_free(card->snd_card); > card->snd_card = NULL; > } > > /* remove and free each DAI */ > (X) soc_remove_link_dais(card); > > for_each_card_links_safe(card, link, _link) > (C) snd_soc_remove_dai_link(card, link); > > ... > } > > At (A), topology calls snd_soc_remove_dai_link(). > Then topology rtd, and its related all data are freed. > > Next, (B) is called, and then, pcm->private_free = soc_pcm_private_free() > is called. > > static void soc_pcm_private_free(struct snd_pcm *pcm) > { > struct snd_soc_pcm_runtime *rtd = pcm->private_data; > > /* need to sync the delayed work before releasing resources */ > flush_delayed_work(&rtd->delayed_work); > snd_soc_pcm_component_free(rtd); > } > > Here, it gets rtd via pcm->private_data. > But, topology related rtd are already freed at (A). > Normal sound card has no damage, becase it frees rtd at (C). > > These are finalizing rtd related data. > Thus, these should be called when rtd was freed, not sound card > was freed. It is very natural and understandable. > > In other words, pcm->private_free = soc_pcm_private_free() > is no longer needed. > > Extra issue is that there is zero chance to call > soc_remove_dai() for topology related dai at (X). > Because (A) removes rtd connection from card too, and, > (X) is based on card connected rtd. > > This means, (X) need to be called before (C) (= for normal sound) > and (A) (= for topology). > > Now, I want to focus this patch which is the reason why > snd_card_free() = (B) is located there. > > commit 4efda5f2130da033aeedc5b3205569893b910de2 > ("ASoC: Fix use-after-free at card unregistration") > > Original snd_card_free() was called last of this function. > But moved to top to avoid use-after-free issue. > The issue was happen at soc_pcm_free() which was pcm->private_free, > today it is updated/renamed to soc_pcm_private_free(). > > In other words, (B) need to be called before (C) (= for normal sound) > and (A) (= for topology), because it needs (not yet freed) rtd. > But, (A) need to be called before (B), > because it needs card->snd_card pointer. > > If we call flush_delayed_work() and snd_soc_pcm_component_free() > (= same as soc_pcm_private_free()) when rtd was freed (= (C), (A)), > there is no reason to call snd_card_free() at top of this function. > It can be called end of this function, again. > > But, in such case, it will likely break unbind again, as Takashi-san > reported. When unbind is performed in a busy state, the code may > release still-in-use resources. > At least we need to call snd_card_disconnect_sync() at the first place. > > The final code will be... > > static void soc_cleanup_card_resources(struct snd_soc_card *card) > { > struct snd_soc_dai_link *link, *_link; > > if (card->snd_card) > (Z) snd_card_disconnect_sync(card->snd_card); > > (X) soc_remove_link_dais(card); > (A) soc_remove_link_components(card); > > for_each_card_links_safe(card, link, _link) > (C) snd_soc_remove_dai_link(card, link); > > ... > if (card->snd_card) { > (B) snd_card_free(card->snd_card); > card->snd_card = NULL; > } > } > > To avoid release still-in-use resources, > call snd_card_disconnect_sync() at (Z). > > (X) is needed for both non-topology and topology. > > topology removes rtd via (A), and > non topology removes rtd via (C). > > snd_card_free() is no longer related to use-after-free issue. > Thus, locating (B) is no problem. > > Fixes: df95a16d2a9626 ("ASoC: soc-core: fix RIP warning on card removal") > Fixes: bc7a9091e5b927 ("ASoC: soc-core: add soc_unbind_dai_link()") > Reported-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > Tested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > --- I didn't look into detail yet, but after applying this patch my Samsung Chromebook Plus started to show different warnings like this, probably caused because in my case the driver is deferring? I'll try to take a look, but if anyone already knows the cause, please let me know. Thanks, Enric [ 10.873336] ------------[ cut here ]------------ [ 10.878507] WARNING: CPU: 4 PID: 45 at kernel/workqueue.c:3032 __flush_work.isra.45+0x210/0x230 [ 10.888226] Modules linked in: snd_soc_rockchip_i2s(+) vctrl_regulator snd_soc_rk3399_gru_sound snd_soc_rt5514 cec gpu_sched snd_soc_max98357a kfifo_buf i2c_hid cros_e c_sensors_core snd_soc_da7219 snd_soc_rockchip_pcm phy_rockchip_pcie snd_soc_rt5514_spi rockchip_saradc sbs_battery pwm_bl cros_usbpd_charger cros_ec_chardev cros_usbpd_l ogger pcie_rockchip_host pwm_cros_ec rockchip_thermal snd_soc_rl6231 ip_tables x_tables ipv6 nf_defrag_ipv6 [ 10.931870] CPU: 4 PID: 45 Comm: kworker/4:1 Not tainted 5.4.0+ #322 [ 10.938967] Hardware name: Google Kevin (DT) [ 10.943738] Workqueue: events deferred_probe_work_func [ 10.949476] pstate: 00000005 (nzcv daif -PAN -UAO) [ 10.954825] pc : __flush_work.isra.45+0x210/0x230 [ 10.960068] lr : flush_delayed_work+0x34/0x58 [ 10.964929] sp : ffff800011d4b930 [ 10.968625] x29: ffff800011d4b930 x28: 00000000fffffdfb [ 10.974547] x27: ffff80001129b530 x26: ffff8000111a6a08 [ 10.980469] x25: 0000000000000003 x24: 0000000000000001 [ 10.986399] x23: ffff800011a7a980 x22: ffff8000118998c8 [ 10.992329] x21: 0000000000000000 x20: ffff800008df1000 [ 10.998260] x19: ffff0000ed60f698 x18: 0000000000000001 [ 11.004190] x17: 0000000000000001 x16: ffff800011e7f000 [ 11.010120] x15: ffffffffffffffff x14: 0000000000000000 [ 11.016051] x13: 0000000000000000 x12: 0000000000000020 [ 11.021981] x11: 0000000000000008 x10: 0101010101010101 [ 11.027912] x9 : 0000000000000000 x8 : 7f7f7f7f7f7f7f7f [ 11.033842] x7 : ffff0000f5557340 x6 : 0080808080808080 [ 11.039778] x5 : dead000000000100 x4 : 0000000000000000 [ 11.045711] x3 : 0000000000000000 x2 : 9def22a3228fd300 [ 11.051642] x1 : 0000000000000000 x0 : 0000000000000000 [ 11.057565] Call trace: [ 11.060294] __flush_work.isra.45+0x210/0x230 [ 11.065157] flush_delayed_work+0x34/0x58 [ 11.069623] soc_free_pcm_runtime.part.13+0x40/0x60 [ 11.075074] snd_soc_remove_dai_link+0x54/0x60 [ 11.075076] soc_cleanup_card_resources+0x160/0x2a8 [ 11.075078] snd_soc_bind_card+0x264/0xa10 [ 11.075080] snd_soc_register_card+0xf4/0x108 [ 11.075088] devm_snd_soc_register_card+0x40/0x90 [ 11.100189] rockchip_sound_probe+0x204/0x2e4 [snd_soc_rk3399_gru_sound] [ 11.107770] platform_drv_probe+0x50/0xa0 [ 11.112244] really_probe+0xd4/0x308 [ 11.116234] driver_probe_device+0x54/0xe8 [ 11.122457] __device_attach_driver+0x80/0xb8 [ 11.127312] bus_for_each_drv+0x78/0xc8 [ 11.131788] __device_attach+0xd4/0x130 [ 11.136070] device_initial_probe+0x10/0x18 [ 11.140738] bus_probe_device+0x90/0x98 [ 11.145020] deferred_probe_work_func+0x6c/0xa0 [ 11.155998] process_one_work+0x1e0/0x358 [ 11.160474] worker_thread+0x40/0x488 [ 11.164562] kthread+0x118/0x120 [ 11.168163] ret_from_fork+0x10/0x18 [ 11.172347] ---[ end trace dc22ca199fcfcf7c ]--- > sound/soc/soc-core.c | 19 +++++++++++-------- > sound/soc/soc-pcm.c | 10 ---------- > 2 files changed, 11 insertions(+), 18 deletions(-) > > diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c > index 977a7bf..e3a53ef 100644 > --- a/sound/soc/soc-core.c > +++ b/sound/soc/soc-core.c > @@ -419,6 +419,9 @@ static void soc_free_pcm_runtime(struct snd_soc_pcm_runtime *rtd) > > list_del(&rtd->list); > > + flush_delayed_work(&rtd->delayed_work); > + snd_soc_pcm_component_free(rtd); > + > /* > * we don't need to call kfree() for rtd->dev > * see > @@ -1945,19 +1948,14 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card, > { > struct snd_soc_dai_link *link, *_link; > > - /* This should be called before snd_card_free() */ > - soc_remove_link_components(card); > - > - /* free the ALSA card at first; this syncs with pending operations */ > - if (card->snd_card) { > - snd_card_free(card->snd_card); > - card->snd_card = NULL; > - } > + if (card->snd_card) > + snd_card_disconnect_sync(card->snd_card); > > snd_soc_dapm_shutdown(card); > > /* remove and free each DAI */ > soc_remove_link_dais(card); > + soc_remove_link_components(card); > > for_each_card_links_safe(card, link, _link) > snd_soc_remove_dai_link(card, link); > @@ -1972,6 +1970,11 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card, > /* remove the card */ > if (card_probed && card->remove) > card->remove(card); > + > + if (card->snd_card) { > + snd_card_free(card->snd_card); > + card->snd_card = NULL; > + } > } > > static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister) > diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c > index c624d30..2c4f50c 100644 > --- a/sound/soc/soc-pcm.c > +++ b/sound/soc/soc-pcm.c > @@ -2892,15 +2892,6 @@ static int dpcm_fe_dai_close(struct snd_pcm_substream *fe_substream) > return ret; > } > > -static void soc_pcm_private_free(struct snd_pcm *pcm) > -{ > - struct snd_soc_pcm_runtime *rtd = pcm->private_data; > - > - /* need to sync the delayed work before releasing resources */ > - flush_delayed_work(&rtd->delayed_work); > - snd_soc_pcm_component_free(rtd); > -} > - > /* create a new pcm */ > int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) > { > @@ -3042,7 +3033,6 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) > return ret; > } > > - pcm->private_free = soc_pcm_private_free; > pcm->no_device_suspend = true; > out: > dev_info(rtd->card->dev, "%s <-> %s mapping ok\n", > -- > 2.7.4 > > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [alsa-devel] [PATCH 2/2] ASoC: soc-pcm: remove soc_pcm_private_free() 2019-12-05 12:16 ` [alsa-devel] [PATCH 2/2] ASoC: soc-pcm: remove soc_pcm_private_free() Enric Balletbo Serra @ 2019-12-05 12:54 ` Daniel Baluta 2019-12-05 15:18 ` Enric Balletbo Serra 0 siblings, 1 reply; 12+ messages in thread From: Daniel Baluta @ 2019-12-05 12:54 UTC (permalink / raw) To: eballetbo, kuninori.morimoto.gx Cc: tiwai, alsa-devel, broonie, pierre-louis.bossart > I didn't look into detail yet, but after applying this patch my > Samsung Chromebook Plus started to show different warnings like this, > probably caused because in my case the driver is deferring? > > I'll try to take a look, but if anyone already knows the cause, > please > let me know. > Hi Enric, Can you try: https://patchwork.kernel.org/patch/11265061/ It should be already in Mark's tree. _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [alsa-devel] [PATCH 2/2] ASoC: soc-pcm: remove soc_pcm_private_free() 2019-12-05 12:54 ` Daniel Baluta @ 2019-12-05 15:18 ` Enric Balletbo Serra 0 siblings, 0 replies; 12+ messages in thread From: Enric Balletbo Serra @ 2019-12-05 15:18 UTC (permalink / raw) To: Daniel Baluta Cc: tiwai, alsa-devel, broonie, pierre-louis.bossart, kuninori.morimoto.gx Hi Daniel, Missatge de Daniel Baluta <daniel.baluta@nxp.com> del dia dj., 5 de des. 2019 a les 13:54: > > > > I didn't look into detail yet, but after applying this patch my > > Samsung Chromebook Plus started to show different warnings like this, > > probably caused because in my case the driver is deferring? > > > > I'll try to take a look, but if anyone already knows the cause, > > please > > let me know. > > > > Hi Enric, > > Can you try: > > https://patchwork.kernel.org/patch/11265061/ > Actually I picked https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/?h=for-5.5&id=4bf2e385aa59c2fae5f880aa25cfd2b470109093 which is supposed to land in this release cycle, so all fine. Thanks, Enric > It should be already in Mark's tree. > > _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [alsa-devel] [PATCH 0/2] ASoC: fixup topology dai_link remove issue 2019-11-18 1:49 [alsa-devel] [PATCH 0/2] ASoC: fixup topology dai_link remove issue Kuninori Morimoto 2019-11-18 1:50 ` [alsa-devel] [PATCH 1/2] ASoC: soc-component: tidyup snd_soc_pcm_component_new/free() parameter Kuninori Morimoto 2019-11-18 1:51 ` [alsa-devel] [PATCH 2/2] ASoC: soc-pcm: remove soc_pcm_private_free() Kuninori Morimoto @ 2019-11-18 15:25 ` Pierre-Louis Bossart 2019-11-19 1:10 ` Kuninori Morimoto 2 siblings, 1 reply; 12+ messages in thread From: Pierre-Louis Bossart @ 2019-11-18 15:25 UTC (permalink / raw) To: Kuninori Morimoto, Mark Brown; +Cc: Takashi Iwai, Linux-ALSA On 11/17/19 7:49 PM, Kuninori Morimoto wrote: > > Hi Mark > Cc Pierre-Louis, Takashi-san > > Currently, I'm focusing to ASoC cleanup / balance-up. > But, it found more unbalance issue, and Intel noticed about it. > These patches fix dai_link remove issue on topology. > > I want to get Acked-by or Reviewed-by from Takashi-san > for 2) patch if possible. > > These are already tested by Intel CI, and all issues were solved. > (https://github.com/thesofproject/linux/pull/1504) > Extra Tested-by / Reviewed-by are very welcome from Intel if you don't mind I'd like to retest this new series, it's based on a different tip and is not exactly the same as before. e.g. we tested this: - /* free the ALSA card at first; this syncs with pending operations */ - if (card->snd_card) { - snd_card_free(card->snd_card); - card->snd_card = NULL; - } + if (card->snd_card) + snd_card_disconnect_sync(card->snd_card); /* remove and free each DAI */ soc_remove_link_dais(card); + soc_remove_link_components(card); and the new code shows this - /* free the ALSA card at first; this syncs with pending operations */ - if (card->snd_card) { - snd_card_free(card->snd_card); - card->snd_card = NULL; - } + if (card->snd_card) + snd_card_disconnect_sync(card->snd_card); snd_soc_dapm_shutdown(card); <<< not tested yet. /* remove and free each DAI */ soc_remove_link_dais(card); + soc_remove_link_components(card); > > Kuninori Morimoto (2): > 1) ASoC: soc-component: tidyup snd_soc_pcm_component_new/free() parameter > 2) ASoC: soc-pcm: remove soc_pcm_private_free() > > include/sound/soc-component.h | 4 ++-- > sound/soc/soc-component.c | 8 +++----- > sound/soc/soc-core.c | 19 +++++++++++-------- > sound/soc/soc-pcm.c | 12 +----------- > 4 files changed, 17 insertions(+), 26 deletions(-) > _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [alsa-devel] [PATCH 0/2] ASoC: fixup topology dai_link remove issue 2019-11-18 15:25 ` [alsa-devel] [PATCH 0/2] ASoC: fixup topology dai_link remove issue Pierre-Louis Bossart @ 2019-11-19 1:10 ` Kuninori Morimoto 2019-11-19 13:37 ` Pierre-Louis Bossart 0 siblings, 1 reply; 12+ messages in thread From: Kuninori Morimoto @ 2019-11-19 1:10 UTC (permalink / raw) To: Pierre-Louis Bossart; +Cc: Takashi Iwai, Linux-ALSA, Mark Brown Hi Pierre-Louis > > These are already tested by Intel CI, and all issues were solved. > > (https://github.com/thesofproject/linux/pull/1504) > > Extra Tested-by / Reviewed-by are very welcome from Intel > > if you don't mind I'd like to retest this new series, it's based on a > different tip and is not exactly the same as before. Yes, of course. Sorry I didn't mention about it. > - /* free the ALSA card at first; this syncs with pending operations */ > - if (card->snd_card) { > - snd_card_free(card->snd_card); > - card->snd_card = NULL; > - } > + if (card->snd_card) > + snd_card_disconnect_sync(card->snd_card); > > snd_soc_dapm_shutdown(card); <<< not tested yet. > > /* remove and free each DAI */ > soc_remove_link_dais(card); > + soc_remove_link_components(card); Yes. It is from 2a6f0892bda954dc2688b002060093ee0fe38528 ("ASoC: soc-core: call snd_soc_dapm_shutdown() at soc_cleanup_card_resources()") Thank you for your help !! Best regards --- Kuninori Morimoto _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [alsa-devel] [PATCH 0/2] ASoC: fixup topology dai_link remove issue 2019-11-19 1:10 ` Kuninori Morimoto @ 2019-11-19 13:37 ` Pierre-Louis Bossart 2019-11-19 18:20 ` Mark Brown 0 siblings, 1 reply; 12+ messages in thread From: Pierre-Louis Bossart @ 2019-11-19 13:37 UTC (permalink / raw) To: Kuninori Morimoto; +Cc: Takashi Iwai, Linux-ALSA, Mark Brown On 11/18/19 7:10 PM, Kuninori Morimoto wrote: > > Hi Pierre-Louis > >>> These are already tested by Intel CI, and all issues were solved. >>> (https://github.com/thesofproject/linux/pull/1504) >>> Extra Tested-by / Reviewed-by are very welcome from Intel >> >> if you don't mind I'd like to retest this new series, it's based on a >> different tip and is not exactly the same as before. > > Yes, of course. > Sorry I didn't mention about it. > >> - /* free the ALSA card at first; this syncs with pending operations */ >> - if (card->snd_card) { >> - snd_card_free(card->snd_card); >> - card->snd_card = NULL; >> - } >> + if (card->snd_card) >> + snd_card_disconnect_sync(card->snd_card); >> >> snd_soc_dapm_shutdown(card); <<< not tested yet. >> >> /* remove and free each DAI */ >> soc_remove_link_dais(card); >> + soc_remove_link_components(card); > > Yes. > It is from > > 2a6f0892bda954dc2688b002060093ee0fe38528 > ("ASoC: soc-core: call snd_soc_dapm_shutdown() at soc_cleanup_card_resources()") No regression detected so from the Intel side we're good with this patchset. Thanks Morimoto-san for this comprehensive analysis, really nice work! _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [alsa-devel] [PATCH 0/2] ASoC: fixup topology dai_link remove issue 2019-11-19 13:37 ` Pierre-Louis Bossart @ 2019-11-19 18:20 ` Mark Brown 0 siblings, 0 replies; 12+ messages in thread From: Mark Brown @ 2019-11-19 18:20 UTC (permalink / raw) To: Pierre-Louis Bossart; +Cc: Takashi Iwai, Linux-ALSA, Kuninori Morimoto [-- Attachment #1.1: Type: text/plain, Size: 278 bytes --] On Tue, Nov 19, 2019 at 07:37:09AM -0600, Pierre-Louis Bossart wrote: > No regression detected so from the Intel side we're good with this patchset. > Thanks Morimoto-san for this comprehensive analysis, really nice work! Yes, thanks indeed for this - it's really great work. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 161 bytes --] _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-12-05 15:19 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-18 1:49 [alsa-devel] [PATCH 0/2] ASoC: fixup topology dai_link remove issue Kuninori Morimoto 2019-11-18 1:50 ` [alsa-devel] [PATCH 1/2] ASoC: soc-component: tidyup snd_soc_pcm_component_new/free() parameter Kuninori Morimoto 2019-11-20 17:18 ` [alsa-devel] Applied "ASoC: soc-component: tidyup snd_soc_pcm_component_new/free() parameter" to the asoc tree Mark Brown 2019-11-18 1:51 ` [alsa-devel] [PATCH 2/2] ASoC: soc-pcm: remove soc_pcm_private_free() Kuninori Morimoto 2019-11-20 17:18 ` [alsa-devel] Applied "ASoC: soc-pcm: remove soc_pcm_private_free()" to the asoc tree Mark Brown 2019-12-05 12:16 ` [alsa-devel] [PATCH 2/2] ASoC: soc-pcm: remove soc_pcm_private_free() Enric Balletbo Serra 2019-12-05 12:54 ` Daniel Baluta 2019-12-05 15:18 ` Enric Balletbo Serra 2019-11-18 15:25 ` [alsa-devel] [PATCH 0/2] ASoC: fixup topology dai_link remove issue Pierre-Louis Bossart 2019-11-19 1:10 ` Kuninori Morimoto 2019-11-19 13:37 ` Pierre-Louis Bossart 2019-11-19 18:20 ` Mark Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).