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

[Joonho Lee]

Hi Takashi,

Thanks for the suggestion and thoughts. I tried the smaller change and hit a
recursive locking issue with linked capture streams.

I used a test driver with two linked capture substreams and made the
later member's pointer callback return SNDRV_PCM_POS_XRUN during
drain. This led to the function calls as follows! :

  snd_pcm_do_drain_init()
    snd_pcm_update_hw_ptr()
      snd_pcm_update_hw_ptr0()
        __snd_pcm_xrun()
          snd_pcm_stop()
            snd_pcm_action()
              snd_pcm_stream_group_ref()

The outer drain action already holds the group locks here. Lockdep
reported the recursive acquisition of group->lock for atomic streams and
group->mutex for nonatomic streams. Both tests remained stuck until
the 45-second timeout. Removing the link let both tests complete.

I went back to the notify=false approach from your first sketch, which is keeping the pointer update inside snd_pcm_do_drain_init(). I also
added the guard to the early SNDRV_PCM_POS_XRUN branch, since that
path still called __snd_pcm_xrun(). Drain init then handles the stop,
while ordinary pointer updates keep their existing behavior.

With this change, all 24 linked test cases passed across both locking
modes, including pointer errors, tail reads and reopen/recovery.
KASAN and lockdep stayed quiet. This was tested with a synthetic
driver under ARM64 QEMU; physical-device testing is still pending.

One thing I wanted to check before sending a revision is, how should
drain report a pointer error?

The current change puts the failing member in SETUP and allows a
healthy linked member to keep its tail in DRAINING. A blocking
capture-only drain returns 0 if the rest of the action succeeds,
and reading the failed member afterwards returns -EBADFD.

Would you prefer that behavior, or should we remember the error,
finish stopping all members, and return -EPIPE from drain? My v2 proposal
returned -EPIPE and moved the linked group to XRUN, so this changes
what userspace sees.