The branch stable/14 has been updated by christos:
URL:
https://cgit.FreeBSD.org/src/commit/?id=134a275eea7a2edf34b4b98cf6f382d7a3618597
commit 134a275eea7a2edf34b4b98cf6f382d7a3618597
Author: Christos Margiolis <chris...@freebsd.org>
AuthorDate: 2024-12-02 16:26:58 +0000
Commit: Christos Margiolis <chris...@freebsd.org>
CommitDate: 2024-12-04 12:03:36 +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
(cherry picked from commit 46a97b9cd6fd4415270afe4070082ae69ee21035)
---
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 761c505fd402..b8f4efea8789 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 65e8c3002026..59d9930712b3 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. */
