The branch main has been updated by christos:
URL:
https://cgit.FreeBSD.org/src/commit/?id=5317480967bfc8bf678e4da3fce81bcb3f5b7836
commit 5317480967bfc8bf678e4da3fce81bcb3f5b7836
Author: Christos Margiolis <chris...@freebsd.org>
AuthorDate: 2024-11-26 14:48:36 +0000
Commit: Christos Margiolis <chris...@freebsd.org>
CommitDate: 2024-11-26 14:48:36 +0000
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
---
sys/dev/sound/pcm/channel.c | 13 +------------
sys/dev/sound/pcm/channel.h | 4 ++--
sys/dev/sound/pcm/sound.c | 4 ++--
3 files changed, 5 insertions(+), 16 deletions(-)
diff --git a/sys/dev/sound/pcm/channel.c b/sys/dev/sound/pcm/channel.c
index 59cdb8cd07f5..f619ddd4bc35 100644
--- a/sys/dev/sound/pcm/channel.c
+++ b/sys/dev/sound/pcm/channel.c
@@ -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);
}
diff --git a/sys/dev/sound/pcm/channel.h b/sys/dev/sound/pcm/channel.h
index 79a8d35b22f7..d226adfba06b 100644
--- a/sys/dev/sound/pcm/channel.h
+++ b/sys/dev/sound/pcm/channel.h
@@ -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" \
diff --git a/sys/dev/sound/pcm/sound.c b/sys/dev/sound/pcm/sound.c
index 4b7bcc64397f..218147b73db0 100644
--- a/sys/dev/sound/pcm/sound.c
+++ b/sys/dev/sound/pcm/sound.c
@@ -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;
}
