Re: Re: [PATCH] Fix the atomic compare and swap

On 5/21/07, John David Anglin <[email protected]> wrote:
> > --- ports/sysdeps/unix/sysv/linux/hppa/bits/atomic.h.orig     2007-05-20 23:15:37.000000000 +0200
> > +++ ports/sysdeps/unix/sysv/linux/hppa/bits/atomic.h  2007-05-20 23:15:59.000000000 +0200
> > @@ -71,10 +71,10 @@
> >       "copy   %5, %%r24                       \n\t"                   \
> >       "ble    " LWS "(%%sr2, %%r0)            \n\t"                   \
> >       "ldi    " LWS_CAS ", %%r20              \n\t"                   \
> > +        "sub %%r0, %%r21, %%r21              \n\t"                   \
> >       "cmpib,=,n " ASM_EAGAIN ",%%r21,0b      \n\t"                   \
>
> Hmmm, I think the original code was better since it keeps the sub
> instruction outside the loop.  The cmpib instruction could be modified
> to negate the ASM_EAGAIN.

The probability of the cmpib branching to 0b is directly proportional
to the contention on the kernel lock selected by the address hash.
Adding instructions to the loop is the equivalent of wasting time in
userpace, so isn't it better to do as much in the loop as possible? I
guess you waste memory bandwidth with the store.

> On the otherhand, I'm now thinking that macros like LWS, LWS_CAS and
> ASM_EAGAIN shouldn't be used as they are names that could be used in
> user code.

I hadn't considered this code would be an external API, but I guess it
is... so these defines should probably go away and the constants
merged into the code?

Cheers,
Carlos.