Date: Sat, 21 Jan 2012 19:26:55 +0200 From: Andriy Gapon <avg@FreeBSD.org> To: current@FreeBSD.org Cc: Gleb Smirnoff <glebius@FreeBSD.org> Subject: Re: locks under printf(9) and WITNESS [Was: new panic in cpu_reset() with WITNESS] Message-ID: <4F1AF55F.4090803@FreeBSD.org> In-Reply-To: <4F1ACD97.5080506@FreeBSD.org> References: <20120117110242.GD12760@glebius.int.ru> <20120120154158.GD16676@FreeBSD.org> <4F1ABFF3.9090305@FreeBSD.org> <4F1ACD97.5080506@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
on 21/01/2012 16:37 Andriy Gapon said the following:
>
> BTW, we have a quite strange situation with spin locks in console output path.
> cnputs_mtx is marked as MTX_NOWITNESS, supposedly because cnputs (printf) can be
> called in any locking context (even during normal operation). But there are a
> number of console-specific locks (scrlock, uart_hwmtx, "syscons video lock")
> which are acquired under cnputs_mtx, but which are *not* marked as
> MTX_NOWITNESS. More, they are specified in the witness order_lists as if we
> certainly know a correct order for them.
> I think that the msgbuf mutex also deserves mentioning in this context.
>
> I think that all of these spin locks should be marked as MTX_NOWITNESS (as long
> as they stay normal spinlocks), because printf(9) should be usable wherever I
> stick it in the code.
>
> P.S. The above only matters for WITNESS and !WITNESS_SKIPSPIN and I am not sure
> if this combination really matters.
>
Here's my take at it:
diff --git a/sys/kern/kern_cons.c b/sys/kern/kern_cons.c
index 5346bc3..97f0f16 100644
--- a/sys/kern/kern_cons.c
+++ b/sys/kern/kern_cons.c
@@ -586,7 +586,7 @@ static void
cn_drvinit(void *unused)
{
- mtx_init(&cnputs_mtx, "cnputs_mtx", NULL, MTX_SPIN | MTX_NOWITNESS);
+ mtx_init(&cnputs_mtx, "cnputs_mtx", NULL, MTX_SPIN);
use_cnputs_mtx = 1;
}
diff --git a/sys/kern/subr_witness.c b/sys/kern/subr_witness.c
index 55cb2d7..39dd94d 100644
--- a/sys/kern/subr_witness.c
+++ b/sys/kern/subr_witness.c
@@ -629,7 +629,6 @@ static struct witness_order_list_entry order_lists[] = {
#endif
{ "rm.mutex_mtx", &lock_class_mtx_spin },
{ "sio", &lock_class_mtx_spin },
- { "scrlock", &lock_class_mtx_spin },
#ifdef __i386__
{ "cy", &lock_class_mtx_spin },
#endif
@@ -638,7 +637,6 @@ static struct witness_order_list_entry order_lists[] = {
{ "rtc_mtx", &lock_class_mtx_spin },
#endif
{ "scc_hwmtx", &lock_class_mtx_spin },
- { "uart_hwmtx", &lock_class_mtx_spin },
{ "fast_taskqueue", &lock_class_mtx_spin },
{ "intr table", &lock_class_mtx_spin },
#ifdef HWPMC_HOOKS
@@ -653,8 +651,8 @@ static struct witness_order_list_entry order_lists[] = {
{ "sched lock", &lock_class_mtx_spin },
{ "td_contested", &lock_class_mtx_spin },
{ "callout", &lock_class_mtx_spin },
+ { "et_hw_mtx", &lock_class_mtx_spin },
{ "entropy harvest mutex", &lock_class_mtx_spin },
- { "syscons video lock", &lock_class_mtx_spin },
#ifdef SMP
{ "smp rendezvous", &lock_class_mtx_spin },
#endif
@@ -662,6 +660,13 @@ static struct witness_order_list_entry order_lists[] = {
{ "tlb0", &lock_class_mtx_spin },
#endif
/*
+ * console locks
+ */
+ { "cnputs_mtx", &lock_class_mtx_spin },
+ { "scrlock", &lock_class_mtx_spin },
+ { "syscons video lock", &lock_class_mtx_spin },
+ { "uart_hwmtx", &lock_class_mtx_spin },
+ /*
* leaf locks
*/
{ "intrcnt", &lock_class_mtx_spin },
How does this look?
--
Andriy Gapon
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4F1AF55F.4090803>