[PATCH] ASoC: soc-compress: Reposition and add pcm_mutex

[PATCH] ASoC: soc-compress: Reposition and add pcm_mutex

[강신형]

[-- Attachment #1: Type: text/plain, Size: 2984 bytes --]

   commit a385cc2614c17ba3002b263c0ca47dfbf6395c78 (HEAD -> fix_compr,
   master)
   Author: s47.kang <[1]s47.kang@samsung.com>
   Date:   Fri Dec 2 15:45:16 2022 +0900

       ASoC: soc-compress.c: Reposition and add pcm_mutex.

       If panic_on_warn is set and compress stream(DPCM) is started,
       then kernel panic will occurs because card->pcm_mutex isn't held
   appropriately in
       soc_compr_set_params_fe, soc_compr_open_fe and soc_compr_free_fe.
       So, I reposition and add pcm_mutex to resolve lockdep error.

       Signed-off-by: Shinhyung Kang <[2]s47.kang@samsung.com>

   diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
   index 870f13e1d389..7bce5088b455 100644
   --- a/sound/soc/soc-compress.c
   +++ b/sound/soc/soc-compress.c
   @@ -149,6 +149,8 @@ static int soc_compr_open_fe(struct
   snd_compr_stream *cstream)
           if (ret < 0)
                   goto be_err;

   +       mutex_lock_nested(&fe->card->pcm_mutex,
   fe->card->pcm_subclass);
   +
           /* calculate valid and active FE <-> BE dpcms */
           dpcm_process_paths(fe, stream, &list, 1);
           fe->dpcm[stream].runtime = fe_substream->runtime;
   @@ -184,7 +186,6 @@ static int soc_compr_open_fe(struct
   snd_compr_stream *cstream)
           fe->dpcm[stream].state = SND_SOC_DPCM_STATE_OPEN;
           fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_NO;

   -       mutex_lock_nested(&fe->card->pcm_mutex,
   fe->card->pcm_subclass);
           snd_soc_runtime_activate(fe, stream);
           mutex_unlock(&fe->card->pcm_mutex);

   @@ -215,7 +216,6 @@ static int soc_compr_free_fe(struct
   snd_compr_stream *cstream)

           mutex_lock_nested(&fe->card->pcm_mutex,
   fe->card->pcm_subclass);
           snd_soc_runtime_deactivate(fe, stream);
   -       mutex_unlock(&fe->card->pcm_mutex);

           fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_FE;

   @@ -234,6 +234,8 @@ static int soc_compr_free_fe(struct
   snd_compr_stream *cstream)

           dpcm_be_disconnect(fe, stream);

   +       mutex_unlock(&fe->card->pcm_mutex);
   +
           fe->dpcm[stream].runtime = NULL;

           snd_soc_link_compr_shutdown(cstream, 0);
   @@ -409,8 +411,9 @@ static int soc_compr_set_params_fe(struct
   snd_compr_stream *cstream,
           ret = snd_soc_link_compr_set_params(cstream);
           if (ret < 0)
                   goto out;
   -
   +       mutex_lock_nested(&fe->card->pcm_mutex,
   fe->card->pcm_subclass);
           dpcm_dapm_stream_event(fe, stream, SND_SOC_DAPM_STREAM_START);
   +       mutex_unlock(&fe->card->pcm_mutex);
           fe->dpcm[stream].state = SND_SOC_DPCM_STATE_PREPARE;

    out:


   [cid:cafe_image_0@s-core.co.kr]

   [update?userid=s47.kang&do=bWFpbElEPTIwMjIxMjAyMDgwMDMyZXBjbXMycDY4ZjU2
   YTZiNDNkN2Q2YTRmMGQyZGU5Mjk1YzBlNGFiNCZyZWNpcGllbnRBZGRyZXNzPWFsc2EtZGV
   2ZWxAYWxzYS1wcm9qZWN0Lm9yZw__]

References

   1. mailto:s47.kang@samsung.com
   2. mailto:s47.kang@samsung.com

[-- Attachment #2: Type: image/gif, Size: 13402 bytes --]

Re: [PATCH] ASoC: soc-compress: Reposition and add pcm_mutex

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 421 bytes --]

On Fri, Dec 02, 2022 at 05:00:32PM +0900, 강신형 wrote:
> <p>commit a385cc2614c17ba3002b263c0ca47dfbf6395c78 (HEAD -&gt; fix_compr, master)<br />
> Author: s47.kang &lt;<a href="mailto:s47.kang@samsung.com">s47.kang@samsung.com</a>&gt;<br />
> Date:&nbsp;&nbsp; Fri Dec 2 15:45:16 2022 +0900</p>

Your mail is HTML only so unfortunately I can't really read it
and none of the tooling will be able to handle it.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH] ASoC: soc-compress: Reposition and add pcm_mutex

[강신형]

From 3f8fd1efedd77331d948b16a8e44ffa6cf0e811f Mon Sep 17 00:00:00 2001

From: "s47.kang" <s47.kang@samsung.com>

Date: Fri, 2 Dec 2022 18:32:35 +0900

Subject: [PATCH] [COMMON] ASoC: soc-compress.c: Reposition and add
pcm_mutex.

 

If panic_on_warn is set and compress stream(DPCM) is started,

then kernel panic will occurs because card->pcm_mutex isn't held
appropriately in

soc_compr_set_params_fe, soc_compr_open_fe and soc_compr_free_fe.

So, I reposition and add pcm_mutex to resolve lockdep error.

 

Change-Id: I15981b4f09b0bff37e7e98e6ee1f809339b2bcc4

---

sound/soc/soc-compress.c | 9 ++++++---

1 file changed, 6 insertions(+), 3 deletions(-)

 

diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c

index 870f13e1d389..7bce5088b455 100644

--- a/sound/soc/soc-compress.c

+++ b/sound/soc/soc-compress.c

@@ -149,6 +149,8 @@ static int soc_compr_open_fe(struct snd_compr_stream
*cstream)

         if (ret < 0)

                    goto be_err;

+         mutex_lock_nested(&fe->card->pcm_mutex, fe->card->pcm_subclass);

+

         /* calculate valid and active FE <-> BE dpcms */

         dpcm_process_paths(fe, stream, &list, 1);

         fe->dpcm[stream].runtime = fe_substream->runtime;

@@ -184,7 +186,6 @@ static int soc_compr_open_fe(struct snd_compr_stream
*cstream)

         fe->dpcm[stream].state = SND_SOC_DPCM_STATE_OPEN;

         fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_NO;

-          mutex_lock_nested(&fe->card->pcm_mutex, fe->card->pcm_subclass);

         snd_soc_runtime_activate(fe, stream);

         mutex_unlock(&fe->card->pcm_mutex);

@@ -215,7 +216,6 @@ static int soc_compr_free_fe(struct snd_compr_stream
*cstream)

          mutex_lock_nested(&fe->card->pcm_mutex, fe->card->pcm_subclass);

         snd_soc_runtime_deactivate(fe, stream);

-          mutex_unlock(&fe->card->pcm_mutex);

          fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_FE;

@@ -234,6 +234,8 @@ static int soc_compr_free_fe(struct snd_compr_stream
*cstream)

          dpcm_be_disconnect(fe, stream);

+         mutex_unlock(&fe->card->pcm_mutex);

+

         fe->dpcm[stream].runtime = NULL;

          snd_soc_link_compr_shutdown(cstream, 0);

@@ -409,8 +411,9 @@ static int soc_compr_set_params_fe(struct
snd_compr_stream *cstream,

         ret = snd_soc_link_compr_set_params(cstream);

         if (ret < 0)

                    goto out;

-

+         mutex_lock_nested(&fe->card->pcm_mutex, fe->card->pcm_subclass);

         dpcm_dapm_stream_event(fe, stream, SND_SOC_DAPM_STREAM_START);

+         mutex_unlock(&fe->card->pcm_mutex);

         fe->dpcm[stream].state = SND_SOC_DPCM_STATE_PREPARE;

 out:

--

2.21.0

 

[PATCH] ASoC: soc-compress: Reposition and add pcm_mutex

[강신형]

From a385cc2614c17ba3002b263c0ca47dfbf6395c78 Mon Sep 17 00:00:00 2001
From: "s47.kang" <s47.kang@samsung.com>
Date: Fri, 2 Dec 2022 15:45:16 +0900
Subject: [PATCH] ASoC: soc-compress.c: Reposition and add pcm_mutex.

If panic_on_warn is set and compress stream(DPCM) is started,
then kernel panic will occurs because card->pcm_mutex isn't held
appropriately in
soc_compr_set_params_fe, soc_compr_open_fe and soc_compr_free_fe.
So, I reposition and add pcm_mutex to resolve lockdep error.

Signed-off-by: Shinhyung Kang <s47.kang@samsung.com>
---
 sound/soc/soc-compress.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index 870f13e1d389..7bce5088b455 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -149,6 +149,8 @@ static int soc_compr_open_fe(struct snd_compr_stream
*cstream)
 	if (ret < 0)
 		goto be_err;
 
+	mutex_lock_nested(&fe->card->pcm_mutex, fe->card->pcm_subclass);
+
 	/* calculate valid and active FE <-> BE dpcms */
 	dpcm_process_paths(fe, stream, &list, 1);
 	fe->dpcm[stream].runtime = fe_substream->runtime;
@@ -184,7 +186,6 @@ static int soc_compr_open_fe(struct snd_compr_stream
*cstream)
 	fe->dpcm[stream].state = SND_SOC_DPCM_STATE_OPEN;
 	fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_NO;
 
-	mutex_lock_nested(&fe->card->pcm_mutex, fe->card->pcm_subclass);
 	snd_soc_runtime_activate(fe, stream);
 	mutex_unlock(&fe->card->pcm_mutex);
 
@@ -215,7 +216,6 @@ static int soc_compr_free_fe(struct snd_compr_stream
*cstream)
 
 	mutex_lock_nested(&fe->card->pcm_mutex, fe->card->pcm_subclass);
 	snd_soc_runtime_deactivate(fe, stream);
-	mutex_unlock(&fe->card->pcm_mutex);
 
 	fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_FE;
 
@@ -234,6 +234,8 @@ static int soc_compr_free_fe(struct snd_compr_stream
*cstream)
 
 	dpcm_be_disconnect(fe, stream);
 
+	mutex_unlock(&fe->card->pcm_mutex);
+
 	fe->dpcm[stream].runtime = NULL;
 
 	snd_soc_link_compr_shutdown(cstream, 0);
@@ -409,8 +411,9 @@ static int soc_compr_set_params_fe(struct
snd_compr_stream *cstream,
 	ret = snd_soc_link_compr_set_params(cstream);
 	if (ret < 0)
 		goto out;
-
+	mutex_lock_nested(&fe->card->pcm_mutex, fe->card->pcm_subclass);
 	dpcm_dapm_stream_event(fe, stream, SND_SOC_DAPM_STREAM_START);
+	mutex_unlock(&fe->card->pcm_mutex);
 	fe->dpcm[stream].state = SND_SOC_DPCM_STATE_PREPARE;
 
 out:
-- 
2.21.0

Re: [PATCH] ASoC: soc-compress: Reposition and add pcm_mutex

[Krzysztof Kozlowski]

On 05/12/2022 08:46, 강신형 wrote:
> From a385cc2614c17ba3002b263c0ca47dfbf6395c78 Mon Sep 17 00:00:00 2001
> From: "s47.kang" <s47.kang@samsung.com>
> Date: Fri, 2 Dec 2022 15:45:16 +0900
> Subject: [PATCH] ASoC: soc-compress.c: Reposition and add pcm_mutex.

This does not match your subject... and having subject twice is also
confusing. Please fix your mailer.

Additionally, this is like third patch, right? So please mark it as v3
(git format-patch helps in this).

> 
> If panic_on_warn is set and compress stream(DPCM) is started,
> then kernel panic will occurs because card->pcm_mutex isn't held
> appropriately in

So the problem is panic_on_warn or wrong synchronization? It's the
second, so skip unrelated information and instead describe the real
problem, e.g. what exactly lockdep reported.

> soc_compr_set_params_fe, soc_compr_open_fe and soc_compr_free_fe.
> So, I reposition and add pcm_mutex to resolve lockdep error.
> 
> Signed-off-by: Shinhyung Kang <s47.kang@samsung.com>
> ---
>  sound/soc/soc-compress.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

Best regards,
Krzysztof

Re: [PATCH] ASoC: soc-compress: Reposition and add pcm_mutex

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 544 bytes --]

On Mon, Dec 05, 2022 at 02:17:42PM +0900, ������ wrote:
> From 3f8fd1efedd77331d948b16a8e44ffa6cf0e811f Mon Sep 17 00:00:00 2001
> 
> From: "s47.kang" <s47.kang@samsung.com>
> 
> Date: Fri, 2 Dec 2022 18:32:35 +0900
> 
> Subject: [PATCH] [COMMON] ASoC: soc-compress.c: Reposition and add
> pcm_mutex.
> 
>  
> 
> If panic_on_warn is set and compress stream(DPCM) is started,

This mail is still really badly corrupted, it's got lots of blank lines
added, other whitespace damage and I think is word wrapped as well.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH] ASoC: soc-compress: Reposition and add pcm_mutex

[강신형]

If panic_on_warn is set and compress stream(DPCM) is started,
then kernel panic occurred because card->pcm_mutex isn't held appropriately.
In the following functions, warning were issued at this line
"snd_soc_dpcm_mutex_assert_held".

static int dpcm_be_connect(struct snd_soc_pcm_runtime *fe,
		struct snd_soc_pcm_runtime *be, int stream)
{
	...
	snd_soc_dpcm_mutex_assert_held(fe);
	...
}

void dpcm_be_disconnect(struct snd_soc_pcm_runtime *fe, int stream)
{
	...
	snd_soc_dpcm_mutex_assert_held(fe);
	...
}

void snd_soc_runtime_action(struct snd_soc_pcm_runtime *rtd,
			    int stream, int action)
{
	...
	snd_soc_dpcm_mutex_assert_held(rtd);
	...
}

int dpcm_dapm_stream_event(struct snd_soc_pcm_runtime *fe, int dir,
	int event)
{
	...
	snd_soc_dpcm_mutex_assert_held(fe);
	...
}

These functions are called by soc_compr_set_params_fe, soc_compr_open_fe
and soc_compr_free_fe
without pcm_mutex locking. And this is call stack.

[  414.527841][ T2179] pc : dpcm_process_paths+0x5a4/0x750
[  414.527848][ T2179] lr : dpcm_process_paths+0x37c/0x750
[  414.527945][ T2179] Call trace:
[  414.527949][ T2179]  dpcm_process_paths+0x5a4/0x750
[  414.527955][ T2179]  soc_compr_open_fe+0xb0/0x2cc
[  414.527972][ T2179]  snd_compr_open+0x180/0x248
[  414.527981][ T2179]  snd_open+0x15c/0x194
[  414.528003][ T2179]  chrdev_open+0x1b0/0x220
[  414.528023][ T2179]  do_dentry_open+0x30c/0x594
[  414.528045][ T2179]  vfs_open+0x34/0x44
[  414.528053][ T2179]  path_openat+0x914/0xb08
[  414.528062][ T2179]  do_filp_open+0xc0/0x170
[  414.528068][ T2179]  do_sys_openat2+0x94/0x18c
[  414.528076][ T2179]  __arm64_sys_openat+0x78/0xa4
[  414.528084][ T2179]  invoke_syscall+0x48/0x10c
[  414.528094][ T2179]  el0_svc_common+0xbc/0x104
[  414.528099][ T2179]  do_el0_svc+0x34/0xd8
[  414.528103][ T2179]  el0_svc+0x34/0xc4
[  414.528125][ T2179]  el0t_64_sync_handler+0x8c/0xfc
[  414.528133][ T2179]  el0t_64_sync+0x1a0/0x1a4
[  414.528142][ T2179] Kernel panic - not syncing: panic_on_warn set ...

So, I reposition and add pcm_mutex to resolve lockdep error.

Signed-off-by: Shinhyung Kang <s47.kang@samsung.com>
---
 sound/soc/soc-compress.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index 870f13e1d389..7bce5088b455 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -149,6 +149,8 @@ static int soc_compr_open_fe(struct snd_compr_stream
*cstream)
 	if (ret < 0)
 		goto be_err;
 
+	mutex_lock_nested(&fe->card->pcm_mutex, fe->card->pcm_subclass);
+
 	/* calculate valid and active FE <-> BE dpcms */
 	dpcm_process_paths(fe, stream, &list, 1);
 	fe->dpcm[stream].runtime = fe_substream->runtime;
@@ -184,7 +186,6 @@ static int soc_compr_open_fe(struct snd_compr_stream
*cstream)
 	fe->dpcm[stream].state = SND_SOC_DPCM_STATE_OPEN;
 	fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_NO;
 
-	mutex_lock_nested(&fe->card->pcm_mutex, fe->card->pcm_subclass);
 	snd_soc_runtime_activate(fe, stream);
 	mutex_unlock(&fe->card->pcm_mutex);
 
@@ -215,7 +216,6 @@ static int soc_compr_free_fe(struct snd_compr_stream
*cstream)
 
 	mutex_lock_nested(&fe->card->pcm_mutex, fe->card->pcm_subclass);
 	snd_soc_runtime_deactivate(fe, stream);
-	mutex_unlock(&fe->card->pcm_mutex);
 
 	fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_FE;
 
@@ -234,6 +234,8 @@ static int soc_compr_free_fe(struct snd_compr_stream
*cstream)
 
 	dpcm_be_disconnect(fe, stream);
 
+	mutex_unlock(&fe->card->pcm_mutex);
+
 	fe->dpcm[stream].runtime = NULL;
 
 	snd_soc_link_compr_shutdown(cstream, 0);
@@ -409,8 +411,9 @@ static int soc_compr_set_params_fe(struct
snd_compr_stream *cstream,
 	ret = snd_soc_link_compr_set_params(cstream);
 	if (ret < 0)
 		goto out;
-
+	mutex_lock_nested(&fe->card->pcm_mutex, fe->card->pcm_subclass);
 	dpcm_dapm_stream_event(fe, stream, SND_SOC_DAPM_STREAM_START);
+	mutex_unlock(&fe->card->pcm_mutex);
 	fe->dpcm[stream].state = SND_SOC_DPCM_STATE_PREPARE;
 
 out:
-- 
2.21.0

Re: [PATCH] ASoC: soc-compress: Reposition and add pcm_mutex

[Mark Brown]

On Thu, 08 Dec 2022 14:18:18 +0900, 강신형 wrote:
> If panic_on_warn is set and compress stream(DPCM) is started,
> then kernel panic occurred because card->pcm_mutex isn't held appropriately.
> In the following functions, warning were issued at this line
> "snd_soc_dpcm_mutex_assert_held".
> 
> static int dpcm_be_connect(struct snd_soc_pcm_runtime *fe,
> 		struct snd_soc_pcm_runtime *be, int stream)
> {
> 	...
> 	snd_soc_dpcm_mutex_assert_held(fe);
> 	...
> }
> 
> [...]

Applied to

   broonie/sound.git for-next

Thanks!

[1/1] ASoC: soc-compress: Reposition and add pcm_mutex
      commit: aa9ff6a4955fdba02b54fbc4386db876603703b7

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