Date: Thu, 8 Jul 2004 13:10:37 -0400 From: John Baldwin <jhb@FreeBSD.org> To: freebsd-current@FreeBSD.org Cc: Daniel Lang <dl@leo.org> Subject: [PATCH] Witness breakage Message-ID: <200407081310.37603.jhb@FreeBSD.org> In-Reply-To: <20040707182018.GA45659@atrbg11.informatik.tu-muenchen.de> References: <20040707182018.GA45659@atrbg11.informatik.tu-muenchen.de>
next in thread | previous in thread | raw e-mail | index | archive | help
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
@@ -701,22 +701,34 @@
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 @ %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@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve" = http://www.FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200407081310.37603.jhb>