Re: [PATCH 1/6] Optimize number of accesses to *adapt_count in lock elision code.

[Dominik Vogt <vogt at linux dot vnet dot ibm dot com>]

On Mon, Sep 02, 2013 at 02:07:25PM -0700, Andi Kleen wrote:
> Dominik Vogt <vogt@linux.vnet.ibm.com> writes:
> 
> > The changes to the original patches before 2.18 led to semantical changes of
> > the adapt_count argument.  In the current code, *adapt_count cannot fall below
> > zero.  When elision is attempted, *adapt_count is always exactly zero.  This
> > fact can be exploited to reduce the number of accesses to *adapt_count and to
> > simplify updating *adapt_count.
> 
> First in hardware <= is exactly the same performance as == 

Yes.  The patch changes that to "==" because it's clearer to the
reader that *adapt_count should not fall below 0 (and if it does,
it's a bug).  I understand that the "<=" is left over from earlier
versions of the patches when *adapt_count _could_ become less than
zero, but now that this is no longer the case it can be replaced
with "==".

> I don't think the > 0 changes are correct. Previously it was a edge
> trigger, you made it a level trigger, which is very different.

The old code

  else if (*adapt_count != aconf.skip_lock_internal_abort)
    *adapt_count = aconf.skip_lock_internal_abort;

would possibly first read *adapt_count.  Since the if-condition at
the start of the functions guarantees that *adapt_count is zero,
the assignement would be executed if

  0 != aconf.skip_lock_internal_abort

Assuming that aconf.skip_lock_internal_abort is always zero or
positive, this is equivalent to

  0 < aconf.skip_lock_internal_abort

I.e. the new code

  else if (aconf.skip_lock_internal_abort > 0)
    *adapt_count = aconf.skip_lock_internal_abort;

Just gets rid of the superfluous potential read access to
*adapt_count.  Superfluous because at that point in the code the
value of *adapt_count is known to be 0, which the compiler cannot
know if you have the "<=" condition at the start of the function.

> Also the compiler takes care of CSEing anyways, no need to 
> do it manually.

I see no reason to write code that is more complex than necessary
and then rely on compiler features to optimize away the inherent
performance penalties if the code can be written simpler, easier
to understand, and not relying on compiler features in the first
place.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany