Date: Thu, 13 Jan 2005 23:15:46 +0100 From: Pierre Beyssac <pb@freebsd.org> To: freebsd-audit@freebsd.org Cc: wpaul@freebsd.org Subject: mutex patch for if_ndis Message-ID: <20050113221546.GA2988@fasterix.frmug.org>
next in thread | raw e-mail | index | archive | help
--pWyiEgJYm5f9v55/
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
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@fasterix.frmug.org pb@fasterix.freenix.org
Free domains: http://www.eu.org/ or mail dns-manager@EU.org
--pWyiEgJYm5f9v55/
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=patch-ndis
--- 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;
--pWyiEgJYm5f9v55/--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050113221546.GA2988>