PERFORCE change 100545 for review

[Ryan Beasley]

http://perforce.freebsd.org/chv.cgi?CH=100545

Change 100545 by ryanb at ryanb_yuki on 2006/07/04 07:10:31

	In sound_oss_sysinfo and dsp_oss_audioinfo, add lock
	wrappers when iterating over PCM devices and audio channels.
	
	Fixed bug in dsp_oss_audioinfo where ai->dev was always set
	to 0, regardless of the real number of the channel probed.

Affected files ...

.. //depot/projects/soc2006/rbeasley_sound/sys/dev/sound/pcm/dsp.c#9 edit
.. //depot/projects/soc2006/rbeasley_sound/sys/dev/sound/pcm/sound.c#7 edit

Differences ...

==== //depot/projects/soc2006/rbeasley_sound/sys/dev/sound/pcm/dsp.c#9 (text+ko) ====

@@ -1315,6 +1315,9 @@
  * opened in read+write) and then assigns them to the si_drv[12] private
  * data fields.  As a result, any information returned linking a channel to
  * a specific character device isn't really accurate.
+ *
+ * @note
+ * Calling threads must not hold any snddev_info or pcm_channel locks.
  * 
  * @param dev		device on which the ioctl was issued
  * @param ai		ioctl request data container
@@ -1336,7 +1339,7 @@
 	struct pcm_channel *ch;
 	struct snddev_info *d;
 	struct cdev *t_cdev;
-	int i, nchan;
+	int i, nchan, ret;
 
 	/* If probing handling device, make sure it's a DSP device. */
 	if ((ai->dev == -1) && (i_dev->si_devsw != &dsp_cdevsw))
@@ -1345,15 +1348,20 @@
 	ch = NULL;
 	t_cdev = NULL;
 	nchan = 0;
-
+	ret = 0;
+	
 	/* Search for the requested audio device. */
 	for (i = 0; i < devclass_get_maxunit(pcm_devclass); i++) {
 		d = devclass_get_softc(pcm_devclass, i);
 		if (d == NULL)
 			continue;
 
+		pcm_inprog(d, 1);
+		pcm_lock(d);
+
 		SLIST_FOREACH(sce, &d->channels, link) {
 			ch = sce->channel;
+			CHN_LOCK(ch);
 			if (ai->dev == -1) {
 				if ((ch == i_dev->si_drv1) ||
 				    (ch == i_dev->si_drv2)) {
@@ -1364,12 +1372,32 @@
 				t_cdev = sce->dsp_devt;
 				goto dspfound;
 			}
+			CHN_UNLOCK(ch);
 			++nchan;
 		}
+
+		pcm_unlock(d);
+		pcm_inprog(d, -1);
 	}
+
+	/* Exhausted the search -- nothing is locked, so return. */
+	return EINVAL;
+
 dspfound:
-	if (t_cdev == NULL)
-		return EINVAL;
+	/* Should've found the device, but something isn't right */
+	if (t_cdev == NULL) {
+		ret = EINVAL;
+		goto out;
+	}
+
+	/*
+	 * At this point, the following synchronization stuff has happened:
+	 *   - a specific PCM device is locked and its "in progress
+	 *     operations" counter has been incremented, so be sure to unlock
+	 *     and decrement when exiting;
+	 *   - a specific audio channel has been locked, so be sure to unlock
+	 *     when exiting;
+	 */
 
 	caps = chn_getcaps(ch);
 
@@ -1379,6 +1407,7 @@
 	 */
 	bzero((void *)ai, sizeof(oss_audioinfo));
 
+	ai->dev = nchan;
 	strlcpy(ai->name, ch->name,  sizeof(ai->name));
 
 	if ((ch->flags & CHN_F_BUSY) == 0)
@@ -1450,6 +1479,11 @@
 	ai->min_rate = caps->minspeed;
 	ai->max_rate = caps->maxspeed;
 
-	return 0;
+out:
+	CHN_UNLOCK(ch);
+	pcm_unlock(d);
+	pcm_inprog(d, -1);
+
+	return ret;
 }
 #endif	/* !OSSV4_EXPERIMENT */

==== //depot/projects/soc2006/rbeasley_sound/sys/dev/sound/pcm/sound.c#7 (text+ko) ====

@@ -1132,6 +1132,9 @@
  * Also includes a bitmask showing which of the above types of devices
  * are open (busy).
  *
+ * @note
+ * Calling threads must not hold any snddev_info or pcm_channel locks.
+ *
  * @author	Ryan Beasley <ryanb at FreeBSD.org>
  */
 void
@@ -1167,6 +1170,11 @@
 			d = devclass_get_softc(pcm_devclass, i);
 			if (!d)
 				continue;
+
+			/* Increment device's "operations in progress" */
+			pcm_inprog(d, 1);
+			pcm_lock(d);
+
 			si->numaudios += d->devcount;
 			++ncards;
 
@@ -1179,6 +1187,9 @@
 				CHN_UNLOCK(c);
 				j++;
 			}
+
+			pcm_unlock(d);
+			pcm_inprog(d, -1);
 		}
 	}