You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by James Peach <ja...@me.com> on 2013/03/18 17:49:55 UTC
128bit compare and swap
Hi Brian,
I finally fixed the autoconf tests for TS_HAS_128BIT_CAS. Unfortunately test_freelist fails on all the platforms that I have access to. I'm assuming that on your platforms it succeeds, so maybe there's something about the version of gcc you are using that makes it work.
Since I don't really know which platforms this is working for, what do you think about reverting this?
J
Re: 128bit compare and swap
Posted by James Peach <jp...@apache.org>.
On Apr 15, 2013, at 9:42 AM, Leif Hedstrom <zw...@apache.org> wrote:
> On 4/15/13 10:33 AM, James Peach wrote:
>> On Mar 19, 2013, at 4:29 PM, Brian Geffon <br...@gmail.com> wrote:
>>
>>> Ok, I'll get both of these committed.
>> this breaks valgrind :(
>
> Theo pointed out this is related to running valgrind under VirtualBox. I confirmed, on my bare-metal Fedora Core box, Valgrind still works fine with current master.
Oh great!
J
Re: 128bit compare and swap
Posted by Leif Hedstrom <zw...@apache.org>.
On 4/15/13 10:33 AM, James Peach wrote:
> On Mar 19, 2013, at 4:29 PM, Brian Geffon <br...@gmail.com> wrote:
>
>> Ok, I'll get both of these committed.
> this breaks valgrind :(
Theo pointed out this is related to running valgrind under VirtualBox. I
confirmed, on my bare-metal Fedora Core box, Valgrind still works fine with
current master.
-- Leif
Re: 128bit compare and swap
Posted by James Peach <jp...@apache.org>.
On Mar 19, 2013, at 4:29 PM, Brian Geffon <br...@gmail.com> wrote:
> Ok, I'll get both of these committed.
this breaks valgrind :(
>
> Brian
>
> On Tue, Mar 19, 2013 at 4:18 PM, John Plevyak <jp...@acm.org> wrote:
>
>> Definitely should do that as well.
>>
>> john
>>
>> On Tue, Mar 19, 2013 at 4:04 PM, Brian Geffon <br...@gmail.com>
>> wrote:
>>
>>> Thanks John!
>>>
>>> Someone else mentioned that we might need to force the head_p to be 16
>> byte
>>> aligned, what are your thoughts?
>>>
>>> - } head_p;
>>> + } __attribute__ ((aligned (16))) head_p;
>>>
>>>
>>> Thanks.
>>> Brian
>>>
>>> On Tue, Mar 19, 2013 at 3:47 PM, John Plevyak <jp...@acm.org> wrote:
>>>
>>>> I believe the problem is with INK_QUEUE_LD which is not atomic:
>>>>
>>>> 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
>>>>
>>>> This fixes the problem.
>>>>
>>>> On Mon, Mar 18, 2013 at 9:31 PM, Brian Geffon <br...@apache.org>
>> wrote:
>>>>
>>>>> Hey, I'm looking into the failing freelist tests using the precise64
>>>>> vagrant box. If I can't figure it out tonight I'll just disable the
>>> check
>>>>> in configure.ac to get things stable again for the time being. If
>>> anyone
>>>>> else has some time to help out with this, I'd really appreciate it.
>>>>>
>>>>> Brian
>>>>>
>>>>> On Mon, Mar 18, 2013 at 9:49 AM, James Peach <ja...@me.com>
>>> wrote:
>>>>>
>>>>>> Hi Brian,
>>>>>>
>>>>>> I finally fixed the autoconf tests for TS_HAS_128BIT_CAS.
>>> Unfortunately
>>>>>> test_freelist fails on all the platforms that I have access to. I'm
>>>>>> assuming that on your platforms it succeeds, so maybe there's
>>> something
>>>>>> about the version of gcc you are using that makes it work.
>>>>>>
>>>>>> Since I don't really know which platforms this is working for, what
>>> do
>>>>> you
>>>>>> think about reverting this?
>>>>>>
>>>>>> J
>>>>>
>>>>
>>>
>>
Re: 128bit compare and swap
Posted by Brian Geffon <br...@gmail.com>.
Ok, I'll get both of these committed.
Brian
On Tue, Mar 19, 2013 at 4:18 PM, John Plevyak <jp...@acm.org> wrote:
> Definitely should do that as well.
>
> john
>
> On Tue, Mar 19, 2013 at 4:04 PM, Brian Geffon <br...@gmail.com>
> wrote:
>
> > Thanks John!
> >
> > Someone else mentioned that we might need to force the head_p to be 16
> byte
> > aligned, what are your thoughts?
> >
> > - } head_p;
> > + } __attribute__ ((aligned (16))) head_p;
> >
> >
> > Thanks.
> > Brian
> >
> > On Tue, Mar 19, 2013 at 3:47 PM, John Plevyak <jp...@acm.org> wrote:
> >
> > > I believe the problem is with INK_QUEUE_LD which is not atomic:
> > >
> > > 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
> > >
> > > This fixes the problem.
> > >
> > > On Mon, Mar 18, 2013 at 9:31 PM, Brian Geffon <br...@apache.org>
> wrote:
> > >
> > > > Hey, I'm looking into the failing freelist tests using the precise64
> > > > vagrant box. If I can't figure it out tonight I'll just disable the
> > check
> > > > in configure.ac to get things stable again for the time being. If
> > anyone
> > > > else has some time to help out with this, I'd really appreciate it.
> > > >
> > > > Brian
> > > >
> > > > On Mon, Mar 18, 2013 at 9:49 AM, James Peach <ja...@me.com>
> > wrote:
> > > >
> > > > > Hi Brian,
> > > > >
> > > > > I finally fixed the autoconf tests for TS_HAS_128BIT_CAS.
> > Unfortunately
> > > > > test_freelist fails on all the platforms that I have access to. I'm
> > > > > assuming that on your platforms it succeeds, so maybe there's
> > something
> > > > > about the version of gcc you are using that makes it work.
> > > > >
> > > > > Since I don't really know which platforms this is working for, what
> > do
> > > > you
> > > > > think about reverting this?
> > > > >
> > > > > J
> > > >
> > >
> >
>
Re: 128bit compare and swap
Posted by John Plevyak <jp...@acm.org>.
Definitely should do that as well.
john
On Tue, Mar 19, 2013 at 4:04 PM, Brian Geffon <br...@gmail.com> wrote:
> Thanks John!
>
> Someone else mentioned that we might need to force the head_p to be 16 byte
> aligned, what are your thoughts?
>
> - } head_p;
> + } __attribute__ ((aligned (16))) head_p;
>
>
> Thanks.
> Brian
>
> On Tue, Mar 19, 2013 at 3:47 PM, John Plevyak <jp...@acm.org> wrote:
>
> > I believe the problem is with INK_QUEUE_LD which is not atomic:
> >
> > 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
> >
> > This fixes the problem.
> >
> > On Mon, Mar 18, 2013 at 9:31 PM, Brian Geffon <br...@apache.org> wrote:
> >
> > > Hey, I'm looking into the failing freelist tests using the precise64
> > > vagrant box. If I can't figure it out tonight I'll just disable the
> check
> > > in configure.ac to get things stable again for the time being. If
> anyone
> > > else has some time to help out with this, I'd really appreciate it.
> > >
> > > Brian
> > >
> > > On Mon, Mar 18, 2013 at 9:49 AM, James Peach <ja...@me.com>
> wrote:
> > >
> > > > Hi Brian,
> > > >
> > > > I finally fixed the autoconf tests for TS_HAS_128BIT_CAS.
> Unfortunately
> > > > test_freelist fails on all the platforms that I have access to. I'm
> > > > assuming that on your platforms it succeeds, so maybe there's
> something
> > > > about the version of gcc you are using that makes it work.
> > > >
> > > > Since I don't really know which platforms this is working for, what
> do
> > > you
> > > > think about reverting this?
> > > >
> > > > J
> > >
> >
>
Re: 128bit compare and swap
Posted by Brian Geffon <br...@gmail.com>.
Thanks John!
Someone else mentioned that we might need to force the head_p to be 16 byte
aligned, what are your thoughts?
- } head_p;
+ } __attribute__ ((aligned (16))) head_p;
Thanks.
Brian
On Tue, Mar 19, 2013 at 3:47 PM, John Plevyak <jp...@acm.org> wrote:
> I believe the problem is with INK_QUEUE_LD which is not atomic:
>
> 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
>
> This fixes the problem.
>
> On Mon, Mar 18, 2013 at 9:31 PM, Brian Geffon <br...@apache.org> wrote:
>
> > Hey, I'm looking into the failing freelist tests using the precise64
> > vagrant box. If I can't figure it out tonight I'll just disable the check
> > in configure.ac to get things stable again for the time being. If anyone
> > else has some time to help out with this, I'd really appreciate it.
> >
> > Brian
> >
> > On Mon, Mar 18, 2013 at 9:49 AM, James Peach <ja...@me.com> wrote:
> >
> > > Hi Brian,
> > >
> > > I finally fixed the autoconf tests for TS_HAS_128BIT_CAS. Unfortunately
> > > test_freelist fails on all the platforms that I have access to. I'm
> > > assuming that on your platforms it succeeds, so maybe there's something
> > > about the version of gcc you are using that makes it work.
> > >
> > > Since I don't really know which platforms this is working for, what do
> > you
> > > think about reverting this?
> > >
> > > J
> >
>
Re: 128bit compare and swap
Posted by John Plevyak <jp...@acm.org>.
I believe the problem is with INK_QUEUE_LD which is not atomic:
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
This fixes the problem.
On Mon, Mar 18, 2013 at 9:31 PM, Brian Geffon <br...@apache.org> wrote:
> Hey, I'm looking into the failing freelist tests using the precise64
> vagrant box. If I can't figure it out tonight I'll just disable the check
> in configure.ac to get things stable again for the time being. If anyone
> else has some time to help out with this, I'd really appreciate it.
>
> Brian
>
> On Mon, Mar 18, 2013 at 9:49 AM, James Peach <ja...@me.com> wrote:
>
> > Hi Brian,
> >
> > I finally fixed the autoconf tests for TS_HAS_128BIT_CAS. Unfortunately
> > test_freelist fails on all the platforms that I have access to. I'm
> > assuming that on your platforms it succeeds, so maybe there's something
> > about the version of gcc you are using that makes it work.
> >
> > Since I don't really know which platforms this is working for, what do
> you
> > think about reverting this?
> >
> > J
>
Re: 128bit compare and swap
Posted by Brian Geffon <br...@apache.org>.
Hey, I'm looking into the failing freelist tests using the precise64
vagrant box. If I can't figure it out tonight I'll just disable the check
in configure.ac to get things stable again for the time being. If anyone
else has some time to help out with this, I'd really appreciate it.
Brian
On Mon, Mar 18, 2013 at 9:49 AM, James Peach <ja...@me.com> wrote:
> Hi Brian,
>
> I finally fixed the autoconf tests for TS_HAS_128BIT_CAS. Unfortunately
> test_freelist fails on all the platforms that I have access to. I'm
> assuming that on your platforms it succeeds, so maybe there's something
> about the version of gcc you are using that makes it work.
>
> Since I don't really know which platforms this is working for, what do you
> think about reverting this?
>
> J