All of lore.kernel.org
 help / color / mirror / Atom feed
From: jamie@shareable.org (Jamie Lokier)
To: linux-arm-kernel@lists.infradead.org
Subject: CAS implementation may be broken
Date: Mon, 23 Nov 2009 22:28:30 +0000	[thread overview]
Message-ID: <20091123222830.GA5598@shareable.org> (raw)
In-Reply-To: <4B08055C.3000408@45mercystreet.com>

Toby Douglass wrote:
> Load-linked/conditional-store architectures solve ABA by having the 
> store fail if the destination has been touched since the load was performed.
> 
> Currently, the code appears to violate this, by repeating the CAS 
> *anyway*.  In fact, the appropriate behaviour would seem to be *not* to 
> loop, but rather, to issue the ldrex/strex *once*, and indicate to the 
> user if the store succeed or failed.

I believe Catalin's explained why it does not work even doing
LDREX/STREX once, because the thread can pause before the LDREX.  So
you must begin fetching pointers after the LDREX.

(At least I think so.  I'm prepared to be shown to be wrong :-)

If you're writing code intended for other LL/SC architectures too, and
following Catalin's suggestion to put LDR between LDREX and STREX,
then you might have to check if the other architectures permit loads
between the LL and SC.

> This requires a prototype change, because the return value is the 
> original value of the destination and so is unable to indicate, when 
> returning that value, if it is returned from a successful or 
> unsuccessful swap.

Nonetheless, such a prototype change might be an improvement anyway.

Some platforms provide compare_and_swap_bool() operations, which do as
you describe: Compare, conditionally store, and return bool to indicate
success.  No loop.

That could be an improvement for some algorithms, because often if the
store doesn't happen, the *inputs* to compare_and_swap() need to be
recalculated anyway before another try is likely to succeed.  What's
the point in having code which expands to two loops:

    do {
       old = get_something;
       new = calc_something;

       /* oldval = compare_and_swap(ptr, old, new); */
       do {
           __asm__("LL/SC" : (failed), (oldval) : (ptr), (old), (new));
       } while (failed && oldval == old);

    } while (oldval != old);

When it can often be a smaller loop, which probably executes a little
faster too in various cases:

    do {
       old = get_something;
       new = calc_something;

       /* oldval = compare_and_swap_bool(ptr, old, new); */
       __asm__("LL/SC" : (failed), (oldval) : (ptr), (old), (new));

    } while (failed);

-- Jamie

  parent reply	other threads:[~2009-11-23 22:28 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-04 18:09 GCC built-in atomic operations and memory barriers Toby Douglass
2009-11-04 19:05 ` Russell King - ARM Linux
2009-11-04 20:12   ` Toby Douglass
2009-11-04 21:03     ` Russell King - ARM Linux
2009-11-06 19:10       ` Toby Douglass
2009-11-04 22:09   ` Gilles Chanteperdrix
2009-11-06 19:17     ` Toby Douglass
2009-11-21 15:21     ` CAS implementation may be broken Toby Douglass
2009-11-23 15:08       ` Russell King - ARM Linux
2009-11-23 19:10         ` Toby Douglass
2009-11-23 20:06           ` Russell King - ARM Linux
2009-11-23 20:34             ` Toby Douglass
2009-11-23 15:13       ` Catalin Marinas
2009-11-24 15:15         ` Toby Douglass
2009-11-24 15:36           ` Russell King - ARM Linux
2009-11-24 16:20             ` Toby Douglass
2009-11-24 16:27             ` Catalin Marinas
2009-11-24 17:14             ` Toby Douglass
2009-11-25  1:24           ` Jamie Lokier
2009-11-26 16:14             ` Toby Douglass
2009-11-27  1:37               ` Jamie Lokier
2009-11-24 15:33         ` Toby Douglass
2009-11-23 15:34       ` Catalin Marinas
2009-11-23 16:40         ` Toby Douglass
2009-11-23 22:28       ` Jamie Lokier [this message]
2009-11-23 23:13         ` Russell King - ARM Linux
2009-11-24  1:32           ` Jamie Lokier
2009-11-24 11:19             ` Catalin Marinas
2009-11-24 22:24               ` Toby Douglass
2009-11-25 11:11                 ` Catalin Marinas
2009-11-25 18:57                   ` Toby Douglass
2009-11-24 22:34               ` Toby Douglass
2009-11-24 22:56                 ` Russell King - ARM Linux
2009-11-25  0:34                   ` Toby Douglass
2009-11-24  9:38           ` Toby Douglass
2009-11-24 15:59         ` Catalin Marinas
2009-11-24 16:34         ` Toby Douglass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20091123222830.GA5598@shareable.org \
    --to=jamie@shareable.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.