Date: Tue, 4 Jul 2006 07:11:23 GMT From: Ryan Beasley <ryanb@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 100545 for review Message-ID: <200607040711.k647BNGn004827@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=100545 Change 100545 by ryanb@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@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); } }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200607040711.k647BNGn004827>