sound: Remove CHN_F_SLEEPING

The KASSERT in chn_sleep() can be triggered if more than one thread
wants to sleep on a given channel at the same time. While this is not
really a common scenario, tools such as stress2, which use fork() and
the child process(es) inherit the parent's FDs as a result, we can end
up triggering such scenarios.

Fix this by removing CHN_F_SLEEPING altogether, which is not very useful
in the first place:
- CHN_BROADCAST() checks cv_waiters already, so there is no need to
  check CHN_F_SLEEPING as well.
- We can check whether cv_waiters is 0 in pcm_killchans(), instead of
  whether CHN_F_SLEEPING is not set.

Reported by:	dougm, pho (stress2)
Sponsored by:	The FreeBSD Foundation
MFC after:	2 days
Reviewed by:	dev_submerge.ch, markj
Differential Revision:	https://reviews.freebsd.org/D47559
This commit is contained in:
Christos Margiolis 2024-11-26 15:48:36 +01:00
parent 6d4c59e261
commit 5317480967
3 changed files with 5 additions and 16 deletions

View File

@ -309,14 +309,7 @@ chn_wakeup(struct pcm_channel *c)
if (CHN_EMPTY(c, children.busy)) {
if (SEL_WAITING(sndbuf_getsel(bs)) && chn_polltrigger(c))
selwakeuppri(sndbuf_getsel(bs), PRIBIO);
if (c->flags & CHN_F_SLEEPING) {
/*
* Ok, I can just panic it right here since it is
* quite obvious that we never allow multiple waiters
* from userland. I'm too generous...
*/
CHN_BROADCAST(&c->intr_cv);
}
CHN_BROADCAST(&c->intr_cv);
} else {
CHN_FOREACH(ch, c, children.busy) {
CHN_LOCK(ch);
@ -332,15 +325,11 @@ chn_sleep(struct pcm_channel *c, int timeout)
int ret;
CHN_LOCKASSERT(c);
KASSERT((c->flags & CHN_F_SLEEPING) == 0,
("%s(): entered with CHN_F_SLEEPING", __func__));
if (c->flags & CHN_F_DEAD)
return (EINVAL);
c->flags |= CHN_F_SLEEPING;
ret = cv_timedwait_sig(&c->intr_cv, c->lock, timeout);
c->flags &= ~CHN_F_SLEEPING;
return ((c->flags & CHN_F_DEAD) ? EINVAL : ret);
}

View File

@ -354,7 +354,7 @@ enum {
#define CHN_F_RUNNING 0x00000004 /* dma is running */
#define CHN_F_TRIGGERED 0x00000008
#define CHN_F_NOTRIGGER 0x00000010
#define CHN_F_SLEEPING 0x00000020
/* unused 0x00000020 */
#define CHN_F_NBIO 0x00000040 /* do non-blocking i/o */
#define CHN_F_MMAP 0x00000080 /* has been mmap()ed */
@ -381,8 +381,8 @@ enum {
"\002ABORTING" \
"\003RUNNING" \
"\004TRIGGERED" \
/* \006 */ \
"\005NOTRIGGER" \
"\006SLEEPING" \
"\007NBIO" \
"\010MMAP" \
"\011BUSY" \

View File

@ -221,8 +221,8 @@ pcm_killchans(struct snddev_info *d)
/* Make sure all channels are stopped. */
CHN_FOREACH(ch, d, channels.pcm) {
CHN_LOCK(ch);
if ((ch->flags & CHN_F_SLEEPING) == 0 &&
CHN_STOPPED(ch) && ch->inprog == 0) {
if (ch->intr_cv.cv_waiters == 0 && CHN_STOPPED(ch) &&
ch->inprog == 0) {
CHN_UNLOCK(ch);
continue;
}