re: kern/46121: audiomp: locking protocol error

[matthew green <mrg%eterna.com.au@localhost>]

The following reply was made to PR kern/46121; it has been noted by GNATS.

From: matthew green <mrg%eterna.com.au@localhost>
To: David Laight <david%l8s.co.uk@localhost>
Cc: ad%netbsd.org@localhost, gnats-bugs%NetBSD.org@localhost, 
kern-bug-people%netbsd.org@localhost,
    gnats-admin%netbsd.org@localhost, netbsd-bugs%netbsd.org@localhost
Subject: re: kern/46121: audiomp: locking protocol error
Date: Tue, 27 Mar 2012 04:44:11 +1100

 > > this looks like it should do the trick.
 > ...
 > >    mutex_enter(sc->sc_lock);
 > > !  can_playback = audio_can_playback(sc);
 > > !  can_capture = audio_can_capture(sc);
 > > !          mutex_exit(sc->sc_lock);
 > > ! 
 > > !  if (can_playback) {
 > 
 > Hmmm... that sort of code tends to look dubious [1].
 > While acquiring a mutex across a function call can protect the underlying
 > data from immediate corruption, it isn't necessarily obvious that
 > the checked condition can't change state after the mutex is released.
 > 
 >      David
 > 
 > [1] As does releaseing a mutex across a function call!
 
 in a general you're right, but this case doesn't fit.  we need the
 lock to ask the hw driver about play/record state, and this is in
 audioattach() so there won't be other activity here yet.  so what
 this really does is push locking around that probably isn't strictly
 necessary for this path, but is necessary for others.
 
 the conditions can change, but only after open() etc.
 
 
 .mrg.