From owner-svn-src-all@freebsd.org Tue Feb 7 23:40:56 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 AA539CD55EA for ; Tue, 7 Feb 2017 23:40:56 +0000 (UTC) (envelope-from steven.hartland@multiplay.co.uk) Received: from mail-wm0-x230.google.com (mail-wm0-x230.google.com [IPv6:2a00:1450:400c:c09::230]) (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 3875F137D for ; Tue, 7 Feb 2017 23:40:55 +0000 (UTC) (envelope-from steven.hartland@multiplay.co.uk) Received: by mail-wm0-x230.google.com with SMTP id c85so181571686wmi.1 for ; Tue, 07 Feb 2017 15:40:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=multiplay-co-uk.20150623.gappssmtp.com; s=20150623; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to; bh=W1GHPbX8CduBoDLkc6uCyrqF/x9FPHBb1xkVQj1Bmgc=; b=jiuQz41pPGL24KRHjryzVqX8czN0nmPOOaKYAEVWCI3WoNh3fXl5p808kTOuNjwIIq FIzs04s+bjYvusNJiNscjx90D94amSTbRelbP/g7lCVNHjnzz8bFdbL7IiFw8qziBbZX 8/VjlOIMz7+4Bvqd3MXtSOccAByOqdgR5ReHP7mgR3uoYz6+MI3Eczyw1//FIcoa7a19 +m4BC//7ZXg6JHTaSZfPB+4aoLsEN/ODe+rjNxpjoN7nfU3/7gIzbcKuXZoPgXgeGxUH GIYvDWvnEghAjJtlI1e6sM4BY+W2e2NfvqsWqGdb1fxg6YIxi8xq2GyEzL7mBzc7zSZ/ UsrQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to; bh=W1GHPbX8CduBoDLkc6uCyrqF/x9FPHBb1xkVQj1Bmgc=; b=P5zc5B3/zW3XdsLLWn06Sx5e5MeC/A7jWy19MssqbHDtcOC9xRJVXYqFITMAC3ibuf lbjn/+0HLdRlChmO6k4nx9kGLBOa4WeOJpJCK1fX9K6jCiUy7aQwpyZ3M+HIoW6dCOPZ ncgS/mjsWUVUM9WwGJPBVyO2ccciXFtGoK9+W08mFdnzadmUtTgkiPNM01/ESHh0D20p rG/o2FznTrucHS8PUDsqSpJCAmspgFxpArVw1QjEEv4dewilx2Jlh03alexs06XGQKo0 9wUkcq58+23LrGzaJ1wFpaxhsQ0Cwd4aPnEZ7FqlUDA3hpb0oOrAqmjGuLN/la0W9BU5 hjhA== X-Gm-Message-State: AMke39lCZmOAJ36r5vfMzlVlPxoGd/HntODPWyDNBcRIuxRwp+/1yssNxplzSOTRCfDsQ12D X-Received: by 10.28.18.130 with SMTP id 124mr16099656wms.8.1486510854307; Tue, 07 Feb 2017 15:40:54 -0800 (PST) Received: from [10.10.1.58] ([185.97.61.26]) by smtp.gmail.com with ESMTPSA id a13sm190495wma.13.2017.02.07.15.40.53 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 07 Feb 2017 15:40:53 -0800 (PST) Subject: Re: svn commit: r313260 - head/sys/kern To: Ed Maste References: <201702050140.v151eRXX090326@repo.freebsd.org> <42c790bb-df12-2a50-6181-24bac5c72d34@multiplay.co.uk> <20170205030006.GB4375@dft-labs.eu> <20170205151746.GA6841@FreeBSD.org> <20170207145750.GD4772@dft-labs.eu> <8e0f5a4c-16ec-842d-f4f7-32c830f43553@multiplay.co.uk> Cc: "svn-src-head@freebsd.org" , "svn-src-all@freebsd.org" , "src-committers@freebsd.org" , Mateusz Guzik From: Steven Hartland Message-ID: <1a7c31e9-561c-9391-47f1-f25e5381b03f@multiplay.co.uk> Date: Tue, 7 Feb 2017 23:40:53 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.23 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 23:40:56 -0000 On 07/02/2017 20:34, Ed Maste wrote: > On 7 February 2017 at 10:30, Steven Hartland > wrote: >> All I'm suggesting is, while one could guess this may be a performance or >> possibly a compatibility thing, the reason is not obvious, so a small piece >> of detail on why the change was done should always be included. >> >> For this one something like the following would be nice: >> >> Switch fget_unlocked to atomic_fcmpset >> >> Improve performance under contention by switching fget_unlocked to >> use atomic_fcmpset. > I agree, and one of the key reasons to do this is so that there's this > tiny bit of context if someone later runs "git blame" or "svn > annotate" and discovers this change for the line containing > atomic_fcmpset. Comments containing "eliminate memory leak" or "remove > unused variable" have a self-evident reason, but I don't believe > that's true for "switch to atomic_fcmpset." > > Repeating the "switch fget_unlocked to..." in the proposed commit > message above feels redundant to me though, and I would suggest: > > | Switch fget_unlocked to atomic_fcmpset > | > | Improves performance under contention. > > or just: > > | Use atmoic_fcmpset to improve performance under contention All those work for me as they clearly state why the change was made, so I hope this is something we can try to improve moving forward :)