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
This commit is contained in:
Christos Margiolis 2024-12-02 17:26:58 +01:00
parent 29ba0cc4d9
commit 46a97b9cd6
4 changed files with 10 additions and 18 deletions

View file

@ -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);
}

View file

@ -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 *);

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->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;
}

View file

@ -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. */