[PATCH] ALSA: pcm: accept the OPEN state for snd_pcm_stop()

* [PATCH] ALSA: pcm: accept the OPEN state for snd_pcm_stop()
@ 2022-01-13 11:31 Jaroslav Kysela
  2022-01-13 12:56 ` Takashi Iwai
  2022-01-13 17:10   ` kernel test robot
  0 siblings, 2 replies; 12+ messages in thread
From: Jaroslav Kysela @ 2022-01-13 11:31 UTC (permalink / raw)
  To: ALSA development; +Cc: Takashi Iwai, Pavel Hofman

It may be useful to completely invalidate streaming under some
circumstances like the USB gadget detach. This case is a bit different
than the complete disconnection. The applications should be notified
that the PCM streaming is no longer available, but the recovery may be
expected.

This patch adds support for SNDRV_PCM_STATE_OPEN passed
to snd_pcm_stop() function. Only the hw_free ioctl is allowed to free
the buffers in this state for a full recovery operation or the PCM file
handle must be closed.

In the OPEN state, ioctls return EBADFD error (with the added hw_free
exception above). The applications which are not aware about this new
state transition (and recovery) will fail with an error. This operation
is expected.

Link: https://lore.kernel.org/alsa-devel/4e17c99b-1d8a-8ebe-972c-9b1299952ff8@ivitera.com/
Cc: Pavel Hofman <pavel.hofman@ivitera.com>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
---
 include/sound/pcm.h     |  1 +
 sound/core/pcm.c        |  1 +
 sound/core/pcm_native.c | 12 +++++++++++-
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 33451f8ff755..4de1c2c91109 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -467,6 +467,7 @@ struct snd_pcm_substream {
 	/* -- assigned files -- */
 	int ref_count;
 	atomic_t mmap_count;
+	atomic_t queued_hw_free;
 	unsigned int f_flags;
 	void (*pcm_release)(struct snd_pcm_substream *);
 	struct pid *pid;
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index 6fd3677685d7..8dc7e99b2113 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -694,6 +694,7 @@ int snd_pcm_new_stream(struct snd_pcm *pcm, int stream, int substream_count)
 		snd_pcm_group_init(&substream->self_group);
 		list_add_tail(&substream->link_list, &substream->self_group.substreams);
 		atomic_set(&substream->mmap_count, 0);
+		atomic_set(&substream->queued_hw_free, 0);
 		prev = substream;
 	}
 	return 0;
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 621883e71194..118ad3f26f4a 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -686,10 +686,13 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
 	snd_pcm_stream_lock_irq(substream);
 	switch (runtime->status->state) {
 	case SNDRV_PCM_STATE_OPEN:
+		if (atomic_read(&substream->queued_hw_free))
+			goto __badfd;
 	case SNDRV_PCM_STATE_SETUP:
 	case SNDRV_PCM_STATE_PREPARED:
 		break;
 	default:
+__badfd:
 		snd_pcm_stream_unlock_irq(substream);
 		return -EBADFD;
 	}
@@ -829,6 +832,7 @@ static int do_hw_free(struct snd_pcm_substream *substream)
 		result = substream->ops->hw_free(substream);
 	if (substream->managed_buffer_alloc)
 		snd_pcm_lib_free_pages(substream);
+	atomic_set(&substream->queued_hw_free, 0);
 	return result;
 }
 
@@ -1454,6 +1458,8 @@ static void snd_pcm_post_stop(struct snd_pcm_substream *substream,
 	}
 	wake_up(&runtime->sleep);
 	wake_up(&runtime->tsleep);
+	if (state == SNDRV_PCM_STATE_OPEN)
+		atomic_set(&substream->queued_hw_free, 1);
 }
 
 static const struct action_ops snd_pcm_action_stop = {
@@ -1469,6 +1475,9 @@ static const struct action_ops snd_pcm_action_stop = {
  *
  * The state of each stream is then changed to the given state unconditionally.
  *
+ * If the requested state is OPEN, the stream is invalidated and
+ * the application must call hw_free to recover the operation.
+ *
  * Return: Zero if successful, or a negative error code.
  */
 int snd_pcm_stop(struct snd_pcm_substream *substream, snd_pcm_state_t state)
@@ -2637,7 +2646,8 @@ void snd_pcm_release_substream(struct snd_pcm_substream *substream)
 
 	snd_pcm_drop(substream);
 	if (substream->hw_opened) {
-		if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
+		if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN ||
+		    atomic_read(&substream->queued_hw_free))
 			do_hw_free(substream);
 		substream->ops->close(substream);
 		substream->hw_opened = 0;
-- 
2.33.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread