Project

General

Profile

Actions

Submit #2804

closed

Blocking IO in sound subsystem.

Added by yellowrabbit2010 about 9 years ago. Updated over 8 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
Driver
Target version:
-
Start date:
03/12/2015
Due date:
% Done:

100%

Estimated time:

Description

Reapply old patch to the new sound system

commit 3e2b80a43e8d3778b07c4680c8a2719cd4c7f621
Author: Hasso Tepper <>
Date: Mon Oct 8 17:55:00 2007 +0000
Dragonfly always passes a flag for every IO operation depending whether
the mode of the operation or of the fd is set to NBIO, but it doesn't
pass down fcntl() changes to the drivers. So, if you open /dev/dsp with
NONBLOCK and later fcntl it to blocking, the sound driver won't be aware
of this fact.

Fix: don't maintain this setting in the sound driver.

Requested and tested by corecode@.


Files

sound.patch (645 Bytes) sound.patch yellowrabbit2010, 03/12/2015 09:57 PM
FIONBIO_test.tar.bz2 (6.7 KB) FIONBIO_test.tar.bz2 tests shamaz, 03/18/2015 07:22 AM
fionbio_master.patch (4.54 KB) fionbio_master.patch patch shamaz, 03/18/2015 07:22 AM
patch-sound_fix_nonblock.diff (3.84 KB) patch-sound_fix_nonblock.diff vadaszi, 03/18/2015 01:47 PM
Actions #1

Updated by shamaz about 9 years ago

Hello. To be honest, this solution seems to be more like a kludge. I think we should check the fcntl system call to see where it calls a specific ioctl method on the device. kern_fcntl in sys/kern/kern_descrip is good place to start. I'll try to make working patch in the day or two.

Updated by shamaz about 9 years ago

Well, this is what I got. My solution is somewhat ugly, but let you be the judge.

fionbio_master.patch makes fcntl call an appropriate ioctl on the file pointer if O_NONBLOCK flag is requested (patch to sys/kern/kern_descrip.c)
All other changes to the kernel were made to ioctl implementations to make them return 0 (no error) if a corresponding device or whatever supports non-blocking mode. These specific implementations do not need to do anything on their own, so they just return 0. PCM subsystem, on the other hand, finally executes its ioctl call and does everything it needs to switch back to non-blocking mode.

To be sure, that this does not break anything, I wrote a set of tests that checks if O_NONBLOCK flag can be set to any of the typical descriptor types: kqueues, pipes, sockets, vnodes (including regular files, some character/block devices, klog device) etc. All of them must silently return if executed (maybe with exception of klog, as you may have not permissions to open /dev/klog).

There is test_dsp.c in the set to test pcm subsystem. It must play 6 second silence and silently quit.

Actions #3

Updated by dillon about 9 years ago

I really don't want to change the fcntl path via fionbio_master.patch. That's a non-starter.

The patch in sound.patch is also a problem if there are multiple applications which have open()d the sound device in different ways because it appears to be modifying a shared structure.

The sound driver really has to handle the blocking or non-blocking I/O on a read-by-read and write-by-write basis, using the flags passed to read and write.

-Matt

Actions #4

Updated by vadaszi about 9 years ago

This patch checks ap->a_ioflag from dsp_read() and dsp_write()'s function argument for the O_NONBLOCK flag (after passing it through several layers of function calls). The CHN_F_NBIO flag that is internally kept by the sound subsystem (and displayed when doing cat /dev/sndstat after setting the hw.snd.verbose sysctl to some value >=2) is accordingly updated on each read/write call.

I had linked to this patch in the irc channel ca. 1 Month ago, and now I'm no longer sure if checking ap->a_ioflag for the O_NONBLOCK bit is actually correct. Most other drivers seem to check ap->a_ioflag for the IO_NDELAY bit instead.

Actions #5

Updated by yellowrabbit2010 over 8 years ago

  • Status changed from In Progress to Closed
  • % Done changed from 0 to 100

fixed by bb3a8342e9142d9204404a9e99d2c469a7dbf83f

Actions

Also available in: Atom PDF