sound/pcm/* bugs (was: Re: page fault panic tracked down (selwakeuppri()) - really sound/pcm/*)

[Don Lewis]

On 18 Jan, Stefan Ehmann wrote:
> On Sat, 2004-01-17 at 10:15, Don Lewis wrote:
>> I think this problem can be caused by a transient malloc() M_NOWAIT
>> failure.
>> 
>> Changes in this version of the patch:
>> 
>> 	When increasing blksz in chn_setblocksize(), increase the size
>> 	of bufsoft before increasing bufhard, and decrease bufsoft after
>> 	decreasing bufhard in an attempt to avoid the buffer overflow in
>> 	feed_vchan_s16().
>> 	
>> 	Buffers are now allocated using M_WAITOK, requiring locks to
>> 	be dropped for the malloc() calls.  This required adding a mutex
>> 	parameter to sndbuf_remalloc().
>> 
>> 	Locking order between parent and child channels is changed.  The
>>         parent is now locked first and then the child.  The list of
>>         children is protected by the parent's lock.  There are still
>>         potential race conditions in the vchan creation/destruction code
>>         because all the locks have to be dropped in order to call
>>         malloc(), etc.
>> 
>> 	Locking cleaned up to eliminate the need for MTX_RECURSE.
>> 
>> 	A new mutex allocator for the channel mutexes was added that
>> 	initializes the mutexes with the MTX_DUPOK flags so that multiple
>> 	channel mutexes of the same type can be held at once.
>> 
>> 	Locking simplification in dsp_ioctl().
>> 
>> 	Added KASSERTs in chn_wrfeed().
> 
> Using this patch (and the small changes to avoid the esd problem) I got
> an assertion triggered in channel.c: chn_wrfeed amt > source size

Doh!

I was sort of afraid that might happen when the buffer size got set back
down to the size I intended.  Dit it happen immediately, or did you get
some sound first?

> #0  doadump () at /usr/src/sys/kern/kern_shutdown.c:240
> #1  0xc04e57c8 in boot (howto=256) at
> /usr/src/sys/kern/kern_shutdown.c:372
> #2  0xc04e5b57 in panic () at /usr/src/sys/kern/kern_shutdown.c:550
> #3  0xc07e36cd in chn_wrfeed (c=0xc37a8880)
>     at /usr/src/sys/dev/sound/pcm/channel.c:219
> #4  0xc07e377f in chn_wrintr (c=0xc37a8880)
>     at /usr/src/sys/dev/sound/pcm/channel.c:239
> #5  0xc07e3f70 in chn_intr (c=0xc37a8880)
>     at /usr/src/sys/dev/sound/pcm/channel.c:485
> #6  0xc07fda2f in csa_intr (p=0xc37a8700)
>     at /usr/src/sys/dev/sound/pci/csapcm.c:623
> #7  0xc07fc724 in csa_intr (arg=0xc37a8600)
>     at /usr/src/sys/dev/sound/pci/csa.c:532
> #8  0xc04d1c62 in ithread_loop (arg=0xc1736b00)
>     at /usr/src/sys/kern/kern_intr.c:544
> #9  0xc04d0c54 in fork_exit (callout=0xc04d1ad0 <ithread_loop>, arg=0x0,
> 
>     frame=0x0) at /usr/src/sys/kern/kern_fork.c:797
> 
> In frame #3 amt evaluates to 2176, bufsize is 2048

I just don't see how this can be happening.  Your hardware has a fixed
4K buffer size.  My changes to chn_setblocksize() should prevent bufsoft
from dropping below this value.

The only place that bufsoft is allocated appears to be in
chn_setblocksize().  Try inserting the following KASSERT() into two
locations in chn_setblocksize(), between the "if" test and the "return
EINVAL" at the beginning of the function (adding the necessary
brackets), and after the "out" label at the end of the function.

        KASSERT(sndbuf_getsize(bs) ==  0 ||
            sndbuf_getsize(bs) >= sndbuf_getsize(b),
            ("%s: bufsoft size %d < bufhard size %d", c->name,
            sndbuf_getsize(bs), sndbuf_getsize(b)));


Hopefully this will catch the problem closer to the source, but I still
don't see how this can be happening ...