linux-arm-kernel.lists.infradead.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).