mutex patch for if_ndis

[Pierre Beyssac]

Hello,

The following patch for -current for if_ndis fixes a panic detected
by the INVARIANTS code, at the msleep in kern_ndis.c:1562.

> Assertion lock == sq->sq_lock failed at ../../../kern/subr_sleepqueue.c:286
> 
> sleepq_add(c1e4c560,c1f7ad68,c1f2bcac,c079edc0,0) at sleep_add+0x13d
> msleep(c1f7ad68,c1f2bcac,259,c079edc0,1f4) at msleep+0x228
> ndis_get_info(c1f7a000,10114,e4a76b30,e4a76b34,c0633b48) at ndis_get_info+0x114
> ndis_ifmedia_sts(c1f7a000,e4a76b30,0,c049f6bc,c0632d20) at ndis_ifmedia_sts+0x45

It seems that the nmb_wkupdpctimer sleep queue can be slept on
simultaneously by more than one thread, which causes sleepq_add()
to fail an assertion (the reason of the panic is that the process
going to sleep is not the process that acquired the lock).

The patch simply replaces the per-process mutex with a dedicated
mutex. It fixes the panic and I have tested it for several months
now but as I'm not a mutex expert, I'd like to confirm that it's a
suitable fix and that I've not left some obvious locking loophole
before I commit it.
-- 
Pierre Beyssac	      	    pb at fasterix.frmug.org pb at fasterix.freenix.org
    Free domains: http://www.eu.org/ or mail dns-manager at EU.org
-------------- next part --------------
--- kern_ndis.c.orig	Thu Sep  2 00:50:33 2004
+++ kern_ndis.c	Sat Sep 25 23:23:41 2004
@@ -110,6 +110,7 @@
 
 static uma_zone_t ndis_packet_zone, ndis_buffer_zone;
 struct mtx ndis_thr_mtx;
+struct mtx ndis_req_mtx;
 static STAILQ_HEAD(ndisqhead, ndis_req) ndis_ttodo;
 struct ndisqhead ndis_itodo;
 struct ndisqhead ndis_free;
@@ -259,6 +260,8 @@
 
 	mtx_init(&ndis_thr_mtx, "NDIS thread lock",
 	   MTX_NDIS_LOCK, MTX_DEF);
+	mtx_init(&ndis_req_mtx, "NDIS request lock",
+	   MTX_NDIS_LOCK, MTX_DEF);
 
 	STAILQ_INIT(&ndis_ttodo);
 	STAILQ_INIT(&ndis_itodo);
@@ -317,6 +320,7 @@
 		free(r, M_DEVBUF);
 	}
 
+	mtx_destroy(&ndis_req_mtx);
 	mtx_destroy(&ndis_thr_mtx);
 
 	return;
@@ -509,8 +513,8 @@
 {
 	int			error;
 
-	PROC_LOCK(p);
-	error = msleep(&p->p_siglist, &p->p_mtx,
+	mtx_lock(&ndis_req_mtx);
+	error = msleep(&p->p_siglist, &ndis_req_mtx,
 	    curthread->td_priority|PDROP, "ndissp", timo);
 	return(error);
 }
@@ -1138,9 +1142,9 @@
 	ntoskrnl_lower_irql(irql);
 
 	if (rval == NDIS_STATUS_PENDING) {
-		PROC_LOCK(curthread->td_proc);
+		mtx_lock(&ndis_req_mtx);
 		error = msleep(&sc->ndis_block.nmb_wkupdpctimer,
-		    &curthread->td_proc->p_mtx,
+		    &ndis_req_mtx,
 		    curthread->td_priority|PDROP,
 		    "ndisset", 5 * hz);
 		rval = sc->ndis_block.nmb_setstat;
@@ -1322,8 +1326,8 @@
 	ntoskrnl_lower_irql(irql);
 
 	if (rval == NDIS_STATUS_PENDING) {
-		PROC_LOCK(curthread->td_proc);
-		msleep(sc, &curthread->td_proc->p_mtx,
+		mtx_lock(&ndis_req_mtx);
+		msleep(sc, &ndis_req_mtx,
 		    curthread->td_priority|PDROP, "ndisrst", 0);
 	}
 
@@ -1558,9 +1562,9 @@
 	/* Wait for requests that block. */
 
 	if (rval == NDIS_STATUS_PENDING) {
-		PROC_LOCK(curthread->td_proc);
+		mtx_lock(&ndis_req_mtx);
 		error = msleep(&sc->ndis_block.nmb_wkupdpctimer,
-		    &curthread->td_proc->p_mtx,
+		    &ndis_req_mtx,
 		    curthread->td_priority|PDROP,
 		    "ndisget", 5 * hz);
 		rval = sc->ndis_block.nmb_getstat;