5.2-RELEASE TODO

[Mathew Kanner]

On Dec 15, Munehiro Matsuda wrote:
> From: Peter Schultz <pmes at bis.midco.net>
> Date: Mon, 15 Dec 2003 09:47:58 -0600
> ::Robert Watson wrote:
> ::> Testing focuses for 5.2-RELEASE
> ::> 
> ::>  +---------------------------------------------------------------------------+
> ::>  |    Issue     |   Status    |Responsible|           Description            |
> ::>  |--------------+-------------+-----------+----------------------------------|
> ::>  |              |             |           |The PCM audio framework and device|
> ::>  |              |             |           |drivers have been locked and free |
> ::>  |PCM locking   |             |           |of Giant for quite a while, but   |
> ::>  |and           |Needs testing|--         |LOR problems persist along with   |
> ::>  |performance   |             |           |reports of poor audio performance |
> ::>  |issues        |             |           |under load. These problems are    |
> ::>  |              |             |           |believed to have been corrected,  |
> ::>  |              |             |           |but more testing is desired.      |
> ::>  |--------------+-------------+-----------+----------------------------------|
> :: >
> ::This longstanding problem has now been fixed as far as I'm concerned. 
> ::Xmms hasn't played mp3s without a hicup for me on -CURRENT since at 
> ::least a year, if not two or more.  It's been so long I can't recall.
> ::
> ::I'm testing with a soundblaster live! value card:
> ::pcm0: <Creative EMU10K1> port 0xef20-0xef3f irq 17 at device 17.0 on pci0
> ::pcm0: <SigmaTel STAC9721/23 AC97 Codec>
> ::
> ::I'm running a -CURRENT SMP kernel on a Tyan S1832DL w/dual PII 350s.  I 
> ::only assume it's working with 5.2 RC1.
> ::
> ::Pete...
> 
> Hi,
> 
> I still get LOR with my sound card and some hicup under disk load, with
> today's 5.2-CURRENT. It's a UP system with following sound card.
> 
> pcm0: <Yamaha DS-1E (YMF744)> port 0xfcac-0xfcaf,0xfc00-0xfc3f mem 0xfecf0000-0xfecf7fff irq 9 at device 9.0 on pci0
> pcm0: <Asahi Kasei AK4543 AC97 Codec>
> 
> Also, here's hand written LOR message:
> 
> lock order reversal
>  1st 0xc3405440 pcm0 (sound softc) @ dev/sound/pci/ds1.c:734
>  2nd 0xc3405080 pcm0:play:0 (pcm play channel) @ dev/sound/pcm/channel.c:480
> Stack backtrace:
> ....
> witness_lock(c3405080,0,c07a8705,1e0,c33f7300) at witness_lock+0x672
> _mtx_lock_flags(c3405080,0,c07a8705,1e0,0) at _mtx_lock_flags+0xba
> chn_intr(c33f7300,104,4,4,c3405240) at chn_intr+0x2f
> ds_intr(c340b4000,0,c07b3baa,21f,c334c1c4) at ds_intr+0xf5
> ithread_loop(c1856900,d03a7d48,c07b3a24,311,3473635f) at ithread_loop+0x192
> fork_exit(c059da00,c1856900,d03a7d48) at fork_exit+0xb4
> fork_trampoline() at fork_trampoline+0x8

	Hello,
	This LOR is mostly harmless.  Please try the attached patch
the avoid it.  Since this bug isn't critical, I won't commit it to 5.2
unless instructed otherwise by the -re team.

	Thanks,
	--Mat

-- 
	Having your book made into a movie is like having your ox
	made into a bouillon cube.
			- Bill Neely
-------------- next part --------------
Index: ds1.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/sound/pci/ds1.c,v
retrieving revision 1.36
diff -u -r1.36 ds1.c
--- ds1.c	2 Sep 2003 17:30:37 -0000	1.36
+++ ds1.c	15 Dec 2003 18:49:02 -0000
@@ -729,8 +729,15 @@
 ds_intr(void *p)
 {
 	struct sc_info *sc = (struct sc_info *)p;
-	u_int32_t i, x;
+	int i, pch, rch;
 
+	/*
+	 * In this particular scenerio, sc->lock only protects
+	 * against hardware access.  Other immutables, like the lock itself and
+	 * the pointers to the pcm channel structure can be accessed safely without
+	 * the lock.
+	 */
+	pch = rch = 0;
 	snd_mtxlock(sc->lock);
 	i = ds_rd(sc, YDSXGR_STATUS, 4);
 	if (i & 0x00008000)
@@ -739,25 +746,26 @@
 		ds_wr(sc, YDSXGR_STATUS, i & 0x80008000, 4);
 		sc->currbank = ds_rd(sc, YDSXGR_CTRLSELECT, 4) & 0x00000001;
 
-		x = 0;
-		for (i = 0; i < DS1_CHANS; i++) {
-			if (sc->pch[i].run) {
-				x = 1;
-				chn_intr(sc->pch[i].channel);
-			}
-		}
-		for (i = 0; i < 2; i++) {
-			if (sc->rch[i].run) {
-				x = 1;
-				chn_intr(sc->rch[i].channel);
-			}
-		}
+		for (i = 0; i < DS1_CHANS; i++)
+			if (sc->pch[i].run)
+				pch |= 1<<i;
+		for (i = 0; i < 2; i++)
+			if (sc->rch[i].run)
+				rch |= 1<<i;
 		i = ds_rd(sc, YDSXGR_MODE, 4);
-		if (x)
+		if ( pch || rch )
 			ds_wr(sc, YDSXGR_MODE, i | 0x00000002, 4);
 
 	}
 	snd_mtxunlock(sc->lock);
+
+	for (i = 0; i<DS1_CHANS; i++)
+		if ( pch & (1<<i) )
+			chn_intr(sc->pch[i].channel);
+	
+	for (i = 0; i<2; i++)
+		if ( rch & (1<<i) )
+			chn_intr(sc->rch[i].channel);
 }
 
 /* -------------------------------------------------------------------- */