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 v2 3/5] ASoC: soc-pcm: replace dpcm_lock with dpcm_mutex Date: Mon, 4 Oct 2021 17:54:39 -0500 [thread overview] Message-ID: <20211004225441.233375-4-pierre-louis.bossart@linux.intel.com> (raw) In-Reply-To: <20211004225441.233375-1-pierre-louis.bossart@linux.intel.com> It's not clear why a spinlock was used, and even less why the 'dpcm_lock' is used with the irqsave/irqrestore: DPCM functions are typically not used in interrupt context. Move to a mutex which will allow us to sleep for longer periods of time and handle non-atomic triggers. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> --- include/sound/soc.h | 2 +- sound/soc/soc-core.c | 2 +- sound/soc/soc-pcm.c | 30 ++++++++++++------------------ 3 files changed, 14 insertions(+), 20 deletions(-) diff --git a/include/sound/soc.h b/include/sound/soc.h index 8e6dd8a257c5..c13d8ec72d62 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -893,7 +893,7 @@ struct snd_soc_card { struct mutex pcm_mutex; enum snd_soc_pcm_subclass pcm_subclass; - spinlock_t dpcm_lock; + struct mutex dpcm_mutex; /* protect all dpcm states and updates */ int (*probe)(struct snd_soc_card *card); int (*late_probe)(struct snd_soc_card *card); diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index c830e96afba2..e94d43c392e0 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2339,7 +2339,7 @@ int snd_soc_register_card(struct snd_soc_card *card) mutex_init(&card->mutex); mutex_init(&card->dapm_mutex); mutex_init(&card->pcm_mutex); - spin_lock_init(&card->dpcm_lock); + mutex_init(&card->dpcm_mutex); return snd_soc_bind_card(card); } diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 360811f8a18c..af12593b033a 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -85,7 +85,6 @@ static ssize_t dpcm_show_state(struct snd_soc_pcm_runtime *fe, struct snd_pcm_hw_params *params = &fe->dpcm[stream].hw_params; struct snd_soc_dpcm *dpcm; ssize_t offset = 0; - unsigned long flags; /* FE state */ offset += scnprintf(buf + offset, size - offset, @@ -113,7 +112,7 @@ static ssize_t dpcm_show_state(struct snd_soc_pcm_runtime *fe, goto out; } - spin_lock_irqsave(&fe->card->dpcm_lock, flags); + mutex_lock(&fe->card->dpcm_mutex); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; params = &dpcm->hw_params; @@ -134,7 +133,7 @@ static ssize_t dpcm_show_state(struct snd_soc_pcm_runtime *fe, params_channels(params), params_rate(params)); } - spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + mutex_unlock(&fe->card->dpcm_mutex); out: return offset; } @@ -1141,7 +1140,6 @@ static int dpcm_be_connect(struct snd_soc_pcm_runtime *fe, struct snd_soc_pcm_runtime *be, int stream) { struct snd_soc_dpcm *dpcm; - unsigned long flags; /* only add new dpcms */ for_each_dpcm_be(fe, stream, dpcm) { @@ -1157,10 +1155,10 @@ static int dpcm_be_connect(struct snd_soc_pcm_runtime *fe, dpcm->fe = fe; be->dpcm[stream].runtime = fe->dpcm[stream].runtime; dpcm->state = SND_SOC_DPCM_LINK_STATE_NEW; - spin_lock_irqsave(&fe->card->dpcm_lock, flags); + mutex_lock(&fe->card->dpcm_mutex); list_add(&dpcm->list_be, &fe->dpcm[stream].be_clients); list_add(&dpcm->list_fe, &be->dpcm[stream].fe_clients); - spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + mutex_unlock(&fe->card->dpcm_mutex); dev_dbg(fe->dev, "connected new DPCM %s path %s %s %s\n", stream ? "capture" : "playback", fe->dai_link->name, @@ -1203,7 +1201,6 @@ static void dpcm_be_reparent(struct snd_soc_pcm_runtime *fe, void dpcm_be_disconnect(struct snd_soc_pcm_runtime *fe, int stream) { struct snd_soc_dpcm *dpcm, *d; - unsigned long flags; for_each_dpcm_be_safe(fe, stream, dpcm, d) { dev_dbg(fe->dev, "ASoC: BE %s disconnect check for %s\n", @@ -1222,10 +1219,10 @@ void dpcm_be_disconnect(struct snd_soc_pcm_runtime *fe, int stream) dpcm_remove_debugfs_state(dpcm); - spin_lock_irqsave(&fe->card->dpcm_lock, flags); + mutex_lock(&fe->card->dpcm_mutex); list_del(&dpcm->list_be); list_del(&dpcm->list_fe); - spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + mutex_unlock(&fe->card->dpcm_mutex); kfree(dpcm); } } @@ -1441,12 +1438,11 @@ int dpcm_process_paths(struct snd_soc_pcm_runtime *fe, void dpcm_clear_pending_state(struct snd_soc_pcm_runtime *fe, int stream) { struct snd_soc_dpcm *dpcm; - unsigned long flags; - spin_lock_irqsave(&fe->card->dpcm_lock, flags); + mutex_lock(&fe->card->dpcm_mutex); for_each_dpcm_be(fe, stream, dpcm) dpcm_set_be_update_state(dpcm->be, stream, SND_SOC_DPCM_UPDATE_NO); - spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + mutex_unlock(&fe->card->dpcm_mutex); } void dpcm_be_dai_stop(struct snd_soc_pcm_runtime *fe, int stream, @@ -2364,7 +2360,6 @@ static int dpcm_run_update_startup(struct snd_soc_pcm_runtime *fe, int stream) struct snd_soc_dpcm *dpcm; enum snd_soc_dpcm_trigger trigger = fe->dai_link->trigger[stream]; int ret = 0; - unsigned long flags; dev_dbg(fe->dev, "ASoC: runtime %s open on FE %s\n", stream ? "capture" : "playback", fe->dai_link->name); @@ -2433,7 +2428,7 @@ static int dpcm_run_update_startup(struct snd_soc_pcm_runtime *fe, int stream) dpcm_be_dai_shutdown(fe, stream); disconnect: /* disconnect any pending BEs */ - spin_lock_irqsave(&fe->card->dpcm_lock, flags); + mutex_lock(&fe->card->dpcm_mutex); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; @@ -2445,7 +2440,7 @@ static int dpcm_run_update_startup(struct snd_soc_pcm_runtime *fe, int stream) be->dpcm[stream].state == SND_SOC_DPCM_STATE_NEW) dpcm->state = SND_SOC_DPCM_LINK_STATE_FREE; } - spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + mutex_unlock(&fe->card->dpcm_mutex); if (ret < 0) dev_err(fe->dev, "ASoC: %s() failed (%d)\n", __func__, ret); @@ -2845,10 +2840,9 @@ static int snd_soc_dpcm_check_state(struct snd_soc_pcm_runtime *fe, struct snd_soc_dpcm *dpcm; int state; int ret = 1; - unsigned long flags; int i; - spin_lock_irqsave(&fe->card->dpcm_lock, flags); + mutex_lock(&fe->card->dpcm_mutex); for_each_dpcm_fe(be, stream, dpcm) { if (dpcm->fe == fe) @@ -2862,7 +2856,7 @@ static int snd_soc_dpcm_check_state(struct snd_soc_pcm_runtime *fe, } } } - spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + mutex_unlock(&fe->card->dpcm_mutex); /* it's safe to do this BE DAI */ return ret; -- 2.25.1
WARNING: multiple messages have this Message-ID (diff)
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> To: alsa-devel@alsa-project.org Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>, tiwai@suse.de, open list <linux-kernel@vger.kernel.org>, Sameer Pujar <spujar@nvidia.com>, Takashi Iwai <tiwai@suse.com>, Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>, Liam Girdwood <lgirdwood@gmail.com>, vkoul@kernel.org, broonie@kernel.org, Gyeongtaek Lee <gt82.lee@samsung.com>, Peter Ujfalusi <peter.ujfalusi@linux.intel.com> Subject: [RFC PATCH v2 3/5] ASoC: soc-pcm: replace dpcm_lock with dpcm_mutex Date: Mon, 4 Oct 2021 17:54:39 -0500 [thread overview] Message-ID: <20211004225441.233375-4-pierre-louis.bossart@linux.intel.com> (raw) In-Reply-To: <20211004225441.233375-1-pierre-louis.bossart@linux.intel.com> It's not clear why a spinlock was used, and even less why the 'dpcm_lock' is used with the irqsave/irqrestore: DPCM functions are typically not used in interrupt context. Move to a mutex which will allow us to sleep for longer periods of time and handle non-atomic triggers. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> --- include/sound/soc.h | 2 +- sound/soc/soc-core.c | 2 +- sound/soc/soc-pcm.c | 30 ++++++++++++------------------ 3 files changed, 14 insertions(+), 20 deletions(-) diff --git a/include/sound/soc.h b/include/sound/soc.h index 8e6dd8a257c5..c13d8ec72d62 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -893,7 +893,7 @@ struct snd_soc_card { struct mutex pcm_mutex; enum snd_soc_pcm_subclass pcm_subclass; - spinlock_t dpcm_lock; + struct mutex dpcm_mutex; /* protect all dpcm states and updates */ int (*probe)(struct snd_soc_card *card); int (*late_probe)(struct snd_soc_card *card); diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index c830e96afba2..e94d43c392e0 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2339,7 +2339,7 @@ int snd_soc_register_card(struct snd_soc_card *card) mutex_init(&card->mutex); mutex_init(&card->dapm_mutex); mutex_init(&card->pcm_mutex); - spin_lock_init(&card->dpcm_lock); + mutex_init(&card->dpcm_mutex); return snd_soc_bind_card(card); } diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 360811f8a18c..af12593b033a 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -85,7 +85,6 @@ static ssize_t dpcm_show_state(struct snd_soc_pcm_runtime *fe, struct snd_pcm_hw_params *params = &fe->dpcm[stream].hw_params; struct snd_soc_dpcm *dpcm; ssize_t offset = 0; - unsigned long flags; /* FE state */ offset += scnprintf(buf + offset, size - offset, @@ -113,7 +112,7 @@ static ssize_t dpcm_show_state(struct snd_soc_pcm_runtime *fe, goto out; } - spin_lock_irqsave(&fe->card->dpcm_lock, flags); + mutex_lock(&fe->card->dpcm_mutex); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; params = &dpcm->hw_params; @@ -134,7 +133,7 @@ static ssize_t dpcm_show_state(struct snd_soc_pcm_runtime *fe, params_channels(params), params_rate(params)); } - spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + mutex_unlock(&fe->card->dpcm_mutex); out: return offset; } @@ -1141,7 +1140,6 @@ static int dpcm_be_connect(struct snd_soc_pcm_runtime *fe, struct snd_soc_pcm_runtime *be, int stream) { struct snd_soc_dpcm *dpcm; - unsigned long flags; /* only add new dpcms */ for_each_dpcm_be(fe, stream, dpcm) { @@ -1157,10 +1155,10 @@ static int dpcm_be_connect(struct snd_soc_pcm_runtime *fe, dpcm->fe = fe; be->dpcm[stream].runtime = fe->dpcm[stream].runtime; dpcm->state = SND_SOC_DPCM_LINK_STATE_NEW; - spin_lock_irqsave(&fe->card->dpcm_lock, flags); + mutex_lock(&fe->card->dpcm_mutex); list_add(&dpcm->list_be, &fe->dpcm[stream].be_clients); list_add(&dpcm->list_fe, &be->dpcm[stream].fe_clients); - spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + mutex_unlock(&fe->card->dpcm_mutex); dev_dbg(fe->dev, "connected new DPCM %s path %s %s %s\n", stream ? "capture" : "playback", fe->dai_link->name, @@ -1203,7 +1201,6 @@ static void dpcm_be_reparent(struct snd_soc_pcm_runtime *fe, void dpcm_be_disconnect(struct snd_soc_pcm_runtime *fe, int stream) { struct snd_soc_dpcm *dpcm, *d; - unsigned long flags; for_each_dpcm_be_safe(fe, stream, dpcm, d) { dev_dbg(fe->dev, "ASoC: BE %s disconnect check for %s\n", @@ -1222,10 +1219,10 @@ void dpcm_be_disconnect(struct snd_soc_pcm_runtime *fe, int stream) dpcm_remove_debugfs_state(dpcm); - spin_lock_irqsave(&fe->card->dpcm_lock, flags); + mutex_lock(&fe->card->dpcm_mutex); list_del(&dpcm->list_be); list_del(&dpcm->list_fe); - spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + mutex_unlock(&fe->card->dpcm_mutex); kfree(dpcm); } } @@ -1441,12 +1438,11 @@ int dpcm_process_paths(struct snd_soc_pcm_runtime *fe, void dpcm_clear_pending_state(struct snd_soc_pcm_runtime *fe, int stream) { struct snd_soc_dpcm *dpcm; - unsigned long flags; - spin_lock_irqsave(&fe->card->dpcm_lock, flags); + mutex_lock(&fe->card->dpcm_mutex); for_each_dpcm_be(fe, stream, dpcm) dpcm_set_be_update_state(dpcm->be, stream, SND_SOC_DPCM_UPDATE_NO); - spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + mutex_unlock(&fe->card->dpcm_mutex); } void dpcm_be_dai_stop(struct snd_soc_pcm_runtime *fe, int stream, @@ -2364,7 +2360,6 @@ static int dpcm_run_update_startup(struct snd_soc_pcm_runtime *fe, int stream) struct snd_soc_dpcm *dpcm; enum snd_soc_dpcm_trigger trigger = fe->dai_link->trigger[stream]; int ret = 0; - unsigned long flags; dev_dbg(fe->dev, "ASoC: runtime %s open on FE %s\n", stream ? "capture" : "playback", fe->dai_link->name); @@ -2433,7 +2428,7 @@ static int dpcm_run_update_startup(struct snd_soc_pcm_runtime *fe, int stream) dpcm_be_dai_shutdown(fe, stream); disconnect: /* disconnect any pending BEs */ - spin_lock_irqsave(&fe->card->dpcm_lock, flags); + mutex_lock(&fe->card->dpcm_mutex); for_each_dpcm_be(fe, stream, dpcm) { struct snd_soc_pcm_runtime *be = dpcm->be; @@ -2445,7 +2440,7 @@ static int dpcm_run_update_startup(struct snd_soc_pcm_runtime *fe, int stream) be->dpcm[stream].state == SND_SOC_DPCM_STATE_NEW) dpcm->state = SND_SOC_DPCM_LINK_STATE_FREE; } - spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + mutex_unlock(&fe->card->dpcm_mutex); if (ret < 0) dev_err(fe->dev, "ASoC: %s() failed (%d)\n", __func__, ret); @@ -2845,10 +2840,9 @@ static int snd_soc_dpcm_check_state(struct snd_soc_pcm_runtime *fe, struct snd_soc_dpcm *dpcm; int state; int ret = 1; - unsigned long flags; int i; - spin_lock_irqsave(&fe->card->dpcm_lock, flags); + mutex_lock(&fe->card->dpcm_mutex); for_each_dpcm_fe(be, stream, dpcm) { if (dpcm->fe == fe) @@ -2862,7 +2856,7 @@ static int snd_soc_dpcm_check_state(struct snd_soc_pcm_runtime *fe, } } } - spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); + mutex_unlock(&fe->card->dpcm_mutex); /* it's safe to do this BE DAI */ return ret; -- 2.25.1
next prev parent reply other threads:[~2021-10-04 22:55 UTC|newest] Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-10-04 22:54 [RFC PATCH v2 0/5] ASoC: soc-pcm: fix trigger race conditions with shared BE Pierre-Louis Bossart 2021-10-04 22:54 ` [RFC PATCH v2 1/5] ASoC: soc-pcm: remove snd_soc_dpcm_fe_can_update() Pierre-Louis Bossart 2021-10-04 22:54 ` Pierre-Louis Bossart 2021-10-04 22:54 ` [RFC PATCH v2 2/5] ASoC: soc-pcm: don't export local functions, use static Pierre-Louis Bossart 2021-10-04 22:54 ` Pierre-Louis Bossart 2021-10-04 22:54 ` Pierre-Louis Bossart [this message] 2021-10-04 22:54 ` [RFC PATCH v2 3/5] ASoC: soc-pcm: replace dpcm_lock with dpcm_mutex Pierre-Louis Bossart 2021-10-04 22:54 ` [RFC PATCH v2 4/5] ASoC: soc-pcm: protect for_each_dpcm_be() loops " Pierre-Louis Bossart 2021-10-04 22:54 ` Pierre-Louis Bossart 2021-10-04 22:54 ` [RFC PATCH v2 5/5] ASoC: soc-pcm: test refcount before triggering Pierre-Louis Bossart 2021-10-04 22:54 ` Pierre-Louis Bossart 2021-10-05 6:36 ` [RFC PATCH v2 0/5] ASoC: soc-pcm: fix trigger race conditions with shared BE Sameer Pujar 2021-10-05 13:17 ` Pierre-Louis Bossart 2021-10-06 14:22 ` Sameer Pujar 2021-10-06 19:47 ` Pierre-Louis Bossart 2021-10-07 11:06 ` Takashi Iwai 2021-10-07 13:31 ` Pierre-Louis Bossart 2021-10-07 14:59 ` Takashi Iwai 2021-10-07 15:24 ` Pierre-Louis Bossart 2021-10-07 15:44 ` Takashi Iwai 2021-10-07 18:13 ` Pierre-Louis Bossart 2021-10-07 21:11 ` Takashi Iwai 2021-10-07 21:27 ` Pierre-Louis Bossart 2021-10-08 6:13 ` Takashi Iwai 2021-10-08 14:41 ` Pierre-Louis Bossart 2021-10-08 14:51 ` Takashi Iwai 2021-10-08 15:41 ` Pierre-Louis Bossart 2021-10-08 19:09 ` Pierre-Louis Bossart 2021-10-11 20:06 ` Pierre-Louis Bossart 2021-10-12 6:34 ` Takashi Iwai 2021-10-12 10:42 ` Takashi Iwai 2021-10-12 13:45 ` Pierre-Louis Bossart 2021-10-12 15:07 ` Takashi Iwai
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=20211004225441.233375-4-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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.