From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Sasha Levin <sashal@kernel.org>,
alsa-devel@alsa-project.org, Mark Brown <broonie@kernel.org>,
Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Subject: [PATCH AUTOSEL 5.14 224/252] ASoC: soc-pcm: protect BE dailink state changes in trigger
Date: Thu, 9 Sep 2021 07:40:38 -0400 [thread overview]
Message-ID: <20210909114106.141462-224-sashal@kernel.org> (raw)
In-Reply-To: <20210909114106.141462-1-sashal@kernel.org>
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
[ Upstream commit 0c75fc7193387776c10f7c7b440d93496e3d5e21 ]
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. The code carefully checks when the BE can be stopped or
hw_free'ed, but the trigger code does not use any mutual exclusion.
Fix by using the same spinlock already used to check FE states, and
set the state before the trigger. In case of errors, the initial
state will be restored.
This patch does not change how the triggers are handled, it only makes
sure the states are handled in critical sections.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Message-Id: <20210817164054.250028-2-pierre-louis.bossart@linux.intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
sound/soc/soc-pcm.c | 103 ++++++++++++++++++++++++++++++++++++--------
1 file changed, 85 insertions(+), 18 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index d1c570ca21ea..b944f56a469a 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -2001,6 +2001,8 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
struct snd_soc_pcm_runtime *be;
struct snd_soc_dpcm *dpcm;
int ret = 0;
+ unsigned long flags;
+ enum snd_soc_dpcm_state state;
for_each_dpcm_be(fe, stream, dpcm) {
struct snd_pcm_substream *be_substream;
@@ -2017,76 +2019,141 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
+ spin_lock_irqsave(&fe->card->dpcm_lock, flags);
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))
+ (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED)) {
+ spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
continue;
+ }
+ state = be->dpcm[stream].state;
+ be->dpcm[stream].state = SND_SOC_DPCM_STATE_START;
+ spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
ret = soc_pcm_trigger(be_substream, cmd);
- if (ret)
+ if (ret) {
+ spin_lock_irqsave(&fe->card->dpcm_lock, flags);
+ be->dpcm[stream].state = state;
+ spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
goto end;
+ }
- 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))
+ spin_lock_irqsave(&fe->card->dpcm_lock, flags);
+ if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_SUSPEND) {
+ spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
continue;
+ }
+
+ state = be->dpcm[stream].state;
+ be->dpcm[stream].state = SND_SOC_DPCM_STATE_START;
+ spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
ret = soc_pcm_trigger(be_substream, cmd);
- if (ret)
+ if (ret) {
+ spin_lock_irqsave(&fe->card->dpcm_lock, flags);
+ be->dpcm[stream].state = state;
+ spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
goto end;
+ }
- 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))
+ spin_lock_irqsave(&fe->card->dpcm_lock, flags);
+ if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED) {
+ spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
continue;
+ }
+
+ state = be->dpcm[stream].state;
+ be->dpcm[stream].state = SND_SOC_DPCM_STATE_START;
+ spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
ret = soc_pcm_trigger(be_substream, cmd);
- if (ret)
+ if (ret) {
+ spin_lock_irqsave(&fe->card->dpcm_lock, flags);
+ be->dpcm[stream].state = state;
+ spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
goto end;
+ }
- be->dpcm[stream].state = SND_SOC_DPCM_STATE_START;
break;
case SNDRV_PCM_TRIGGER_STOP:
+ spin_lock_irqsave(&fe->card->dpcm_lock, flags);
if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_START) &&
- (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED))
+ (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED)) {
+ spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
continue;
+ }
+ spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream))
continue;
+ spin_lock_irqsave(&fe->card->dpcm_lock, flags);
+ state = be->dpcm[stream].state;
+ be->dpcm[stream].state = SND_SOC_DPCM_STATE_STOP;
+ spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
+
ret = soc_pcm_trigger(be_substream, cmd);
- if (ret)
+ if (ret) {
+ spin_lock_irqsave(&fe->card->dpcm_lock, flags);
+ be->dpcm[stream].state = state;
+ spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
goto end;
+ }
- 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)
+ spin_lock_irqsave(&fe->card->dpcm_lock, flags);
+ if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START) {
+ spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
continue;
+ }
+ spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream))
continue;
+ spin_lock_irqsave(&fe->card->dpcm_lock, flags);
+ state = be->dpcm[stream].state;
+ be->dpcm[stream].state = SND_SOC_DPCM_STATE_STOP;
+ spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
+
ret = soc_pcm_trigger(be_substream, cmd);
- if (ret)
+ if (ret) {
+ spin_lock_irqsave(&fe->card->dpcm_lock, flags);
+ be->dpcm[stream].state = state;
+ spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
goto end;
+ }
- 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)
+ spin_lock_irqsave(&fe->card->dpcm_lock, flags);
+ if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START) {
+ spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
continue;
+ }
+ spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
if (!snd_soc_dpcm_can_be_free_stop(fe, be, stream))
continue;
+ spin_lock_irqsave(&fe->card->dpcm_lock, flags);
+ state = be->dpcm[stream].state;
+ be->dpcm[stream].state = SND_SOC_DPCM_STATE_PAUSED;
+ spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
+
ret = soc_pcm_trigger(be_substream, cmd);
- if (ret)
+ if (ret) {
+ spin_lock_irqsave(&fe->card->dpcm_lock, flags);
+ be->dpcm[stream].state = state;
+ spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
goto end;
+ }
- be->dpcm[stream].state = SND_SOC_DPCM_STATE_PAUSED;
break;
}
}
--
2.30.2
prev parent reply other threads:[~2021-09-09 11:48 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20210909114106.141462-1-sashal@kernel.org>
2021-09-09 11:37 ` [PATCH AUTOSEL 5.14 012/252] ASoC: ti: davinci-mcasp: Fix DIT mode support Sasha Levin
2021-09-09 11:37 ` [PATCH AUTOSEL 5.14 013/252] ASoC: atmel: ATMEL drivers don't need HAS_DMA Sasha Levin
2021-09-09 11:38 ` [PATCH AUTOSEL 5.14 100/252] ASoC: Intel: bytcr_rt5640: Move "Platform Clock" routes to the maps for the matching in-/output Sasha Levin
2021-09-09 11:38 ` [PATCH AUTOSEL 5.14 102/252] ASoC: Intel: update sof_pcm512x quirks Sasha Levin
2021-09-09 11:39 ` [PATCH AUTOSEL 5.14 132/252] ALSA: pci: cs46xx: Fix set up buffer type properly Sasha Levin
2021-09-09 11:53 ` Takashi Iwai
2021-09-09 11:39 ` [PATCH AUTOSEL 5.14 166/252] ALSA: hda: Drop workaround for a hang at shutdown again Sasha Levin
2021-09-09 11:50 ` Takashi Iwai
2021-09-09 11:39 ` [PATCH AUTOSEL 5.14 179/252] ASoC: rsnd: adg: clearly handle clock error / NULL case Sasha Levin
2021-09-09 11:40 ` [PATCH AUTOSEL 5.14 198/252] ASoC: intel: atom: Revert PCM buffer address setup workaround again Sasha Levin
2021-09-09 11:51 ` Takashi Iwai
2021-09-09 11:40 ` [PATCH AUTOSEL 5.14 199/252] soundwire: intel: fix potential race condition during power down Sasha Levin
2021-09-09 11:40 ` [PATCH AUTOSEL 5.14 200/252] ASoC: Intel: Skylake: Fix module configuration for KPB and MIXER Sasha Levin
2021-09-09 11:40 ` [PATCH AUTOSEL 5.14 201/252] ASoC: Intel: Skylake: Fix passing loadable flag for module Sasha Levin
2021-09-09 11:40 ` [PATCH AUTOSEL 5.14 222/252] ASoC: rockchip: i2s: Fix regmap_ops hang Sasha Levin
2021-09-09 11:40 ` [PATCH AUTOSEL 5.14 223/252] ASoC: rockchip: i2s: Fixup config for DAIFMT_DSP_A/B Sasha Levin
2021-09-09 11:40 ` Sasha Levin [this message]
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=20210909114106.141462-224-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pierre-louis.bossart@linux.intel.com \
--cc=stable@vger.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).