[alsa-devel] [PATCH] ASoC: soc-core: call snd_soc_unbind_card() under mutex_lock;
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
commit 34ac3c3eb8f0c07 ("ASoC: core: lock client_mutex while removing link components") added mutex_lock() at soc_remove_link_components().
Is is called from snd_soc_unbind_card()
snd_soc_unbind_card() => soc_remove_link_components() soc_cleanup_card_resources() soc_remove_dai_links() => soc_remove_link_components()
And, there are 2 way to call it.
(1) snd_soc_unregister_component() ** mutex_lock() snd_soc_component_del_unlocked() => snd_soc_unbind_card() ** mutex_unlock()
(2) snd_soc_unregister_card() => snd_soc_unbind_card()
(1) case is already using mutex_lock() when it calles snd_soc_unbind_card(), thus, we will get lockdep warning.
commit 495f926c68ddb90 ("ASoC: core: Fix deadlock in snd_soc_instantiate_card()") tried to fixup it, but still not enough. We still have lockdep warning when we try unbind/bind.
We need mutex_lock() under snd_soc_unregister_card() instead of snd_remove_link_components()/snd_soc_unbind_card().
Fixes: 34ac3c3eb8f0c07 ("ASoC: core: lock client_mutex while removing link components") Fixes: 495f926c68ddb90 ("ASoC: core: Fix deadlock in snd_soc_instantiate_card()") Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 2353886..2a408cc 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2747,14 +2747,12 @@ static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister) snd_soc_dapm_shutdown(card); snd_soc_flush_all_delayed_work(card);
- mutex_lock(&client_mutex); /* remove all components used by DAI links on this card */ for_each_comp_order(order) { for_each_card_rtds(card, rtd) { soc_remove_link_components(card, rtd, order); } } - mutex_unlock(&client_mutex);
soc_cleanup_card_resources(card); if (!unregister) @@ -2773,7 +2771,9 @@ static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister) */ int snd_soc_unregister_card(struct snd_soc_card *card) { + mutex_lock(&client_mutex); snd_soc_unbind_card(card, true); + mutex_unlock(&client_mutex); dev_dbg(card->dev, "ASoC: Unregistered card '%s'\n", card->name);
return 0;
Hi Ranjani
Can you please confirm this patch ? It is no problem if I unbind "sound card" 1st, but, have issue if I unbind "CPU driver" 1st.
----------------------- # echo ec500000.sound > /sys/bus/platform/drivers/rcar_sound/unbind [ 29.972081] [ 29.973581] ============================================ [ 29.978891] WARNING: possible recursive locking detected [ 29.984203] 5.2.0-rc5+ #312 Not tainted [ 29.988036] -------------------------------------------- [ 29.993346] sh/190 is trying to acquire lock: [ 29.997700] (____ptrval____) (client_mutex){+.+.}, at: snd_soc_unbind_card.part.9+0x78/0xf8 [ 30.006070] [ 30.006070] but task is already holding lock: [ 30.011901] (____ptrval____) (client_mutex){+.+.}, at: snd_soc_unregister_component+0x54/0xe8 [ 30.020432] [ 30.020432] other info that might help us debug this: [ 30.026959] Possible unsafe locking scenario: [ 30.026959] [ 30.032876] CPU0 [ 30.035318] ---- [ 30.037759] lock(client_mutex); [ 30.041071] lock(client_mutex); [ 30.044383] [ 30.044383] *** DEADLOCK *** [ 30.044383] [ 30.050301] May be due to missing lock nesting notation [ 30.050301] [ 30.057088] 4 locks held by sh/190: [ 30.060572] #0: (____ptrval____) (sb_writers#6){.+.+}, at: vfs_write+0x198/0x1b8 [ 30.068063] #1: (____ptrval____) (&of->mutex){+.+.}, at: kernfs_fop_write+0xc0/0x1e8 [ 30.075900] #2: (____ptrval____) (&dev->mutex){....}, at: __device_driver_lock+0x38/0x68 [ 30.084085] #3: (____ptrval____) (client_mutex){+.+.}, at: snd_soc_unregister_component+0x54/0xe8 [ 30.093049] [ 30.093049] stack backtrace: [ 30.097406] CPU: 2 PID: 190 Comm: sh Not tainted 5.2.0-rc5+ #312 [ 30.103411] Hardware name: Renesas H3ULCB Kingfisher board based on r8a7795 ES2.0+ (DT) [ 30.111414] Call trace: [ 30.113860] dump_backtrace+0x0/0x140 [ 30.117520] show_stack+0x24/0x30 [ 30.120837] dump_stack+0xc8/0x114 [ 30.124238] __lock_acquire+0x1de4/0x1e48 [ 30.128244] lock_acquire+0xdc/0x258 [ 30.131819] __mutex_lock+0x80/0x7c8 [ 30.135392] mutex_lock_nested+0x3c/0x50 [ 30.139312] snd_soc_unbind_card.part.9+0x78/0xf8 [ 30.144015] snd_soc_unregister_component+0xe4/0xe8 [ 30.148894] devm_component_release+0x20/0x30 [ 30.153250] release_nodes+0x1c8/0x240 [ 30.156996] devres_release_all+0x3c/0x58 [ 30.161004] device_release_driver_internal+0x100/0x1c0 [ 30.166226] device_driver_detach+0x28/0x38 [ 30.170407] unbind_store+0x94/0x100 [ 30.173979] drv_attr_store+0x40/0x58 [ 30.177640] sysfs_kf_write+0x50/0x78 [ 30.181299] kernfs_fop_write+0xf0/0x1e8 [ 30.185219] __vfs_write+0x48/0x90 [ 30.188617] vfs_write+0xac/0x1b8 [ 30.191928] ksys_write+0x74/0xf8 [ 30.195240] __arm64_sys_write+0x24/0x30 [ 30.199163] el0_svc_common.constprop.0+0x98/0x170 [ 30.203952] el0_svc_compat_handler+0x2c/0x38 [ 30.208307] el0_svc_compat+0x8/0x10 -----------------------
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
commit 34ac3c3eb8f0c07 ("ASoC: core: lock client_mutex while removing link components") added mutex_lock() at soc_remove_link_components().
Is is called from snd_soc_unbind_card()
snd_soc_unbind_card() => soc_remove_link_components() soc_cleanup_card_resources() soc_remove_dai_links() => soc_remove_link_components()
And, there are 2 way to call it.
(1) snd_soc_unregister_component() ** mutex_lock() snd_soc_component_del_unlocked() => snd_soc_unbind_card() ** mutex_unlock()
(2) snd_soc_unregister_card() => snd_soc_unbind_card()
(1) case is already using mutex_lock() when it calles snd_soc_unbind_card(), thus, we will get lockdep warning.
commit 495f926c68ddb90 ("ASoC: core: Fix deadlock in snd_soc_instantiate_card()") tried to fixup it, but still not enough. We still have lockdep warning when we try unbind/bind.
We need mutex_lock() under snd_soc_unregister_card() instead of snd_remove_link_components()/snd_soc_unbind_card().
Fixes: 34ac3c3eb8f0c07 ("ASoC: core: lock client_mutex while removing link components") Fixes: 495f926c68ddb90 ("ASoC: core: Fix deadlock in snd_soc_instantiate_card()") Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
sound/soc/soc-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 2353886..2a408cc 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2747,14 +2747,12 @@ static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister) snd_soc_dapm_shutdown(card); snd_soc_flush_all_delayed_work(card);
mutex_lock(&client_mutex);/* remove all components used by DAI links on this card */ for_each_comp_order(order) { for_each_card_rtds(card, rtd) { soc_remove_link_components(card, rtd, order); } }
mutex_unlock(&client_mutex);soc_cleanup_card_resources(card); if (!unregister)
@@ -2773,7 +2771,9 @@ static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister) */ int snd_soc_unregister_card(struct snd_soc_card *card) {
mutex_lock(&client_mutex); snd_soc_unbind_card(card, true);
mutex_unlock(&client_mutex); dev_dbg(card->dev, "ASoC: Unregistered card '%s'\n", card->name);
return 0;
-- 2.7.4
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
commit 34ac3c3eb8f0c07 ("ASoC: core: lock client_mutex while removing link components") added mutex_lock() at soc_remove_link_components().
Is is called from snd_soc_unbind_card()
snd_soc_unbind_card() => soc_remove_link_components() soc_cleanup_card_resources() soc_remove_dai_links() => soc_remove_link_components()
And, there are 2 way to call it.
(1) snd_soc_unregister_component() ** mutex_lock() snd_soc_component_del_unlocked() => snd_soc_unbind_card() ** mutex_unlock()
(2) snd_soc_unregister_card() => snd_soc_unbind_card()
(1) case is already using mutex_lock() when it calles snd_soc_unbind_card(), thus, we will get lockdep warning.
Thanks, morimoto-san. You are correct. Case 1 will result in a lockdep warning. This patch looks good.
Thanks, Ranjani
commit 495f926c68ddb90 ("ASoC: core: Fix deadlock in snd_soc_instantiate_card()") tried to fixup it, but still not enough. We still have lockdep warning when we try unbind/bind.
We need mutex_lock() under snd_soc_unregister_card() instead of snd_remove_link_components()/snd_soc_unbind_card().
Fixes: 34ac3c3eb8f0c07 ("ASoC: core: lock client_mutex while removing link components") Fixes: 495f926c68ddb90 ("ASoC: core: Fix deadlock in snd_soc_instantiate_card()") Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
sound/soc/soc-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 2353886..2a408cc 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2747,14 +2747,12 @@ static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister) snd_soc_dapm_shutdown(card); snd_soc_flush_all_delayed_work(card);
mutex_lock(&client_mutex);*/ for_each_comp_order(order) { for_each_card_rtds(card, rtd) { soc_remove_link_components(card, rtd, order); } }
mutex_unlock(&client_mutex);soc_cleanup_card_resources(card); if (!unregister)
@@ -2773,7 +2771,9 @@ static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister) */ int snd_soc_unregister_card(struct snd_soc_card *card) {
- mutex_lock(&client_mutex); snd_soc_unbind_card(card, true);
- mutex_unlock(&client_mutex); dev_dbg(card->dev, "ASoC: Unregistered card '%s'\n", card-
name);
return 0;
2.7.4
Hi Ranjani
Thank you for your confirming. Can you give it your Acked-by ? It is easy for Mark, I think.
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
commit 34ac3c3eb8f0c07 ("ASoC: core: lock client_mutex while removing link components") added mutex_lock() at soc_remove_link_components().
Is is called from snd_soc_unbind_card()
snd_soc_unbind_card() => soc_remove_link_components() soc_cleanup_card_resources() soc_remove_dai_links() => soc_remove_link_components()
And, there are 2 way to call it.
(1) snd_soc_unregister_component() ** mutex_lock() snd_soc_component_del_unlocked() => snd_soc_unbind_card() ** mutex_unlock()
(2) snd_soc_unregister_card() => snd_soc_unbind_card()
(1) case is already using mutex_lock() when it calles snd_soc_unbind_card(), thus, we will get lockdep warning.
Thanks, morimoto-san. You are correct. Case 1 will result in a lockdep warning. This patch looks good.
Thanks, Ranjani
commit 495f926c68ddb90 ("ASoC: core: Fix deadlock in snd_soc_instantiate_card()") tried to fixup it, but still not enough. We still have lockdep warning when we try unbind/bind.
We need mutex_lock() under snd_soc_unregister_card() instead of snd_remove_link_components()/snd_soc_unbind_card().
Fixes: 34ac3c3eb8f0c07 ("ASoC: core: lock client_mutex while removing link components") Fixes: 495f926c68ddb90 ("ASoC: core: Fix deadlock in snd_soc_instantiate_card()") Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
sound/soc/soc-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 2353886..2a408cc 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2747,14 +2747,12 @@ static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister) snd_soc_dapm_shutdown(card); snd_soc_flush_all_delayed_work(card);
mutex_lock(&client_mutex);*/ for_each_comp_order(order) { for_each_card_rtds(card, rtd) { soc_remove_link_components(card, rtd, order); } }
mutex_unlock(&client_mutex);soc_cleanup_card_resources(card); if (!unregister)
@@ -2773,7 +2771,9 @@ static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister) */ int snd_soc_unregister_card(struct snd_soc_card *card) {
- mutex_lock(&client_mutex); snd_soc_unbind_card(card, true);
- mutex_unlock(&client_mutex); dev_dbg(card->dev, "ASoC: Unregistered card '%s'\n", card-
name);
return 0;
2.7.4
The patch
ASoC: soc-core: call snd_soc_unbind_card() under mutex_lock;
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.2
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 b545542a0b866f7975254e41c595836e9bc0ff2f Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Wed, 19 Jun 2019 10:07:19 +0900 Subject: [PATCH] ASoC: soc-core: call snd_soc_unbind_card() under mutex_lock;
commit 34ac3c3eb8f0c07 ("ASoC: core: lock client_mutex while removing link components") added mutex_lock() at soc_remove_link_components().
Is is called from snd_soc_unbind_card()
snd_soc_unbind_card() => soc_remove_link_components() soc_cleanup_card_resources() soc_remove_dai_links() => soc_remove_link_components()
And, there are 2 way to call it.
(1) snd_soc_unregister_component() ** mutex_lock() snd_soc_component_del_unlocked() => snd_soc_unbind_card() ** mutex_unlock()
(2) snd_soc_unregister_card() => snd_soc_unbind_card()
(1) case is already using mutex_lock() when it calles snd_soc_unbind_card(), thus, we will get lockdep warning.
commit 495f926c68ddb90 ("ASoC: core: Fix deadlock in snd_soc_instantiate_card()") tried to fixup it, but still not enough. We still have lockdep warning when we try unbind/bind.
We need mutex_lock() under snd_soc_unregister_card() instead of snd_remove_link_components()/snd_soc_unbind_card().
Fixes: 34ac3c3eb8f0c07 ("ASoC: core: lock client_mutex while removing link components") Fixes: 495f926c68ddb90 ("ASoC: core: Fix deadlock in snd_soc_instantiate_card()") Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/soc-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 41c0cfaf2db5..9138fcb15cd3 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2837,14 +2837,12 @@ static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister) snd_soc_dapm_shutdown(card); snd_soc_flush_all_delayed_work(card);
- mutex_lock(&client_mutex); /* remove all components used by DAI links on this card */ for_each_comp_order(order) { for_each_card_rtds(card, rtd) { soc_remove_link_components(card, rtd, order); } } - mutex_unlock(&client_mutex);
soc_cleanup_card_resources(card); if (!unregister) @@ -2863,7 +2861,9 @@ static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister) */ int snd_soc_unregister_card(struct snd_soc_card *card) { + mutex_lock(&client_mutex); snd_soc_unbind_card(card, true); + mutex_unlock(&client_mutex); dev_dbg(card->dev, "ASoC: Unregistered card '%s'\n", card->name);
return 0;
On Wed, 2019-06-19 at 10:07 +0900, Kuninori Morimoto wrote:
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
commit 34ac3c3eb8f0c07 ("ASoC: core: lock client_mutex while removing link components") added mutex_lock() at soc_remove_link_components().
Is is called from snd_soc_unbind_card()
snd_soc_unbind_card() => soc_remove_link_components() soc_cleanup_card_resources() soc_remove_dai_links() => soc_remove_link_components()
And, there are 2 way to call it.
(1) snd_soc_unregister_component() ** mutex_lock() snd_soc_component_del_unlocked() => snd_soc_unbind_card() ** mutex_unlock()
(2) snd_soc_unregister_card() => snd_soc_unbind_card()
(1) case is already using mutex_lock() when it calles snd_soc_unbind_card(), thus, we will get lockdep warning.
commit 495f926c68ddb90 ("ASoC: core: Fix deadlock in snd_soc_instantiate_card()") tried to fixup it, but still not enough. We still have lockdep warning when we try unbind/bind.
We need mutex_lock() under snd_soc_unregister_card() instead of snd_remove_link_components()/snd_soc_unbind_card().
Fixes: 34ac3c3eb8f0c07 ("ASoC: core: lock client_mutex while removing link components") Fixes: 495f926c68ddb90 ("ASoC: core: Fix deadlock in snd_soc_instantiate_card()") Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Acked-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com
sound/soc/soc-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 2353886..2a408cc 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2747,14 +2747,12 @@ static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister) snd_soc_dapm_shutdown(card); snd_soc_flush_all_delayed_work(card);
mutex_lock(&client_mutex);*/ for_each_comp_order(order) { for_each_card_rtds(card, rtd) { soc_remove_link_components(card, rtd, order); } }
mutex_unlock(&client_mutex);soc_cleanup_card_resources(card); if (!unregister)
@@ -2773,7 +2771,9 @@ static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister) */ int snd_soc_unregister_card(struct snd_soc_card *card) {
- mutex_lock(&client_mutex); snd_soc_unbind_card(card, true);
- mutex_unlock(&client_mutex); dev_dbg(card->dev, "ASoC: Unregistered card '%s'\n", card-
name);
return 0;
participants (3)
-
Kuninori Morimoto -
Mark Brown -
Ranjani Sridharan