From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.9 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,UNWANTED_LANGUAGE_BODY, URIBL_BLOCKED,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CB80BC4338F for ; Tue, 17 Aug 2021 16:41:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A930A60F11 for ; Tue, 17 Aug 2021 16:41:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231163AbhHQQlu (ORCPT ); Tue, 17 Aug 2021 12:41:50 -0400 Received: from mga06.intel.com ([134.134.136.31]:38258 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230354AbhHQQlt (ORCPT ); Tue, 17 Aug 2021 12:41:49 -0400 X-IronPort-AV: E=McAfee;i="6200,9189,10079"; a="277170637" X-IronPort-AV: E=Sophos;i="5.84,329,1620716400"; d="scan'208";a="277170637" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Aug 2021 09:41:16 -0700 X-IronPort-AV: E=Sophos;i="5.84,329,1620716400"; d="scan'208";a="449342716" Received: from aaadelek-mobl3.amr.corp.intel.com (HELO pbossart-mobl3.intel.com) ([10.212.12.89]) by fmsmga007-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Aug 2021 09:41:14 -0700 From: Pierre-Louis Bossart To: alsa-devel@alsa-project.org Cc: tiwai@suse.de, broonie@kernel.org, vkoul@kernel.org, liam.r.girdwood@linux.intel.com, Kuninori Morimoto , Pierre-Louis Bossart , Liam Girdwood , Jaroslav Kysela , Takashi Iwai , linux-kernel@vger.kernel.org (open list) Subject: [RFC PATCH 1/2] ASoC: soc-pcm: protect BE dailink state changes in trigger Date: Tue, 17 Aug 2021 11:40:53 -0500 Message-Id: <20210817164054.250028-2-pierre-louis.bossart@linux.intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210817164054.250028-1-pierre-louis.bossart@linux.intel.com> References: <20210817164054.250028-1-pierre-louis.bossart@linux.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 --- 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 48f71bb81a2f..0717f39d2eec 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1999,6 +1999,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; @@ -2015,76 +2017,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.25.1