git: 46a97b9cd6fd - main - sound: Do not access cv_waiters

The branch main has been updated by christos:

URL: https://cgit.FreeBSD.org/src/commit/?id=46a97b9cd6fd4415270afe4070082ae69ee21035

commit 46a97b9cd6fd4415270afe4070082ae69ee21035
Author:     Christos Margiolis <christos@FreeBSD.org>
AuthorDate: 2024-12-02 16:26:58 +0000
Commit:     Christos Margiolis <christos@FreeBSD.org>
CommitDate: 2024-12-02 16:26:58 +0000

    sound: Do not access cv_waiters
    
    Remove uses of cv_waiters in PCM_RELEASE and CHN_BROADCAST, and also use
    a counter around cv_timedwait_sig() in chn_sleep(), which is checked in
    pcm_killchans(), as opposed to reading cv_waiters directly, which is a
    layering violation.
    
    While here, move CHN_BROADCAST below the channel lock operations.
    
    Reported by:    avg, jhb, markj
    Sponsored by:   The FreeBSD Foundation
    MFC after:      2 days
    Reviewed by:    dev_submerge.ch, avg
    Differential Revision:  https://reviews.freebsd.org/D47780
---
 sys/dev/sound/pcm/channel.c |  2 ++
 sys/dev/sound/pcm/channel.h |  9 ++++-----
 sys/dev/sound/pcm/sound.c   |  4 ++--
 sys/dev/sound/pcm/sound.h   | 13 ++-----------
 4 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/sys/dev/sound/pcm/channel.c b/sys/dev/sound/pcm/channel.c
index 728284b055e5..925a82bb170f 100644
--- a/sys/dev/sound/pcm/channel.c
+++ b/sys/dev/sound/pcm/channel.c
@@ -329,7 +329,9 @@ chn_sleep(struct pcm_channel *c, int timeout)
 	if (c->flags & CHN_F_DEAD)
 		return (EINVAL);
 
+	c->sleeping++;
 	ret = cv_timedwait_sig(&c->intr_cv, c->lock, timeout);
+	c->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 47089aca0745..67f5019f4727 100644
--- a/sys/dev/sound/pcm/channel.h
+++ b/sys/dev/sound/pcm/channel.h
@@ -117,6 +117,8 @@ struct pcm_channel {
 	 * lock.
 	 */
 	unsigned int inprog;
+	/* Incrememnt/decrement around cv_timedwait_sig() in chn_sleep(). */
+	unsigned int sleeping;
 	/**
 	 * Special channel operations should examine @c inprog after acquiring
 	 * lock.  If zero, operations may continue.  Else, thread should
@@ -242,11 +244,6 @@ struct pcm_channel {
 	(x)->parentchannel->bufhard != NULL) ?				\
 	(x)->parentchannel->bufhard : (y))
 
-#define CHN_BROADCAST(x)	do {					\
-	if ((x)->cv_waiters != 0)					\
-		cv_broadcastpri(x, PRIBIO);				\
-} while (0)
-
 #include "channel_if.h"
 
 int chn_reinit(struct pcm_channel *c);
@@ -320,6 +317,8 @@ int chn_getpeaks(struct pcm_channel *c, int *lpeak, int *rpeak);
 #define CHN_LOCKASSERT(c)	mtx_assert((c)->lock, MA_OWNED)
 #define CHN_UNLOCKASSERT(c)	mtx_assert((c)->lock, MA_NOTOWNED)
 
+#define CHN_BROADCAST(x)	cv_broadcastpri(x, PRIBIO)
+
 int snd_fmtvalid(uint32_t fmt, uint32_t *fmtlist);
 
 uint32_t snd_str2afmt(const char *);
diff --git a/sys/dev/sound/pcm/sound.c b/sys/dev/sound/pcm/sound.c
index 218147b73db0..2d57852e036d 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->intr_cv.cv_waiters == 0 && CHN_STOPPED(ch) &&
-			    ch->inprog == 0) {
+			if (ch->inprog == 0 && ch->sleeping == 0 &&
+			    CHN_STOPPED(ch)) {
 				CHN_UNLOCK(ch);
 				continue;
 			}
diff --git a/sys/dev/sound/pcm/sound.h b/sys/dev/sound/pcm/sound.h
index abbfc1111805..fe04898c17a5 100644
--- a/sys/dev/sound/pcm/sound.h
+++ b/sys/dev/sound/pcm/sound.h
@@ -359,15 +359,7 @@ int	sound_oss_card_info(oss_card_info *);
 		    __func__, __LINE__);				\
 	if ((x)->flags & SD_F_BUSY) {					\
 		(x)->flags &= ~SD_F_BUSY;				\
-		if ((x)->cv.cv_waiters != 0) {				\
-			if ((x)->cv.cv_waiters > 1 && snd_verbose > 3)	\
-				device_printf((x)->dev,			\
-				    "%s(%d): [PCM RELEASE] "		\
-				    "cv_waiters=%d > 1!\n",		\
-				    __func__, __LINE__,			\
-				    (x)->cv.cv_waiters);		\
-			cv_broadcast(&(x)->cv);				\
-		}							\
+		cv_broadcast(&(x)->cv);					\
 	} else								\
 		panic("%s(%d): [PCM RELEASE] Releasing non-BUSY cv!",	\
 		    __func__, __LINE__);				\
@@ -459,8 +451,7 @@ int	sound_oss_card_info(oss_card_info *);
 	    ("%s(%d): [PCM RELEASE] Releasing non-BUSY cv!",		\
 	    __func__, __LINE__));					\
 	(x)->flags &= ~SD_F_BUSY;					\
-	if ((x)->cv.cv_waiters != 0)					\
-		cv_broadcast(&(x)->cv);					\
+	cv_broadcast(&(x)->cv);						\
 } while (0)
 
 /* Quick version, for shorter path. */