* [PATCH] ASoC: soc-core: call snd_soc_unbind_card() under mutex_lock;
@ 2019-06-10 0:49 Kuninori Morimoto
2019-06-10 16:27 ` Ranjani Sridharan
0 siblings, 1 reply; 8+ messages in thread
From: Kuninori Morimoto @ 2019-06-10 0:49 UTC (permalink / raw)
To: Mark Brown, Ranjani Sridharan; +Cc: Linux-ALSA
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.
We need mutex_lock() under snd_soc_unregister_card()
instead of snd_remove_link_components().
Fixes: 34ac3c3eb8f0c07 ("ASoC: core: lock client_mutex while removing link components")
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 94a36ee..1679990 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1018,14 +1018,12 @@ static void soc_remove_link_components(struct snd_soc_card *card,
struct snd_soc_component *component;
struct snd_soc_rtdcom_list *rtdcom;
- mutex_lock(&client_mutex);
for_each_rtdcom(rtd, rtdcom) {
component = rtdcom->component;
if (component->driver->remove_order == order)
soc_remove_component(component);
}
- mutex_unlock(&client_mutex);
}
static void soc_remove_dai_links(struct snd_soc_card *card)
@@ -2774,7 +2772,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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] ASoC: soc-core: call snd_soc_unbind_card() under mutex_lock;
2019-06-10 0:49 [PATCH] ASoC: soc-core: call snd_soc_unbind_card() under mutex_lock; Kuninori Morimoto
@ 2019-06-10 16:27 ` Ranjani Sridharan
2019-06-11 0:27 ` Kuninori Morimoto
0 siblings, 1 reply; 8+ messages in thread
From: Ranjani Sridharan @ 2019-06-10 16:27 UTC (permalink / raw)
To: Kuninori Morimoto, Mark Brown; +Cc: Linux-ALSA
On Mon, 2019-06-10 at 09:49 +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.
> We need mutex_lock() under snd_soc_unregister_card()
> instead of snd_remove_link_components().
Thanks, Morimoto-san. I submitted a solution to fix the deadlock just a
couple of days ago as well (
https://mailman.alsa-project.org/pipermail/alsa-devel/2019-June/150764.html
).
I am fine with either solution though.
Thanks,
Ranjani
>
> Fixes: 34ac3c3eb8f0c07 ("ASoC: core: lock client_mutex while removing
> link components")
> 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 94a36ee..1679990 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -1018,14 +1018,12 @@ static void soc_remove_link_components(struct
> snd_soc_card *card,
> struct snd_soc_component *component;
> struct snd_soc_rtdcom_list *rtdcom;
>
> - mutex_lock(&client_mutex);
> for_each_rtdcom(rtd, rtdcom) {
> component = rtdcom->component;
>
> if (component->driver->remove_order == order)
> soc_remove_component(component);
> }
> - mutex_unlock(&client_mutex);
> }
>
> static void soc_remove_dai_links(struct snd_soc_card *card)
> @@ -2774,7 +2772,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;
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ASoC: soc-core: call snd_soc_unbind_card() under mutex_lock;
2019-06-10 16:27 ` Ranjani Sridharan
@ 2019-06-11 0:27 ` Kuninori Morimoto
0 siblings, 0 replies; 8+ messages in thread
From: Kuninori Morimoto @ 2019-06-11 0:27 UTC (permalink / raw)
To: Ranjani Sridharan; +Cc: Linux-ALSA, Mark Brown
Hi Ranjani
Thank you for your feedback
> Thanks, Morimoto-san. I submitted a solution to fix the deadlock just a
> couple of days ago as well (
> https://mailman.alsa-project.org/pipermail/alsa-devel/2019-June/150764.html
> ).
>
> I am fine with either solution though.
Hmm... If my understanding was correct,
mutex_lock under snd_soc_unbind_card() is not good / not enough.
My patch can solve these, but please double check it.
# To be honest, current ALSA SoC is very difficult to read.
# Thus, I'm working to cleanup code, actually. I want to post it.
# I'm waiting for posted patches to be applied
1) snd_soc_unbind_card() is used from snd_soc_component_del_unlocked()
which is used from __snd_soc_unregister_component().
It is using mutex_lock() already
__snd_soc_unregister_component()
=> mutex_lock()
snd_soc_component_del_unlocked()
snd_soc_unbind_card()
=> mutex_lock()
soc_remove_link_components()
=> mutex_unlock()
(*2) soc_cleanup_card_resources()
=> mutex_unlock()
(2) soc_cleanup_card_resources() also calls soc_remove_link_components()
(see above (*2))
soc_cleanup_card_resources()
soc_remove_dai_links()
=> soc_remove_link_components()
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ASoC: soc-core: call snd_soc_unbind_card() under mutex_lock;
2019-06-19 1:07 Kuninori Morimoto
2019-06-21 0:58 ` Kuninori Morimoto
@ 2019-06-25 1:13 ` Ranjani Sridharan
1 sibling, 0 replies; 8+ messages in thread
From: Ranjani Sridharan @ 2019-06-25 1:13 UTC (permalink / raw)
To: Kuninori Morimoto, Mark Brown; +Cc: Linux-ALSA
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);
> /* 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;
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ASoC: soc-core: call snd_soc_unbind_card() under mutex_lock;
2019-06-21 21:19 ` Ranjani Sridharan
@ 2019-06-24 1:09 ` Kuninori Morimoto
0 siblings, 0 replies; 8+ messages in thread
From: Kuninori Morimoto @ 2019-06-24 1:09 UTC (permalink / raw)
To: Ranjani Sridharan; +Cc: Linux-ALSA, Mark Brown
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);
> > > /* 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
> > >
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ASoC: soc-core: call snd_soc_unbind_card() under mutex_lock;
2019-06-21 0:58 ` Kuninori Morimoto
@ 2019-06-21 21:19 ` Ranjani Sridharan
2019-06-24 1:09 ` Kuninori Morimoto
0 siblings, 1 reply; 8+ messages in thread
From: Ranjani Sridharan @ 2019-06-21 21:19 UTC (permalink / raw)
To: Kuninori Morimoto; +Cc: Linux-ALSA, Mark Brown
> > 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);
> > /* 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
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ASoC: soc-core: call snd_soc_unbind_card() under mutex_lock;
2019-06-19 1:07 Kuninori Morimoto
@ 2019-06-21 0:58 ` Kuninori Morimoto
2019-06-21 21:19 ` Ranjani Sridharan
2019-06-25 1:13 ` Ranjani Sridharan
1 sibling, 1 reply; 8+ messages in thread
From: Kuninori Morimoto @ 2019-06-21 0:58 UTC (permalink / raw)
To: Ranjani Sridharan; +Cc: Linux-ALSA, Mark Brown
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] ASoC: soc-core: call snd_soc_unbind_card() under mutex_lock;
@ 2019-06-19 1:07 Kuninori Morimoto
2019-06-21 0:58 ` Kuninori Morimoto
2019-06-25 1:13 ` Ranjani Sridharan
0 siblings, 2 replies; 8+ messages in thread
From: Kuninori Morimoto @ 2019-06-19 1:07 UTC (permalink / raw)
To: Mark Brown, Ranjani Sridharan; +Cc: Linux-ALSA
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-06-25 1:13 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-10 0:49 [PATCH] ASoC: soc-core: call snd_soc_unbind_card() under mutex_lock; Kuninori Morimoto
2019-06-10 16:27 ` Ranjani Sridharan
2019-06-11 0:27 ` Kuninori Morimoto
2019-06-19 1:07 Kuninori Morimoto
2019-06-21 0:58 ` Kuninori Morimoto
2019-06-21 21:19 ` Ranjani Sridharan
2019-06-24 1:09 ` Kuninori Morimoto
2019-06-25 1:13 ` Ranjani Sridharan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.