Message ID | 5cef9f66.1c69fb81.39f30.21e8@mx.google.com |
---|---|
State | New |
Headers | show |
Series | next/master boot bisection: next-20190528 on sun8i-h3-libretech-all-h3-cc | expand |
On Thu, 30 May 2019 11:16:22 +0200, kernelci.org bot wrote: > > * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * > * This automated bisection report was sent to you on the basis * > * that you may be involved with the breaking commit it has * > * found. No manual investigation has been done to verify it, * > * and the root cause of the problem may be somewhere else. * > * Hope this helps! * > * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * > > next/master boot bisection: next-20190528 on sun8i-h3-libretech-all-h3-cc > > Summary: > Start: 531b0a360899 Add linux-next specific files for 20190528 > Details: https://kernelci.org/boot/id/5cece0fd59b5144bc47a362b > Plain log: https://storage.kernelci.org//next/master/next-20190528/arm/sunxi_defconfig/gcc-8/lab-baylibre/boot-sun8i-h3-libretech-all-h3-cc.txt > HTML log: https://storage.kernelci.org//next/master/next-20190528/arm/sunxi_defconfig/gcc-8/lab-baylibre/boot-sun8i-h3-libretech-all-h3-cc.html > Result: 34ac3c3eb8f0 ASoC: core: lock client_mutex while removing link components > > Checks: > revert: PASS > verify: PASS > > Parameters: > Tree: next > URL: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git > Branch: master > Target: sun8i-h3-libretech-all-h3-cc > CPU arch: arm > Lab: lab-baylibre > Compiler: gcc-8 > Config: sunxi_defconfig > Test suite: boot > > Breaking commit found: > > ------------------------------------------------------------------------------- > commit 34ac3c3eb8f0c07252ceddf0a22dd240e5c91ccb > Author: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> > Date: Thu May 23 10:12:01 2019 -0700 > > ASoC: core: lock client_mutex while removing link components > > Removing link components results in topology unloading. So, > acquire the client_mutex before removing components in > soc_remove_link_components. This will prevent the lockdep warning > seen when dai links are removed during topology removal. > > Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> > Signed-off-by: Mark Brown <broonie@kernel.org> > > diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c > index 2403bec2fccf..7c9415987ac7 100644 > --- a/sound/soc/soc-core.c > +++ b/sound/soc/soc-core.c > @@ -1005,12 +1005,14 @@ 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); > } Indeed this dead-locks in the error path of snd_soc_instantiate_card(): snd_soc_instantiate_card() -> mutex_lock(&client_mutex); .... -> soc_cleanup_card_resources(); -> soc_remove_dai_links(); -> soc_remove_link_components(); mutex_lock(&client_mutex); Ranjani, which code path your patch tries to address? Maybe better to wrap client_mutex() in the caller side like snd_soc_unbind_card()? thanks, Takashi > > static void soc_remove_dai_links(struct snd_soc_card *card) > ------------------------------------------------------------------------------- > > > Git bisection log: > > ------------------------------------------------------------------------------- > git bisect start > # good: [cd6c84d8f0cdc911df435bb075ba22ce3c605b07] Linux 5.2-rc2 > git bisect good cd6c84d8f0cdc911df435bb075ba22ce3c605b07 > # bad: [531b0a360899269bd99a38ba9852a8ba46852bcd] Add linux-next specific files for 20190528 > git bisect bad 531b0a360899269bd99a38ba9852a8ba46852bcd > # bad: [0b61d4c3b7d7938ef0014778c328e3f65c0d6d57] Merge remote-tracking branch 'crypto/master' > git bisect bad 0b61d4c3b7d7938ef0014778c328e3f65c0d6d57 > # bad: [6179e21b065dc0f592cd3d9d3676bd64d4278025] Merge remote-tracking branch 'xtensa/xtensa-for-next' > git bisect bad 6179e21b065dc0f592cd3d9d3676bd64d4278025 > # bad: [3e085f66fe7e93575f2a583a3d434415cef2d860] Merge remote-tracking branch 'amlogic/for-next' > git bisect bad 3e085f66fe7e93575f2a583a3d434415cef2d860 > # bad: [b9afa223a3420432bc483d2b43429c88c6a5d0e0] Merge remote-tracking branch 'staging.current/staging-linus' > git bisect bad b9afa223a3420432bc483d2b43429c88c6a5d0e0 > # good: [fc6557648e19dbd207dc815c6e09fc6452f01e63] Merge remote-tracking branch 'bpf/master' > git bisect good fc6557648e19dbd207dc815c6e09fc6452f01e63 > # bad: [6c3f2a0e0f236f31b47d63ab7d3f4ec889821d0d] Merge remote-tracking branch 'spi-fixes/for-linus' > git bisect bad 6c3f2a0e0f236f31b47d63ab7d3f4ec889821d0d > # bad: [20a5f9c8649d74407aa657ce5b76cf8b0bbb17e3] Merge branch 'asoc-5.2' into asoc-linus > git bisect bad 20a5f9c8649d74407aa657ce5b76cf8b0bbb17e3 > # good: [ad6eecbfc01c987e0253371f274c3872042e4350] ASoC: cs42xx8: Add regcache mask dirty > git bisect good ad6eecbfc01c987e0253371f274c3872042e4350 > # good: [069d037aea98ffa64c26d4b1dc958fb8f39f5c2b] ASoC: simple-card: Fix configuration of DAI format > git bisect good 069d037aea98ffa64c26d4b1dc958fb8f39f5c2b > # good: [df9366131a452296d040a7a496d93108f1fc240c] ASoC: Intel: sof-rt5682: fix AMP quirk support > git bisect good df9366131a452296d040a7a496d93108f1fc240c > # bad: [34ac3c3eb8f0c07252ceddf0a22dd240e5c91ccb] ASoC: core: lock client_mutex while removing link components > git bisect bad 34ac3c3eb8f0c07252ceddf0a22dd240e5c91ccb > # good: [4819d06292c9b57eabdd6d1603e49a27baf183be] ASoC: simple-card: Restore original configuration of DAI format > git bisect good 4819d06292c9b57eabdd6d1603e49a27baf183be > # first bad commit: [34ac3c3eb8f0c07252ceddf0a22dd240e5c91ccb] ASoC: core: lock client_mutex while removing link components > ------------------------------------------------------------------------------- > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel >
On 30/05/2019 16:53, Takashi Iwai wrote: > On Thu, 30 May 2019 11:16:22 +0200, > kernelci.org bot wrote: >> >> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * >> * This automated bisection report was sent to you on the basis * >> * that you may be involved with the breaking commit it has * >> * found. No manual investigation has been done to verify it, * >> * and the root cause of the problem may be somewhere else. * >> * Hope this helps! * >> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * >> >> next/master boot bisection: next-20190528 on sun8i-h3-libretech-all-h3-cc >> >> Summary: >> Start: 531b0a360899 Add linux-next specific files for 20190528 >> Details: https://kernelci.org/boot/id/5cece0fd59b5144bc47a362b >> Plain log: https://storage.kernelci.org//next/master/next-20190528/arm/sunxi_defconfig/gcc-8/lab-baylibre/boot-sun8i-h3-libretech-all-h3-cc.txt >> HTML log: https://storage.kernelci.org//next/master/next-20190528/arm/sunxi_defconfig/gcc-8/lab-baylibre/boot-sun8i-h3-libretech-all-h3-cc.html >> Result: 34ac3c3eb8f0 ASoC: core: lock client_mutex while removing link components >> >> Checks: >> revert: PASS >> verify: PASS >> >> Parameters: >> Tree: next >> URL: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git >> Branch: master >> Target: sun8i-h3-libretech-all-h3-cc >> CPU arch: arm >> Lab: lab-baylibre >> Compiler: gcc-8 >> Config: sunxi_defconfig >> Test suite: boot >> >> Breaking commit found: >> >> ------------------------------------------------------------------------------- >> commit 34ac3c3eb8f0c07252ceddf0a22dd240e5c91ccb >> Author: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> >> Date: Thu May 23 10:12:01 2019 -0700 >> >> ASoC: core: lock client_mutex while removing link components >> >> Removing link components results in topology unloading. So, >> acquire the client_mutex before removing components in >> soc_remove_link_components. This will prevent the lockdep warning >> seen when dai links are removed during topology removal. >> >> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> >> Signed-off-by: Mark Brown <broonie@kernel.org> >> >> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c >> index 2403bec2fccf..7c9415987ac7 100644 >> --- a/sound/soc/soc-core.c >> +++ b/sound/soc/soc-core.c >> @@ -1005,12 +1005,14 @@ 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); >> } > > Indeed this dead-locks in the error path of > snd_soc_instantiate_card(): > > snd_soc_instantiate_card() -> > mutex_lock(&client_mutex); > .... > -> soc_cleanup_card_resources(); > -> soc_remove_dai_links(); > -> soc_remove_link_components(); > mutex_lock(&client_mutex); > > > Ranjani, which code path your patch tries to address? Maybe better to > wrap client_mutex() in the caller side like snd_soc_unbind_card()? Is anyone looking into this issue? It is still occurring in next-20190606, there was a bisection today which landed on the same commit. There just hasn't been any new bisection reports because they have been temporarily disabled while we fix some issues on kernelci.org. Thanks, Guillaume >> static void soc_remove_dai_links(struct snd_soc_card *card) >> ------------------------------------------------------------------------------- >> >> >> Git bisection log: >> >> ------------------------------------------------------------------------------- >> git bisect start >> # good: [cd6c84d8f0cdc911df435bb075ba22ce3c605b07] Linux 5.2-rc2 >> git bisect good cd6c84d8f0cdc911df435bb075ba22ce3c605b07 >> # bad: [531b0a360899269bd99a38ba9852a8ba46852bcd] Add linux-next specific files for 20190528 >> git bisect bad 531b0a360899269bd99a38ba9852a8ba46852bcd >> # bad: [0b61d4c3b7d7938ef0014778c328e3f65c0d6d57] Merge remote-tracking branch 'crypto/master' >> git bisect bad 0b61d4c3b7d7938ef0014778c328e3f65c0d6d57 >> # bad: [6179e21b065dc0f592cd3d9d3676bd64d4278025] Merge remote-tracking branch 'xtensa/xtensa-for-next' >> git bisect bad 6179e21b065dc0f592cd3d9d3676bd64d4278025 >> # bad: [3e085f66fe7e93575f2a583a3d434415cef2d860] Merge remote-tracking branch 'amlogic/for-next' >> git bisect bad 3e085f66fe7e93575f2a583a3d434415cef2d860 >> # bad: [b9afa223a3420432bc483d2b43429c88c6a5d0e0] Merge remote-tracking branch 'staging.current/staging-linus' >> git bisect bad b9afa223a3420432bc483d2b43429c88c6a5d0e0 >> # good: [fc6557648e19dbd207dc815c6e09fc6452f01e63] Merge remote-tracking branch 'bpf/master' >> git bisect good fc6557648e19dbd207dc815c6e09fc6452f01e63 >> # bad: [6c3f2a0e0f236f31b47d63ab7d3f4ec889821d0d] Merge remote-tracking branch 'spi-fixes/for-linus' >> git bisect bad 6c3f2a0e0f236f31b47d63ab7d3f4ec889821d0d >> # bad: [20a5f9c8649d74407aa657ce5b76cf8b0bbb17e3] Merge branch 'asoc-5.2' into asoc-linus >> git bisect bad 20a5f9c8649d74407aa657ce5b76cf8b0bbb17e3 >> # good: [ad6eecbfc01c987e0253371f274c3872042e4350] ASoC: cs42xx8: Add regcache mask dirty >> git bisect good ad6eecbfc01c987e0253371f274c3872042e4350 >> # good: [069d037aea98ffa64c26d4b1dc958fb8f39f5c2b] ASoC: simple-card: Fix configuration of DAI format >> git bisect good 069d037aea98ffa64c26d4b1dc958fb8f39f5c2b >> # good: [df9366131a452296d040a7a496d93108f1fc240c] ASoC: Intel: sof-rt5682: fix AMP quirk support >> git bisect good df9366131a452296d040a7a496d93108f1fc240c >> # bad: [34ac3c3eb8f0c07252ceddf0a22dd240e5c91ccb] ASoC: core: lock client_mutex while removing link components >> git bisect bad 34ac3c3eb8f0c07252ceddf0a22dd240e5c91ccb >> # good: [4819d06292c9b57eabdd6d1603e49a27baf183be] ASoC: simple-card: Restore original configuration of DAI format >> git bisect good 4819d06292c9b57eabdd6d1603e49a27baf183be >> # first bad commit: [34ac3c3eb8f0c07252ceddf0a22dd240e5c91ccb] ASoC: core: lock client_mutex while removing link components >> ------------------------------------------------------------------------------- >> _______________________________________________ >> Alsa-devel mailing list >> Alsa-devel@alsa-project.org >> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel >>
On Fri, Jun 07, 2019 at 05:31:12PM +0100, Guillaume Tucker wrote: > On 30/05/2019 16:53, Takashi Iwai wrote: > >> + 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); > > Ranjani, which code path your patch tries to address? Maybe better to > > wrap client_mutex() in the caller side like snd_soc_unbind_card()? > Is anyone looking into this issue? > It is still occurring in next-20190606, there was a bisection > today which landed on the same commit. There just hasn't been > any new bisection reports because they have been temporarily > disabled while we fix some issues on kernelci.org. I was expecting that Ranjani or one of the other Intel people was looking into it...
On 6/7/19 2:00 PM, Mark Brown wrote: > On Fri, Jun 07, 2019 at 05:31:12PM +0100, Guillaume Tucker wrote: >> On 30/05/2019 16:53, Takashi Iwai wrote: > >>>> + 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); > >>> Ranjani, which code path your patch tries to address? Maybe better to >>> wrap client_mutex() in the caller side like snd_soc_unbind_card()? > >> Is anyone looking into this issue? > >> It is still occurring in next-20190606, there was a bisection >> today which landed on the same commit. There just hasn't been >> any new bisection reports because they have been temporarily >> disabled while we fix some issues on kernelci.org. > > I was expecting that Ranjani or one of the other Intel people was > looking into it... Ack. We've all been underwater this week and this wasn't addressed, sorry about the delay. It's probably wise to revert this commit at this point while we look for an alternate solution? There was an initial proposal submitted on GitHub [1] (patch attached) which implemented what Takashi suggested in his comments. This proposal was later optimized further, it could be that the optimization was one bridge too far. Could you let us know if this attached patch has any negative effects on non-Intel platforms? Thanks! [1] https://github.com/thesofproject/linux/commit/9fd09dd417bc8be7a4a8bdd1621558151f8d117b From 9fd09dd417bc8be7a4a8bdd1621558151f8d117b Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> Date: Wed, 22 May 2019 10:52:40 -0700 Subject: [PATCH] ASoC: core: lock client_mutex while removing link components Removing link components results in topology unloading. So, acquire the client_mutex before removing components in snd_soc_unbind_card(). This will prevent lockdep warning when the dai link is removed. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> --- sound/soc/soc-core.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 2403bec2fccf3..5609398f05d80 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2839,12 +2839,14 @@ 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)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 2403bec2fccf..7c9415987ac7 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1005,12 +1005,14 @@ 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)