From owner-freebsd-threads@FreeBSD.ORG Tue Dec 20 22:26:19 2011 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 12CFE106564A for ; Tue, 20 Dec 2011 22:26:19 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mx1.stack.nl (relay02.stack.nl [IPv6:2001:610:1108:5010::104]) by mx1.freebsd.org (Postfix) with ESMTP id A12528FC12 for ; Tue, 20 Dec 2011 22:26:18 +0000 (UTC) Received: from snail.stack.nl (snail.stack.nl [IPv6:2001:610:1108:5010::131]) by mx1.stack.nl (Postfix) with ESMTP id 0639F359311 for ; Tue, 20 Dec 2011 23:26:18 +0100 (CET) Received: by snail.stack.nl (Postfix, from userid 1677) id E204028468; Tue, 20 Dec 2011 23:26:17 +0100 (CET) Date: Tue, 20 Dec 2011 23:26:17 +0100 From: Jilles Tjoelker To: freebsd-threads@freebsd.org Message-ID: <20111220222617.GC75886@stack.nl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) Subject: [patch] Fix unchecked atomic_cmpset when unlocking mutex X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Dec 2011 22:26:19 -0000 When unlocking a contested mutex that is not PI or PP, the mutex is made available for new lockers faster by unlocking it in userland and only calling the kernel to wake sleeping threads. This seems to make sense. Although the atomic_cmpset here should never fail because this thread owns the mutex and other threads may only set UMUTEX_CONTESTED which is already set, it seems prudent anyway to check for this. If the atomic_cmpset fails, use the slow path that unlocks in the kernel. Index: lib/libthr/thread/thr_umtx.c =================================================================== --- lib/libthr/thread/thr_umtx.c (revision 228504) +++ lib/libthr/thread/thr_umtx.c (working copy) @@ -154,10 +154,9 @@ { #ifndef __ia64__ /* XXX this logic has a race-condition on ia64. */ - if ((mtx->m_flags & (UMUTEX_PRIO_PROTECT | UMUTEX_PRIO_INHERIT)) == 0) { - atomic_cmpset_rel_32(&mtx->m_owner, id | UMUTEX_CONTESTED, UMUTEX_CONTESTED); + if ((mtx->m_flags & (UMUTEX_PRIO_PROTECT | UMUTEX_PRIO_INHERIT)) == 0 && + atomic_cmpset_rel_32(&mtx->m_owner, id | UMUTEX_CONTESTED, UMUTEX_CONTESTED)) return _umtx_op_err(mtx, UMTX_OP_MUTEX_WAKE, 0, 0, 0); - } #endif /* __ia64__ */ return _umtx_op_err(mtx, UMTX_OP_MUTEX_UNLOCK, 0, 0, 0); } -- Jilles Tjoelker