Grokbase › Groups › Go › golang-dev › December 2012
FAQ
Reviewers: dvyukov, iant,

Message:
Hello dvyukov, iant (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/


Description:
sync/atomic: document that users must deal with 64-bit alignment

Update issue 599.

Please review this at https://codereview.appspot.com/7001056/

Affected files:
M src/pkg/sync/atomic/doc.go


Index: src/pkg/sync/atomic/doc.go
===================================================================
--- a/src/pkg/sync/atomic/doc.go
+++ b/src/pkg/sync/atomic/doc.go
@@ -41,6 +41,10 @@
// BUG(rsc): On ARM, the 64-bit functions use instructions unavailable
before ARM 11.
//
// On x86-32, the 64-bit functions use instructions unavailable before the
Pentium MMX.
+//
+// On both ARM and x86-32, it is the caller's responsibility to arrange
for 64-bit
+// alignment of 64-bit words accessed atomically. The first word in an
allocated
+// struct or slice can be relied upon to be 64-bit aligned.

// CompareAndSwapInt32 executes the compare-and-swap operation for an
int32 value.
func CompareAndSwapInt32(addr *int32, old, new int32) (swapped bool)

Search Discussions

  • Iant at Dec 23, 2012 at 6:59 am ⇧
    LGTM

    Can we provide any guarantees about int64 global variables?


    https://codereview.appspot.com/7001056/
  • Dmitry Vyukov at Dec 23, 2012 at 10:05 am ⇧
    We can fix it in the library. It's possible to implement atomic
    operations on non-aligned vars. All LOCK prefixed operations are
    atomic regardless of alignment. It will incur one additional branch
    for aligned vars, for non-aligned var it will LOCK prefixed
    instructions + there is huge overhead for LOCK prefixed operations
    that occupy 2 cache lines (it can be 5000+ cycles vs 15 cycles).
    I don't know about LL/SC on ARM.
    On Sun, Dec 23, 2012 at 10:59 AM, wrote:
    LGTM

    Can we provide any guarantees about int64 global variables?


    https://codereview.appspot.com/7001056/
  • Russ Cox at Dec 23, 2012 at 7:21 pm ⇧
    I'm happy to add global variables to the list of aligned things. We align
    all >=8-byte variables to 8-byte boundaries.

    "We can fix it" + "huge overhead" doesn't sound like a fix. I think it's
    fine to tell people who use sync/atomic that they have to work a little bit
    for it. Especially since it might not be possible on ARM or other future
    platforms.

    Russ
  • Dmitry Vyukov at Dec 24, 2012 at 8:07 am ⇧

    On Sun, Dec 23, 2012 at 11:21 PM, Russ Cox wrote:
    I'm happy to add global variables to the list of aligned things. We align
    all >=8-byte variables to 8-byte boundaries.

    "We can fix it" + "huge overhead" doesn't sound like a fix. I think it's
    fine to tell people who use sync/atomic that they have to work a little bit
    for it. Especially since it might not be possible on ARM or other future
    platforms.
    It's possible on any platform with global mutex hashtable. But it will
    be slower and not "lock-free".

    What I afraid is that it can lead to extremely hard to diagnose issues
    and the comment won't actually help people. Perhaps add a runtime
    check for alignment.
  • Minux at Dec 24, 2012 at 8:21 am ⇧

    On Monday, December 24, 2012, Dmitry Vyukov wrote:

    On Sun, Dec 23, 2012 at 11:21 PM, Russ Cox <rsc@golang.org <javascript:;>>
    wrote:
    I'm happy to add global variables to the list of aligned things. We align
    all >=8-byte variables to 8-byte boundaries.

    "We can fix it" + "huge overhead" doesn't sound like a fix. I think it's
    fine to tell people who use sync/atomic that they have to work a little bit
    for it. Especially since it might not be possible on ARM or other future
    platforms.
    It's possible on any platform with global mutex hashtable. But it will
    be slower and not "lock-free".
    using that defeated the very purpose of using atomic instructions.
    (we already uses that for 64-bit arm atomics in runtime unconditionally,
    and we should fix that to use native instr. or kernel helper if avail.
    i want to move part or all of core atomics package into runtime,
    as their functionality is currently duplicated in runtime. what do you
    think?)
    What I afraid is that it can lead to extremely hard to diagnose issues
    and the comment won't actually help people. Perhaps add a runtime
    check for alignment.
    agreed. we should panic if the address is not properly aligned.
    the problem is, should we panic on x86 where unaligned atomics
    is tolerated? i think we should, to provide an uniform api and help
    people who develop on 386 for arm.
  • Dmitry Vyukov at Dec 24, 2012 at 8:33 am ⇧

    On Mon, Dec 24, 2012 at 12:21 PM, minux wrote:
    I'm happy to add global variables to the list of aligned things. We
    align
    all >=8-byte variables to 8-byte boundaries.

    "We can fix it" + "huge overhead" doesn't sound like a fix. I think it's
    fine to tell people who use sync/atomic that they have to work a little
    bit
    for it. Especially since it might not be possible on ARM or other future
    platforms.
    It's possible on any platform with global mutex hashtable. But it will
    be slower and not "lock-free".
    using that defeated the very purpose of using atomic instructions.
    It's a difficult question, because hardware uses mutexes in implement
    atomic ops anyway :)
    However, of course if we add software mutexes, it will bring blocking
    to OS level.
    (we already uses that for 64-bit arm atomics in runtime unconditionally,
    and we should fix that to use native instr. or kernel helper if avail.
    i want to move part or all of core atomics package into runtime,
    as their functionality is currently duplicated in runtime. what do you
    think?)
    When I proposed it some time ago, Russ said that he does not want to
    unify them because one is C called and another is Go called.
    It's possible with an additional level of abstraction :) Maybe Russ
    has changed his mind...

    What I afraid is that it can lead to extremely hard to diagnose issues
    and the comment won't actually help people. Perhaps add a runtime
    check for alignment.
    agreed. we should panic if the address is not properly aligned.
    the problem is, should we panic on x86 where unaligned atomics
    is tolerated? i think we should, to provide an uniform api and help
    people who develop on 386 for arm.
    64-bit atomic load/store are not atomic on 386/amd64 (when unaligned).
    That was the original issue than I hit with GC on 386 with
    GOMAXPROCS>1. So there is the same problem.
  • Minux at Dec 24, 2012 at 9:23 am ⇧

    On Monday, December 24, 2012, Dmitry Vyukov wrote:
    64-bit atomic load/store are not atomic on 386/amd64 (when unaligned).
    That was the original issue than I hit with GC on 386 with
    GOMAXPROCS>1. So there is the same problem.
    i was confused by your saying "All LOCK prefixed operations are
    atomic regardless of alignment", so you mean the instruction itself
    is atomic whereas the memory access is not?
  • Dmitry Vyukov at Dec 24, 2012 at 9:28 am ⇧

    On Mon, Dec 24, 2012 at 1:23 PM, minux wrote:
    On Monday, December 24, 2012, Dmitry Vyukov wrote:

    64-bit atomic load/store are not atomic on 386/amd64 (when unaligned).
    That was the original issue than I hit with GC on 386 with
    GOMAXPROCS>1. So there is the same problem.
    i was confused by your saying "All LOCK prefixed operations are
    atomic regardless of alignment", so you mean the instruction itself
    is atomic whereas the memory access is not?
    64-bit atomic load on amd64/386 does not use LOCK prefixed instructions.
  • Minux at Dec 24, 2012 at 9:37 am ⇧

    On Monday, December 24, 2012, Dmitry Vyukov wrote:

    On Mon, Dec 24, 2012 at 1:23 PM, minux <minux.ma@gmail.com <javascript:;>>
    wrote:
    On Monday, December 24, 2012, Dmitry Vyukov wrote:
    64-bit atomic load/store are not atomic on 386/amd64 (when unaligned).
    That was the original issue than I hit with GC on 386 with
    GOMAXPROCS>1. So there is the same problem.
    i was confused by your saying "All LOCK prefixed operations are
    atomic regardless of alignment", so you mean the instruction itself
    is atomic whereas the memory access is not?
    64-bit atomic load on amd64/386 does not use LOCK prefixed instructions.
    but cas does use lock prefix. i think we can use cas to provide
    atomic loads just like we do on ARM? is that what you have in mind
    when saying we can fix the library?
  • Dmitry Vyukov at Dec 24, 2012 at 10:05 am ⇧

    On Mon, Dec 24, 2012 at 1:37 PM, minux wrote:
    On Monday, December 24, 2012, Dmitry Vyukov wrote:
    On Mon, Dec 24, 2012 at 1:23 PM, minux wrote:
    On Monday, December 24, 2012, Dmitry Vyukov wrote:
    64-bit atomic load/store are not atomic on 386/amd64 (when unaligned).
    That was the original issue than I hit with GC on 386 with
    GOMAXPROCS>1. So there is the same problem.
    i was confused by your saying "All LOCK prefixed operations are
    atomic regardless of alignment", so you mean the instruction itself
    is atomic whereas the memory access is not?
    64-bit atomic load on amd64/386 does not use LOCK prefixed instructions.
    but cas does use lock prefix. i think we can use cas to provide
    atomic loads just like we do on ARM? is that what you have in mind
    when saying we can fix the library?
    Yes, that's what I meant. It can hundreds of times slower.

    I think on ARM one needs just LDX, not the whole CAS. However I
    understand that there are issues with portability.
  • Minux Ma at Dec 23, 2012 at 1:14 pm ⇧
    https://codereview.appspot.com/7001056/diff/5001/src/pkg/sync/atomic/doc.go
    File src/pkg/sync/atomic/doc.go (right):

    https://codereview.appspot.com/7001056/diff/5001/src/pkg/sync/atomic/doc.go#newcode41
    src/pkg/sync/atomic/doc.go:41: // BUG(rsc): On ARM, the 64-bit functions
    use instructions unavailable before ARM 11.
    while you're at it, please remove this BUG, as it's no longer true
    for Linux/ARM (though it's still true for FreeBSD/ARM).

    https://codereview.appspot.com/7001056/
  • Minux Ma at Dec 23, 2012 at 1:22 pm ⇧

    On 2012/12/23 10:05:24, dvyukov wrote:
    We can fix it in the library. It's possible to implement atomic
    operations on non-aligned vars. All LOCK prefixed operations are
    atomic regardless of alignment. It will incur one additional branch
    for aligned vars, for non-aligned var it will LOCK prefixed
    instructions + there is huge overhead for LOCK prefixed operations
    that occupy 2 cache lines (it can be 5000+ cycles vs 15 cycles).
    I don't know about LL/SC on ARM.
    ARM ARM specifies LDREXD/STREXD must have 64-bit alignment,
    or alignment fault will result.

    https://codereview.appspot.com/7001056/

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedDec 22, '12 at 11:53p
activeDec 24, '12 at 10:05a
posts13
users4
websitegolang.org

4 users in discussion

Dmitry Vyukov: 5 posts Minux Ma: 5 posts Russ Cox: 2 posts Iant: 1 post

Content

People

Support

Translate

site design / logo © 2022 Grokbase