Re: [PATCH] Improve spinlock inline assembly for x86.

On January 18, 2016 10:42:42 PM GMT+01:00, Kevin Grittner <kgrittn@gmail.com> wrote:
>On Mon, Jan 18, 2016 at 3:04 PM, Andres Freund <andres@anarazel.de>
>wrote:
>> On January 18, 2016 7:27:59 PM GMT+01:00, Robert Haas
><robertmhaas@gmail.com> wrote:
>>> On Sun, Jan 17, 2016 at 6:38 AM, Andreas Seltenreich
><seltenreich@gmx.de> wrote:
>
>>>> While discussing issues with its developers, it was pointed out to
>me
>>>> that our spinlock inline assembly is less than optimal.  Attached
>is
>>>> a patch that addresses this.
>
>>> I did a Google search and everybody seems to agree that the LOCK
>>> prefix is redundant.  I found a source agreeing with the idea that
>it
>>> doesn't clobber registers
>
>>> So I guess it would be safe to change this.  Scares me slightly,
>>> though.
>>
>> Clobbers IIRC are implicit on x86 anyway. Rather doubt that the
>> space for the prefix makes any sorry of practical difference, but
>> there indeed seems no reason to have it.
>
>I took a look at this and agree that the shorter, simpler code
>proposed in this patch should make no *logical* difference, and
>looks like it *should* have a neutral or beneficial affect; but
>performance tuning in general, and spinlock performance in
>particular, is full of surprises.  We have seen customers suffer
>poor scaling on their brand new monster machines because of the
>interaction between NUMA scheduling and our spinlock
>implementation, and seen those problems go away with an upgrade
>from pre-3.8 to post-3.8 kernels.  I would be hesitant to accept
>this change without seeing a benchmark on a large NUMA machine with
>4 or more memory nodes, on Linux kernels both before and after 3.8,
>to make sure that the effects are at least neutral.

Unconvinced. You get just as much churn by changing code elsewhere, which often causes code movement and alignment
changes.

--- 
Please excuse brevity and formatting - I am writing this on my mobile phone.