All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: henrik.enquist@gmail.com
Cc: Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
	Mark Brown <broonie@kernel.org>, Shuah Khan <shuah@kernel.org>,
	linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-kselftest@vger.kernel.org,
	Pavel Hofman <pavel.hofman@ivitera.com>,
	stable@vger.kernel.org
Subject: Re: [PATCH 0/2] ALSA: aloop: Fix notify mode, add a selftest
Date: Sun, 04 Oct 2026 20:37:53 +0200	[thread overview]
Message-ID: <874if1ia66.wl-tiwai@suse.de> (raw)
In-Reply-To: <20261003-aloop-notify-fix-v1-0-ba5dfb831bf5@gmail.com>

On Sat, 03 Oct 2026 15:26:10 +0200,
Henrik Enquist via B4 Relay wrote:
> 
> Hi,
> 
> snd-aloop has a notify mode (the pcm_notify module parameter, or the
> "PCM Notify" control per cable) where the playback side may switch
> format, rate and channels while a capture is running. The driver then
> stops the capture and updates the "PCM Slave" controls, so the capture
> application can reopen with the new parameters. alsaloop enables it in
> its slave mode, and I'd like to use it in CamillaDSP, which captures
> from a loopback and needs to follow whatever the player outputs.
> 
> This hasn't worked since 2018. Commit 898dfe4687f4 ("ALSA: aloop: Fix
> racy hw constraints adjustment") moved the hw rules over to the shared
> cable->hw. That fixed a real race, but it also pins the playback side
> to the capture's parameters in notify mode. Pavel reported it on
> alsa-devel in 2020 [1], and Jaroslav reproduced it and pointed at the
> same commit:
> 
>   It seems that 898dfe4687f4 from Takashi broke this functionality
>   (tied the cable parameters more strictly, so the playback cannot set
>   freely own parameters for the pcm_notify=1 case). We need to find
>   another way to detach capture stream in this case.
> 
> The thread ended there, without a patch.
> 
> The breakage is quiet, which probably explains why it lasted. A player
> that probes the playback device sees only the capture's rate and
> format, and resamples to them. aplay prints a warning, most players
> don't. There's no rate change, no stopped capture and no event.
> 
> Patch 1 skips the hw rules for the playback side when notify is on,
> which gives it back the freedom loopback_open() still intends it to
> have. That makes the capture stop in loopback_check_format() reachable
> again. That path was hardened this year by 826af7fa62e3 ("ALSA: aloop:
> Fix racy access at PCM trigger") and e5c33cdc6f40 ("ALSA: aloop: Fix
> peer runtime UAF during format-change stop"), and with those in place
> I'm comfortable turning it back on. They need to go first in any
> stable backport.
> 
> The period bytes rule is skipped as well. It came later, for the sound
> timer source, but without skipping it a playback that switches rate
> keeps the old capture's period size. The stream then runs with a period
> that doesn't match the timer, and every switch fills dmesg with "Period
> size ... not corresponding to timer resolution".
> 
> Patch 2 adds a selftest, so this doesn't break silently again. It
> doesn't need to go to stable, so it can go via for-next if that's
> easier.
> 
> Testing: for-linus at b4e7fc36e31f, arm64 VM, kernel with KASAN,
> lockdep, DEBUG_ATOMIC_SLEEP and DEBUG_LIST, comparing the patched
> driver with the unpatched one built from the same tree. With the
> jiffies, hrtimer and sound timer (via snd-dummy) sources:
> - With notify on, rate, format and channel changes on the playback
>   side now go through, stop the capture and update the controls. With
>   notify off they're refused as before.
> - A capture that reopens at "PCM Slave Rate" after each stop follows a
>   player alternating between 44.1k and 96k files, 20 of 20 switches,
>   with both RW and mmap access. Unpatched: 10 of 20, the playback stays
>   pinned to the capture's rate.
> - A capture opened second is still pinned to the playback.
> - 2 minutes of random open/start/close on both ends per source gave no
>   KASAN or lockdep reports. The sound timer source logs an occasional
>   "snd_timer_stop ... failed with -16", at the same rate unpatched.
> - The new selftest fails 2 of 4 tests unpatched, and passes 4 of 4
>   patched, 50 runs in a row.
> Not tested: real hardware, other architectures, pause/resume, more
> than two substreams.
> 
> One question, following up on Takashi's point in that thread about
> telling user space that a stream got invalidated. After the stop, the
> capture drains and then reads return -EBADFD. alsa-lib now documents
> -ENODATA as meaning the PCM must be completely restarted, which looks
> like the right signal here. Would you want aloop to report that
> instead? I left it out since it changes what user space sees, but I'm
> happy to do it as a follow-up.
> 
> AI disclosure: I used Claude Code (Claude Opus 5.5) for this. Starting
> from my description of the problem and the 2020 thread, it did the
> root cause analysis from the driver source and git history, set up the
> test kernels and VM, wrote the test scripts, ran the measurements, and
> wrote both patches and their changelogs. I reviewed all of it, and both
> patches carry an Assisted-by tag.
> 
> [1] https://lore.kernel.org/all/b4af9071-f8d7-5b47-4d7a-c5743bd67394@ivitera.com/
> 
> Henrik
> 
> ---
> Henrik Enquist (2):
>       ALSA: aloop: Don't constrain playback params in notify mode
>       selftests/alsa: Add a test for snd-aloop notify mode

Applied both patches to for-next branch now.  Thanks.


Takashi

      parent reply	other threads:[~2026-10-04 18:38 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-03 13:26 [PATCH 0/2] ALSA: aloop: Fix notify mode, add a selftest Henrik Enquist via B4 Relay
2026-10-03 13:26 ` Henrik Enquist
2026-10-03 13:26 ` [PATCH 1/2] ALSA: aloop: Don't constrain playback params in notify mode Henrik Enquist via B4 Relay
2026-10-03 13:26   ` Henrik Enquist
2026-10-03 13:26 ` [PATCH 2/2] selftests/alsa: Add a test for snd-aloop " Henrik Enquist via B4 Relay
2026-10-03 13:26   ` Henrik Enquist
2026-10-04 18:37 ` Takashi Iwai [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=874if1ia66.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=broonie@kernel.org \
    --cc=henrik.enquist@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=pavel.hofman@ivitera.com \
    --cc=perex@perex.cz \
    --cc=shuah@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.