Alsa-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: alsa-devel@alsa-project.org
Subject: [alsa-devel] [PATCH 6/8] ALSA: pcm: Add the support for sync-stop operation
Date: Sun, 17 Nov 2019 09:53:06 +0100
Message-ID: <20191117085308.23915-7-tiwai@suse.de> (raw)
In-Reply-To: <20191117085308.23915-1-tiwai@suse.de>

The standard programming model of a PCM sound driver is to process
snd_pcm_period_elapsed() from an interrupt handler.  When a running
stream is stopped, PCM core calls the trigger-STOP PCM ops, sets the
stream state to SETUP, and moves on to the next step.  This is
performed in an atomic manner -- this could be called from the interrupt
context, after all.

The problem is that, if the stream goes further and reaches to the
CLOSE state immediately, the stream might be still being processed in
snd_pcm_period_elapsed() in the interrupt context, and hits a NULL
dereference.  Such a crash happens because of the atomic operation,
and we can't wait until the stream-stop finishes.

For addressing such a problem, this commit adds a new PCM ops,
sync_stop.  This gets called at the appropriate places that need a
sync with the stream-stop, i.e. at hw_params, prepare and hw_free.

Some drivers already have a similar mechanism implemented locally, and
we'll refactor the code later.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/pcm.h     |  2 ++
 sound/core/pcm_native.c | 15 +++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 25563317782c..8a89fa6fdd5e 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -59,6 +59,7 @@ struct snd_pcm_ops {
 	int (*hw_free)(struct snd_pcm_substream *substream);
 	int (*prepare)(struct snd_pcm_substream *substream);
 	int (*trigger)(struct snd_pcm_substream *substream, int cmd);
+	int (*sync_stop)(struct snd_pcm_substream *substream);
 	snd_pcm_uframes_t (*pointer)(struct snd_pcm_substream *substream);
 	int (*get_time_info)(struct snd_pcm_substream *substream,
 			struct timespec *system_ts, struct timespec *audio_ts,
@@ -395,6 +396,7 @@ struct snd_pcm_runtime {
 	wait_queue_head_t sleep;	/* poll sleep */
 	wait_queue_head_t tsleep;	/* transfer sleep */
 	struct fasync_struct *fasync;
+	bool stop_operating;		/* sync_stop will be called */
 
 	/* -- private section -- */
 	void *private_data;
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 704ea75199e4..163d621ff238 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -568,6 +568,15 @@ static inline void snd_pcm_timer_notify(struct snd_pcm_substream *substream,
 #endif
 }
 
+static void snd_pcm_sync_stop(struct snd_pcm_substream *substream)
+{
+	if (substream->runtime->stop_operating) {
+		substream->runtime->stop_operating = false;
+		if (substream->ops->sync_stop)
+			substream->ops->sync_stop(substream);
+	}
+}
+
 /**
  * snd_pcm_hw_param_choose - choose a configuration defined by @params
  * @pcm: PCM instance
@@ -660,6 +669,8 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
 		if (atomic_read(&substream->mmap_count))
 			return -EBADFD;
 
+	snd_pcm_sync_stop(substream);
+
 	params->rmask = ~0U;
 	err = snd_pcm_hw_refine(substream, params);
 	if (err < 0)
@@ -788,6 +799,7 @@ static int snd_pcm_hw_free(struct snd_pcm_substream *substream)
 	snd_pcm_stream_unlock_irq(substream);
 	if (atomic_read(&substream->mmap_count))
 		return -EBADFD;
+	snd_pcm_sync_stop(substream);
 	if (substream->ops->hw_free)
 		result = substream->ops->hw_free(substream);
 	if (substream->managed_buffer_alloc)
@@ -1313,6 +1325,7 @@ static void snd_pcm_post_stop(struct snd_pcm_substream *substream, int state)
 		runtime->status->state = state;
 		snd_pcm_timer_notify(substream, SNDRV_TIMER_EVENT_MSTOP);
 	}
+	runtime->stop_operating = true;
 	wake_up(&runtime->sleep);
 	wake_up(&runtime->tsleep);
 }
@@ -1589,6 +1602,7 @@ static void snd_pcm_post_resume(struct snd_pcm_substream *substream, int state)
 	snd_pcm_trigger_tstamp(substream);
 	runtime->status->state = runtime->status->suspended_state;
 	snd_pcm_timer_notify(substream, SNDRV_TIMER_EVENT_MRESUME);
+	snd_pcm_sync_stop(substream);
 }
 
 static const struct action_ops snd_pcm_action_resume = {
@@ -1709,6 +1723,7 @@ static int snd_pcm_pre_prepare(struct snd_pcm_substream *substream,
 static int snd_pcm_do_prepare(struct snd_pcm_substream *substream, int state)
 {
 	int err;
+	snd_pcm_sync_stop(substream);
 	err = substream->ops->prepare(substream);
 	if (err < 0)
 		return err;
-- 
2.16.4

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

  parent reply index

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-17  8:53 [alsa-devel] [PATCH 0/8] ALSA: pcm: API cleanups and extensions Takashi Iwai
2019-11-17  8:53 ` [alsa-devel] [PATCH 1/8] ALSA: pcm: Introduce managed buffer allocation mode Takashi Iwai
2019-11-18 16:24   ` Pierre-Louis Bossart
2019-11-18 18:46     ` Takashi Iwai
2019-11-17  8:53 ` [alsa-devel] [PATCH 2/8] ALSA: docs: Update for " Takashi Iwai
2019-11-17  8:53 ` [alsa-devel] [PATCH 3/8] ALSA: pcm: Allow NULL ioctl ops Takashi Iwai
2019-11-17  8:53 ` [alsa-devel] [PATCH 4/8] ALSA: docs: Update document about the default PCM " Takashi Iwai
2019-11-17  8:53 ` [alsa-devel] [PATCH 5/8] ALSA: pcm: Move PCM_RUNTIME_CHECK() macro into local header Takashi Iwai
2019-11-17  9:42   ` kbuild test robot
2019-11-17 10:05     ` Takashi Iwai
2019-11-17 10:28   ` kbuild test robot
2019-11-17  8:53 ` Takashi Iwai [this message]
2019-11-18 16:33   ` [alsa-devel] [PATCH 6/8] ALSA: pcm: Add the support for sync-stop operation Pierre-Louis Bossart
2019-11-18 18:47     ` Takashi Iwai
2019-11-17  8:53 ` [alsa-devel] [PATCH 7/8] ALSA: pcm: Add card sync_irq field Takashi Iwai
2019-11-18 16:38   ` Pierre-Louis Bossart
2019-11-18 18:52     ` Takashi Iwai
2019-11-18 19:20       ` Sridharan, Ranjani
2019-11-18 19:49         ` Takashi Iwai
2019-11-18 19:55           ` Sridharan, Ranjani
2019-11-18 20:40             ` Takashi Iwai
2019-11-18 23:47               ` Ranjani Sridharan
2019-11-19  6:44                 ` Takashi Iwai
2019-11-19  7:40                   ` Ranjani Sridharan
2019-11-19  8:24                     ` Takashi Iwai
2019-11-19  9:39                       ` Takashi Iwai
2019-11-19 16:36                       ` Ranjani Sridharan
2019-11-19 21:27                         ` Takashi Iwai
2019-11-19 21:43                           ` Sridharan, Ranjani
2019-11-21 19:22                             ` Sridharan, Ranjani
2019-11-21 20:34                               ` Takashi Iwai
2019-11-21 20:46                                 ` Sridharan, Ranjani
2019-11-21 21:13                                   ` Takashi Iwai
2019-11-21 21:17                                     ` Sridharan, Ranjani
2019-11-21 21:28                                       ` Takashi Iwai
2019-11-21 21:45                                         ` Sridharan, Ranjani
2019-11-22  4:08                                     ` Jie, Yang
2019-11-17  8:53 ` [alsa-devel] [PATCH 8/8] ALSA: docs: Update about the new PCM sync_stop ops Takashi Iwai

Reply instructions:

You may reply publically 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=20191117085308.23915-7-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.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

Alsa-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/alsa-devel/0 alsa-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 alsa-devel alsa-devel/ https://lore.kernel.org/alsa-devel \
		alsa-devel@alsa-project.org
	public-inbox-index alsa-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.alsa-project.alsa-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git