[PATCH] Witness breakage

On Wednesday 07 July 2004 02:20 pm, Daniel Lang wrote:
> Hi,
>
> as announced, here is the PR I just filed: kern/68779
> it includes a gdb stack trace and some very basic analysys.
> The crash dump and kernel are currently available, so if
> anyone is interested in some particular data, please let me
> know in the next few hours.
>
> Thanks and best regards,
>  Daniel

Ok, I think I've found at least one bug in witness that came in with the 
witness_checkorder() changes a few months ago that can be triggered by 
preemption because of thread migration.  For those seeing witness problems, 
please try this patch:

--- //depot/projects/smpng/sys/kern/subr_witness.c	2004/06/23 20:40:08
+++ //depot/user/jhb/lock/kern/subr_witness.c	2004/07/08 17:04:53
_at__at_ -701,22 +701,34 _at__at_
 	if (class->lc_flags & LC_SLEEPLOCK) {
 		/*
 		 * Since spin locks include a critical section, this check
-		 * impliclty enforces a lock order of all sleep locks before
+		 * impliclity enforces a lock order of all sleep locks before
 		 * all spin locks.
 		 */
 		if (td->td_critnest != 0)
 			panic("blockable sleep lock (%s) %s _at_ %s:%d",
 			    class->lc_name, lock->lo_name, file, line);
+
+		/*
+		 * If this is the first lock acquired then just return as
+		 * no order checking is needed.
+		 */
+		if (td->td_sleeplocks == NULL)
+			return;
 		lock_list = &td->td_sleeplocks;
-	} else
+	} else {
+		/*
+		 * If this is the first lock, just return as no order
+		 * checking is needed.  We check this in both if clauses
+		 * here as unifying the check would require us to use a
+		 * critical section to ensure we don't migrate while doing
+		 * the check.  Note that if this is not the first lock, we
+		 * are already in a critical section and are safe for the
+		 * rest of the check.
+		 */
+		if (PCPU_GET(spinlocks) == NULL)
+			return;
 		lock_list = PCPU_PTR(spinlocks);
-
-	/*
-	 * Is this the first lock acquired?  If so, then no order checking
-	 * is needed.
-	 */
-	if (*lock_list == NULL)
-		return;
+	}
 
 	/*
 	 * Check to see if we are recursing on a lock we already own.  If

-- 
John Baldwin <jhb_at_FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org