| Message ID | 87imtev0l2.wl-kuninori.morimoto.gx@renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: soc-core: call snd_soc_unbind_card() under mutex_lock; | expand |
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;
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()
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;