From owner-freebsd-current@FreeBSD.ORG Fri Jan 11 20:42:07 2013 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 4E250948 for ; Fri, 11 Jan 2013 20:42:07 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id 2B167F77 for ; Fri, 11 Jan 2013 20:42:07 +0000 (UTC) Received: from ralph.baldwin.cx (c-68-39-198-164.hsd1.de.comcast.net [68.39.198.164]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 99E83B9B9; Fri, 11 Jan 2013 15:42:06 -0500 (EST) From: John Baldwin To: freebsd-current@freebsd.org Subject: Re: Spurious witness warning when destroying spin mtx Date: Fri, 11 Jan 2013 11:44:09 -0500 User-Agent: KMail/1.13.7 (FreeBSD/9.1-PRERELEASE; KDE/4.8.4; amd64; ; ) References: In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201301111144.10060.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Fri, 11 Jan 2013 15:42:06 -0500 (EST) Cc: Ryan Stone X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.14 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: Fri, 11 Jan 2013 20:42:07 -0000 On Friday, November 23, 2012 10:08:28 PM Ryan Stone wrote: > Today I saw a spurious witness warning for "acquiring duplicate lock of > same type". The root cause is that when running mtx_destroy on a spinlock > that is held by the current thread, mtx_destroy calls spinlock_exit() > before calling WITNESS_UNLOCK, which opens up a window in which the CPU can > be interrupted and attempt to acquire another spinlock of the same type as > the one being destroyed. This patch should fix it: > > diff --git a/sys/kern/kern_mutex.c b/sys/kern/kern_mutex.c > index 2f13863..96f43f8 100644 > --- a/sys/kern/kern_mutex.c > +++ b/sys/kern/kern_mutex.c > @@ -918,16 +918,16 @@ _mtx_destroy(volatile uintptr_t *c) > else { > MPASS((m->mtx_lock & (MTX_RECURSED|MTX_CONTESTED)) == 0); > > + lock_profile_release_lock(&m->lock_object); > + /* Tell witness this isn't locked to make it happy. */ > + WITNESS_UNLOCK(&m->lock_object, LOP_EXCLUSIVE, __FILE__, > + __LINE__); > + > /* Perform the non-mtx related part of mtx_unlock_spin(). > */ if (LOCK_CLASS(&m->lock_object) == &lock_class_mtx_spin) > spinlock_exit(); > else > curthread->td_locks--; > - > - lock_profile_release_lock(&m->lock_object); > - /* Tell witness this isn't locked to make it happy. */ > - WITNESS_UNLOCK(&m->lock_object, LOP_EXCLUSIVE, __FILE__, > - __LINE__); > } > > m->mtx_lock = MTX_DESTROYED Ah, I would tweak this slightly perhaps to match mtx_unlock() and mtx_unlock_spin(): if (LOCK_CLASS() == &lock_class_mtx_sleep) curthread->td_locks--; /* lock profile and witness stuff */ if (LOCK_CLASS() == &lock_class_mtx_spin) spinlock_exit(); You are correct that it is broken for the spin case. -- John Baldwin