On 3/14/07, Johnny Billquist <bqt@softjar.se> wrote:
> Jason Thorpe wrote:
> >
> > On Mar 13, 2007, at 5:42 PM, Bang Jun-Young wrote:
> >
> >> On 3/13/07, Jason Thorpe <thorpej@shagadelic.org> wrote:
> >>
> >>> The API is borrowed from Solaris.  There is one major difference,
> >>> however.  Solaris defines "cas" as COMPARE-AND-SWAP, returning the
> >>> previous value of the memory cell.  I have defined it as COMPARE-AND-
> >>> STORE, returning true if the ops succeeds, or false otherwise.  The
> >>> reason for this is because some platforms will have extreme  difficulty
> >>> implementing COMPARE-AND-SWAP, whereas COMPARE-AND-STORE is somewhat
> >>> easier for these platforms.
> >>
> >> uint32_t
> >> atomic_compare_and_swap_32(volatile uint32_t *target, uint32_t cmp,
> >> uint32_t new)
> >> {
> >>    uint32_t old = *target;
> >>    (void) atomic_compare_and_store_32(target, cmp, new);
> >>    return old;
> >> }
> >>
> >> I think atomic_compare_and_swap_*() might not be so difficult to
> >> implement or
> >> expensive as you expected. :-)
> >
> > You're missing the memory barrier before fetching the old target.
> > Extra memory barriers are expensive.
>
> Memory barrier or not, I wouldn't expect the above code to work anyway.
> The value of the target could very well change between the read and the
> actual store.
> So you could actually have a situation where old == cmp, but the store
> didn't take place, since the value of target changed before the
> atomic_compare_and_store. Thus, you might think that you did a write,
> but in reality you didn't.
> The value of *target can change between the read and the atomic modify.
>
> Or am I missing something?

atomic_compare_and_store() doesn't guarantee that it returns the old
value of *target you expect, either. If *target was modified within
atomic_compare_and_store() before the actual atomic instruction was
executed, the modified value would be returned instead of the expected
one.

You should check the return value to detect if the call has been actually
successful. Here I'm reproducing my second mail for clarification:

       while (i > (max = arc.hash_chain_max) &&
           max != atomic_cas_32(&arc.hash_chain_max, max, i)) {
               continue;
       }

Jun-Young