Date: Tue, 7 Feb 2017 23:40:53 +0000 From: Steven Hartland <steven.hartland@multiplay.co.uk> To: Ed Maste <emaste@freebsd.org> Cc: "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, Mateusz Guzik <mjg@freebsd.org> Subject: Re: svn commit: r313260 - head/sys/kern Message-ID: <1a7c31e9-561c-9391-47f1-f25e5381b03f@multiplay.co.uk> In-Reply-To: <CAPyFy2CcS4GxS-7VgWM49teOT0ZR8s0TVC%2BvVbnDZg_caHO2VA@mail.gmail.com> 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> <CAPyFy2CcS4GxS-7VgWM49teOT0ZR8s0TVC%2BvVbnDZg_caHO2VA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 07/02/2017 20:34, Ed Maste wrote: > On 7 February 2017 at 10:30, Steven Hartland > <steven.hartland@multiplay.co.uk> 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 :)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1a7c31e9-561c-9391-47f1-f25e5381b03f>