Re: [PATCH v29 17/17] ALSA: virtio: preserve playback progress across pause and buffer access

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] ALSA: virtio: ephemeral patch numbers and test notes in commit
  message
--

commit 828a40dca88211506643733b87d449d53fefdce3
Author: Joonho Lee <mynameisjune111@gmail.com>

> ALSA: virtio: preserve playback progress across pause and buffer access
>
> TX completions can arrive after PAUSE closes transfer admission.
> Preserve that progress and schedule catch-up when the stream resumes or
> a rejected pause restores RUNNING. Cache device latency in stream-owned
> state so a late completion does not need to dereference the ALSA
> runtime. Clear both the cache and runtime delay on prepare and before
> RESET observes the hardware pointer.
>
> A playback pointer callback can also invoke ACK while the core has
> closed buffer-access admission, after the pointer helper has already
> accounted for completed hardware bytes. Track that callback context and
> refill only previously committed data using pcm_indirect.appl_ptr. Keep
> rejecting other ACKs while the gate is closed, and use the
> explicit-pointer helper from patch 16 for the refill. If
> runtime->control is absent, route the condition through the common
> playback validator and XRUN error path rather than returning before
> transfer failure handling.
>
> Use the existing indirect hw_queue_size to keep outstanding TX below one
> physical ring: buffer_bytes - period_bytes. Equal modulo cursors then
> mean no unobserved completion, and a non-interrupt HWSYNC cannot consume
> a hidden full-ring wrap before deferred period work. Continue using the
> ordinary ALSA pointer update for silence, drain, stop-threshold and
> wakeup policy.
>
> Check for pending progress or a latched transfer error again in the
> period worker under the PCM stream lock. A queued notification can
> outlive a paused RESET or an earlier pointer observation. Calling the
> period helper without new progress after resume can turn equal cursors
> into a false wrap when the core has backdated its pointer timestamp.
>
> This reduces queued buffering by one period. With two periods, only one
> period can be queued. Worst-case scheduling tolerance needs
> qualification.
>
> The final candidate was tested on ARM64 QEMU with KASAN and lockdep.
> RW/MMAP boundary pause, release, HWSYNC/poll, drop and reopen passed
> with two- and eight-period rings. RW/MMAP also passed accepted RESET
> from PREPARED followed by DRAIN and reopen, and accepted RESET while
> RUNNING after the full 2,048-frame buffer became available.
>
> A separate paired RW/MMAP gate fixture reproduced the baseline failure.
> The generic ALSA indirect helper invokes ACK for each pointer callback
> but masks its internal -EBADFD. Once hw_ptr == hw_io with no completion
> outstanding, virtio period-work progress suppresses further callbacks,
> so there is no ACK to retry the refill. Baseline stopped after 3,072
> completed bytes with 1,024 bytes in sw_ready. The candidate drained
> sw_ready and completed 4,096 bytes without any further client ACK. A
> test-only hook changed appl_ptr to 2,560 while only 2,048 frames were
> committed; the candidate refilled only committed data. The hostile
> mutation used a test-only kernel hook because ARM64 control-page mmap is
> unsupported. KASAN and lockdep had no diagnostics. KCSAN and physical
> audio were not tested. The tree through patch 15 was inspected but was
> not run as a separate control. Full-series runtime is not claimed here.
>
> Assisted-by: LLM
> Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>

[Severity: Low]
This isn't a bug, but the commit message refers to mailing-list series
numbers ("patch 15" and "patch 16"), which lose their meaning once the
commits are applied to git history; naming
snd_pcm_indirect_playback_transfer_appl_ptr() or the prior commit subject
directly would be clearer.  Also, the open-ended note ("Worst-case
scheduling tolerance needs qualification") and test-runner disclaimers
("The tree through patch 15 was inspected...", "Full-series runtime is not
claimed here.") belong in the cover letter or below the --- separator
rather than the permanent changelog.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112053446.73504-1-mynameisjune111@gmail.com?part=17