svn commit: r313260 - head/sys/kern

Steven Hartland steven.hartland at multiplay.co.uk
Tue Feb 7 23:40:56 UTC 2017


On 07/02/2017 20:34, Ed Maste wrote:
> On 7 February 2017 at 10:30, Steven Hartland
> <steven.hartland at 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 :)


More information about the svn-src-all mailing list