cvs commit: src/sys/dev/sound/pcm buffer.c

[Brian Fundakowski Feldman]

Don Lewis <truckman at FreeBSD.org> wrote:
> On 28 Apr, Brian Feldman wrote:
> > green       2004/04/28 19:51:59 PDT
> > 
> >   FreeBSD src repository
> > 
> >   Modified files:
> >     sys/dev/sound/pcm    buffer.c 
> >   Log:
> >   Don't do malloc(M_WAITOK) for sound buffers while locks are held.
> >   
> >   Revision  Changes    Path
> >   1.23      +1 -1      src/sys/dev/sound/pcm/buffer.c
> 
> The correct fix is to not hold the offending lock across the
> sndbuf_create() call and nuke the (tmpbuf == NULL) test.  At present, if
> the malloc() call fails, the channel will not be relocked, and my panic
> the system with an MTX_ASSERT() failure.  I wouldn't bet that the ENOMEM
> failure is properly handled.

I do believe that the failure is handled correctly (if not optimally); the 
old channel buffer won't be removed if the allocation of the new one failed,
and all sndbuf_resize() callers already need to handle errors.  The locking 
that sndbuf_resize() does is on the channel, itself, which in all cases 
should be unlocked again on return.

> I pretty much know how I want to fix the locking, but haven't had time
> to do it.  There are a bunch of other places in the sound code with
> similar problems.

Well, I would "currently" like it just not to have the option of hanging my 
system by calling malloc(9) illegally ;-)  I hope you do have time to fix it 
soon.

-- 
Brian Fundakowski Feldman                           \'[ FreeBSD ]''''''''''\
  <> green at FreeBSD.org                               \  The Power to Serve! \
 Opinions expressed are my own.                       \,,,,,,,,,,,,,,,,,,,,,,\