Hi Pierre-Louis
Thank you for your report
> >> + for_each_rtd_components(rtd, rtdcom, component) {
> >> + pr_err("plb: %s processing component\n", __func__);
> >> + if (!component)
> >> + pr_err("plb: %s component is NULL\n", __func__);
> >
> > Could you perhaps add traces of which components are being accessed at
> > each stage? We might want to go through and just add something like
> > that in the code anyway to help figure things out.
>
> I tried to add more traces but couldn't triangulate on a clear issue,
> and the traces have an Heisenbug effect.
>
> So I switched to higher-level code analysis: it turns out that
> soc_dai_link_remove() routine is called from both topology and on card
> cleanup.
>
> The patch 06/19 in this series essentially forces the pcm_runtimes to
> be freed in both cases, so possibly twice for topology-managed
> dailinks - or using information that's been freed already.
>
> I 'fixed' this by adding an additional parameter to avoid doing the
> pcm runtime free from the topology (as was done before), and the
> kernel oops goes away. My tests have been running for 45mn now, when
> without change I get a kernel oops in less than 10-20 cycles (but
> still more than apparently our CI tracks, something to improve).
>
> I pushed the code on GitHub to check if there are any negative points
> reported by the Intel CI, should be complete shortly:
> https://github.com/thesofproject/linux/pull/1469
>
> I am not sure the suggested fix is correct, I don't fully get what the
> topology and card cleanups should do and how the work is split, if at
> all.
BTW, I guess your kernel is appling this patch,
but, is it correct ?
df95a16d2a9626dcfc3f2b3671c9b91fa076c997
("ASoC: soc-core: fix RIP warning on card removal")
If so, I think I could understand the issue.
But, before explaining detail,
I want to confirm that my solution is correct or not first.
Can you please check attached patch ?
Then, please remove above RIP warning patch.
It is not clean patch, but OK to confirming, so far.
I think
- no kernel Oops
- no RIP warning
Thank you for your help !!
Best regards
---
Kuninori Morimoto
-------------- next part --------------
From 0d825237eea4baad0c489e1c43a58373f41a3632 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx at renesas.com>
Date: Wed, 13 Nov 2019 11:58:18 +0900
Subject: [PATCH] topology die fixup patch candidate 1
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx at renesas.com>
---
include/sound/soc-component.h | 2 +-
sound/soc/soc-component.c | 5 ++---
sound/soc/soc-core.c | 14 ++++++++------
sound/soc/soc-pcm.c | 10 ----------
4 files changed, 11 insertions(+), 20 deletions(-)
diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h
index 6aa3ecb..cf726a5 100644
--- a/include/sound/soc-component.h
+++ b/include/sound/soc-component.h
@@ -413,6 +413,6 @@ struct page *snd_soc_pcm_component_page(struct snd_pcm_substream *substream,
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);
+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..195979f 100644
--- a/sound/soc/soc-component.c
+++ b/sound/soc/soc-component.c
@@ -516,13 +516,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-core.c b/sound/soc/soc-core.c
index 1e8dd19..8e7ff7d 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -386,6 +386,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
@@ -1965,12 +1968,6 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card)
{
struct snd_soc_dai_link *link, *_link;
- /* 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;
- }
-
/* remove and free each DAI */
soc_remove_link_dais(card);
soc_remove_link_components(card);
@@ -1988,6 +1985,11 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card)
/* remove the card */
if (card->remove)
card->remove(card);
+
+ if (card->snd_card) {
+ snd_card_free(card->snd_card);
+ card->snd_card = NULL;
+ }
}
static int snd_soc_instantiate_card(struct snd_soc_card *card)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 1c00119..a60e381 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -2883,15 +2883,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(pcm);
-}
-
/* create a new pcm */
int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
{
@@ -3033,7 +3024,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