Re: git: e254ef87a30b - main - sound: Merge chn_intr() with chn_intr_locked()

On Sun Nov 23, 2025 at 2:34 PM CET, Roman Bogorodskiy wrote:
>   Christos Margiolis wrote:
>
>> On Sun Nov 23, 2025 at 7:54 AM CET, Roman Bogorodskiy wrote:
>> >   Christos Margiolis wrote:
>> >
>> >> The branch main has been updated by christos:
>> >> 
>> >> URL: https://cgit.FreeBSD.org/src/commit/?id=e254ef87a30bfcaabc6e4d8e0ecf05f6949a4f06
>> >> 
>> >> commit e254ef87a30bfcaabc6e4d8e0ecf05f6949a4f06
>> >> Author:     Christos Margiolis <christos@FreeBSD.org>
>> >> AuthorDate: 2025-11-21 16:14:28 +0000
>> >> Commit:     Christos Margiolis <christos@FreeBSD.org>
>> >> CommitDate: 2025-11-21 16:14:47 +0000
>> >> 
>> >>     sound: Merge chn_intr() with chn_intr_locked()
>> >>     
>> >>     There is no scenario where chn_intr() is called with the channel lock
>> >>     already held.
>> >>     
>> >>     No functional change intended.
>> >>     
>> >>     Sponsored by:   The FreeBSD Foundation
>> >>     MFC after:      1 week
>> >>     Reviewed by:    kib, markj
>> >>     Differential Revision:  https://reviews.freebsd.org/D53854
>> >> ---
>> >>  sys/dev/sound/pcm/channel.c | 20 ++------------------
>> >>  sys/dev/sound/pcm/channel.h |  1 -
>> >>  2 files changed, 2 insertions(+), 19 deletions(-)
>> >
>> > With this change my system panics as soon as I start firefox:
>> >
>> > panic: _mtx_lock_sleep: recursed on non-recursive mutex dsp2.play.0 @ /usr/src/sys/dev/sound/pcm/channel.c:586
>> >
>> > Things work fine again with this commit reverted.
>> >
>> > Roman
>> 
>> Which driver are you using for your sound card?
>> 
>> Christos
>
> I'm using snd_uaudio(4). If that's important, I have other pcm devices
> as well:
>
> $ cat /dev/sndstat 
> Installed devices:
> pcm0: <Realtek ALC897 (Rear Analog)> (play/rec)
> pcm1: <Realtek ALC897 (Analog)> (play/rec)
> pcm2: <Generic Philips SPA 20/93> (play/rec) default
> No devices installed from userspace.
> $ dmesg |grep pcm
> pcm2 on uaudio0
> pcm0: <Realtek ALC897 (Rear Analog)> at nid 20 and 24 on hdaa0
> pcm1: <Realtek ALC897 (Analog)> at nid 27 and 26 on hdaa0
> pcm2 on uaudio0
> $ 
>
> Roman

Oh, right. I just saw snd_uaudio(4) uses the channel lock (the one your
panic says we recursed in), to lock the usb transfer callback, which at
some point calls chn_intr(). I actually do not like that snd_uaudio(4)
uses sound(4)'s locks to do USB stuff.

I just reverted the commit, and will look into solving this afterwards.

Christos