[PATCH] ASoC: sti: Fix deadlock via snd_pcm_stop_xrun() call
This is essentially a revert of the commit dc865fb9e7c2 ("ASoC: sti: Use snd_pcm_stop_xrun() helper"), which converted the manual snd_pcm_stop() calls with snd_pcm_stop_xrun().
The commit above introduced a deadlock as snd_pcm_stop_xrun() itself takes the PCM stream lock while the caller already holds it. Since the conversion was done only for consistency reason and the open-call with snd_pcm_stop() to the XRUN state is a correct usage, let's revert the commit back as the fix.
Fixes: dc865fb9e7c2 ("ASoC: sti: Use snd_pcm_stop_xrun() helper") Reported-by: Daniel Palmer daniel@0x0f.com Cc: Arnaud POULIQUEN arnaud.pouliquen@st.com Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20220315091319.3351522-1-daniel@0x0f.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/soc/sti/uniperif_player.c | 6 +++--- sound/soc/sti/uniperif_reader.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/soc/sti/uniperif_player.c b/sound/soc/sti/uniperif_player.c index 2ed92c990b97..dd9013c47664 100644 --- a/sound/soc/sti/uniperif_player.c +++ b/sound/soc/sti/uniperif_player.c @@ -91,7 +91,7 @@ static irqreturn_t uni_player_irq_handler(int irq, void *dev_id) SET_UNIPERIF_ITM_BCLR_FIFO_ERROR(player);
/* Stop the player */ - snd_pcm_stop_xrun(player->substream); + snd_pcm_stop(player->substream, SNDRV_PCM_STATE_XRUN); }
ret = IRQ_HANDLED; @@ -105,7 +105,7 @@ static irqreturn_t uni_player_irq_handler(int irq, void *dev_id) SET_UNIPERIF_ITM_BCLR_DMA_ERROR(player);
/* Stop the player */ - snd_pcm_stop_xrun(player->substream); + snd_pcm_stop(player->substream, SNDRV_PCM_STATE_XRUN);
ret = IRQ_HANDLED; } @@ -138,7 +138,7 @@ static irqreturn_t uni_player_irq_handler(int irq, void *dev_id) dev_err(player->dev, "Underflow recovery failed\n");
/* Stop the player */ - snd_pcm_stop_xrun(player->substream); + snd_pcm_stop(player->substream, SNDRV_PCM_STATE_XRUN);
ret = IRQ_HANDLED; } diff --git a/sound/soc/sti/uniperif_reader.c b/sound/soc/sti/uniperif_reader.c index 136059331211..065c5f0d1f5f 100644 --- a/sound/soc/sti/uniperif_reader.c +++ b/sound/soc/sti/uniperif_reader.c @@ -65,7 +65,7 @@ static irqreturn_t uni_reader_irq_handler(int irq, void *dev_id) if (unlikely(status & UNIPERIF_ITS_FIFO_ERROR_MASK(reader))) { dev_err(reader->dev, "FIFO error detected\n");
- snd_pcm_stop_xrun(reader->substream); + snd_pcm_stop(reader->substream, SNDRV_PCM_STATE_XRUN);
ret = IRQ_HANDLED; }
On 3/15/22 17:41, Takashi Iwai wrote:
This is essentially a revert of the commit dc865fb9e7c2 ("ASoC: sti: Use snd_pcm_stop_xrun() helper"), which converted the manual snd_pcm_stop() calls with snd_pcm_stop_xrun().
The commit above introduced a deadlock as snd_pcm_stop_xrun() itself takes the PCM stream lock while the caller already holds it. Since the conversion was done only for consistency reason and the open-call with snd_pcm_stop() to the XRUN state is a correct usage, let's revert the commit back as the fix.
Fixes: dc865fb9e7c2 ("ASoC: sti: Use snd_pcm_stop_xrun() helper") Reported-by: Daniel Palmer daniel@0x0f.com Cc: Arnaud POULIQUEN arnaud.pouliquen@st.com Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20220315091319.3351522-1-daniel@0x0f.com Signed-off-by: Takashi Iwai tiwai@suse.de
Thanks for the fix!
Reviewed-by: Arnaud Pouliquen arnaud.pouliquen@foss.st.com
Regards, Arnaud
sound/soc/sti/uniperif_player.c | 6 +++--- sound/soc/sti/uniperif_reader.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/soc/sti/uniperif_player.c b/sound/soc/sti/uniperif_player.c index 2ed92c990b97..dd9013c47664 100644 --- a/sound/soc/sti/uniperif_player.c +++ b/sound/soc/sti/uniperif_player.c @@ -91,7 +91,7 @@ static irqreturn_t uni_player_irq_handler(int irq, void *dev_id) SET_UNIPERIF_ITM_BCLR_FIFO_ERROR(player);
/* Stop the player */
snd_pcm_stop_xrun(player->substream);
snd_pcm_stop(player->substream, SNDRV_PCM_STATE_XRUN);
}
ret = IRQ_HANDLED;
@@ -105,7 +105,7 @@ static irqreturn_t uni_player_irq_handler(int irq, void *dev_id) SET_UNIPERIF_ITM_BCLR_DMA_ERROR(player);
/* Stop the player */
snd_pcm_stop_xrun(player->substream);
snd_pcm_stop(player->substream, SNDRV_PCM_STATE_XRUN);
ret = IRQ_HANDLED; }
@@ -138,7 +138,7 @@ static irqreturn_t uni_player_irq_handler(int irq, void *dev_id) dev_err(player->dev, "Underflow recovery failed\n");
/* Stop the player */
snd_pcm_stop_xrun(player->substream);
snd_pcm_stop(player->substream, SNDRV_PCM_STATE_XRUN);
ret = IRQ_HANDLED; }
diff --git a/sound/soc/sti/uniperif_reader.c b/sound/soc/sti/uniperif_reader.c index 136059331211..065c5f0d1f5f 100644 --- a/sound/soc/sti/uniperif_reader.c +++ b/sound/soc/sti/uniperif_reader.c @@ -65,7 +65,7 @@ static irqreturn_t uni_reader_irq_handler(int irq, void *dev_id) if (unlikely(status & UNIPERIF_ITS_FIFO_ERROR_MASK(reader))) { dev_err(reader->dev, "FIFO error detected\n");
snd_pcm_stop_xrun(reader->substream);
snd_pcm_stop(reader->substream, SNDRV_PCM_STATE_XRUN);
ret = IRQ_HANDLED; }
On Tue, 15 Mar 2022 17:41:58 +0100, Takashi Iwai wrote:
This is essentially a revert of the commit dc865fb9e7c2 ("ASoC: sti: Use snd_pcm_stop_xrun() helper"), which converted the manual snd_pcm_stop() calls with snd_pcm_stop_xrun().
The commit above introduced a deadlock as snd_pcm_stop_xrun() itself takes the PCM stream lock while the caller already holds it. Since the conversion was done only for consistency reason and the open-call with snd_pcm_stop() to the XRUN state is a correct usage, let's revert the commit back as the fix.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: sti: Fix deadlock via snd_pcm_stop_xrun() call commit: 455c5653f50e10b4f460ef24e99f0044fbe3401c
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
participants (3)
-
Arnaud POULIQUEN
-
Mark Brown
-
Takashi Iwai