From owner-freebsd-hackers  Sat Sep 30 19:59:33 1995
Return-Path: owner-hackers
Received: (from root@localhost)
          by freefall.freebsd.org (8.6.12/8.6.6) id TAA28708
          for hackers-outgoing; Sat, 30 Sep 1995 19:59:33 -0700
Received: from sbstark.cs.sunysb.edu (sbstark.cs.sunysb.edu [130.245.1.47])
          by freefall.freebsd.org (8.6.12/8.6.6) with ESMTP id TAA28702
          for <hackers@freebsd.org>; Sat, 30 Sep 1995 19:59:27 -0700
Received: (from root@localhost) by sbstark.cs.sunysb.edu (8.6.12/8.6.9) with UUCP id WAA24028 for hackers@freebsd.org; Sat, 30 Sep 1995 22:57:46 -0400
Received: (from gene@localhost) by starkhome.cs.sunysb.edu (8.6.11/8.6.9) id WAA12297; Sat, 30 Sep 1995 22:54:48 -0400
Date: Sat, 30 Sep 1995 22:54:48 -0400
From: Gene Stark <gene@starkhome.cs.sunysb.edu>
Message-Id: <199510010254.WAA12297@starkhome.cs.sunysb.edu>
To: hackers@freebsd.org
Subject: Problems with VAT and Sound Code
Sender: owner-hackers@freebsd.org
Precedence: bulk

Even with the sound code now in -stable, I am still unable to get VAT
to work with an SB-16.  The symptom is that there is no sound.  If you
select one of the audio tests, you hear nothing until you delete the
VAT window, at which time there is a very brief spurt of the test tone.
During the time there is no sound in the speakers, VAT appears to operate
normally.  If you talk into the mike you can see the little level meter
bounce around, etc.  A ps on VAT indicates it is spending its time in
select.

On the off chance that I might spot a problem in the audio select()
code, I started looking at that part of the drivers.  My first impression
is that there are a *lot* of potential race conditions due to incorrect
positioning of DISABLE_INTR and ENABLE_INTR.

For example, in audio.c (line 494) we have:

      if (wr_buff_no[dev] != -1)
	return 1;		/* There is space in the current buffer */

      return DMAbuf_select (dev, file, sel_type, wait);
      break;

However, interrupts are not disabled, leaving the possibility of a
race if an interrupt occurs between testing no space in the buffer
and sleeping in DMAbuf_select().

In dmabuf.c (line 986) we have:

      if (!space_in_queue (dev))
	{
	  DISABLE_INTR (flags);
#if defined(__FreeBSD__)
	  selrecord(wait, &selinfo[dev]);
#else
	  dev_sleep_flag[dev].mode = WK_SLEEP;
	  select_wait (&dev_sleeper[dev], wait);
#endif
	  RESTORE_INTR (flags);
	  return 0;
	}
      return 1;
      break;

Again, the queue space is being checked outside of the critical section,
allowing a race and potential indefinite sleep.

Of course, my impressions here could be due to the fact that I am not
familiar with the code (just started looking at it), but it sure looks
like trouble to me.

Can anyone confirm or refute the correctness of these observations?
I thought by now most of the drivers with incorrect handling of critical
sections had been rooted out and fixed.

							- Gene Stark