svn commit: r313260 - head/sys/kern

[Steven Hartland]

On 05/02/2017 15:17, 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.
Indeed these are self explanatory
>> 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.
Absolutely, I would encourage everyone to not only think about others 
making similar changes but also providing education for those who may 
uses similar code in other areas.

If said changes where using older code as an example, without knowing 
otherwise they may not use the updated methodologies.

Sharing the detail you have done above is fantastic, allowing others to 
take note without having to do the research that the may well not have 
time for, with the result being improved code quality moving forward; so 
thanks for that :)

>
>>> 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. ;-)
>
Sounds like a good guide.

     Regards
     Steve