Re: _mtx_lock_sleep: recursed on non-recursive mutex pcm0

On  9 Jun, Fredrik Lindberg wrote:
> On Wed, Jun 08, 2005 at 02:39:04PM -0700, Don Lewis (truckman_at_FreeBSD.org) wrote:
>> On  8 Jun, Fredrik Lindberg wrote:
>> > Hi
>> > 
>> > When trying to play sound from two different sources at the same time I get an
>> > instant panic with the following message
>> >  
>> > panic: _mtx_lock_sleep: recursed on non-recursive mutex pcm0 _at_ /usr/src/sys/dev/sound/pcm/sound.c:381
>> > 
>> > This is with a <Intel ICH3 (82801CA)> (AC97) on a few hours old current and with
>> > sound and snd_ich compiled staticly into the kernel.
>> > It only occurs when hw.snd.autovchans is set to a number greater than 1.
>> > 
> [cut backtrace]
> 
>> > The content of frame 5 yields the following
>> > (kgdb) f 5
>> > #5  0xc0506dfc in pcm_chn_create (d=0xc1979a00, parent=0x0, cls=0x0, dir=2, 
>> >     devinfo=0x0) at /usr/src/sys/dev/sound/pcm/sound.c:381
>> > 381             snd_mtxlock(d->lock);
>> > 
>> > d->lock is initialized as a non-recursive mutex in snd_mtxcreate (sound.c:78) 
>> > called from pcm_register (sound.c:655).
>> > While the following patch fixes the panic and let me play sound from different
>> > sources at the same time without any problems, I'm really not sure this is
>> > the correct solution since the mutex was initialized as a non-recursive mutex but
>> > recursion happens anyway. Perhaps somebody with more experience in the sound system
>> > could look at this.
>> 
>> Try manually creating the vchans ahead of time by setting the
>> hw.snd.pcm0.vchans to the desired value.  There are a number of locking
>> bugs and possible race conditions in the top half of the sound code,
>> especially in the channel creation code.  Pre-creating the vchans
>> exercises a different code path that should not have this particular
>> bug.
> 
> Yes, manually creating vchans works without problems. So the issue is most
> likley, as you said, somewhere during channel creation.
> 
>> 
>> I'm guessing that your kernel does not have the WITNESS option enabled,
>> otherwise WITNESS should be complaining about calls to malloc() while
>> holding a mutex.
> 
> I ran this with WITNESS enabled too, and yes it complains a few times about malloc.
> 
> malloc(M_WAITOK) of "16", forcing M_NOWAIT with the following non-sleepable locks held:
> exclusive sleep mutex pcm0 (sound cdev) r = 0 (0xc1be9bc0) locked _at_ /usr/src/sys/dev/sound/pcm/dsp.c:214

The M_WAITOK -> M_NOWAIT translation probably means that this code path
could panic if one of the malloc() calls failed due to a memory
shortage.


> I'm not posting the full backtrace, since quite large, but the importat 
> bits are similar to the backtrace produced from the crash dump.
> 
> malloc(8,c081c400,102,100,c07ca0ea) at malloc+0xd9
> vchan_create(c1bf0180,0,c07ca0ea,bf,3) at vchan_create+0x4e
> pcm_chnalloc(c1979800,1,2e7,ffffffff,0) at pcm_chnalloc+0x114
> dsp_open(c1a8ec00,2,2000,c2072c80,c1a8ec00) at dsp_open+0x2dd
> 
> I'll try to dig around some more and see if I can come up with something. 

The pcm0 mutex needs to be dropped before calling vchan_create() (the
other paths to vchan_create() do this) so that no mutexes are held
across the malloc() calls and to avoid the recursion on the pcm0 mutex.
The problem then is that then there is nothing preventing this code from
being re-entered and hosing things in some other way.

The locking scheme in the top half of the sound code is pretty broken
and needs to be re-written.  I've got some ideas on how to do it, but
ENOTIME to work on it.  I think its also likely that bugs are lurking in
the channel creation and destruction code due to the way the code is
structured and the way operations are ordered.

I'd recommend just manually creating the vchans as a workaround.