From owner-freebsd-current@FreeBSD.ORG Sat Jan 21 17:26:58 2012 Return-Path: Delivered-To: current@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 2B32B1065674; Sat, 21 Jan 2012 17:26:58 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id 420448FC24; Sat, 21 Jan 2012 17:26:56 +0000 (UTC) Received: from porto.starpoint.kiev.ua (porto-e.starpoint.kiev.ua [212.40.38.100]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id TAA15641; Sat, 21 Jan 2012 19:26:56 +0200 (EET) (envelope-from avg@FreeBSD.org) Received: from localhost ([127.0.0.1]) by porto.starpoint.kiev.ua with esmtp (Exim 4.34 (FreeBSD)) id 1RoeiN-000JXG-Iu; Sat, 21 Jan 2012 19:26:55 +0200 Message-ID: <4F1AF55F.4090803@FreeBSD.org> Date: Sat, 21 Jan 2012 19:26:55 +0200 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:9.0) Gecko/20111222 Thunderbird/9.0 MIME-Version: 1.0 To: current@FreeBSD.org References: <20120117110242.GD12760@glebius.int.ru> <20120120154158.GD16676@FreeBSD.org> <4F1ABFF3.9090305@FreeBSD.org> <4F1ACD97.5080506@FreeBSD.org> In-Reply-To: <4F1ACD97.5080506@FreeBSD.org> X-Enigmail-Version: undefined Content-Type: text/plain; charset=KOI8-R Content-Transfer-Encoding: 7bit Cc: Gleb Smirnoff Subject: Re: locks under printf(9) and WITNESS [Was: new panic in cpu_reset() with WITNESS] X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 21 Jan 2012 17:26:58 -0000 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