PERFORCE change 101208 for review

[Paolo Pisati]

http://perforce.freebsd.org/chv.cgi?CH=101208

Change 101208 by piso at piso_newluxor on 2006/07/10 15:33:30

	Correct the execution logic of interrupts as pointed out
	by jhb:
	
	-if a filter completely served the interrupt (FILTER_HANDLED),
	return immediately and eoi the source.
	
	-if a filter wants to run the handler in ithread
	(FILTER_SCHEDULE_ITHREAD) stamp ih_need, mask & eoi the source 
	and schedule the ithread.
	
	-if no handlers with filter were present, then run
	all the filter-less handlers (if present).
	
	In kern_intr.c::ithread_execute_handlers() execute only
	sw & hw interrupts that have ih_need set.
	
	Unfortunately, the screen of my laptop still doesn't get 
	refreshed properly:
	
	it does boot fine, but after it mounts the disk, the screen
	isn't refreshed anymore up to the next printf() from kernel
	land.
	In fact, the only way to see what's going on is to break
	to ddb (CTRL+ALT+ESC), the debugger tries to print something 
	on the screen and magically all the output that was previously 
	missing gets flushed to screen and i can see that it reached 
	the login prompt fine, i can log, issue commands and so on
	(everything blindly of course) with no problems.

Affected files ...

.. //depot/projects/soc2006/intr_filter/i386/i386/intr_machdep.c#6 edit
.. //depot/projects/soc2006/intr_filter/kern/kern_intr.c#6 edit

Differences ...

==== //depot/projects/soc2006/intr_filter/i386/i386/intr_machdep.c#6 (text+ko) ====

@@ -215,18 +215,19 @@
 	thread = intr_filter_loop(ie, frame);
 	
 	/*
-	 * If there are any threaded handlers that need to run,
-	 * mask the source as well as sending it an EOI.  Otherwise,
-	 * just send it an EOI but leave it unmasked.
+	 * If the interrupt was fully served, send it an EOI but leave it 
+	 * unmasked. Otherwise, if there are any threaded handlers that need 
+	 * to run or it was a stray interrupt, mask the source as well as 
+	 * sending it an EOI.
 	 */
-	if (thread)
+	if (thread & FILTER_HANDLED)
+		isrc->is_pic->pic_eoi_source(isrc);		
+	else
 		isrc->is_pic->pic_disable_source(isrc, PIC_EOI);
-	else
-		isrc->is_pic->pic_eoi_source(isrc);
 	critical_exit();
 
 	/* Schedule the ithread if needed. */
-	if (thread) {
+	if (thread & FILTER_SCHEDULE_THREAD) {
 		error = intr_event_schedule_thread(ie);
 		KASSERT(error == 0, ("bad stray interrupt"));
 	}

==== //depot/projects/soc2006/intr_filter/kern/kern_intr.c#6 (text+ko) ====

@@ -638,29 +638,17 @@
 		}
 
 		/*
-                 * For software interrupt threads, we only execute
-                 * handlers that have their need flag set.  Hardware
-                 * interrupt threads always invoke all of their handlers.
+                 * Execute handlers that have their need flag set.
                  */		
-		if (ie->ie_flags & IE_SOFT) {
-			if (!ih->ih_need)
-				continue;
-			else
-				atomic_store_rel_int(&ih->ih_need, 0);
-		}
+		if (!ih->ih_need)
+			continue;
+		else
+			atomic_store_rel_int(&ih->ih_need, 0);
 
 		/* Fast handlers are handled in primary interrupt context. */
 		if (ih->ih_flags & IH_FAST)
                         continue;
 
-		/* Execute handlers of filters that need it. */
-		if (ih->ih_filter != NULL) {
-			if (!ih->ih_need)
-				continue;
-			else
-				atomic_store_rel_int(&ih->ih_need, 0);
-		}
-				
 		/* Execute this handler. */
 		CTR6(KTR_INTR, "%s: pid %d exec %p(%p) for %s flg=%x",
 		    __func__, p->p_pid, (void *)ih->ih_handler, ih->ih_argument,
@@ -785,10 +773,11 @@
 intr_filter_loop(struct intr_event *ie, struct trapframe *frame) {
 	struct intr_handler *ih;
 	void *arg;
-	int ret, ret2 = 0;
+	int ret, ret2, thread_only;
 
+	ret = 0;
+	thread_only = 0;
 	TAILQ_FOREACH(ih, &ie->ie_handlers, ih_next) {
-		ih->ih_need = 0;
 		/*
 		 * Execute fast interrupt handlers directly.
 		 * To support clock handlers, if a handler registers
@@ -801,28 +790,48 @@
 		     ih->ih_filter, ih->ih_handler, arg, ih->ih_name);
 
 		if (ih->ih_filter != NULL)
-			ret = ih->ih_filter(arg);
+			ret2 = ih->ih_filter(arg);
 		else { 		
-			/* legacy ithread handler */
-			ret2 = 1;
+			/* Legacy ithread only handler. */
+			thread_only = 1;
 			continue;
 		}
 
-		/* try with the next handler... */
-		if (ret == FILTER_STRAY) 
+		/* Mark handler for later execution in ithread. */
+		if (ret2 == FILTER_SCHEDULE_THREAD) {
+			ih->ih_need = 1; 
+			ret |= FILTER_SCHEDULE_THREAD;
 			continue;
+		}
+
+		/* Interrupt served in filter. */
+		if (ret2 == FILTER_HANDLED) {
+			ret |= FILTER_HANDLED;
+			return (ret);
+		}
+	}
 
-		/* interrupt served in filter */
-		if (ret == FILTER_HANDLED)
-			continue;
+	/*
+	 * A filter did claim the interrupt but didn't shut it up
+	 * fully, so schedule the ithread.
+	 */	
+	if (ret != 0)
+		return (ret);
 
-		/* mark handler for later execution */
-		if (ret == FILTER_SCHEDULE_THREAD) {
-			ih->ih_need = 1; 
-			ret2 = 1;
+	/*
+	 * No filters handled the interrupt and we have at least
+	 * one handler without a filter.  In this case, we schedule
+	 * all of the filter-less handlers to run in the ithread.
+	 */	
+	if (thread_only) {
+		TAILQ_FOREACH(ih, &ie->ie_handlers, ih_next) {
+			if (ih->ih_filter != NULL)
+				continue;
+			ih->ih_need = 1;
 		}
+		return (FILTER_SCHEDULE_THREAD);
 	}
-	return(ret2);
+	return (FILTER_STRAY);
 }