From owner-freebsd-current@freebsd.org Fri May 27 20:34:28 2016 Return-Path: Delivered-To: freebsd-current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id C8335B4D24E for ; Fri, 27 May 2016 20:34:28 +0000 (UTC) (envelope-from vangyzen@FreeBSD.org) Received: from smtp.vangyzen.net (hotblack.vangyzen.net [IPv6:2607:fc50:1000:7400:216:3eff:fe72:314f]) by mx1.freebsd.org (Postfix) with ESMTP id B4C011785 for ; Fri, 27 May 2016 20:34:28 +0000 (UTC) (envelope-from vangyzen@FreeBSD.org) Received: from sweettea.beer.town (unknown [76.164.8.130]) by smtp.vangyzen.net (Postfix) with ESMTPSA id 4B57856547; Fri, 27 May 2016 15:34:28 -0500 (CDT) Subject: Re: [PATCH] microoptimize locking primitives by avoiding unnecessary atomic ops To: Mateusz Guzik , freebsd-current@freebsd.org References: <20160527191700.GA23039@dft-labs.eu> From: Eric van Gyzen Message-ID: <84b595b1-7fb4-efd9-dc2f-9faac90da1e0@FreeBSD.org> Date: Fri, 27 May 2016 15:34:25 -0500 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 MIME-Version: 1.0 In-Reply-To: <20160527191700.GA23039@dft-labs.eu> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.22 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, 27 May 2016 20:34:28 -0000 On 05/27/16 02:17 PM, Mateusz Guzik wrote: > Hello there, > > quite some time ago I posted a trivial patch to locking primitives. What > they do is the inline part tries an atomic op and if that fails the > actual function is called, which immediately tries the same op. > > The obvious optimisation checks for the availability of the lock first. > > There concerns about the way it was done previously by relying on > volatile behaving in a specific way. > > Later a simplified version was posted which should not have the concern, > but the thread died. > > I refer you to https://lists.freebsd.org/pipermail/freebsd-current/2015-November/058100.html > for simple benchmark results. > > I would like to get the patch in before 11 freeze. This makes sense to me, and the patch looks good. Please consider adding a comment to each location that explains why the extra condition is tested before the atomic op. Without such a comment, I am concerned that your changes will be garbage collected later, because the extra condition would seem superfluous to someone less familiar with the code. Eric