From owner-svn-src-head@freebsd.org Sat Feb 18 22:59:46 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 B81E3CE5A0B; Sat, 18 Feb 2017 22:59:46 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-wr0-x243.google.com (mail-wr0-x243.google.com [IPv6:2a00:1450:400c:c0c::243]) (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 4E12E1D87; Sat, 18 Feb 2017 22:59:46 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by mail-wr0-x243.google.com with SMTP id q39so8557367wrb.2; Sat, 18 Feb 2017 14:59:46 -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:mail-followup-to:references :mime-version:content-disposition:in-reply-to:user-agent; bh=Xws76ODFOvxKhlWJVz2K0OnEKHYLRgsxuNXguUb3WaU=; b=riyFnzfzL2hULu2rgp9kiOP8/ODJC0CscYOz6boRBhc7YTiCzbfT/gYWSXdJnwNTpa A0oGw2dDiAggdJY+4oHzTG94g7io78mUzECG/UJ1ZL6F2qfg2uDynQaEneddT2e1VHBm 68dVhn4GfALV1LNtmTUAlSvdXkTgdF/HuvXN5VWioxIiRznwjq2WTJ0ZbNwXAZuj43R4 FpeXxlPI0/6jLg4Zn5eJRSFkpW2h8OaAJ0wpOkGfaSqQ55lZsLAWdX5VVk8uY4epDzf7 P0ZZhCXCbqLdmXsdDTGeMFJtwrv0mnRMic1fud9aLniSGSx7abcT63/X6e/vsrg0KNmE N4+A== 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 :mail-followup-to:references:mime-version:content-disposition :in-reply-to:user-agent; bh=Xws76ODFOvxKhlWJVz2K0OnEKHYLRgsxuNXguUb3WaU=; b=ZYZkV5dvokkN8SagGQefpl88qDgkoKDxFjIGbuephdjzSFfbBb0znS7zM0yl7H6clQ vuad3HCQ+KFp+0ftZ1LBtrSLq5CAZg7xBXihL6acX+JG1VMdb8gAJIjX2Te1JVXpNuhR 4vzaMgp6gVDnuZ20Ahv/LjKmSyLoyMIDvSrMUl7lQnd3o3JVV5u7G/ZYiwLzrl4litue 3sneRqBSgh6PKzqOKwGLZK00vRkDMKOdri9IiUAt6+oyX4yhilD+BsElSz0yJdMGThub M95arM8vknY8KHiJcHbf5HIcEClGMeIrHtex+QmET8k4R9E7MLzPgv3P2EKnxMzyV84n DLzg== X-Gm-Message-State: AMke39kfIPZ9h9w6lzdieIkiOt5xOUEYiDWc/Pn0qMGVwa+hbC/o5NfvWJdgvRcrRnNnBA== X-Received: by 10.223.150.110 with SMTP id c43mr4244778wra.5.1487458783727; Sat, 18 Feb 2017 14:59:43 -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 40sm18541534wry.22.2017.02.18.14.59.42 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Sat, 18 Feb 2017 14:59:42 -0800 (PST) Date: Sat, 18 Feb 2017 23:59:40 +0100 From: Mateusz Guzik To: Mark Millard Cc: mjg@freebsd.org, Justin Hibbits , svn-src-head@freebsd.org, FreeBSD PowerPC ML , FreeBSD Current Subject: Re: svn commit: r313268 - head/sys/kern [through -r313271 for atomic_fcmpset use and later: fails on PowerMac G5 "Quad Core"; -r313266 works] Message-ID: <20170218225940.GB24384@dft-labs.eu> Mail-Followup-To: Mateusz Guzik , Mark Millard , mjg@freebsd.org, Justin Hibbits , svn-src-head@freebsd.org, FreeBSD PowerPC ML , FreeBSD Current References: <2FD12B8F-2255-470A-98D4-2DCE9C7495F5@dsl-only.net> <20170218205825.GA24384@dft-labs.eu> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: 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: Sat, 18 Feb 2017 22:59:46 -0000 On Sat, Feb 18, 2017 at 01:58:49PM -0800, Mark Millard wrote: > On 2017-Feb-18, at 12:58 PM, Mateusz Guzik wrote: > > Well either the primitive itself is buggy or the somewhat (now) unusual > > condition of not providing the failed value (but possibly a stale one) > > is not handled correctly in locking code. > > > > That said, I would start with putting barriers "on both sides" of > > powerpc's fcmpset for debugging purposes and if the problem persists I > > can add some debugs to locking priitmives. > > > > I currently have the only powerpc64 that I have access > to for now doing a test that will likely finish tonight > sometime (if it has no problems). > > Also I'm not so familiar with powerpc64 details as to be > able insert proper barriers and the like off the top of > my head: It is more of a research subject for me. > This was a suggestion to jhibbits@. Looking at the code it is not hard to slap them in for testing purposes, or maybe there is an "obvious now that I look at it" braino in there, or maybe he has a better idea. Now that I wrote it I can get myself access to powerpc boxes. While I wont be able to run bsd on them, I can hack around in userapce and see. That's unless jhibbits@ steps in. I have no clue about ppc. > It looks like contexts like __rw_wlock_hard(c,v,tid,file,line) > now needs the caller to do an equivalent of: > > __rw_wlock_hard(c,RW_READ_VALUE(rwlock2rw(c)),file,line) > > in order for the code behavior to match the old behavior > that was based on the original local-v's initialization > before v was used: > > rw = rwlock2rw(c); > v = RW_READ_VALUE(rw); /* this line no longer exists */ > > This means that checking for equivalence is no longer > local to the routine but involves checking all the > usage of the routine. > Not reading the argument locally was the entire point of introducing fcmpset. Otherwise the 'v' argument would be a waste of time. Some primitives can attempt grabbing the lock and if they fail, we have the lock value to work with (e.g. check who owns the lock and see if they are running). In particular amd64 will give us the value it found. An explicit read requires whoever owns the cachelilne to lose the exclusive ownership and if the lock is contended (multiple cpus doing fcmpset), this makes the cachelilne ping-pong between cores. This destroys performance especially on systems with many cores and especially so with multiple numa nodes. Other primitives don't have inline variants. This concerns read-write locks which try to: retry: r = lock_value(lock); if (!locked(r)) { if (!cmpset(lock, r, r + ONE_READER)) goto retry; } That is, if multiple cpus try to get the lock for reading, one will fail and willl lbe forced to compute the new value to be set. The longer the time between attempts the more likely it is other core showed up trying to do the same thing with the same value, causing another failed attempt. So here there are no inlilnes so that the time is shorter and fcmpset alllows NOT reading the lock value explicitely - it was already provided by hardware. Note this is still significantly slower than it has to be in principle - the lock can 'blilndly increment by ONE_READER and see what happens', but that requires several changes and is a subject for another e-mail. I'm working on it though. -- Mateusz Guzik