Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] ALSA: virtio-snd: unlatched XRUN in virtsnd_pcm_trigger() on pause release --
commit 84dba89c8170c6b6702f2442748d799772bffa04 Author: Joonho Lee <[email protected]> ALSA: virtio-snd: fix dsnoop capture with staged buffers This commit adds free-running staged-capture policy handling when stop_threshold equals boundary so dsnoop capture streams do not fail with ring_capacity rejections. It also reconstructs unread ring occupancy when returning from free-running to bounded capture mode and returns -EPIPE if outstanding unacknowledged data already fills the ring. > diff --git a/sound/virtio/virtio_pcm_msg.c b/sound/virtio/virtio_pcm_msg.c > index 663f7695b5be8..e7258bce2e6f0 100644 > --- a/sound/virtio/virtio_pcm_msg.c > +++ b/sound/virtio/virtio_pcm_msg.c [ ... ] > +/* Caller holds vss->lock. */ > +int virtsnd_pcm_capture_refresh_mode_locked(struct virtio_pcm_substream *vss, > + bool *free_running) > +{ > + struct snd_pcm_runtime *runtime; > + snd_pcm_uframes_t stop_threshold; > + u64 outstanding; > + bool new_free_running; > + > + if (!vss->capture_staging || !vss->runtime_active || !vss->substream) > + return -EBADFD; > + > + runtime = vss->substream->runtime; > + if (!runtime || !vss->buffer_bytes || !vss->capture_boundary) > + return -EBADFD; > + > + /* Validate the currently committed policy before observing a > transition. */ > + if (!virtsnd_pcm_capture_counters_valid(vss)) > + return -EIO; > + > + stop_threshold = READ_ONCE(runtime->stop_threshold); > + new_free_running = stop_threshold == vss->capture_boundary; > + if (new_free_running != vss->capture_free_running) { > + outstanding = vss->capture_copied_total - > + vss->capture_acked_total; > + if (new_free_running) { > + vss->capture_ready = 0; > + } else { > + if (outstanding >= vss->buffer_bytes) > + return -EPIPE; > + vss->capture_ready = outstanding; > + } > + vss->capture_free_running = new_free_running; > + } > + > + *free_running = new_free_running; > + return 0; > +} [ ... ] > +int virtsnd_pcm_capture_send(struct virtio_pcm_substream *vss) > +{ > + bool free_running; > + int rc; > + > + rc = virtsnd_pcm_capture_refresh_mode_locked(vss, &free_running); > + if (rc) > + return rc; > + > + return virtsnd_pcm_capture_enqueue(vss, free_running); > +} [Severity: Medium] Can a staged capture substream get stuck in SNDRV_PCM_STATE_PAUSED with vss->stopped = true and vss->xfer_xrun = false when virtsnd_pcm_capture_refresh_mode_locked() returns -EPIPE during pause release? Suppose a staged capture stream (with virtsnd_capture_pause_restart=false) runs in free-running mode until capture_copied_total - capture_acked_total >= vss->buffer_bytes, is paused via SNDRV_PCM_IOCTL_PAUSE(1), and then has stop_threshold updated below boundary via snd_pcm_sw_params(). Because the substream is in SNDRV_PCM_STATE_PAUSED, snd_pcm_running(substream) is false and snd_pcm_sw_params() skips snd_pcm_update_state(). When the stream is subsequently resumed via SNDRV_PCM_IOCTL_PAUSE(0), snd_pcm_do_pause() calls virtsnd_pcm_trigger(substream, SNDRV_PCM_TRIGGER_PAUSE_RELEASE), which calls virtsnd_pcm_capture_send() -> virtsnd_pcm_capture_refresh_mode_locked() and receives -EPIPE: sound/virtio/virtio_pcm_ops.c:virtsnd_pcm_trigger() { ... case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: ... if (vss->direction == SNDRV_PCM_STREAM_CAPTURE) { if (vss->capture_staging) { if (!vss->runtime_active) rc = -EBADFD; else rc = virtsnd_pcm_capture_send(vss); } else { rc = virtsnd_pcm_msg_send(vss, 0, vss->buffer_bytes); } } if (rc) { vss->stopped = true; substream->runtime->stop_operating = true; return rc; } } Unlike snd_pcm_do_start(), snd_pcm_do_pause() does not transition runtime->state to SNDRV_PCM_STATE_XRUN on -EPIPE; snd_pcm_action_single() invokes snd_pcm_undo_pause() and leaves runtime->state in SNDRV_PCM_STATE_PAUSED. In sound/virtio/virtio_pcm.c:virtsnd_pcm_period_elapsed(), transitioning a paused staged-capture stream to SNDRV_PCM_STATE_XRUN relies on vss->xfer_xrun being latched and vss->elapsed_period being scheduled: sound/virtio/virtio_pcm.c:virtsnd_pcm_period_elapsed() { ... if (!snd_pcm_running(substream)) { scoped_guard(spinlock_irqsave, &vss->lock) { runtime = substream->runtime; paused_xrun = runtime && runtime->state == SNDRV_PCM_STATE_PAUSED && vss->runtime_active && vss->xfer_xrun; } if (paused_xrun) { /* Wake readers without sending STOP for a paused stream. */ snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN); } return; } } Because virtsnd_pcm_trigger() sets vss->stopped = true without setting vss->xfer_xrun = true or scheduling vss->elapsed_period (unlike the virtsnd_pcm_pause_restart_start() -> virtsnd_pcm_pause_restart_fail() path), won't any reader blocked in wait_for_avail() remain asleep in SNDRV_PCM_STATE_PAUSED while subsequent pause-release attempts fail with -EBADFD? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
