From owner-svn-src-head@freebsd.org Thu Feb 2 01:42:10 2017 Return-Path: Delivered-To: svn-src-head@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 7B3BCCCB3DC; Thu, 2 Feb 2017 01:42:10 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-wm0-x244.google.com (mail-wm0-x244.google.com [IPv6:2a00:1450:400c:c09::244]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 0F63912D9; Thu, 2 Feb 2017 01:42:10 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by mail-wm0-x244.google.com with SMTP id u63so742613wmu.2; Wed, 01 Feb 2017 17:42:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=GSHFNumzS5QN/8ZgHZ7rCbyGYOgZ6o+180vaVjkPfkM=; b=kqcFT2x80zSKnDPZZwnckz2cxXHF1pzkI35cUAv/BtrNtMxakASprVnghcPJemMyzp NAR419MzKMgvfXEfjm15l/9xV00nKDI+GL+VOOBJgCzSVVhyxk6anUBNHlMK0sCpwiyW 79s0ztw3H+kVeqj4rlMyYu9CfCW1LNyWHjK30qM1Cak9HsbmMXIait6erwzP3+lmK9Bk cEDhnwUuURlA4Qzh2tKvjT6FnKMbcHA67xzHu8LgBeB00bNY1lbiCHpsJifACb58lVI5 Fv3BpspYylXhswdhwVjebFd9KydzMphNAIbBecrYy1Wbu/vEXOKmbzW85rx09DyijyGB Nyxw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=GSHFNumzS5QN/8ZgHZ7rCbyGYOgZ6o+180vaVjkPfkM=; b=Ze6rLca/n/iVajEjqFxZ5jCuCzkHzOvaeOELd7c+GmkB5c7eVKKrSQ5uyLmCCrAvun zU6AQpL1wwXWbqEO3+j+oXJsNpNYdVbgbmkwr1GmLsxEhuCVYk07aq3BpGD4KrgBaccg IrfBCzpHfBcvAeASANw7mM4Rl8A5kbBMbo5vEXL7igDzrmV52TGKwvnqqqF+7qw5/A6E y2TSIYOcttjgkdqb0qRxiEvpGTanhCHw2y5fPrxFX8BjIsGW4ACjcsKGuqmcH5Y36UsL 5UC86zpgUV8+Zl8VeH/b2OtkMbCPyOd3wCC85wHWB22gyzCp0wbYOotslFFC9eCTqqwJ FvAA== X-Gm-Message-State: AIkVDXJPzQZDcVeqgmEQz+lHWqcqFx9nz/nUJpycNo1YKofV8Edg22sTbugp9MM3zL1jeA== X-Received: by 10.28.209.202 with SMTP id i193mr5512474wmg.20.1485999728343; Wed, 01 Feb 2017 17:42:08 -0800 (PST) Received: from dft-labs.eu (n1x0n-1-pt.tunnel.tserv5.lon1.ipv6.he.net. [2001:470:1f08:1f7::2]) by smtp.gmail.com with ESMTPSA id c133sm262603wmd.13.2017.02.01.17.42.07 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Wed, 01 Feb 2017 17:42:07 -0800 (PST) Date: Thu, 2 Feb 2017 02:42:04 +0100 From: Mateusz Guzik To: Bruce Evans Cc: Mateusz Guzik , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r312975 - head/sys/i386/include Message-ID: <20170202014204.GA992@dft-labs.eu> References: <201701300224.v0U2Osj1010421@repo.freebsd.org> <20170130142123.V953@besplex.bde.org> <20170201214349.H1136@besplex.bde.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20170201214349.H1136@besplex.bde.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Feb 2017 01:42:10 -0000 On Wed, Feb 01, 2017 at 10:12:57PM +1100, Bruce Evans wrote: > On Mon, 30 Jan 2017, Bruce Evans wrote: > > >On Mon, 30 Jan 2017, Mateusz Guzik wrote: > > > >>Log: > >> i386: add atomic_fcmpset > >> > >> Tested by: pho > > > >This is has some bugs and style bugs. > > This is still broken. The invalid asm breaks building at least atomic.o > with gcc-4.2.1. > > Tested fix: > > X Index: i386/include/atomic.h > X =================================================================== > X --- i386/include/atomic.h (revision 313007) > X +++ i386/include/atomic.h (working copy) > X @@ -225,9 +225,9 @@ > X " cmpxchgl %3,%1 ; " > X " sete %0 ; " > X "# atomic_cmpset_int" > X - : "=r" (res), /* 0 */ > X + : "=q" (res), /* 0 */ > X "+m" (*dst), /* 1 */ > X - "+a" (*expect) /* 2 */ > X + "+a" (*expect) /* 2 */ > X : "r" (src) /* 3 */ > X : "memory", "cc"); > X return (res); > Uh, I ended up with the same fix. Committed in r313080. Sorry for breakage in the first place. > The semantics of fcmpset seem to be undocumented. On x86, *expect is > updated non-atomically by a store in the output parameter. I think > cmpxchg updates the "a" register atomically, but then the output > parameter causes this to be stored non-atomically to *expect. A better > API would somehow return the "a" register and let the caller store it > if it wants. Ordinary cmpset can be built on this by not storing, and > the caller can do the store atomically to a different place if *expect > is too volatile to be atomic. > The primitive was modeled after atomic_compare_exchange_* from c11 atomics. I don't see what's the benefit of storing the result separately. As it is, the primitive fits nicely into loops like "inc not zero". Like this: r = *counter; for (;;) { if (r == 0) break; if (atomic_fcmpset_int(counter, &r, r + 1)) break; // r we can loop back to re-test r } > Maybe just decouple the input parameter from the output parameter. The > following works right (for an amd64 API): > > Y static __inline int > Y atomic_xfcmpset_long(volatile u_long *dst, u_long *expect_out, u_long expect_in, > Y u_long src) > Y { > Y u_long expect; > Y u_char res; > Y Y expect = expect_in; > Y __asm __volatile( > Y " " MPLOCKED " " > Y " cmpxchgq %3,%1 ; " > Y " sete %0 ; " > Y "# atomic_fcmpset_long" > Y : "=r" (res), /* 0 */ > Y "+m" (*dst), /* 1 */ > Y "+a" (expect) /* 2 */ > Y : "r" (src) /* 3 */ > Y : "memory", "cc"); > Y *expect_out = expect; > > If the caller doesn't want to use *expect_out, it passes a pointer to an > unused local variable. The compiler can then optimize away the store > since it is not hidden in the asm. > _fcmpset is specifically for callers who want the value back. Ones which don't can use the _cmpset variant. -- Mateusz Guzik