linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: alsa-devel@alsa-project.org
Cc: tiwai@suse.de, broonie@kernel.org, vkoul@kernel.org,
	Sameer Pujar <spujar@nvidia.com>,
	Gyeongtaek Lee <gt82.lee@samsung.com>,
	Peter Ujfalusi <peter.ujfalusi@linux.intel.com>,
	Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
	linux-kernel@vger.kernel.org (open list)
Subject: [RFC PATCH v3 11/13] ASoC: soc-pcm: serialize BE triggers
Date: Wed, 13 Oct 2021 09:30:48 -0500	[thread overview]
Message-ID: <20211013143050.244444-12-pierre-louis.bossart@linux.intel.com> (raw)
In-Reply-To: <20211013143050.244444-1-pierre-louis.bossart@linux.intel.com>

When more than one FE is connected to a BE, e.g. in a mixing use case,
the BE can be triggered multiple times when the FE are opened/started
concurrently. This race condition is problematic in the case of
SoundWire BE dailinks, and this is not desirable in a general
case.

This patch relies on the existing BE PCM lock, which takes atomicity into
account. The locking model assumes that all interactions start with
the FE, so that there is no deadlock between FE and BE locks.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/soc-pcm.c | 49 +++++++++++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 5f2368059e14..d115e9409c14 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -41,6 +41,12 @@ void snd_soc_dpcm_fe_unlock_irq(struct snd_soc_pcm_runtime *fe, int stream)
 }
 EXPORT_SYMBOL_GPL(snd_soc_dpcm_fe_unlock_irq);
 
+#define snd_soc_dpcm_be_lock_irqsave(be, stream, flags) \
+	snd_pcm_stream_lock_irqsave(snd_soc_dpcm_get_substream(be, stream), flags)
+
+#define snd_soc_dpcm_be_unlock_irqrestore(be, stream, flags) \
+	snd_pcm_stream_unlock_irqrestore(snd_soc_dpcm_get_substream(be, stream), flags)
+
 /* can this BE stop and free */
 static int snd_soc_dpcm_can_be_free_stop_locked(struct snd_soc_pcm_runtime *fe,
 						struct snd_soc_pcm_runtime *be, int stream);
@@ -2071,6 +2077,7 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
 {
 	struct snd_soc_pcm_runtime *be;
 	struct snd_soc_dpcm *dpcm;
+	unsigned long flags;
 	int ret = 0;
 
 	for_each_dpcm_be(fe, stream, dpcm) {
@@ -2086,85 +2093,89 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
 		dev_dbg(be->dev, "ASoC: trigger BE %s cmd %d\n",
 			be->dai_link->name, cmd);
 
+		snd_soc_dpcm_be_lock_irqsave(be, stream, flags);
 		switch (cmd) {
 		case SNDRV_PCM_TRIGGER_START:
 			if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_PREPARE) &&
 			    (be->dpcm[stream].state != SND_SOC_DPCM_STATE_STOP) &&
 			    (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED))
-				continue;
+				goto unlock_be;
 
 			ret = soc_pcm_trigger(be_substream, cmd);
 			if (ret)
-				goto end;
+				goto unlock_be;
 
 			be->dpcm[stream].state = SND_SOC_DPCM_STATE_START;
 			break;
 		case SNDRV_PCM_TRIGGER_RESUME:
 			if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_SUSPEND))
-				continue;
+				goto unlock_be;
 
 			ret = soc_pcm_trigger(be_substream, cmd);
 			if (ret)
-				goto end;
+				goto unlock_be;
 
 			be->dpcm[stream].state = SND_SOC_DPCM_STATE_START;
 			break;
 		case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 			if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED))
-				continue;
+				goto unlock_be;
 
 			ret = soc_pcm_trigger(be_substream, cmd);
 			if (ret)
-				goto end;
+				goto unlock_be;
 
 			be->dpcm[stream].state = SND_SOC_DPCM_STATE_START;
 			break;
 		case SNDRV_PCM_TRIGGER_STOP:
 			if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_START) &&
 			    (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED))
-				continue;
+				goto unlock_be;
 
 			if (!snd_soc_dpcm_can_be_free_stop_locked(fe, be, stream))
-				continue;
+				goto unlock_be;
 
 			ret = soc_pcm_trigger(be_substream, cmd);
 			if (ret)
-				goto end;
+				goto unlock_be;
 
 			be->dpcm[stream].state = SND_SOC_DPCM_STATE_STOP;
 			break;
 		case SNDRV_PCM_TRIGGER_SUSPEND:
 			if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START)
-				continue;
+				goto unlock_be;
 
 			if (!snd_soc_dpcm_can_be_free_stop_locked(fe, be, stream))
-				continue;
+				goto unlock_be;
 
 			ret = soc_pcm_trigger(be_substream, cmd);
 			if (ret)
-				goto end;
+				goto unlock_be;
 
 			be->dpcm[stream].state = SND_SOC_DPCM_STATE_SUSPEND;
 			break;
 		case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
 			if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START)
-				continue;
+				goto unlock_be;
 
 			if (!snd_soc_dpcm_can_be_free_stop_locked(fe, be, stream))
-				continue;
+				goto unlock_be;
 
 			ret = soc_pcm_trigger(be_substream, cmd);
 			if (ret)
-				goto end;
+				goto unlock_be;
 
 			be->dpcm[stream].state = SND_SOC_DPCM_STATE_PAUSED;
 			break;
 		}
+unlock_be:
+		snd_soc_dpcm_be_unlock_irqrestore(be, stream, flags);
+		if (ret < 0) {
+			dev_err(fe->dev, "ASoC: %s() failed at %s (%d)\n",
+				__func__, be->dai_link->name, ret);
+			break;
+		}
 	}
-end:
-	if (ret < 0)
-		dev_err(fe->dev, "ASoC: %s() failed at %s (%d)\n",
-			__func__, be->dai_link->name, ret);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(dpcm_be_dai_trigger);
-- 
2.25.1


  parent reply	other threads:[~2021-10-13 14:33 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20211013143050.244444-1-pierre-louis.bossart@linux.intel.com>
2021-10-13 14:30 ` [RFC PATCH v3 01/13] ASoC: soc-pcm: remove snd_soc_dpcm_fe_can_update() Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 02/13] ASoC: soc-pcm: don't export local functions, use static Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 03/13] ASoC: soc-pcm: use proper indentation on 'continue' Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 04/13] ASoC: soc-pcm: introduce snd_soc_dpcm_fe_lock_irq/unlock_irq() Pierre-Louis Bossart
2021-10-15  6:24   ` Sameer Pujar
2021-10-15 12:24     ` Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 05/13] ASoC: soc-pcm: align BE 'atomicity' with that of the FE Pierre-Louis Bossart
2021-10-15  6:24   ` Sameer Pujar
2021-10-15  7:39     ` Takashi Iwai
2021-10-15 11:22       ` Pierre-Louis Bossart
2021-10-15 12:04         ` Pierre-Louis Bossart
2021-10-15 15:38         ` Takashi Iwai
2021-10-15 16:22           ` Pierre-Louis Bossart
2021-10-15 16:56             ` Takashi Iwai
2021-10-15 17:08               ` Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 06/13] ASoC: soc-pcm: remove dpcm spin_lock, use PCM stream lock Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 07/13] ASoC: soc-pcm: protect for_each_dpcm_be() loops Pierre-Louis Bossart
2021-10-15  6:24   ` Sameer Pujar
2021-10-15 11:02     ` Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 08/13] ASoC: soc-compress: " Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 09/13] ASoC: sh: rcar: " Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 10/13] ASoC: fsl: asrc_dma: " Pierre-Louis Bossart
2021-10-13 14:30 ` Pierre-Louis Bossart [this message]
2021-10-13 14:30 ` [RFC PATCH v3 12/13] ASoC: soc-pcm: test refcount before triggering Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 13/13] ASoC: soc-pcm: fix BE handling of PAUSE_RELEASE Pierre-Louis Bossart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211013143050.244444-12-pierre-louis.bossart@linux.intel.com \
    --to=pierre-louis.bossart@linux.intel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=gt82.lee@samsung.com \
    --cc=kuninori.morimoto.gx@renesas.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=perex@perex.cz \
    --cc=peter.ujfalusi@linux.intel.com \
    --cc=spujar@nvidia.com \
    --cc=tiwai@suse.com \
    --cc=tiwai@suse.de \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).