From owner-freebsd-current@FreeBSD.ORG  Fri Jul 20 02:17:07 2007
Return-Path: <owner-freebsd-current@FreeBSD.ORG>
Delivered-To: current@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34])
	by hub.freebsd.org (Postfix) with ESMTP id 9C32D16A419
	for <current@freebsd.org>; Fri, 20 Jul 2007 02:17:07 +0000 (UTC)
	(envelope-from thompsa@FreeBSD.org)
Received: from heff.fud.org.nz (203-109-251-39.static.bliink.ihug.co.nz
	[203.109.251.39])
	by mx1.freebsd.org (Postfix) with ESMTP id 38D1413C45B
	for <current@freebsd.org>; Fri, 20 Jul 2007 02:17:07 +0000 (UTC)
	(envelope-from thompsa@FreeBSD.org)
Received: by heff.fud.org.nz (Postfix, from userid 1001)
	id 5E4061CC58; Fri, 20 Jul 2007 14:17:06 +1200 (NZST)
Date: Fri, 20 Jul 2007 14:17:06 +1200
From: Andrew Thompson <thompsa@FreeBSD.org>
To: FreeBSD Current <current@freebsd.org>
Message-ID: <20070720021706.GB69737@heff.fud.org.nz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.5.13 (2006-08-11)
Cc: 
Subject: ndis kthread teardown
X-BeenThere: freebsd-current@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: Discussions about the use of FreeBSD-current
	<freebsd-current.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-current>, 
	<mailto:freebsd-current-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-current>
List-Post: <mailto:freebsd-current@freebsd.org>
List-Help: <mailto:freebsd-current-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-current>,
	<mailto:freebsd-current-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Fri, 20 Jul 2007 02:17:07 -0000

Hi,


There seems to be a race in the ndis code when stopping the
ntoskrnl_workitem threads. It sets a flag, wakes up the thread, and then
sleeps on the proc for the wakeup from kthread_exit.

I have found that the kthread can exit before
ntoskrnl_destroy_workitem_threads() gets to sleep on the proc, this
means that it will sleep indefinitely. Here is a patch that fixes it for
me, it reuses the kq_exit flag to identify that its already closed.


Andrew


Index: subr_ntoskrnl.c
===================================================================
RCS file: /home/ncvs/src/sys/compat/ndis/subr_ntoskrnl.c,v
retrieving revision 1.89
diff -u -p -r1.89 subr_ntoskrnl.c
--- subr_ntoskrnl.c	5 Jun 2007 00:00:50 -0000	1.89
+++ subr_ntoskrnl.c	20 Jul 2007 02:05:43 -0000
@@ -2778,6 +2778,7 @@ ntoskrnl_workitem_thread(arg)
 		KeAcquireSpinLock(&kq->kq_lock, &irql);
 
 		if (kq->kq_exit) {
+			kq->kq_exit = 0;
 			KeReleaseSpinLock(&kq->kq_lock, irql);
 			break;
 		}
@@ -2814,7 +2815,8 @@ ntoskrnl_destroy_workitem_threads(void)
 		kq = wq_queues + i;
 		kq->kq_exit = 1;
 		KeSetEvent(&kq->kq_proc, IO_NO_INCREMENT, FALSE);	
-		tsleep(kq->kq_td->td_proc, PWAIT, "waitiw", 0);
+		if (kq->kq_exit)
+			tsleep(kq->kq_td->td_proc, PWAIT, "waitiw", 0);
 	}
 
 	return;
@@ -3842,6 +3844,7 @@ ntoskrnl_dpc_thread(arg)
 		KeAcquireSpinLock(&kq->kq_lock, &irql);
 
 		if (kq->kq_exit) {
+			kq->kq_exit = 0;
 			KeReleaseSpinLock(&kq->kq_lock, irql);
 			break;
 		}
@@ -3891,7 +3894,8 @@ ntoskrnl_destroy_dpc_threads(void)
 		KeInitializeDpc(&dpc, NULL, NULL);
 		KeSetTargetProcessorDpc(&dpc, i);
 		KeInsertQueueDpc(&dpc, NULL, NULL);
-		tsleep(kq->kq_td->td_proc, PWAIT, "dpcw", 0);
+		if (kq->kq_exit)
+			tsleep(kq->kq_td->td_proc, PWAIT, "dpcw", 0);
 	}
 
 	return;