Re: [RFC PATCH 2/2] ALSA: pcm: refresh capture pointers before drain

[Takashi Iwai <tiwai@suse.de>]

On Thu, 15 Oct 2026 01:52:03 +0200,
Joonho Lee wrote:
> 
> Hi Takashi,
> 
> Thanks, I've reworked the local version along those lines.
> 
> The helper takes snd_pcm_link_rwsem for reading and updates each running
> member under its own stream lock. It uses snd_pcm_update_hw_ptr() for both
> playback and capture. I call it after the existing pause-resume step so a
> paused capture stream gets its final pointer update too. The pause and
> drain-init actions remain outside the link rwsem.
> 
> A pointer error now follows the usual XRUN path and returns -EPIPE before
> drain-init. This restores the group-wide error handling from v2 and removes
> the "notify" boolean argument introduced in v3 proposal.
> 
> While checking this version, I also confirmed three problems in the first
> patch: 
> 1) a reader already waiting for data could lose the capture tail
> 2) finishing a capture read could stop a linked playback stream
> 3) DELAYand SYNC_PTR(HWSYNC) rejected capture DRAINING. The local fixes preserve
> the unread count when waking the reader, finish capture with
> snd_pcm_drain_done(), and use the cached pointer for capture hwsync.
> 
> I reproduced all three failures with the helper plus the old first patch,
> then checked the fixes in both atomic and nonatomic modes. The blocked
> reader now receives all 64 final frames, capture EOF leaves linked playback
> in DRAINING, and both position queries succeed without another pointer
> callback. The 26 linked-stream checks, including paused drain and pointer
> errors, and the 10 existing lifecycle/reset cases also passed under ARM64
> QEMU with KASAN and lockdep, but physical real-world device testing is still pending in this time.
> 
> There is one concurrency point I'd like to check. The helper releases the
> link rwsem before drain-init, and snd_pcm_link() allows another RUNNING
> stream to join in that interval. The new member could enter drain-init
> without being visited by the refresh loop.
> My question is, should we handle concurrent linking across these two steps, or leave that outside this change's scope?

I guess we can leave this for now.  The invalid state like OPEN or
SUSPENDED is checked at snd_pcm_pre_drain_init().


thanks,

Takashi