Re: Kernel panic during boot in usb_add_task

[Matthew Dillon <dillon@xxxxxxxxxxxxxxxxxxxx>]

:http://opengrok.creo.hu/dragonfly/xref/src/sys/bus/usb/usb.c#374
:
:More specifically:
:
:   http://opengrok.creo.hu/dragonfly/xref/src/sys/sys/queue.h#428
:
:   *(head)->tqh_last = (elm);
:
:This expands to:
:
:   *(&taskq->tasks)->tgh_last = task;
:
:There a NULL pointer is dereferenced somehow.

     It looks like memory corruption.  USB may be issuing duplicate TAILQ
     operations on its taskq structure(s).

     Please try this patch.  If it doesn't work then one or more task
     structure is probably being ripped out from under the usb code,
     probably by somthing related to uhci_timeout() as you previously
     reported.  My guess is that uhci_detach() is ripping the structure
     out without waiting for the abort to finish but lets see if this
     patch has any effect first.

						-Matt

Index: usb.c
===================================================================
RCS file: /cvs/src/sys/bus/usb/usb.c,v
retrieving revision 1.39
diff -u -p -r1.39 usb.c
--- usb.c	3 Jul 2007 19:28:16 -0000	1.39
+++ usb.c	27 Nov 2007 17:50:58 -0000
@@ -368,15 +368,21 @@ 	struct usb_taskq *taskq;
 
 	crit_enter();
 
+	/*
+	 * Wait if task is currently executing
+	 */
+	while (task->queue == -2)
+		tsleep(task, 0, "usbwttsk", hz);
+
 	taskq = &usb_taskq[queue];
 	if (task->queue == -1) {
 		DPRINTFN(2,("usb_add_task: task=%p\n", task));
 		TAILQ_INSERT_TAIL(&taskq->tasks, task, next);
 		task->queue = queue;
+		wakeup(&taskq->tasks);
 	} else {
 		DPRINTFN(3,("usb_add_task: task=%p on q\n", task));
 	}
-	wakeup(&taskq->tasks);
 
 	crit_exit();
 }
@@ -389,18 +395,27 @@ 	struct usb_taskq *taskq;
 
 	crit_enter();
 
+	/*
+	 * Wait if task is currently executing
+	 */
+	while (task->queue == -2)
+		tsleep(task, 0, "usbwttsk", hz);
+
 	taskq = &usb_taskq[queue];
 	if (task->queue == -1) {
 		DPRINTFN(2,("usb_add_task: task=%p\n", task));
 		TAILQ_INSERT_TAIL(&taskq->tasks, task, next);
 		task->queue = queue;
+		wakeup(&taskq->tasks);
 	} else {
 		DPRINTFN(3,("usb_add_task: task=%p on q\n", task));
 	}
-	wakeup(&taskq->tasks);
 
-	/* Wait until task is finished */
-	tsleep((&taskq->tasks + 1), 0, "usbdotsk", time_out);
+	/*
+	 * Wait until task is completely finished
+	 */
+	while (task->queue != -1)
+		tsleep(task, 0, "usbwttsk", hz);
 
 	crit_exit();
 }
@@ -409,6 +424,16 @@ void
 usb_rem_task(usbd_device_handle dev, struct usb_task *task)
 {
 	crit_enter();
+
+	/*
+	 * Wait if task is currently executing, we can't remove it now!
+	 */
+	while (task->queue == -2)
+		tsleep(task, 0, "usbwttsk", hz);
+
+	/*
+	 * Remove the task if it has not started executing yet.
+	 */
 	if (task->queue != -1) {
 		TAILQ_REMOVE(&usb_taskq[task->queue].tasks, task, next);
 		task->queue = -1;
@@ -475,20 +500,30 @@ 	taskq = arg;
 	DPRINTF(("usb_task_thread: start taskq %s\n", taskq->name));
 
 	while (usb_ndevs > 0) {
-		task = TAILQ_FIRST(&taskq->tasks);
-		if (task == NULL) {
+		/*
+		 * Wait for work
+		 */
+		if ((task = TAILQ_FIRST(&taskq->tasks)) == NULL) {
 			tsleep(&taskq->tasks, 0, "usbtsk", 0);
-			task = TAILQ_FIRST(&taskq->tasks);
+			continue;
 		}
+
+		/*
+		 * De-queue, mark as currently executing and run the
+		 * function.
+		 */
 		DPRINTFN(2,("usb_task_thread: woke up task=%p\n", task));
-		if (task != NULL) {
-			TAILQ_REMOVE(&taskq->tasks, task, next);
-			task->queue = -1;
-			crit_exit();
-			task->fun(task->arg);
-			crit_enter();
-			wakeup((&taskq->tasks + 1));
-		}
+		TAILQ_REMOVE(&taskq->tasks, task, next);
+		task->queue = -2;
+		crit_exit();
+		task->fun(task->arg);
+
+		/*
+		 * Mark as complete and wakeup anyone waiting.
+		 */
+		crit_enter();
+		task->queue = -1;
+		wakeup(task);
 	}
 
 	crit_exit();