https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=289441
Damjan Jovanovic <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |patch --- Comment #2 from Damjan Jovanovic <[email protected]> --- (In reply to Damjan Jovanovic from comment #1) This patch fixes the problem - simply setting CHN_F_NBIO after calling chn_reset() (instead of before), is enough to make write() non-blocking like it should be: ---snip--- diff --git a/sys/dev/sound/pcm/dsp.c b/sys/dev/sound/pcm/dsp.c index aa6ad4a59778..f1c6fc840edb 100644 --- a/sys/dev/sound/pcm/dsp.c +++ b/sys/dev/sound/pcm/dsp.c @@ -221,17 +221,17 @@ dsp_chn_alloc(struct snddev_info *d, struct pcm_channel **ch, int direction, return (ENODEV); } - (*ch)->flags |= CHN_F_BUSY; - if (flags & O_NONBLOCK) - (*ch)->flags |= CHN_F_NBIO; - if (flags & O_EXCL) - (*ch)->flags |= CHN_F_EXCLUSIVE; (*ch)->pid = pid; strlcpy((*ch)->comm, (comm != NULL) ? comm : CHN_COMM_UNKNOWN, sizeof((*ch)->comm)); if ((err = chn_reset(*ch, (*ch)->format, (*ch)->speed)) != 0) return (err); + (*ch)->flags |= CHN_F_BUSY; + if (flags & O_NONBLOCK) + (*ch)->flags |= CHN_F_NBIO; + if (flags & O_EXCL) + (*ch)->flags |= CHN_F_EXCLUSIVE; chn_vpc_reset(*ch, SND_VOL_C_PCM, 0); CHN_UNLOCK(*ch); ---snip--- So yes, what I proposed in comment 1, where chn_reset() clears CHN_F_NBIO, causing later calls to write() to block, must be correct. But is this enough? Setting CHN_F_NBIO after calling chn_reset(), like the above patch does, will fix open(). But will it remain working? The chn_reset() function is called from a large number of other places. Here is its hierarchy of callers: chn_reset() dsp_chn_alloc() dsp_open() dsp_close() chn_init() pcm_addchan() (device drivers?) vchan_create() dsp_chn_alloc() (but happens before setting CHN_F_NBIO) chn_notify() chn_resizebuf() chn_setlatency() chn_reset() chn_notify() dsp_oss_policy() dsp_ioctl() with SNDCTL_DSP_POLICY chn_setblocksize() dsp_ioctl() with AIOSSIZE or SNDCTL_DSP_SETBLKSIZE or SNDCTL_DSP_SETFRAGMENT vchan_trigger() sysctl_dev_pcm_vchanrate() sysctl_dev_pcm_vchanformat() vchan_create() dsp_chn_alloc() (but happens before setting CHN_F_NBIO) vchan_destroy() dsp_close() vchan_create() (on failure) And device drivers also call some of those functions. So could chn_reset() get called while a channel is in use, clearing the CHN_F_NBIO flag despite the fact open() set it correctly? If so, then it might be better to add CHN_F_NBIO to the CHN_F_RESET flags instead, so that chn_reset() preserves it. But if I understand correctly, after a channel is closed, it can be reused by another process. If so, then where should CHN_F_NBIO get cleared, so the next user starts with a blocking channel? If my concerns are valid, then this patch might be better. Add CHN_F_NBIO to the CHN_F_RESET flags, so chn_reset() preserves it, but in dsp_chn_alloc(), also clear CHN_F_NBIO if O_NONBLOCK is unset, so that reused channels have the correct initial blocking mode: ---snip--- diff --git a/sys/dev/sound/pcm/channel.h b/sys/dev/sound/pcm/channel.h index fab182b22774..9ad21d219001 100644 --- a/sys/dev/sound/pcm/channel.h +++ b/sys/dev/sound/pcm/channel.h @@ -408,7 +408,7 @@ enum { #define CHN_F_RESET (CHN_F_BUSY | CHN_F_DEAD | \ CHN_F_VIRTUAL | CHN_F_HAS_VCHAN | \ - CHN_F_VCHAN_DYNAMIC | \ + CHN_F_VCHAN_DYNAMIC | CHN_F_NBIO | \ CHN_F_PASSTHROUGH | CHN_F_EXCLUSIVE) #define CHN_F_MMAP_INVALID (CHN_F_DEAD | CHN_F_RUNNING) diff --git a/sys/dev/sound/pcm/dsp.c b/sys/dev/sound/pcm/dsp.c index aa6ad4a59778..8a972b42a70d 100644 --- a/sys/dev/sound/pcm/dsp.c +++ b/sys/dev/sound/pcm/dsp.c @@ -222,6 +222,7 @@ dsp_chn_alloc(struct snddev_info *d, struct pcm_channel **ch, int direction, } (*ch)->flags |= CHN_F_BUSY; + (*ch)->flags &= ~CHN_F_NBIO; if (flags & O_NONBLOCK) (*ch)->flags |= CHN_F_NBIO; if (flags & O_EXCL) ---snip--- Can someone more familiar with OSS internals please comment? -- You are receiving this mail because: You are the assignee for the bug.
