From owner-svn-src-all@freebsd.org Tue Feb 7 14:57:55 2017 Return-Path: Delivered-To: svn-src-all@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 D12CBCD48C0; Tue, 7 Feb 2017 14:57:55 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-wm0-x242.google.com (mail-wm0-x242.google.com [IPv6:2a00:1450:400c:c09::242]) (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 3B87E1F8F; Tue, 7 Feb 2017 14:57:55 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by mail-wm0-x242.google.com with SMTP id c85so28778673wmi.1; Tue, 07 Feb 2017 06:57:55 -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=Oiyb5Dsp6PWIbbIreyhlFLh+joIJaJOb+9wekeAhX3A=; b=CNZnB2b17szKjrjE2Q7RC+539xvIWGbtnbPTxVxs7QKCY8iCo7MHByu8rd5kSrpmOc epC3mZrD4Jtlrscd2Zv7mcnPg6z82KI+9FKbYt0a5ErL0w3xrgZMBJR2anX0p48of4bH 1xPMPgIIVRs54GO9ajd8Fopt9b6SrZO2AwZ11H/5xf6lIAh84J7Rni2iczaHDQ36WuJs li0GYe7aL/49lDSsIs1FQMUOr++5yLgyARYv65O/VumezKpxYMGJ8f4roomTDTVL4iN5 XJ3438m4fxo1HzLqiYRAOcSwMzfK+DgCNOjb7v7k6A8coR49RFuY0kEDwtohFsMUgVEF nbhA== 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=Oiyb5Dsp6PWIbbIreyhlFLh+joIJaJOb+9wekeAhX3A=; b=OEHyZt1ny2RI4bLwjkWIrHUtdmrQ2tyf8l83Ok2DPtTALcb+3GQb0vD/fQNgU+Qu6O sdxx2p260GhLVA66pnNfAuKd2YAnFTK+steHutK0yT9U8J7nBukT9+8bLUKxsDNhuhnW UNgRDjWCqnrqbleD8W4ix/NYOHZCB8rhm86FwHcy8eFw7bg+Qgnrzr7+LWe+THMxpi1N fbRxTEBDRq0lDNYDbiThw+FSE2aka0DBQN1+PUh3MUCj96Z/slBr+4h3nI/O0hMpbdIv UzVNmaCruxPqsSEzo0IoxKXe78qQVj9ev8eiBZbEPZvdMhoV3nhkqF/uIcCyVCleldmN AUNQ== X-Gm-Message-State: AMke39m9VD2o9R4knGUeVFaTLlG5gEnyMH6fqyb1eURFA3yQXh91bzaxXmCIvxTCrvUNsQ== X-Received: by 10.28.47.15 with SMTP id v15mr12929996wmv.67.1486479473685; Tue, 07 Feb 2017 06:57:53 -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 x135sm19233327wme.23.2017.02.07.06.57.52 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Tue, 07 Feb 2017 06:57:53 -0800 (PST) Date: Tue, 7 Feb 2017 15:57:50 +0100 From: Mateusz Guzik To: Alexey Dokuchaev Cc: Steven Hartland , svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Mateusz Guzik Subject: Re: svn commit: r313260 - head/sys/kern Message-ID: <20170207145750.GD4772@dft-labs.eu> References: <201702050140.v151eRXX090326@repo.freebsd.org> <42c790bb-df12-2a50-6181-24bac5c72d34@multiplay.co.uk> <20170205030006.GB4375@dft-labs.eu> <20170205151746.GA6841@FreeBSD.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20170205151746.GA6841@FreeBSD.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 07 Feb 2017 14:57:55 -0000 On Sun, Feb 05, 2017 at 03:17:46PM +0000, Alexey Dokuchaev wrote: > On Sun, Feb 05, 2017 at 04:00:06AM +0100, Mateusz Guzik wrote: > > For instance, plugging an unused variable, a memory leak, doing a > > lockless check first etc. are all pretty standard and unless there is > > something unusual going on (e.g. complicated circumstances leading to a > > leak) there is not much to explain. In particular, I don't see why > > anyone would explain why leaks are bad on each commit plugging one. > > Right; these (unused variable, resource leaks) usually do not warrant > elaborate explanation. > > [ Some linefeeds below were trimmed for brevity ] > > The gist is as follows: there are plenty of cases where the kernel wants > > to atomically replace the value of a particular variable. Sometimes, > > like in this commit, we want to bump the counter by 1, but only if the > > current value is not 0. For that we need to read the value, see if it is > > 0 and if not, try to replace what we read with what we read + 1. We > > cannot just increment as the value could have changed to 0 in the > > meantime. > > But this also means that multiple cpus doing the same operation on the > > same variable will trip on each other - one will succeed while the rest > > will have to retry. > > Prior to this commit, each retry attempt would explicitly re-read the > > value. This induces cache coherency traffic slowing everyone down. > > amd64 has the nice property of giving us the value it found eleminating > > the need to explicitly re-read it. There is similar story on i386 and > > sparc. > > Other architectures may also benefit from this, but that I did not > > benchmark. > > > > In short[,] under contention atomic_fcmpset is going to be faster than > > atomic_cmpset. > > I did not benchmark this particular change, but a switch of the sort > > easily gives 10%+ in microbenchmarks on amd64. > > That said, while one can argue this optimizes the code, it really > > depessimizes it as something of the sort should have been already > > employed. > > Given the above, IMHO it's quite far from an obvious or of manpage-lookup > thing, and thus requires proper explanation in the commit log. > If the aformenteiond explanation is necessary, the place for it is in the man page. There are already several commits with fcmpset and there will be more to come. I don't see why any of them would convey the information. > > > While on this subject are there any official guidelines to writing > > > commit messages, if no should we create some? > > > > I'm unaware of any. > > We might not have official guidelines, but 30%-what/70%-why rule would > apply perfectly here. ;-) > > ./danfe -- Mateusz Guzik