Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] alsa: virtio: state desync on msg send failure --
commit e0fc1ff7e6c503fbe54480dc784a6aac80eec85b Author: Joonho Lee <[email protected]> ALSA: virtio: Notify queued PCM messages after send errors virtsnd_pcm_msg_send() advances a message length before adding it to the virtqueue. If a later add fails, the error path returns without restoring that length or notifying the device about earlier messages added by the same call. The device can therefore leave successfully queued messages unprocessed, while the failed message appears already submitted. > diff --git a/sound/virtio/virtio_pcm_msg.c b/sound/virtio/virtio_pcm_msg.c > index 72103f6067169..5371f4985fd06 100644 > --- a/sound/virtio/virtio_pcm_msg.c > +++ b/sound/virtio/virtio_pcm_msg.c > @@ -260,6 +260,18 @@ int virtsnd_pcm_msg_send(struct virtio_pcm_substream > *vss, unsigned long offset, > dev_err(&vdev->dev, > "SID %u: failed to send I/O message\n", > vss->sid); > + msg->length -= n; [Severity: High] Does this length rollback desynchronize the virtio message state from the ALSA indirect layer? When virtsnd_pcm_msg_send() fails and returns an error, its caller, virtsnd_pcm_trans_copy(), ignores the return value: sound/virtio/virtio_pcm_ops.c:virtsnd_pcm_trans_copy() { struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream); virtsnd_pcm_msg_send(vss, rec->sw_data, bytes); } Because virtsnd_pcm_trans_copy() returns void, the ALSA indirect layer (snd_pcm_indirect_playback_transfer()) assumes success and unconditionally advances its software offset pointers. Since the ALSA pointer advanced but the virtio message length rolled back, the two states become out of sync. On the next wrap-around, will the skipped message artificially reach period_bytes with old/new mixed data and be submitted to the virtqueue out of order? Should the driver set substream->runtime->stop_operating = true inside virtsnd_pcm_msg_send() to properly abort the stream when an I/O message fails? > + > + /* > + * Do not leave earlier messages queued without > notifying > + * the device when a later message could not be > added. > + */ > + if (msg_count != vss->msg_count && > + !(vss->features & > + (1U << VIRTIO_SND_PCM_F_MSG_POLLING)) && > + virtqueue_kick_prepare(vqueue)) > + virtqueue_notify(vqueue); > + > return rc; > } > -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
