kern/50613: unpausing audio(4) causes already written data to be dropped

[Onno van der Linden <o.vd.linden%quicknet.nl@localhost>]

>Number:         50613
>Category:       kern
>Synopsis:       unpausing audio(4) causes already written data to be dropped
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Jan 02 23:10:00 +0000 2016
>Originator:     Onno van der Linden
>Release:        NetBSD 7.99.25
>Organization:

>Environment:
	<The following information is extracted from your kernel. Please>
	<append output of "ldd", "ident" where relevant (multiple lines).>
System: NetBSD sheep 7.99.25 NetBSD 7.99.25 (SHEEP) #8: Sat Jan 2 18:34:49 MET 2016 onno@sheep:/usr/src/sys/arch/i386/compile/SHEEP i386
Architecture: i386
Machine: i386
>Description:
Writing several buffers to the audio device in a paused state and
unpausing after the kernel internal audio buffers are full will cause
the previously written buffers to be dropped.
>How-To-Repeat:
Port sndio-1.0.1 to NetBSD (pkgsrc-wip and a patch for libsndio/sio_sun.c from me) ,run the daemon on a kernel compiled with AUDIO_DEBUG=3 , play a wav with aucat from the same package and in /var/log/messages I see:

Jan  1 10:21:02 sheep /netbsd: audio_write: sc=0xc2c30800 count=3840 used=26880(hi=30720)
--- Notice the used=26880, 7 blocks have already been written to /dev/audio,
--- but the output hasn't started yet because the device is still paused
Jan  1 10:21:02 sheep /netbsd: audio_poll: events=0x1c mode=5
--- Here comes the call from sio_sun_autostart() to unpause
Jan  1 10:21:02 sheep /netbsd: audio_ioctl(136,'A',22)
Jan  1 10:21:02 sheep /netbsd: AUDIO_SETINFO mode=0x5
Jan  1 10:21:02 sheep /netbsd: audiosetinfo sc=0xc2c30800 ai=0xc2df2058
--- But before audio output is started, the ringbuffer gets a reset
--- which means that all the blocks written before in pause mode are lost
Jan  1 10:21:02 sheep /netbsd: audio_initbufs: mode=0x5
Jan  1 10:21:02 sheep /netbsd: audio_init_ringbuffer: MI blksize=3840
Jan  1 10:21:02 sheep /netbsd: audio_init_ringbuffer: final blksize=3840
Jan  1 10:21:02 sheep /netbsd: audio_init_ringbuffer: MI blksize=3840
Jan  1 10:21:02 sheep /netbsd: audio_init_ringbuffer: final blksize=3840
Jan  1 10:21:02 sheep /netbsd: audiostartp: start=0xda69c000 used=0(hi=30720 blk=3840) mmapped=0
--- Because of the used=0 audiostartp only does a "wakeup and return"
--- And now sndiod has a complete different view from the kernel resulting
--- in a watchdog timeout message from sndiod

>Fix:

The cause for this is the 1.196 commit (Sat Jun 11 08:14:19 2005 UTC)
saying 
"Make sure buffer sizes are initialized correctly even when the pause state
is explicitly set. Without this, the machine would crash in the audio interupt
when the driver needs to divide by the block size (e.g., cs4281.c/cs4280.c).
Idea for the fix by yamt."

Possible fix below but I wonder if it's really necessary.
Isn't audio_attach_mi() already setting the blocksize ?
Or is there a window of opportunity between enabling the
interrupt and audio_attach_mi() ?
audio_attach_mi() did exist when the 1.196 fix was made
so I might be wrong.


--- /usr/src/sys/dev/audio.c.orig	2015-12-31 16:27:13.000000000 +0100
+++ /usr/src/sys/dev/audio.c	2016-01-01 14:29:48.000000000 +0100
@@ -3579,7 +3579,7 @@
 	int oldpblksize, oldrblksize;
 	u_int gain;
 	bool rbus, pbus;
-	bool cleared, modechange, pausechange;
+	bool cleared, modechange, pausechange, initbufs;
 	u_char balance;
 
 	KASSERT(mutex_owned(sc->sc_lock));
@@ -3597,6 +3597,7 @@
 	cleared = false;
 	modechange = false;
 	pausechange = false;
+	initbufs = true;
 
 	pp = sc->sc_pparams;	/* Temporary encoding storage in */
 	rp = sc->sc_rparams;	/* case setting the modes fails. */
@@ -3830,11 +3831,15 @@
 		sc->sc_pr.pause = p->pause;
 		pbus = !p->pause;
 		pausechange = true;
+		if (sc->sc_pr.maxblks > 0)
+			initbufs = false;
 	}
 	if (SPECIFIED_CH(r->pause)) {
 		sc->sc_rr.pause = r->pause;
 		rbus = !r->pause;
 		pausechange = true;
+		if (sc->sc_rr.maxblks > 0)
+			initbufs = false;
 	}
 
 	if (SPECIFIED(ai->blocksize)) {
@@ -3898,7 +3903,7 @@
 		int init_error;
 
 		mutex_enter(sc->sc_intr_lock);
-		init_error = audio_initbufs(sc);
+		init_error = initbufs ? audio_initbufs(sc): 0;
 		if (init_error) goto err;
 		if (sc->sc_pr.blksize != oldpblksize ||
 		    sc->sc_rr.blksize != oldrblksize ||

>Unformatted:
 	<Please check that the above is correct for the bug being reported,>
 	<and append source date of snapshot, if applicable (one line).>