You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by jp...@apache.org on 2013/03/21 21:54:22 UTC

[1/3] git commit: Fix bug with 128 bit compare and swap.

Updated Branches:
  refs/heads/master 1a2ebccb1 -> f41323e01


Fix bug with 128 bit compare and swap.


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/4a090831
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/4a090831
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/4a090831

Branch: refs/heads/master
Commit: 4a0908314acb301f1e9eb691bbcd8d956e86c321
Parents: 57ffdf5
Author: jplevyak@apache.org <jp...@apache.org>
Authored: Thu Mar 21 10:10:16 2013 -0700
Committer: John Plevyak <jp...@acm.org>
Committed: Thu Mar 21 10:10:16 2013 -0700

----------------------------------------------------------------------
 lib/ts/ink_queue.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/4a090831/lib/ts/ink_queue.h
----------------------------------------------------------------------
diff --git a/lib/ts/ink_queue.h b/lib/ts/ink_queue.h
index 6ac9945..20fbf9a 100644
--- a/lib/ts/ink_queue.h
+++ b/lib/ts/ink_queue.h
@@ -72,7 +72,7 @@ extern "C"
 #endif
 
 #if TS_HAS_128BIT_CAS
-#define INK_QUEUE_LD(dst,src) *((__int128_t*)&(dst)) = *((__int128_t*)&(src))
+#define INK_QUEUE_LD(dst,src) *(__int128_t*)&(dst) = __sync_fetch_and_add((__int128_t*)&(src), 0)
 #else
 #define INK_QUEUE_LD(dst,src) INK_QUEUE_LD64(dst,src)
 #endif


Re: [1/3] git commit: Fix bug with 128 bit compare and swap.

Posted by James Peach <jp...@apache.org>.
On Mar 25, 2013, at 12:37 PM, Brian Geffon <br...@apache.org> wrote:

> That's great news. When you commit the change w/ sync_val_compare_and_swap
> will you reenable 128 bit freelist cas in configure.ac?

Done.

> 
> Brian
> 
> On Mon, Mar 25, 2013 at 12:26 PM, James Peach <jp...@apache.org> wrote:
> 
>> On Mar 25, 2013, at 11:46 AM, John Plevyak <jp...@acm.org> wrote:
>> 
>>> Sure, whatever works.  I did a little looking around, but I couldn't find
>>> any other way to do an atomic read of 128 bits on x86_64.
>> 
>> Yeh, __sync_val_compare_and_swap seems to work ...
>> 
>>> 
>>> john
>>> 
>>> On Mon, Mar 25, 2013 at 11:44 AM, Brian Geffon <br...@apache.org>
>> wrote:
>>> 
>>>> It turns out that __sync_fetch_and_add(__int128_t*, int) just uses
>>>> cmpxchg16b under the covers anyway...
>>>> 
>>>> int main ( void ) {
>>>> __int128_t src=10, dest=20;
>>>> *(__int128_t*)&(dest) = __sync_fetch_and_add((__int128_t*)&src, 0);
>>>> 
>>>> return 0;
>>>> }
>>>> 
>>>> When building using gcc -mcx16 -S -o test.S test.c, we get the following
>>>> assembly:
>>>> 
>>>> main:
>>>> .LFB0:
>>>>       .cfi_startproc
>>>>       pushq   %rbp
>>>>       .cfi_def_cfa_offset 16
>>>>       .cfi_offset 6, -16
>>>>       movq    %rsp, %rbp
>>>>       .cfi_def_cfa_register 6
>>>>       pushq   %rbx
>>>>       movq    $10, -32(%rbp)
>>>>       movq    $0, -24(%rbp)
>>>>       movq    $20, -48(%rbp)
>>>>       movq    $0, -40(%rbp)
>>>>       leaq    -32(%rbp), %r8
>>>>       movq    (%r8), %rax
>>>>       movq    8(%r8), %rdx
>>>> .L2:
>>>>       movq    %rax, %rsi
>>>>       movq    %rdx, %rdi
>>>>       movq    %rax, %rcx
>>>>       movq    %rdx, %rbx
>>>>       .cfi_offset 3, -24
>>>>       addq    $0, %rcx
>>>>       adcq    $0, %rbx
>>>>       movq    %rcx, %r9
>>>>       movq    %rbx, %rcx
>>>>       movq    %r9, %rbx
>>>>       lock cmpxchg16b (%r8)
>>>>       jne     .L2
>>>>       movq    %rsi, %rax
>>>>       movq    %rdi, %rdx
>>>>       movq    %rax, -48(%rbp)
>>>>       movq    %rdx, -40(%rbp)
>>>>       movl    $0, %eax
>>>>       popq    %rbx
>>>>       leave
>>>>       .cfi_def_cfa 7, 8
>>>>       ret
>>>>       .cfi_endproc
>>>> 
>>>> So instead of __sync_fetch_and_add() it appears we can use
>>>> __sync_val_compare_and_swap() which should have no problems with
>> clang...?
>>>> 
>>>> Brian
>>>> 
>>>> 
>>>> On Mon, Mar 25, 2013 at 8:43 AM, James Peach <jp...@apache.org> wrote:
>>>> 
>>>>> On Mar 24, 2013, at 2:11 PM, Brian Geffon <br...@apache.org> wrote:
>>>>> 
>>>>>> I'm confused, I reenabled 128bit cas in configure.ac and verified
>> that
>>>>> it
>>>>>> is building with cmpxchg16b and it's running perfectly. The
>>>> test_freelist
>>>>>> regression tests pass with no problems. So what were the issues
>>>> remaining
>>>>>> that required the revert? How can I possibly reproduce them so I can
>>>> get
>>>>>> this back on track?
>>>>> 
>>>>> From my testing, the changes from John and Weijin fixed all the tests.
>>>> The
>>>>> last remaining issue that I have is that John's fix doesn't build with
>>>>> clang. I guess that we can disable this code path with clang though ...
>>>>> 
>>>>>> 
>>>>>> Brian
>>>>>> 
>>>>>> 
>>>>>> On Fri, Mar 22, 2013 at 10:06 AM, James Peach <ja...@me.com>
>>>> wrote:
>>>>>> 
>>>>>>> On Mar 21, 2013, at 1:54 PM, jplevyak@apache.org wrote:
>>>>>>> 
>>>>>>>> Updated Branches:
>>>>>>>> refs/heads/master 1a2ebccb1 -> f41323e01
>>>>>>>> 
>>>>>>>> 
>>>>>>>> Fix bug with 128 bit compare and swap.
>>>>>>> 
>>>>>>> Hmm, this doesn't build with clang ... it makes clang generate a call
>>>>> to a
>>>>>>> ___sync_fetch_and_add_1 stub that doesn't exist:
>>>>>>> 
>>>>>>> Undefined symbols for architecture x86_64:
>>>>>>> "___sync_fetch_and_add_1", referenced from:
>>>>>>>    _ink_freelist_new in ink_queue.o
>>>>>>>    _ink_freelist_free in ink_queue.o
>>>>>>>    _ink_atomiclist_pop in ink_queue.o
>>>>>>>    _ink_atomiclist_popall in ink_queue.o
>>>>>>>    _ink_atomiclist_push in ink_queue.o
>>>>>>>    _ink_atomiclist_remove in ink_queue.o
>>>>>>> 
>>>>>>> I have an email out to the clang devs, but no response yet.
>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>>> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
>>>>>>>> Commit:
>>>>>>> http://git-wip-us.apache.org/repos/asf/trafficserver/commit/4a090831
>>>>>>>> Tree:
>>>>> http://git-wip-us.apache.org/repos/asf/trafficserver/tree/4a090831
>>>>>>>> Diff:
>>>>> http://git-wip-us.apache.org/repos/asf/trafficserver/diff/4a090831
>>>>>>>> 
>>>>>>>> Branch: refs/heads/master
>>>>>>>> Commit: 4a0908314acb301f1e9eb691bbcd8d956e86c321
>>>>>>>> Parents: 57ffdf5
>>>>>>>> Author: jplevyak@apache.org <jp...@apache.org>
>>>>>>>> Authored: Thu Mar 21 10:10:16 2013 -0700
>>>>>>>> Committer: John Plevyak <jp...@acm.org>
>>>>>>>> Committed: Thu Mar 21 10:10:16 2013 -0700
>>>>>>>> 
>>>>>>>> 
>>>> ----------------------------------------------------------------------
>>>>>>>> lib/ts/ink_queue.h |    2 +-
>>>>>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>>>>>> 
>>>> ----------------------------------------------------------------------
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>> 
>>>> 
>> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/4a090831/lib/ts/ink_queue.h
>>>>>>>> 
>>>> ----------------------------------------------------------------------
>>>>>>>> diff --git a/lib/ts/ink_queue.h b/lib/ts/ink_queue.h
>>>>>>>> index 6ac9945..20fbf9a 100644
>>>>>>>> --- a/lib/ts/ink_queue.h
>>>>>>>> +++ b/lib/ts/ink_queue.h
>>>>>>>> @@ -72,7 +72,7 @@ extern "C"
>>>>>>>> #endif
>>>>>>>> 
>>>>>>>> #if TS_HAS_128BIT_CAS
>>>>>>>> -#define INK_QUEUE_LD(dst,src) *((__int128_t*)&(dst)) =
>>>>>>> *((__int128_t*)&(src))
>>>>>>>> +#define INK_QUEUE_LD(dst,src) *(__int128_t*)&(dst) =
>>>>>>> __sync_fetch_and_add((__int128_t*)&(src), 0)
>>>>>>>> #else
>>>>>>>> #define INK_QUEUE_LD(dst,src) INK_QUEUE_LD64(dst,src)
>>>>>>>> #endif
>>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>> 
>>>>> 
>>>> 
>> 
>> 


Re: [1/3] git commit: Fix bug with 128 bit compare and swap.

Posted by Brian Geffon <br...@apache.org>.
That's great news. When you commit the change w/ sync_val_compare_and_swap
will you reenable 128 bit freelist cas in configure.ac?

Brian

On Mon, Mar 25, 2013 at 12:26 PM, James Peach <jp...@apache.org> wrote:

> On Mar 25, 2013, at 11:46 AM, John Plevyak <jp...@acm.org> wrote:
>
> > Sure, whatever works.  I did a little looking around, but I couldn't find
> > any other way to do an atomic read of 128 bits on x86_64.
>
> Yeh, __sync_val_compare_and_swap seems to work ...
>
> >
> > john
> >
> > On Mon, Mar 25, 2013 at 11:44 AM, Brian Geffon <br...@apache.org>
> wrote:
> >
> >> It turns out that __sync_fetch_and_add(__int128_t*, int) just uses
> >> cmpxchg16b under the covers anyway...
> >>
> >> int main ( void ) {
> >> __int128_t src=10, dest=20;
> >> *(__int128_t*)&(dest) = __sync_fetch_and_add((__int128_t*)&src, 0);
> >>
> >> return 0;
> >> }
> >>
> >> When building using gcc -mcx16 -S -o test.S test.c, we get the following
> >> assembly:
> >>
> >> main:
> >> .LFB0:
> >>        .cfi_startproc
> >>        pushq   %rbp
> >>        .cfi_def_cfa_offset 16
> >>        .cfi_offset 6, -16
> >>        movq    %rsp, %rbp
> >>        .cfi_def_cfa_register 6
> >>        pushq   %rbx
> >>        movq    $10, -32(%rbp)
> >>        movq    $0, -24(%rbp)
> >>        movq    $20, -48(%rbp)
> >>        movq    $0, -40(%rbp)
> >>        leaq    -32(%rbp), %r8
> >>        movq    (%r8), %rax
> >>        movq    8(%r8), %rdx
> >> .L2:
> >>        movq    %rax, %rsi
> >>        movq    %rdx, %rdi
> >>        movq    %rax, %rcx
> >>        movq    %rdx, %rbx
> >>        .cfi_offset 3, -24
> >>        addq    $0, %rcx
> >>        adcq    $0, %rbx
> >>        movq    %rcx, %r9
> >>        movq    %rbx, %rcx
> >>        movq    %r9, %rbx
> >>        lock cmpxchg16b (%r8)
> >>        jne     .L2
> >>        movq    %rsi, %rax
> >>        movq    %rdi, %rdx
> >>        movq    %rax, -48(%rbp)
> >>        movq    %rdx, -40(%rbp)
> >>        movl    $0, %eax
> >>        popq    %rbx
> >>        leave
> >>        .cfi_def_cfa 7, 8
> >>        ret
> >>        .cfi_endproc
> >>
> >> So instead of __sync_fetch_and_add() it appears we can use
> >> __sync_val_compare_and_swap() which should have no problems with
> clang...?
> >>
> >> Brian
> >>
> >>
> >> On Mon, Mar 25, 2013 at 8:43 AM, James Peach <jp...@apache.org> wrote:
> >>
> >>> On Mar 24, 2013, at 2:11 PM, Brian Geffon <br...@apache.org> wrote:
> >>>
> >>>> I'm confused, I reenabled 128bit cas in configure.ac and verified
> that
> >>> it
> >>>> is building with cmpxchg16b and it's running perfectly. The
> >> test_freelist
> >>>> regression tests pass with no problems. So what were the issues
> >> remaining
> >>>> that required the revert? How can I possibly reproduce them so I can
> >> get
> >>>> this back on track?
> >>>
> >>> From my testing, the changes from John and Weijin fixed all the tests.
> >> The
> >>> last remaining issue that I have is that John's fix doesn't build with
> >>> clang. I guess that we can disable this code path with clang though ...
> >>>
> >>>>
> >>>> Brian
> >>>>
> >>>>
> >>>> On Fri, Mar 22, 2013 at 10:06 AM, James Peach <ja...@me.com>
> >> wrote:
> >>>>
> >>>>> On Mar 21, 2013, at 1:54 PM, jplevyak@apache.org wrote:
> >>>>>
> >>>>>> Updated Branches:
> >>>>>> refs/heads/master 1a2ebccb1 -> f41323e01
> >>>>>>
> >>>>>>
> >>>>>> Fix bug with 128 bit compare and swap.
> >>>>>
> >>>>> Hmm, this doesn't build with clang ... it makes clang generate a call
> >>> to a
> >>>>> ___sync_fetch_and_add_1 stub that doesn't exist:
> >>>>>
> >>>>> Undefined symbols for architecture x86_64:
> >>>>> "___sync_fetch_and_add_1", referenced from:
> >>>>>     _ink_freelist_new in ink_queue.o
> >>>>>     _ink_freelist_free in ink_queue.o
> >>>>>     _ink_atomiclist_pop in ink_queue.o
> >>>>>     _ink_atomiclist_popall in ink_queue.o
> >>>>>     _ink_atomiclist_push in ink_queue.o
> >>>>>     _ink_atomiclist_remove in ink_queue.o
> >>>>>
> >>>>> I have an email out to the clang devs, but no response yet.
> >>>>>
> >>>>>>
> >>>>>>
> >>>>>> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> >>>>>> Commit:
> >>>>> http://git-wip-us.apache.org/repos/asf/trafficserver/commit/4a090831
> >>>>>> Tree:
> >>> http://git-wip-us.apache.org/repos/asf/trafficserver/tree/4a090831
> >>>>>> Diff:
> >>> http://git-wip-us.apache.org/repos/asf/trafficserver/diff/4a090831
> >>>>>>
> >>>>>> Branch: refs/heads/master
> >>>>>> Commit: 4a0908314acb301f1e9eb691bbcd8d956e86c321
> >>>>>> Parents: 57ffdf5
> >>>>>> Author: jplevyak@apache.org <jp...@apache.org>
> >>>>>> Authored: Thu Mar 21 10:10:16 2013 -0700
> >>>>>> Committer: John Plevyak <jp...@acm.org>
> >>>>>> Committed: Thu Mar 21 10:10:16 2013 -0700
> >>>>>>
> >>>>>>
> >> ----------------------------------------------------------------------
> >>>>>> lib/ts/ink_queue.h |    2 +-
> >>>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>>>>>
> >> ----------------------------------------------------------------------
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>
> >>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/4a090831/lib/ts/ink_queue.h
> >>>>>>
> >> ----------------------------------------------------------------------
> >>>>>> diff --git a/lib/ts/ink_queue.h b/lib/ts/ink_queue.h
> >>>>>> index 6ac9945..20fbf9a 100644
> >>>>>> --- a/lib/ts/ink_queue.h
> >>>>>> +++ b/lib/ts/ink_queue.h
> >>>>>> @@ -72,7 +72,7 @@ extern "C"
> >>>>>> #endif
> >>>>>>
> >>>>>> #if TS_HAS_128BIT_CAS
> >>>>>> -#define INK_QUEUE_LD(dst,src) *((__int128_t*)&(dst)) =
> >>>>> *((__int128_t*)&(src))
> >>>>>> +#define INK_QUEUE_LD(dst,src) *(__int128_t*)&(dst) =
> >>>>> __sync_fetch_and_add((__int128_t*)&(src), 0)
> >>>>>> #else
> >>>>>> #define INK_QUEUE_LD(dst,src) INK_QUEUE_LD64(dst,src)
> >>>>>> #endif
> >>>>>>
> >>>>>
> >>>>>
> >>>
> >>>
> >>
>
>

Re: [1/3] git commit: Fix bug with 128 bit compare and swap.

Posted by James Peach <jp...@apache.org>.
On Mar 25, 2013, at 11:46 AM, John Plevyak <jp...@acm.org> wrote:

> Sure, whatever works.  I did a little looking around, but I couldn't find
> any other way to do an atomic read of 128 bits on x86_64.

Yeh, __sync_val_compare_and_swap seems to work ...

> 
> john
> 
> On Mon, Mar 25, 2013 at 11:44 AM, Brian Geffon <br...@apache.org> wrote:
> 
>> It turns out that __sync_fetch_and_add(__int128_t*, int) just uses
>> cmpxchg16b under the covers anyway...
>> 
>> int main ( void ) {
>> __int128_t src=10, dest=20;
>> *(__int128_t*)&(dest) = __sync_fetch_and_add((__int128_t*)&src, 0);
>> 
>> return 0;
>> }
>> 
>> When building using gcc -mcx16 -S -o test.S test.c, we get the following
>> assembly:
>> 
>> main:
>> .LFB0:
>>        .cfi_startproc
>>        pushq   %rbp
>>        .cfi_def_cfa_offset 16
>>        .cfi_offset 6, -16
>>        movq    %rsp, %rbp
>>        .cfi_def_cfa_register 6
>>        pushq   %rbx
>>        movq    $10, -32(%rbp)
>>        movq    $0, -24(%rbp)
>>        movq    $20, -48(%rbp)
>>        movq    $0, -40(%rbp)
>>        leaq    -32(%rbp), %r8
>>        movq    (%r8), %rax
>>        movq    8(%r8), %rdx
>> .L2:
>>        movq    %rax, %rsi
>>        movq    %rdx, %rdi
>>        movq    %rax, %rcx
>>        movq    %rdx, %rbx
>>        .cfi_offset 3, -24
>>        addq    $0, %rcx
>>        adcq    $0, %rbx
>>        movq    %rcx, %r9
>>        movq    %rbx, %rcx
>>        movq    %r9, %rbx
>>        lock cmpxchg16b (%r8)
>>        jne     .L2
>>        movq    %rsi, %rax
>>        movq    %rdi, %rdx
>>        movq    %rax, -48(%rbp)
>>        movq    %rdx, -40(%rbp)
>>        movl    $0, %eax
>>        popq    %rbx
>>        leave
>>        .cfi_def_cfa 7, 8
>>        ret
>>        .cfi_endproc
>> 
>> So instead of __sync_fetch_and_add() it appears we can use
>> __sync_val_compare_and_swap() which should have no problems with clang...?
>> 
>> Brian
>> 
>> 
>> On Mon, Mar 25, 2013 at 8:43 AM, James Peach <jp...@apache.org> wrote:
>> 
>>> On Mar 24, 2013, at 2:11 PM, Brian Geffon <br...@apache.org> wrote:
>>> 
>>>> I'm confused, I reenabled 128bit cas in configure.ac and verified that
>>> it
>>>> is building with cmpxchg16b and it's running perfectly. The
>> test_freelist
>>>> regression tests pass with no problems. So what were the issues
>> remaining
>>>> that required the revert? How can I possibly reproduce them so I can
>> get
>>>> this back on track?
>>> 
>>> From my testing, the changes from John and Weijin fixed all the tests.
>> The
>>> last remaining issue that I have is that John's fix doesn't build with
>>> clang. I guess that we can disable this code path with clang though ...
>>> 
>>>> 
>>>> Brian
>>>> 
>>>> 
>>>> On Fri, Mar 22, 2013 at 10:06 AM, James Peach <ja...@me.com>
>> wrote:
>>>> 
>>>>> On Mar 21, 2013, at 1:54 PM, jplevyak@apache.org wrote:
>>>>> 
>>>>>> Updated Branches:
>>>>>> refs/heads/master 1a2ebccb1 -> f41323e01
>>>>>> 
>>>>>> 
>>>>>> Fix bug with 128 bit compare and swap.
>>>>> 
>>>>> Hmm, this doesn't build with clang ... it makes clang generate a call
>>> to a
>>>>> ___sync_fetch_and_add_1 stub that doesn't exist:
>>>>> 
>>>>> Undefined symbols for architecture x86_64:
>>>>> "___sync_fetch_and_add_1", referenced from:
>>>>>     _ink_freelist_new in ink_queue.o
>>>>>     _ink_freelist_free in ink_queue.o
>>>>>     _ink_atomiclist_pop in ink_queue.o
>>>>>     _ink_atomiclist_popall in ink_queue.o
>>>>>     _ink_atomiclist_push in ink_queue.o
>>>>>     _ink_atomiclist_remove in ink_queue.o
>>>>> 
>>>>> I have an email out to the clang devs, but no response yet.
>>>>> 
>>>>>> 
>>>>>> 
>>>>>> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
>>>>>> Commit:
>>>>> http://git-wip-us.apache.org/repos/asf/trafficserver/commit/4a090831
>>>>>> Tree:
>>> http://git-wip-us.apache.org/repos/asf/trafficserver/tree/4a090831
>>>>>> Diff:
>>> http://git-wip-us.apache.org/repos/asf/trafficserver/diff/4a090831
>>>>>> 
>>>>>> Branch: refs/heads/master
>>>>>> Commit: 4a0908314acb301f1e9eb691bbcd8d956e86c321
>>>>>> Parents: 57ffdf5
>>>>>> Author: jplevyak@apache.org <jp...@apache.org>
>>>>>> Authored: Thu Mar 21 10:10:16 2013 -0700
>>>>>> Committer: John Plevyak <jp...@acm.org>
>>>>>> Committed: Thu Mar 21 10:10:16 2013 -0700
>>>>>> 
>>>>>> 
>> ----------------------------------------------------------------------
>>>>>> lib/ts/ink_queue.h |    2 +-
>>>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>>>> 
>> ----------------------------------------------------------------------
>>>>>> 
>>>>>> 
>>>>>> 
>>>>> 
>>> 
>> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/4a090831/lib/ts/ink_queue.h
>>>>>> 
>> ----------------------------------------------------------------------
>>>>>> diff --git a/lib/ts/ink_queue.h b/lib/ts/ink_queue.h
>>>>>> index 6ac9945..20fbf9a 100644
>>>>>> --- a/lib/ts/ink_queue.h
>>>>>> +++ b/lib/ts/ink_queue.h
>>>>>> @@ -72,7 +72,7 @@ extern "C"
>>>>>> #endif
>>>>>> 
>>>>>> #if TS_HAS_128BIT_CAS
>>>>>> -#define INK_QUEUE_LD(dst,src) *((__int128_t*)&(dst)) =
>>>>> *((__int128_t*)&(src))
>>>>>> +#define INK_QUEUE_LD(dst,src) *(__int128_t*)&(dst) =
>>>>> __sync_fetch_and_add((__int128_t*)&(src), 0)
>>>>>> #else
>>>>>> #define INK_QUEUE_LD(dst,src) INK_QUEUE_LD64(dst,src)
>>>>>> #endif
>>>>>> 
>>>>> 
>>>>> 
>>> 
>>> 
>> 


Re: [1/3] git commit: Fix bug with 128 bit compare and swap.

Posted by John Plevyak <jp...@acm.org>.
Sure, whatever works.  I did a little looking around, but I couldn't find
any other way to do an atomic read of 128 bits on x86_64.

john

On Mon, Mar 25, 2013 at 11:44 AM, Brian Geffon <br...@apache.org> wrote:

> It turns out that __sync_fetch_and_add(__int128_t*, int) just uses
> cmpxchg16b under the covers anyway...
>
> int main ( void ) {
> __int128_t src=10, dest=20;
> *(__int128_t*)&(dest) = __sync_fetch_and_add((__int128_t*)&src, 0);
>
> return 0;
> }
>
> When building using gcc -mcx16 -S -o test.S test.c, we get the following
> assembly:
>
> main:
> .LFB0:
>         .cfi_startproc
>         pushq   %rbp
>         .cfi_def_cfa_offset 16
>         .cfi_offset 6, -16
>         movq    %rsp, %rbp
>         .cfi_def_cfa_register 6
>         pushq   %rbx
>         movq    $10, -32(%rbp)
>         movq    $0, -24(%rbp)
>         movq    $20, -48(%rbp)
>         movq    $0, -40(%rbp)
>         leaq    -32(%rbp), %r8
>         movq    (%r8), %rax
>         movq    8(%r8), %rdx
> .L2:
>         movq    %rax, %rsi
>         movq    %rdx, %rdi
>         movq    %rax, %rcx
>         movq    %rdx, %rbx
>         .cfi_offset 3, -24
>         addq    $0, %rcx
>         adcq    $0, %rbx
>         movq    %rcx, %r9
>         movq    %rbx, %rcx
>         movq    %r9, %rbx
>         lock cmpxchg16b (%r8)
>         jne     .L2
>         movq    %rsi, %rax
>         movq    %rdi, %rdx
>         movq    %rax, -48(%rbp)
>         movq    %rdx, -40(%rbp)
>         movl    $0, %eax
>         popq    %rbx
>         leave
>         .cfi_def_cfa 7, 8
>         ret
>         .cfi_endproc
>
> So instead of __sync_fetch_and_add() it appears we can use
> __sync_val_compare_and_swap() which should have no problems with clang...?
>
> Brian
>
>
> On Mon, Mar 25, 2013 at 8:43 AM, James Peach <jp...@apache.org> wrote:
>
> > On Mar 24, 2013, at 2:11 PM, Brian Geffon <br...@apache.org> wrote:
> >
> > > I'm confused, I reenabled 128bit cas in configure.ac and verified that
> > it
> > > is building with cmpxchg16b and it's running perfectly. The
> test_freelist
> > > regression tests pass with no problems. So what were the issues
> remaining
> > > that required the revert? How can I possibly reproduce them so I can
> get
> > > this back on track?
> >
> > From my testing, the changes from John and Weijin fixed all the tests.
> The
> > last remaining issue that I have is that John's fix doesn't build with
> > clang. I guess that we can disable this code path with clang though ...
> >
> > >
> > > Brian
> > >
> > >
> > > On Fri, Mar 22, 2013 at 10:06 AM, James Peach <ja...@me.com>
> wrote:
> > >
> > >> On Mar 21, 2013, at 1:54 PM, jplevyak@apache.org wrote:
> > >>
> > >>> Updated Branches:
> > >>> refs/heads/master 1a2ebccb1 -> f41323e01
> > >>>
> > >>>
> > >>> Fix bug with 128 bit compare and swap.
> > >>
> > >> Hmm, this doesn't build with clang ... it makes clang generate a call
> > to a
> > >> ___sync_fetch_and_add_1 stub that doesn't exist:
> > >>
> > >> Undefined symbols for architecture x86_64:
> > >>  "___sync_fetch_and_add_1", referenced from:
> > >>      _ink_freelist_new in ink_queue.o
> > >>      _ink_freelist_free in ink_queue.o
> > >>      _ink_atomiclist_pop in ink_queue.o
> > >>      _ink_atomiclist_popall in ink_queue.o
> > >>      _ink_atomiclist_push in ink_queue.o
> > >>      _ink_atomiclist_remove in ink_queue.o
> > >>
> > >> I have an email out to the clang devs, but no response yet.
> > >>
> > >>>
> > >>>
> > >>> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> > >>> Commit:
> > >> http://git-wip-us.apache.org/repos/asf/trafficserver/commit/4a090831
> > >>> Tree:
> > http://git-wip-us.apache.org/repos/asf/trafficserver/tree/4a090831
> > >>> Diff:
> > http://git-wip-us.apache.org/repos/asf/trafficserver/diff/4a090831
> > >>>
> > >>> Branch: refs/heads/master
> > >>> Commit: 4a0908314acb301f1e9eb691bbcd8d956e86c321
> > >>> Parents: 57ffdf5
> > >>> Author: jplevyak@apache.org <jp...@apache.org>
> > >>> Authored: Thu Mar 21 10:10:16 2013 -0700
> > >>> Committer: John Plevyak <jp...@acm.org>
> > >>> Committed: Thu Mar 21 10:10:16 2013 -0700
> > >>>
> > >>>
> ----------------------------------------------------------------------
> > >>> lib/ts/ink_queue.h |    2 +-
> > >>> 1 files changed, 1 insertions(+), 1 deletions(-)
> > >>>
> ----------------------------------------------------------------------
> > >>>
> > >>>
> > >>>
> > >>
> >
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/4a090831/lib/ts/ink_queue.h
> > >>>
> ----------------------------------------------------------------------
> > >>> diff --git a/lib/ts/ink_queue.h b/lib/ts/ink_queue.h
> > >>> index 6ac9945..20fbf9a 100644
> > >>> --- a/lib/ts/ink_queue.h
> > >>> +++ b/lib/ts/ink_queue.h
> > >>> @@ -72,7 +72,7 @@ extern "C"
> > >>> #endif
> > >>>
> > >>> #if TS_HAS_128BIT_CAS
> > >>> -#define INK_QUEUE_LD(dst,src) *((__int128_t*)&(dst)) =
> > >> *((__int128_t*)&(src))
> > >>> +#define INK_QUEUE_LD(dst,src) *(__int128_t*)&(dst) =
> > >> __sync_fetch_and_add((__int128_t*)&(src), 0)
> > >>> #else
> > >>> #define INK_QUEUE_LD(dst,src) INK_QUEUE_LD64(dst,src)
> > >>> #endif
> > >>>
> > >>
> > >>
> >
> >
>

Re: [1/3] git commit: Fix bug with 128 bit compare and swap.

Posted by Brian Geffon <br...@apache.org>.
It turns out that __sync_fetch_and_add(__int128_t*, int) just uses
cmpxchg16b under the covers anyway...

int main ( void ) {
__int128_t src=10, dest=20;
*(__int128_t*)&(dest) = __sync_fetch_and_add((__int128_t*)&src, 0);

return 0;
}

When building using gcc -mcx16 -S -o test.S test.c, we get the following
assembly:

main:
.LFB0:
        .cfi_startproc
        pushq   %rbp
        .cfi_def_cfa_offset 16
        .cfi_offset 6, -16
        movq    %rsp, %rbp
        .cfi_def_cfa_register 6
        pushq   %rbx
        movq    $10, -32(%rbp)
        movq    $0, -24(%rbp)
        movq    $20, -48(%rbp)
        movq    $0, -40(%rbp)
        leaq    -32(%rbp), %r8
        movq    (%r8), %rax
        movq    8(%r8), %rdx
.L2:
        movq    %rax, %rsi
        movq    %rdx, %rdi
        movq    %rax, %rcx
        movq    %rdx, %rbx
        .cfi_offset 3, -24
        addq    $0, %rcx
        adcq    $0, %rbx
        movq    %rcx, %r9
        movq    %rbx, %rcx
        movq    %r9, %rbx
        lock cmpxchg16b (%r8)
        jne     .L2
        movq    %rsi, %rax
        movq    %rdi, %rdx
        movq    %rax, -48(%rbp)
        movq    %rdx, -40(%rbp)
        movl    $0, %eax
        popq    %rbx
        leave
        .cfi_def_cfa 7, 8
        ret
        .cfi_endproc

So instead of __sync_fetch_and_add() it appears we can use
__sync_val_compare_and_swap() which should have no problems with clang...?

Brian


On Mon, Mar 25, 2013 at 8:43 AM, James Peach <jp...@apache.org> wrote:

> On Mar 24, 2013, at 2:11 PM, Brian Geffon <br...@apache.org> wrote:
>
> > I'm confused, I reenabled 128bit cas in configure.ac and verified that
> it
> > is building with cmpxchg16b and it's running perfectly. The test_freelist
> > regression tests pass with no problems. So what were the issues remaining
> > that required the revert? How can I possibly reproduce them so I can get
> > this back on track?
>
> From my testing, the changes from John and Weijin fixed all the tests. The
> last remaining issue that I have is that John's fix doesn't build with
> clang. I guess that we can disable this code path with clang though ...
>
> >
> > Brian
> >
> >
> > On Fri, Mar 22, 2013 at 10:06 AM, James Peach <ja...@me.com> wrote:
> >
> >> On Mar 21, 2013, at 1:54 PM, jplevyak@apache.org wrote:
> >>
> >>> Updated Branches:
> >>> refs/heads/master 1a2ebccb1 -> f41323e01
> >>>
> >>>
> >>> Fix bug with 128 bit compare and swap.
> >>
> >> Hmm, this doesn't build with clang ... it makes clang generate a call
> to a
> >> ___sync_fetch_and_add_1 stub that doesn't exist:
> >>
> >> Undefined symbols for architecture x86_64:
> >>  "___sync_fetch_and_add_1", referenced from:
> >>      _ink_freelist_new in ink_queue.o
> >>      _ink_freelist_free in ink_queue.o
> >>      _ink_atomiclist_pop in ink_queue.o
> >>      _ink_atomiclist_popall in ink_queue.o
> >>      _ink_atomiclist_push in ink_queue.o
> >>      _ink_atomiclist_remove in ink_queue.o
> >>
> >> I have an email out to the clang devs, but no response yet.
> >>
> >>>
> >>>
> >>> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> >>> Commit:
> >> http://git-wip-us.apache.org/repos/asf/trafficserver/commit/4a090831
> >>> Tree:
> http://git-wip-us.apache.org/repos/asf/trafficserver/tree/4a090831
> >>> Diff:
> http://git-wip-us.apache.org/repos/asf/trafficserver/diff/4a090831
> >>>
> >>> Branch: refs/heads/master
> >>> Commit: 4a0908314acb301f1e9eb691bbcd8d956e86c321
> >>> Parents: 57ffdf5
> >>> Author: jplevyak@apache.org <jp...@apache.org>
> >>> Authored: Thu Mar 21 10:10:16 2013 -0700
> >>> Committer: John Plevyak <jp...@acm.org>
> >>> Committed: Thu Mar 21 10:10:16 2013 -0700
> >>>
> >>> ----------------------------------------------------------------------
> >>> lib/ts/ink_queue.h |    2 +-
> >>> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>> ----------------------------------------------------------------------
> >>>
> >>>
> >>>
> >>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/4a090831/lib/ts/ink_queue.h
> >>> ----------------------------------------------------------------------
> >>> diff --git a/lib/ts/ink_queue.h b/lib/ts/ink_queue.h
> >>> index 6ac9945..20fbf9a 100644
> >>> --- a/lib/ts/ink_queue.h
> >>> +++ b/lib/ts/ink_queue.h
> >>> @@ -72,7 +72,7 @@ extern "C"
> >>> #endif
> >>>
> >>> #if TS_HAS_128BIT_CAS
> >>> -#define INK_QUEUE_LD(dst,src) *((__int128_t*)&(dst)) =
> >> *((__int128_t*)&(src))
> >>> +#define INK_QUEUE_LD(dst,src) *(__int128_t*)&(dst) =
> >> __sync_fetch_and_add((__int128_t*)&(src), 0)
> >>> #else
> >>> #define INK_QUEUE_LD(dst,src) INK_QUEUE_LD64(dst,src)
> >>> #endif
> >>>
> >>
> >>
>
>

Re: [1/3] git commit: Fix bug with 128 bit compare and swap.

Posted by James Peach <jp...@apache.org>.
On Mar 24, 2013, at 2:11 PM, Brian Geffon <br...@apache.org> wrote:

> I'm confused, I reenabled 128bit cas in configure.ac and verified that it
> is building with cmpxchg16b and it's running perfectly. The test_freelist
> regression tests pass with no problems. So what were the issues remaining
> that required the revert? How can I possibly reproduce them so I can get
> this back on track?

>From my testing, the changes from John and Weijin fixed all the tests. The last remaining issue that I have is that John's fix doesn't build with clang. I guess that we can disable this code path with clang though ...

> 
> Brian
> 
> 
> On Fri, Mar 22, 2013 at 10:06 AM, James Peach <ja...@me.com> wrote:
> 
>> On Mar 21, 2013, at 1:54 PM, jplevyak@apache.org wrote:
>> 
>>> Updated Branches:
>>> refs/heads/master 1a2ebccb1 -> f41323e01
>>> 
>>> 
>>> Fix bug with 128 bit compare and swap.
>> 
>> Hmm, this doesn't build with clang ... it makes clang generate a call to a
>> ___sync_fetch_and_add_1 stub that doesn't exist:
>> 
>> Undefined symbols for architecture x86_64:
>>  "___sync_fetch_and_add_1", referenced from:
>>      _ink_freelist_new in ink_queue.o
>>      _ink_freelist_free in ink_queue.o
>>      _ink_atomiclist_pop in ink_queue.o
>>      _ink_atomiclist_popall in ink_queue.o
>>      _ink_atomiclist_push in ink_queue.o
>>      _ink_atomiclist_remove in ink_queue.o
>> 
>> I have an email out to the clang devs, but no response yet.
>> 
>>> 
>>> 
>>> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
>>> Commit:
>> http://git-wip-us.apache.org/repos/asf/trafficserver/commit/4a090831
>>> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/4a090831
>>> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/4a090831
>>> 
>>> Branch: refs/heads/master
>>> Commit: 4a0908314acb301f1e9eb691bbcd8d956e86c321
>>> Parents: 57ffdf5
>>> Author: jplevyak@apache.org <jp...@apache.org>
>>> Authored: Thu Mar 21 10:10:16 2013 -0700
>>> Committer: John Plevyak <jp...@acm.org>
>>> Committed: Thu Mar 21 10:10:16 2013 -0700
>>> 
>>> ----------------------------------------------------------------------
>>> lib/ts/ink_queue.h |    2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>> ----------------------------------------------------------------------
>>> 
>>> 
>>> 
>> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/4a090831/lib/ts/ink_queue.h
>>> ----------------------------------------------------------------------
>>> diff --git a/lib/ts/ink_queue.h b/lib/ts/ink_queue.h
>>> index 6ac9945..20fbf9a 100644
>>> --- a/lib/ts/ink_queue.h
>>> +++ b/lib/ts/ink_queue.h
>>> @@ -72,7 +72,7 @@ extern "C"
>>> #endif
>>> 
>>> #if TS_HAS_128BIT_CAS
>>> -#define INK_QUEUE_LD(dst,src) *((__int128_t*)&(dst)) =
>> *((__int128_t*)&(src))
>>> +#define INK_QUEUE_LD(dst,src) *(__int128_t*)&(dst) =
>> __sync_fetch_and_add((__int128_t*)&(src), 0)
>>> #else
>>> #define INK_QUEUE_LD(dst,src) INK_QUEUE_LD64(dst,src)
>>> #endif
>>> 
>> 
>> 


Re: [1/3] git commit: Fix bug with 128 bit compare and swap.

Posted by Brian Geffon <br...@apache.org>.
I'm confused, I reenabled 128bit cas in configure.ac and verified that it
is building with cmpxchg16b and it's running perfectly. The test_freelist
regression tests pass with no problems. So what were the issues remaining
that required the revert? How can I possibly reproduce them so I can get
this back on track?

Brian


On Fri, Mar 22, 2013 at 10:06 AM, James Peach <ja...@me.com> wrote:

> On Mar 21, 2013, at 1:54 PM, jplevyak@apache.org wrote:
>
> > Updated Branches:
> >  refs/heads/master 1a2ebccb1 -> f41323e01
> >
> >
> > Fix bug with 128 bit compare and swap.
>
> Hmm, this doesn't build with clang ... it makes clang generate a call to a
> ___sync_fetch_and_add_1 stub that doesn't exist:
>
> Undefined symbols for architecture x86_64:
>   "___sync_fetch_and_add_1", referenced from:
>       _ink_freelist_new in ink_queue.o
>       _ink_freelist_free in ink_queue.o
>       _ink_atomiclist_pop in ink_queue.o
>       _ink_atomiclist_popall in ink_queue.o
>       _ink_atomiclist_push in ink_queue.o
>       _ink_atomiclist_remove in ink_queue.o
>
> I have an email out to the clang devs, but no response yet.
>
> >
> >
> > Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> > Commit:
> http://git-wip-us.apache.org/repos/asf/trafficserver/commit/4a090831
> > Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/4a090831
> > Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/4a090831
> >
> > Branch: refs/heads/master
> > Commit: 4a0908314acb301f1e9eb691bbcd8d956e86c321
> > Parents: 57ffdf5
> > Author: jplevyak@apache.org <jp...@apache.org>
> > Authored: Thu Mar 21 10:10:16 2013 -0700
> > Committer: John Plevyak <jp...@acm.org>
> > Committed: Thu Mar 21 10:10:16 2013 -0700
> >
> > ----------------------------------------------------------------------
> > lib/ts/ink_queue.h |    2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> > ----------------------------------------------------------------------
> >
> >
> >
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/4a090831/lib/ts/ink_queue.h
> > ----------------------------------------------------------------------
> > diff --git a/lib/ts/ink_queue.h b/lib/ts/ink_queue.h
> > index 6ac9945..20fbf9a 100644
> > --- a/lib/ts/ink_queue.h
> > +++ b/lib/ts/ink_queue.h
> > @@ -72,7 +72,7 @@ extern "C"
> > #endif
> >
> > #if TS_HAS_128BIT_CAS
> > -#define INK_QUEUE_LD(dst,src) *((__int128_t*)&(dst)) =
> *((__int128_t*)&(src))
> > +#define INK_QUEUE_LD(dst,src) *(__int128_t*)&(dst) =
> __sync_fetch_and_add((__int128_t*)&(src), 0)
> > #else
> > #define INK_QUEUE_LD(dst,src) INK_QUEUE_LD64(dst,src)
> > #endif
> >
>
>

Re: [1/3] git commit: Fix bug with 128 bit compare and swap.

Posted by James Peach <ja...@me.com>.
On Mar 21, 2013, at 1:54 PM, jplevyak@apache.org wrote:

> Updated Branches:
>  refs/heads/master 1a2ebccb1 -> f41323e01
> 
> 
> Fix bug with 128 bit compare and swap.

Hmm, this doesn't build with clang ... it makes clang generate a call to a ___sync_fetch_and_add_1 stub that doesn't exist:

Undefined symbols for architecture x86_64:
  "___sync_fetch_and_add_1", referenced from:
      _ink_freelist_new in ink_queue.o
      _ink_freelist_free in ink_queue.o
      _ink_atomiclist_pop in ink_queue.o
      _ink_atomiclist_popall in ink_queue.o
      _ink_atomiclist_push in ink_queue.o
      _ink_atomiclist_remove in ink_queue.o

I have an email out to the clang devs, but no response yet.

> 
> 
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/4a090831
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/4a090831
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/4a090831
> 
> Branch: refs/heads/master
> Commit: 4a0908314acb301f1e9eb691bbcd8d956e86c321
> Parents: 57ffdf5
> Author: jplevyak@apache.org <jp...@apache.org>
> Authored: Thu Mar 21 10:10:16 2013 -0700
> Committer: John Plevyak <jp...@acm.org>
> Committed: Thu Mar 21 10:10:16 2013 -0700
> 
> ----------------------------------------------------------------------
> lib/ts/ink_queue.h |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> ----------------------------------------------------------------------
> 
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/4a090831/lib/ts/ink_queue.h
> ----------------------------------------------------------------------
> diff --git a/lib/ts/ink_queue.h b/lib/ts/ink_queue.h
> index 6ac9945..20fbf9a 100644
> --- a/lib/ts/ink_queue.h
> +++ b/lib/ts/ink_queue.h
> @@ -72,7 +72,7 @@ extern "C"
> #endif
> 
> #if TS_HAS_128BIT_CAS
> -#define INK_QUEUE_LD(dst,src) *((__int128_t*)&(dst)) = *((__int128_t*)&(src))
> +#define INK_QUEUE_LD(dst,src) *(__int128_t*)&(dst) = __sync_fetch_and_add((__int128_t*)&(src), 0)
> #else
> #define INK_QUEUE_LD(dst,src) INK_QUEUE_LD64(dst,src)
> #endif
> 


[3/3] git commit: Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/trafficserver

Posted by jp...@apache.org.
Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/trafficserver


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/f41323e0
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/f41323e0
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/f41323e0

Branch: refs/heads/master
Commit: f41323e010480a7e206c680b3fc3b06ac8a4675a
Parents: 5a34bbd 1a2ebcc
Author: John Plevyak <jp...@apache.org>
Authored: Thu Mar 21 13:53:59 2013 -0700
Committer: John Plevyak <jp...@acm.org>
Committed: Thu Mar 21 13:53:59 2013 -0700

----------------------------------------------------------------------
 CHANGES                                   |    3 +
 iocore/eventsystem/P_UnixEventProcessor.h |    4 +-
 mgmt/Main.cc                              |   18 --
 mgmt/cluster/ClusterCom.cc                |   12 +-
 mgmt/cluster/VMap.cc                      |  341 ++----------------------
 mgmt/cluster/VMap.h                       |   13 +-
 proxy/config/vaddrs.config.default        |   11 +-
 7 files changed, 46 insertions(+), 356 deletions(-)
----------------------------------------------------------------------



[2/3] git commit: Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/trafficserver

Posted by jp...@apache.org.
Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/trafficserver


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/5a34bbd7
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/5a34bbd7
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/5a34bbd7

Branch: refs/heads/master
Commit: 5a34bbd7e5030494ce12285331d63ca582d1f051
Parents: 4a09083 bd8fd4f
Author: John Plevyak <jp...@apache.org>
Authored: Thu Mar 21 10:37:08 2013 -0700
Committer: John Plevyak <jp...@acm.org>
Committed: Thu Mar 21 10:37:08 2013 -0700

----------------------------------------------------------------------
 configure.ac |   52 +++++++++++++++++++++++++++-------------------------
 1 files changed, 27 insertions(+), 25 deletions(-)
----------------------------------------------------------------------



Re: [1/3] git commit: Fix bug with 128 bit compare and swap.

Posted by weijin <we...@126.com>.
well done, John.

should we also changed ink_queue_load_64 to
void
ink_queue_load_64(void *dst, void *src)
{
#if (defined(__i386__) || defined(__arm__)) && (SIZEOF_VOIDP == 4)
   __sync_fetch_and_add((int64_t*)&(src), 0)
#else
   *(void**)dst = *(void**)src;
#endif
}

If not, could you please explain the reasons ? thanks.

On 03/22/2013 04:54 AM, jplevyak@apache.org wrote:
> Updated Branches:
>    refs/heads/master 1a2ebccb1 -> f41323e01
>
>
> Fix bug with 128 bit compare and swap.
>
>
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/4a090831
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/4a090831
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/4a090831
>
> Branch: refs/heads/master
> Commit: 4a0908314acb301f1e9eb691bbcd8d956e86c321
> Parents: 57ffdf5
> Author: jplevyak@apache.org <jp...@apache.org>
> Authored: Thu Mar 21 10:10:16 2013 -0700
> Committer: John Plevyak <jp...@acm.org>
> Committed: Thu Mar 21 10:10:16 2013 -0700
>
> ----------------------------------------------------------------------
>   lib/ts/ink_queue.h |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/4a090831/lib/ts/ink_queue.h
> ----------------------------------------------------------------------
> diff --git a/lib/ts/ink_queue.h b/lib/ts/ink_queue.h
> index 6ac9945..20fbf9a 100644
> --- a/lib/ts/ink_queue.h
> +++ b/lib/ts/ink_queue.h
> @@ -72,7 +72,7 @@ extern "C"
>   #endif
>   
>   #if TS_HAS_128BIT_CAS
> -#define INK_QUEUE_LD(dst,src) *((__int128_t*)&(dst)) = *((__int128_t*)&(src))
> +#define INK_QUEUE_LD(dst,src) *(__int128_t*)&(dst) = __sync_fetch_and_add((__int128_t*)&(src), 0)
>   #else
>   #define INK_QUEUE_LD(dst,src) INK_QUEUE_LD64(dst,src)
>   #endif
>
>