All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH v4 0/2] ALSA: pcm: preserve capture tail during drain
@ 2026-10-15  0:35 Joonho Lee
  2026-10-15  0:36 ` [RFC PATCH v4 1/2] ALSA: pcm: allow capture reads while draining Joonho Lee
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Joonho Lee @ 2026-10-15  0:35 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Takashi Iwai, Jaroslav Kysela, linux-sound, linux-kernel,
	linux-api

This v4 replaces both patches in the core capture-drain v3 series. It
follows Takashi Iwai's suggestion to refresh linked pointers in a helper
before drain-init and fixes three tail-handling problems in patch 1.

Patch 1 allows buffered capture reads in DRAINING. When drain wakes a
reader waiting for data, wait_for_avail() returns the remaining frame
count. Capture EOF uses snd_pcm_drain_done(), so a linked playback stream
can continue draining. DELAY and SYNC_PTR(HWSYNC) use the cached final
capture pointer without calling the driver again after capture stops.
Sashiko's review identified these three problems in the earlier patch.

Patch 2 takes snd_pcm_link_rwsem for reading and updates each running
member under its own stream lock. It calls snd_pcm_update_hw_ptr() for
both playback and capture. The helper runs after the existing pause-resume
step, allowing a paused capture stream to receive its final update.
The pause and drain-init actions remain outside the link rwsem.

A pointer error follows the ordinary XRUN path and returns -EPIPE before
drain-init. This restores v2's group-wide error handling. V3 instead put
the failing member in SETUP and could return success from blocking drain
while another member retained its tail. V4 removes v3's notify argument
and the conditional notifications in the pointer update code.

Previous core v3:
https://lore.kernel.org/all/rfc-v3-core-capture-drain-0.cf2e307fcac34b6a@gmail.com/
Takashi's latest suggestion:
https://lore.kernel.org/all/87jynkcn60.wl-tiwai@suse.de/
My implementation and test report:
https://lore.kernel.org/all/core-drain-helper-reply.0c1099dbd0ca4a37@gmail.com/

The separate five-patch virtio-snd lifetime v3 series is unchanged:
https://lore.kernel.org/all/cover.1791881787.git.mynameisjune111@gmail.com/
Apply those five patches to the public base below, then these two patches.
Remove both old core patches before applying v4. Core has no dependency
on the generic control-wait or VZ compatibility RFC. The lifetime stack
is pinned here to reproduce the tested integration. Neither companion
RFC is revised by this series. A future VZ revision will need the new
core prerequisite IDs.

Validation used ARM64 QEMU with KASAN and lockdep:
- 26 linked-stream checks passed across atomic and nonatomic modes,
  including paused drain, first/later member pointer errors, normal XRUN
  handling, tail reads, and unlink/prepare/reopen recovery.
- The three patch-1 failures were reproduced with the helper and the old
  first patch in both locking modes. All six corresponding fixed cases
  passed: a blocked reader received all 64 final frames, capture EOF
  left linked playback in DRAINING, and both position queries succeeded
  without another pointer callback.
- Four additional blocked-reader runs compared baseline and fixed modules
  in both modes with a stronger gate: thread sleep and a pointer callback
  after START. They confirmed the baseline loss and fixed 64-frame read.
- The 10 existing lifecycle and timeout/reset scenarios passed, including
  held STOP/RELEASE, SIGKILL, reset, and subsequent I/O recovery.
- No KASAN, lockdep, or panic diagnostics appeared in the 54 selected
  logs, which include the baseline behavioral-failure comparisons.

The full Image and modules were first built with the new helper and the
old first patch. After fixing the three tail issues, sound/core was
rebuilt with W=1 against that same configuration and symbol versions,
and the corrected PCM module was used for the runs above. The module
build had no warnings or errors. The full build had 517 existing
-Woverride-init warnings in arch/arm64 outside sound/. The exported v4
core source matches the tested source byte for byte.

These tests use synthetic drivers and modified fault/reset backends.
Physical-device and Virtualization.framework microphone testing of this
core revision is still pending. Concurrent LINK, wraparound, and
forward/rewind behavior have not been exercised by this test set.
The updated reproducer and logs are local; the earlier public package
contains an older core revision. There is no new reproducer upload for
this candidate.

One design question remains: the helper releases the link rwsem before
drain-init, and snd_pcm_link() permits another RUNNING stream to join in
that interval. That member could enter drain-init without a refresh by
the helper. Should this change protect the membership across both steps,
or should concurrent linking be handled separately? I have reviewed this
interval in the source but have not reproduced it with a stress test.

Do the restored group-wide XRUN/-EPIPE behavior and cached capture hwsync
semantics look appropriate? Which physical-driver coverage would you
want before this can move beyond RFC?

LLM assistance was used for implementation, reproducer development,
analysis, and documentation. I reviewed the changes and test evidence.
Assisted-by: LLM

Joonho Lee (2):
  ALSA: pcm: allow capture reads while draining
  ALSA: pcm: refresh linked pointers before draining

 sound/core/pcm_lib.c    | 16 ++++++++++------
 sound/core/pcm_native.c | 27 ++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 7 deletions(-)

base-commit: 8d70e364e866bdfca81eee24d0688fac9472cfb9
prerequisite-patch-id: 39fb327a004c966ab90f66a34a64bbdbb3ffb475
prerequisite-patch-id: 7c6bc344656fd839ae0d697ba18d66ac0b47827a
prerequisite-patch-id: 9dd290ac3147fa7b49a475d7df8a6172778f2860
prerequisite-patch-id: f4c24a5e441343a690f4ac2437f181c815bc23d0
prerequisite-patch-id: ab4a5b770247a65272ab7af18a75a147daf81287

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

* [RFC PATCH v4 1/2] ALSA: pcm: allow capture reads while draining
  2026-10-15  0:35 [RFC PATCH v4 0/2] ALSA: pcm: preserve capture tail during drain Joonho Lee
@ 2026-10-15  0:36 ` Joonho Lee
  2026-10-15  0:36 ` [RFC PATCH v4 2/2] ALSA: pcm: refresh linked pointers before draining Joonho Lee
  2026-10-15 15:54 ` [RFC PATCH v4 0/2] ALSA: pcm: preserve capture tail during drain Takashi Iwai
  2 siblings, 0 replies; 4+ messages in thread
From: Joonho Lee @ 2026-10-15  0:36 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Takashi Iwai, Jaroslav Kysela, linux-sound, linux-kernel,
	linux-api

Capture drain can leave buffered samples in DRAINING, but the transfer
state check rejects reads in that state. Allow capture reads to consume
the remaining frames while preserving the playback state check.

When drain wakes a blocked capture reader, return the actual available
frame count from wait_for_avail(). A zero count would make the transfer
loop discard a nonempty tail. Finish capture EOF with snd_pcm_drain_done()
so the state transition applies to that substream and leaves a linked
playback stream draining its queued data.

Allow capture hwsync to use the cached final pointer in DRAINING. Capture
has already stopped, so DELAY and SYNC_PTR(HWSYNC) can report its remaining
frames without another driver pointer callback. Playback retains its
existing pointer update.

Synthetic atomic and nonatomic tests reproduce dropped tail frames,
linked playback stopping at capture EOF, and rejected position queries.
With this change, a blocked read gets all 64 tail frames, playback stays
DRAINING at capture EOF, and the queries succeed with no extra callback.

Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
 sound/core/pcm_lib.c    | 16 ++++++++++------
 sound/core/pcm_native.c |  2 +-
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index 4d665b4148d7..e8f6a9da4a4c 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -2009,8 +2009,8 @@ static int wait_for_avail(struct snd_pcm_substream *substream,
 		case SNDRV_PCM_STATE_DRAINING:
 			if (is_playback)
 				err = -EPIPE;
-			else 
-				avail = 0; /* indicate draining */
+			else
+				avail = snd_pcm_avail(substream);
 			goto _endloop;
 		case SNDRV_PCM_STATE_OPEN:
 		case SNDRV_PCM_STATE_SETUP:
@@ -2208,13 +2208,17 @@ static int pcm_sanity_check(struct snd_pcm_substream *substream)
 	return 0;
 }
 
-static int pcm_accessible_state(struct snd_pcm_runtime *runtime)
+static int pcm_accessible_state(struct snd_pcm_substream *substream)
 {
+	struct snd_pcm_runtime *runtime = substream->runtime;
+
 	switch (runtime->state) {
 	case SNDRV_PCM_STATE_PREPARED:
 	case SNDRV_PCM_STATE_RUNNING:
 	case SNDRV_PCM_STATE_PAUSED:
 		return 0;
+	case SNDRV_PCM_STATE_DRAINING:
+		return substream->stream == SNDRV_PCM_STREAM_CAPTURE ? 0 : -EBADFD;
 	case SNDRV_PCM_STATE_XRUN:
 		return -EPIPE;
 	case SNDRV_PCM_STATE_SUSPENDED:
@@ -2320,7 +2324,7 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream,
 	nonblock = !!(substream->f_flags & O_NONBLOCK);
 
 	snd_pcm_stream_lock_irq(substream);
-	err = pcm_accessible_state(runtime);
+	err = pcm_accessible_state(substream);
 	if (err < 0)
 		goto _end_unlock;
 
@@ -2348,7 +2352,7 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream,
 		if (!avail) {
 			if (!is_playback &&
 			    runtime->state == SNDRV_PCM_STATE_DRAINING) {
-				snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP);
+				snd_pcm_drain_done(substream);
 				goto _end_unlock;
 			}
 			if (nonblock) {
@@ -2388,7 +2392,7 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream,
 		atomic_dec(&runtime->buffer_accessing);
 		if (err < 0)
 			goto _end_unlock;
-		err = pcm_accessible_state(runtime);
+		err = pcm_accessible_state(substream);
 		if (err < 0)
 			goto _end_unlock;
 		appl_ptr += frames;
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index defbb2977efe..657e77d33b21 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -3014,7 +3014,7 @@ static int do_pcm_hwsync(struct snd_pcm_substream *substream)
 	switch (substream->runtime->state) {
 	case SNDRV_PCM_STATE_DRAINING:
 		if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
-			return -EBADFD;
+			return 0;
 		fallthrough;
 	case SNDRV_PCM_STATE_RUNNING:
 		return snd_pcm_update_hw_ptr(substream);

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

* [RFC PATCH v4 2/2] ALSA: pcm: refresh linked pointers before draining
  2026-10-15  0:35 [RFC PATCH v4 0/2] ALSA: pcm: preserve capture tail during drain Joonho Lee
  2026-10-15  0:36 ` [RFC PATCH v4 1/2] ALSA: pcm: allow capture reads while draining Joonho Lee
@ 2026-10-15  0:36 ` Joonho Lee
  2026-10-15 15:54 ` [RFC PATCH v4 0/2] ALSA: pcm: preserve capture tail during drain Takashi Iwai
  2 siblings, 0 replies; 4+ messages in thread
From: Joonho Lee @ 2026-10-15  0:36 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Takashi Iwai, Jaroslav Kysela, linux-sound, linux-kernel,
	linux-api

Capture hardware can produce samples after the last PCM pointer update.
Drain must refresh those pointers before stopping capture, or its final
availability check can miss the remaining frames. With linked hardware,
the first STOP may freeze other members, so refresh all running members
before entering the drain action.

Factor the traversal into snd_pcm_update_linked_hw_ptr(). Hold
snd_pcm_link_rwsem for reading to keep membership stable during that
traversal, and take each member's stream lock for its ordinary
snd_pcm_update_hw_ptr() call. Include running playback streams and invoke
the helper after pause resume so paused capture also receives its final
update. Keep pause and drain-init outside the helper's rwsem scope.

Updating a pointer inside the group drain action can enter the XRUN stop
path and recursively acquire the group lock. The helper runs before that
action, so an update error follows the existing group XRUN path and is
returned to the caller before drain-init.

Suggested-by: Takashi Iwai <tiwai@suse.de>
Link: https://lore.kernel.org/all/87jynkcn60.wl-tiwai@suse.de/
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
 sound/core/pcm_native.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 657e77d33b21..9ee7245f4707 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -2071,6 +2071,25 @@ static int snd_pcm_prepare(struct snd_pcm_substream *substream,
 					(snd_pcm_state_t)f_flags);
 }
 
+static int snd_pcm_update_linked_hw_ptr(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_substream *s;
+
+	/* Keep group membership stable while locking each stream separately. */
+	guard(rwsem_read)(&snd_pcm_link_rwsem);
+	snd_pcm_group_for_each_entry(s, substream) {
+		int err;
+
+		guard(pcm_stream_lock_irq)(s);
+		if (!snd_pcm_running(s))
+			continue;
+		err = snd_pcm_update_hw_ptr(s);
+		if (err < 0)
+			return err;
+	}
+	return 0;
+}
+
 /*
  * drain ioctl
  */
@@ -2180,7 +2199,13 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream,
 	/* resume pause */
 	if (runtime->state == SNDRV_PCM_STATE_PAUSED)
 		snd_pcm_pause(substream, false);
+	snd_pcm_stream_unlock_irq(substream);
 
+	result = snd_pcm_update_linked_hw_ptr(substream);
+	if (result < 0)
+		return result;
+
+	snd_pcm_stream_lock_irq(substream);
 	/* pre-start/stop - all running streams are changed to DRAINING state */
 	result = snd_pcm_action(&snd_pcm_action_drain_init, substream,
 				ACTION_ARG_IGNORE);

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

* Re: [RFC PATCH v4 0/2] ALSA: pcm: preserve capture tail during drain
  2026-10-15  0:35 [RFC PATCH v4 0/2] ALSA: pcm: preserve capture tail during drain Joonho Lee
  2026-10-15  0:36 ` [RFC PATCH v4 1/2] ALSA: pcm: allow capture reads while draining Joonho Lee
  2026-10-15  0:36 ` [RFC PATCH v4 2/2] ALSA: pcm: refresh linked pointers before draining Joonho Lee
@ 2026-10-15 15:54 ` Takashi Iwai
  2 siblings, 0 replies; 4+ messages in thread
From: Takashi Iwai @ 2026-10-15 15:54 UTC (permalink / raw)
  To: Joonho Lee
  Cc: Takashi Iwai, Takashi Iwai, Jaroslav Kysela, linux-sound,
	linux-kernel, linux-api

On Thu, 15 Oct 2026 02:35:59 +0200,
Joonho Lee wrote:
> 
> This v4 replaces both patches in the core capture-drain v3 series. It
> follows Takashi Iwai's suggestion to refresh linked pointers in a helper
> before drain-init and fixes three tail-handling problems in patch 1.
> 
> Patch 1 allows buffered capture reads in DRAINING. When drain wakes a
> reader waiting for data, wait_for_avail() returns the remaining frame
> count. Capture EOF uses snd_pcm_drain_done(), so a linked playback stream
> can continue draining. DELAY and SYNC_PTR(HWSYNC) use the cached final
> capture pointer without calling the driver again after capture stops.
> Sashiko's review identified these three problems in the earlier patch.
> 
> Patch 2 takes snd_pcm_link_rwsem for reading and updates each running
> member under its own stream lock. It calls snd_pcm_update_hw_ptr() for
> both playback and capture. The helper runs after the existing pause-resume
> step, allowing a paused capture stream to receive its final update.
> The pause and drain-init actions remain outside the link rwsem.
> 
> A pointer error follows the ordinary XRUN path and returns -EPIPE before
> drain-init. This restores v2's group-wide error handling. V3 instead put
> the failing member in SETUP and could return success from blocking drain
> while another member retained its tail. V4 removes v3's notify argument
> and the conditional notifications in the pointer update code.
> 
> Previous core v3:
> https://lore.kernel.org/all/rfc-v3-core-capture-drain-0.cf2e307fcac34b6a@gmail.com/
> Takashi's latest suggestion:
> https://lore.kernel.org/all/87jynkcn60.wl-tiwai@suse.de/
> My implementation and test report:
> https://lore.kernel.org/all/core-drain-helper-reply.0c1099dbd0ca4a37@gmail.com/
> 
> The separate five-patch virtio-snd lifetime v3 series is unchanged:
> https://lore.kernel.org/all/cover.1791881787.git.mynameisjune111@gmail.com/
> Apply those five patches to the public base below, then these two patches.
> Remove both old core patches before applying v4. Core has no dependency
> on the generic control-wait or VZ compatibility RFC. The lifetime stack
> is pinned here to reproduce the tested integration. Neither companion
> RFC is revised by this series. A future VZ revision will need the new
> core prerequisite IDs.
> 
> Validation used ARM64 QEMU with KASAN and lockdep:
> - 26 linked-stream checks passed across atomic and nonatomic modes,
>   including paused drain, first/later member pointer errors, normal XRUN
>   handling, tail reads, and unlink/prepare/reopen recovery.
> - The three patch-1 failures were reproduced with the helper and the old
>   first patch in both locking modes. All six corresponding fixed cases
>   passed: a blocked reader received all 64 final frames, capture EOF
>   left linked playback in DRAINING, and both position queries succeeded
>   without another pointer callback.
> - Four additional blocked-reader runs compared baseline and fixed modules
>   in both modes with a stronger gate: thread sleep and a pointer callback
>   after START. They confirmed the baseline loss and fixed 64-frame read.
> - The 10 existing lifecycle and timeout/reset scenarios passed, including
>   held STOP/RELEASE, SIGKILL, reset, and subsequent I/O recovery.
> - No KASAN, lockdep, or panic diagnostics appeared in the 54 selected
>   logs, which include the baseline behavioral-failure comparisons.
> 
> The full Image and modules were first built with the new helper and the
> old first patch. After fixing the three tail issues, sound/core was
> rebuilt with W=1 against that same configuration and symbol versions,
> and the corrected PCM module was used for the runs above. The module
> build had no warnings or errors. The full build had 517 existing
> -Woverride-init warnings in arch/arm64 outside sound/. The exported v4
> core source matches the tested source byte for byte.
> 
> These tests use synthetic drivers and modified fault/reset backends.
> Physical-device and Virtualization.framework microphone testing of this
> core revision is still pending. Concurrent LINK, wraparound, and
> forward/rewind behavior have not been exercised by this test set.
> The updated reproducer and logs are local; the earlier public package
> contains an older core revision. There is no new reproducer upload for
> this candidate.
> 
> One design question remains: the helper releases the link rwsem before
> drain-init, and snd_pcm_link() permits another RUNNING stream to join in
> that interval. That member could enter drain-init without a refresh by
> the helper. Should this change protect the membership across both steps,
> or should concurrent linking be handled separately? I have reviewed this
> interval in the source but have not reproduced it with a stress test.
> 
> Do the restored group-wide XRUN/-EPIPE behavior and cached capture hwsync
> semantics look appropriate? Which physical-driver coverage would you
> want before this can move beyond RFC?
> 
> LLM assistance was used for implementation, reproducer development,
> analysis, and documentation. I reviewed the changes and test evidence.
> Assisted-by: LLM
> 
> Joonho Lee (2):
>   ALSA: pcm: allow capture reads while draining
>   ALSA: pcm: refresh linked pointers before draining

Both patches look OK.  Once when you submit a non-RFC patch set, I can
try merging.


thanks,

Takashi

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

end of thread, other threads:[~2026-10-15 15:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-15  0:35 [RFC PATCH v4 0/2] ALSA: pcm: preserve capture tail during drain Joonho Lee
2026-10-15  0:36 ` [RFC PATCH v4 1/2] ALSA: pcm: allow capture reads while draining Joonho Lee
2026-10-15  0:36 ` [RFC PATCH v4 2/2] ALSA: pcm: refresh linked pointers before draining Joonho Lee
2026-10-15 15:54 ` [RFC PATCH v4 0/2] ALSA: pcm: preserve capture tail during drain Takashi Iwai

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.