You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Jeff Trawick <tr...@gmail.com> on 2013/11/22 21:57:20 UTC
Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number
server/mpm/event/event.c server/mpm/eventopt/eventopt.c
On Fri, Nov 22, 2013 at 3:40 PM, Jeff Trawick <tr...@gmail.com> wrote:
> On Fri, Nov 22, 2013 at 3:24 PM, Jeff Trawick <tr...@gmail.com> wrote:
>
>> On Fri, Nov 22, 2013 at 2:52 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>
>>> Note, the only think changed in event now (via
>>> https://svn.apache.org/viewvc?view=revision&revision=1542560)
>>> is that event *checks* that atomics work as required for
>>> event... if the check fails, it means that event has
>>> been "broken" on that system, assuming it ever hit
>>> blocked idlers, for a *long* time...
>>>
>>
>> Got it... fdqueue.c is asking for trouble...
>>
>> I'm using atomic/unix/ia32.c with icc too.
>>
>> Need to compare generated code... I hate stuff like "int foo() {
>> unsigned char x; ... return x; }"
>>
>>
> APR is documented as returning "zero if the value becomes zero on
> decrement, otherwise non-zero".
>
> With gcc we use get __sync_sub_and_fetch(), which returns the new value
> after the decrement.
>
> With icc we use the assembler implementation in APR. I think that's
> returning 1 instead of -1.
>
It does a decrement long, which sets the zero flag as appropriate. Then it
does set-not-equal to set what becomes the return value to 0 if the result
of the decrement was 0 and 1 otherwise.
It "should be easy" to make it return the new value like the
__sync_sub_and_fetch() version does :)
>
> Here is where fdqueue needs to be able to see a negative return code:
>
> apr_status_t ap_queue_info_try_get_idler(fd_queue_info_t * queue_info)
> {
> int prev_idlers;
> prev_idlers = apr_atomic_dec32((apr_uint32_t *)&(queue_info->idlers));
> if (prev_idlers <= 0) {
> apr_atomic_inc32((apr_uint32_t *)&(queue_info->idlers)); /*
> back out dec */
> return APR_EAGAIN;
> }
> return APR_SUCCESS;
> }
>
> ("prev" in the ia32.c version of apr_atomic_dec32() and "prev_idlers" here
> is deceiving.)
>
>
>>
>>>
>>> You should be seeing it in trunk as well...
>>>
>>>
>>> On Nov 22, 2013, at 2:43 PM, Jeff Trawick <tr...@gmail.com> wrote:
>>>
>>> > On Fri, Nov 22, 2013 at 2:39 PM, Jim Jagielski <ji...@jagunet.com>
>>> wrote:
>>> >
>>> > On Nov 22, 2013, at 2:22 PM, Jeff Trawick <tr...@gmail.com> wrote:
>>> >
>>> > > On Sat, Nov 17, 2012 at 6:00 AM, Ruediger Pluem <rp...@apache.org>
>>> wrote:
>>> > >
>>> > >
>>> > > jim@apache.org wrote:
>>> > > > + i = apr_atomic_dec32(&foo);
>>> > > > + if (i >= 0) {
>>> > >
>>> > > Why can we expect i < 0? apr_atomic_dec32 returns 0 if the dec
>>> causes foo to become zero and it returns non zero
>>> > > otherwise. Shouldn't this behavior the same across all platforms?
>>> And if not should that be fixed in APR?
>>> > >
>>> > > icc (Intel) builds of httpd 2.4.7 event MPM (with apr-1.5.0) bomb
>>> here.
>>> > >
>>> > > --enable-nonportable-atomics is specified for apr, though I haven't
>>> checked what that does with icc.
>>> > >
>>> >
>>> > As noted back with the orig update, this test is due to the
>>> > fdqueue code in the new event:
>>> >
>>> > apr_status_t ap_queue_info_set_idle(fd_queue_info_t * queue_info,
>>> > apr_pool_t * pool_to_recycle)
>>> > {
>>> > apr_status_t rv;
>>> > int prev_idlers;
>>> >
>>> > ap_push_pool(queue_info, pool_to_recycle);
>>> >
>>> > /* Atomically increment the count of idle workers */
>>> > /*
>>> > * TODO: The atomics expect unsigned whereas we're using signed.
>>> > * Need to double check that they work as expected or else
>>> > * rework how we determine blocked.
>>> > * UPDATE: Correct operation is performed during open_logs()
>>> > */
>>> > prev_idlers = apr_atomic_inc32((apr_uint32_t
>>> *)&(queue_info->idlers));
>>> >
>>> > /* If other threads are waiting on a worker, wake one up */
>>> > if (prev_idlers < 0) {
>>> >
>>> >
>>> > See the comments ("The atomics expect unsigned whereas...") for
>>> > the reason, etc.
>>> >
>>> > When you say "icc (Intel) builds of httpd 2.4.7 event MPM (with
>>> apr-1.5.0) bomb here."
>>> > do you mean that you get the 'atomics not working as expected' error
>>> > (and the internal server error) or that it core dumps?
>>> >
>>> >
>>> > "atomics not working as expected"
>>> >
>>> > Let me see what code is used...
>>> >
>>> > --
>>> > Born in Roswell... married an alien...
>>> > http://emptyhammock.com/
>>>
>>>
>>
>>
>> --
>> Born in Roswell... married an alien...
>> http://emptyhammock.com/
>>
>
>
>
> --
> Born in Roswell... married an alien...
> http://emptyhammock.com/
>
--
Born in Roswell... married an alien...
http://emptyhammock.com/
Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number
server/mpm/event/event.c server/mpm/eventopt/eventopt.c
Posted by Rainer Jung <ra...@kippdata.de>.
On 22.11.2013 23:03, Jim Jagielski wrote:
> Anyone ever try OpenPA?
>
> https://trac.mcs.anl.gov/projects/openpa/
>
> It's under MIT, fwiw.
Haven't tried it but the README
http://git.mcs.anl.gov/radix/openpa.git/blob_plain/HEAD:/README
indicates only platform support based on gcc plus Windows (but they say
they haven't set up a build system for it) plus Solaris (but that seems
to be Solaris 10+ only, at least my oldest Solaris 8 doesn't seem to
have the needed implementation).
Regards,
Rainer
Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number
server/mpm/event/event.c server/mpm/eventopt/eventopt.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Sun, Nov 24, 2013 at 7:33 AM, Eric Covener <co...@gmail.com> wrote:
> On Sun, Nov 24, 2013 at 5:24 AM, Rainer Jung <ra...@kippdata.de>
> wrote:
> > On 24.11.2013 01:03, Eric Covener wrote:
> >>>> I'm curious what other scenarios will fail though. I can try Sun
> Studio on
> >>>> Solaris 10 x86_64 (32-bit and 64-bit builds) "soon". But Sun Studio
> on
> >>>> SPARC presumably uses different explicit code in APR and I don't have
> access
> >>>> to that.
> >>>
> >>> I will try to get that one up and running on 2.4.7.
> >>
> >> No startup error with 2.4.7 + event on sparc32 or sparc64 with sun
> >> studio 12 (cc 5.9)
> >
> > For the sake of completeness: was apr build with the nonportable atomics
> > switch enabled? Don't know whether that really matters, but want to make
> > sure we understand the test case.
>
> Defaults before, retested with no difference under
> --enable-nonportable-atomics
>
Solaris 10 on Intel/AMD, Sun Studio, --enable-nonportable-atomics, 32-bit
and 64-bit, 2.4.x HEAD, apr 1.5.x HEAD, apr-util 1.5.x HEAD
no problem; uses Solaris-specific atomic implementation, which uses the
return-new-value version of decrement
The portable, "mutex" version of atomics clearly returns the new value too,
though I didn't test with it.
--
Born in Roswell... married an alien...
http://emptyhammock.com/
Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number
server/mpm/event/event.c server/mpm/eventopt/eventopt.c
Posted by Eric Covener <co...@gmail.com>.
On Sun, Nov 24, 2013 at 5:24 AM, Rainer Jung <ra...@kippdata.de> wrote:
> On 24.11.2013 01:03, Eric Covener wrote:
>>>> I'm curious what other scenarios will fail though. I can try Sun Studio on
>>>> Solaris 10 x86_64 (32-bit and 64-bit builds) "soon". But Sun Studio on
>>>> SPARC presumably uses different explicit code in APR and I don't have access
>>>> to that.
>>>
>>> I will try to get that one up and running on 2.4.7.
>>
>> No startup error with 2.4.7 + event on sparc32 or sparc64 with sun
>> studio 12 (cc 5.9)
>
> For the sake of completeness: was apr build with the nonportable atomics
> switch enabled? Don't know whether that really matters, but want to make
> sure we understand the test case.
Defaults before, retested with no difference under --enable-nonportable-atomics
Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number
server/mpm/event/event.c server/mpm/eventopt/eventopt.c
Posted by Rainer Jung <ra...@kippdata.de>.
On 24.11.2013 01:03, Eric Covener wrote:
>>> I'm curious what other scenarios will fail though. I can try Sun Studio on
>>> Solaris 10 x86_64 (32-bit and 64-bit builds) "soon". But Sun Studio on
>>> SPARC presumably uses different explicit code in APR and I don't have access
>>> to that.
>>
>> I will try to get that one up and running on 2.4.7.
>
> No startup error with 2.4.7 + event on sparc32 or sparc64 with sun
> studio 12 (cc 5.9)
For the sake of completeness: was apr build with the nonportable atomics
switch enabled? Don't know whether that really matters, but want to make
sure we understand the test case.
Regards,
Rainer
Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number
server/mpm/event/event.c server/mpm/eventopt/eventopt.c
Posted by Eric Covener <co...@gmail.com>.
>> I'm curious what other scenarios will fail though. I can try Sun Studio on
>> Solaris 10 x86_64 (32-bit and 64-bit builds) "soon". But Sun Studio on
>> SPARC presumably uses different explicit code in APR and I don't have access
>> to that.
>
> I will try to get that one up and running on 2.4.7.
No startup error with 2.4.7 + event on sparc32 or sparc64 with sun
studio 12 (cc 5.9)
Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number
server/mpm/event/event.c server/mpm/eventopt/eventopt.c
Posted by Eric Covener <co...@gmail.com>.
On Sat, Nov 23, 2013 at 3:56 PM, Jeff Trawick <tr...@gmail.com> wrote:
> On Sat, Nov 23, 2013 at 3:45 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>
>> I'm just curious why people never saw this on trunk...
>> That code path is almost exactly a year old. No one has
>> hit this in 12 months??
>
>
> l rarely try trunk except on
>
> * Windows, which uses a system mechanism regardless of compiler
> * Linux or FreeBSD, 64-bit gcc builds
>
> --/--
>
> I'm curious what other scenarios will fail though. I can try Sun Studio on
> Solaris 10 x86_64 (32-bit and 64-bit builds) "soon". But Sun Studio on
> SPARC presumably uses different explicit code in APR and I don't have access
> to that.
I will try to get that one up and running on 2.4.7.
Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number
server/mpm/event/event.c server/mpm/eventopt/eventopt.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Sat, Nov 23, 2013 at 3:45 PM, Jim Jagielski <ji...@jagunet.com> wrote:
> I'm just curious why people never saw this on trunk...
> That code path is almost exactly a year old. No one has
> hit this in 12 months??
>
l rarely try trunk except on
* Windows, which uses a system mechanism regardless of compiler
* Linux or FreeBSD, 64-bit gcc builds
--/--
I'm curious what other scenarios will fail though. I can try Sun Studio on
Solaris 10 x86_64 (32-bit and 64-bit builds) "soon". But Sun Studio on
SPARC presumably uses different explicit code in APR and I don't have
access to that.
Right now it looks reasonable to change the ia32.c and os390/atomic.c
implementations in APR as described before, but there's no substitute for
actually trying the other atomic implementations, which look safe to me but
with little certainty.
> On Nov 23, 2013, at 12:45 PM, Rainer Jung <ra...@kippdata.de> wrote:
>
> > On 23.11.2013 14:15, Jeff Trawick wrote:
> >> On Sat, Nov 23, 2013 at 8:04 AM, Rainer Jung <rainer.jung@kippdata.de
> >> <ma...@kippdata.de>> wrote:
> >>
> >> On 22.11.2013 23:03, Jim Jagielski wrote:
> >>> Anyone ever try OpenPA?
> >>>
> >>> https://trac.mcs.anl.gov/projects/openpa/
> >>>
> >>> It's under MIT, fwiw.
> >>
> >> Haven't tried it but the README
> >>
> >> http://git.mcs.anl.gov/radix/openpa.git/blob_plain/HEAD:/README
> >>
> >> indicates only platform support based on gcc plus Windows (but they
> say
> >> they haven't set up a build system for it) plus Solaris (but that
> seems
> >> to be Solaris 10+ only, at least my oldest Solaris 8 doesn't seem to
> >> have the needed implementation).
> >>
> >> Regards,
> >>
> >> Rainer
> >>
> >>
> >> By the way, Rainer, in your l*m*n test combinations, which compiler(s)
> >> did you use on Solaris while confirming that 2.4.7 Event works at least
> >> a little?
> >
> > I'm only building with gcc. On Solaris 8 with gcc 4.1.2, on Solaris 10
> > the latest and greatest (4.8.2).
> >
> > Will post the full 2.4.7 results a bit later (only for the sake of
> > completeness), but the builds are also broken for event on Linux 32 Bits
> > even when using gcc:
> >
> > [Thu Nov 21 11:34:53.149765 2013] [mpm_event:crit] [pid 18492:tid
> > 3080439472] AH02405: atomics not working as expected
> >
> > Sorry for not having checked progress during the tests, but the last
> > combinations just finished right now.
> >
> > Regards,
> >
> > Rainer
> >
>
>
--
Born in Roswell... married an alien...
http://emptyhammock.com/
Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number
server/mpm/event/event.c server/mpm/eventopt/eventopt.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Sat, Nov 23, 2013 at 3:45 PM, Jim Jagielski <ji...@jagunet.com> wrote:
> I'm just curious why people never saw this on trunk...
> That code path is almost exactly a year old. No one has
> hit this in 12 months??
>
l rarely try trunk except on
* Windows, which uses a system mechanism regardless of compiler
* Linux or FreeBSD, 64-bit gcc builds
--/--
I'm curious what other scenarios will fail though. I can try Sun Studio on
Solaris 10 x86_64 (32-bit and 64-bit builds) "soon". But Sun Studio on
SPARC presumably uses different explicit code in APR and I don't have
access to that.
Right now it looks reasonable to change the ia32.c and os390/atomic.c
implementations in APR as described before, but there's no substitute for
actually trying the other atomic implementations, which look safe to me but
with little certainty.
> On Nov 23, 2013, at 12:45 PM, Rainer Jung <ra...@kippdata.de> wrote:
>
> > On 23.11.2013 14:15, Jeff Trawick wrote:
> >> On Sat, Nov 23, 2013 at 8:04 AM, Rainer Jung <rainer.jung@kippdata.de
> >> <ma...@kippdata.de>> wrote:
> >>
> >> On 22.11.2013 23:03, Jim Jagielski wrote:
> >>> Anyone ever try OpenPA?
> >>>
> >>> https://trac.mcs.anl.gov/projects/openpa/
> >>>
> >>> It's under MIT, fwiw.
> >>
> >> Haven't tried it but the README
> >>
> >> http://git.mcs.anl.gov/radix/openpa.git/blob_plain/HEAD:/README
> >>
> >> indicates only platform support based on gcc plus Windows (but they
> say
> >> they haven't set up a build system for it) plus Solaris (but that
> seems
> >> to be Solaris 10+ only, at least my oldest Solaris 8 doesn't seem to
> >> have the needed implementation).
> >>
> >> Regards,
> >>
> >> Rainer
> >>
> >>
> >> By the way, Rainer, in your l*m*n test combinations, which compiler(s)
> >> did you use on Solaris while confirming that 2.4.7 Event works at least
> >> a little?
> >
> > I'm only building with gcc. On Solaris 8 with gcc 4.1.2, on Solaris 10
> > the latest and greatest (4.8.2).
> >
> > Will post the full 2.4.7 results a bit later (only for the sake of
> > completeness), but the builds are also broken for event on Linux 32 Bits
> > even when using gcc:
> >
> > [Thu Nov 21 11:34:53.149765 2013] [mpm_event:crit] [pid 18492:tid
> > 3080439472] AH02405: atomics not working as expected
> >
> > Sorry for not having checked progress during the tests, but the last
> > combinations just finished right now.
> >
> > Regards,
> >
> > Rainer
> >
>
>
--
Born in Roswell... married an alien...
http://emptyhammock.com/
Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number server/mpm/event/event.c server/mpm/eventopt/eventopt.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
I'm just curious why people never saw this on trunk...
That code path is almost exactly a year old. No one has
hit this in 12 months??
On Nov 23, 2013, at 12:45 PM, Rainer Jung <ra...@kippdata.de> wrote:
> On 23.11.2013 14:15, Jeff Trawick wrote:
>> On Sat, Nov 23, 2013 at 8:04 AM, Rainer Jung <rainer.jung@kippdata.de
>> <ma...@kippdata.de>> wrote:
>>
>> On 22.11.2013 23:03, Jim Jagielski wrote:
>>> Anyone ever try OpenPA?
>>>
>>> https://trac.mcs.anl.gov/projects/openpa/
>>>
>>> It's under MIT, fwiw.
>>
>> Haven't tried it but the README
>>
>> http://git.mcs.anl.gov/radix/openpa.git/blob_plain/HEAD:/README
>>
>> indicates only platform support based on gcc plus Windows (but they say
>> they haven't set up a build system for it) plus Solaris (but that seems
>> to be Solaris 10+ only, at least my oldest Solaris 8 doesn't seem to
>> have the needed implementation).
>>
>> Regards,
>>
>> Rainer
>>
>>
>> By the way, Rainer, in your l*m*n test combinations, which compiler(s)
>> did you use on Solaris while confirming that 2.4.7 Event works at least
>> a little?
>
> I'm only building with gcc. On Solaris 8 with gcc 4.1.2, on Solaris 10
> the latest and greatest (4.8.2).
>
> Will post the full 2.4.7 results a bit later (only for the sake of
> completeness), but the builds are also broken for event on Linux 32 Bits
> even when using gcc:
>
> [Thu Nov 21 11:34:53.149765 2013] [mpm_event:crit] [pid 18492:tid
> 3080439472] AH02405: atomics not working as expected
>
> Sorry for not having checked progress during the tests, but the last
> combinations just finished right now.
>
> Regards,
>
> Rainer
>
Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number server/mpm/event/event.c server/mpm/eventopt/eventopt.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
I'm just curious why people never saw this on trunk...
That code path is almost exactly a year old. No one has
hit this in 12 months??
On Nov 23, 2013, at 12:45 PM, Rainer Jung <ra...@kippdata.de> wrote:
> On 23.11.2013 14:15, Jeff Trawick wrote:
>> On Sat, Nov 23, 2013 at 8:04 AM, Rainer Jung <rainer.jung@kippdata.de
>> <ma...@kippdata.de>> wrote:
>>
>> On 22.11.2013 23:03, Jim Jagielski wrote:
>>> Anyone ever try OpenPA?
>>>
>>> https://trac.mcs.anl.gov/projects/openpa/
>>>
>>> It's under MIT, fwiw.
>>
>> Haven't tried it but the README
>>
>> http://git.mcs.anl.gov/radix/openpa.git/blob_plain/HEAD:/README
>>
>> indicates only platform support based on gcc plus Windows (but they say
>> they haven't set up a build system for it) plus Solaris (but that seems
>> to be Solaris 10+ only, at least my oldest Solaris 8 doesn't seem to
>> have the needed implementation).
>>
>> Regards,
>>
>> Rainer
>>
>>
>> By the way, Rainer, in your l*m*n test combinations, which compiler(s)
>> did you use on Solaris while confirming that 2.4.7 Event works at least
>> a little?
>
> I'm only building with gcc. On Solaris 8 with gcc 4.1.2, on Solaris 10
> the latest and greatest (4.8.2).
>
> Will post the full 2.4.7 results a bit later (only for the sake of
> completeness), but the builds are also broken for event on Linux 32 Bits
> even when using gcc:
>
> [Thu Nov 21 11:34:53.149765 2013] [mpm_event:crit] [pid 18492:tid
> 3080439472] AH02405: atomics not working as expected
>
> Sorry for not having checked progress during the tests, but the last
> combinations just finished right now.
>
> Regards,
>
> Rainer
>
Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number
server/mpm/event/event.c server/mpm/eventopt/eventopt.c
Posted by Rainer Jung <ra...@kippdata.de>.
On 23.11.2013 14:15, Jeff Trawick wrote:
> On Sat, Nov 23, 2013 at 8:04 AM, Rainer Jung <rainer.jung@kippdata.de
> <ma...@kippdata.de>> wrote:
>
> On 22.11.2013 23:03, Jim Jagielski wrote:
> > Anyone ever try OpenPA?
> >
> > https://trac.mcs.anl.gov/projects/openpa/
> >
> > It's under MIT, fwiw.
>
> Haven't tried it but the README
>
> http://git.mcs.anl.gov/radix/openpa.git/blob_plain/HEAD:/README
>
> indicates only platform support based on gcc plus Windows (but they say
> they haven't set up a build system for it) plus Solaris (but that seems
> to be Solaris 10+ only, at least my oldest Solaris 8 doesn't seem to
> have the needed implementation).
>
> Regards,
>
> Rainer
>
>
> By the way, Rainer, in your l*m*n test combinations, which compiler(s)
> did you use on Solaris while confirming that 2.4.7 Event works at least
> a little?
I'm only building with gcc. On Solaris 8 with gcc 4.1.2, on Solaris 10
the latest and greatest (4.8.2).
Will post the full 2.4.7 results a bit later (only for the sake of
completeness), but the builds are also broken for event on Linux 32 Bits
even when using gcc:
[Thu Nov 21 11:34:53.149765 2013] [mpm_event:crit] [pid 18492:tid
3080439472] AH02405: atomics not working as expected
Sorry for not having checked progress during the tests, but the last
combinations just finished right now.
Regards,
Rainer
Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number
server/mpm/event/event.c server/mpm/eventopt/eventopt.c
Posted by Rainer Jung <ra...@kippdata.de>.
On 23.11.2013 14:15, Jeff Trawick wrote:
> On Sat, Nov 23, 2013 at 8:04 AM, Rainer Jung <rainer.jung@kippdata.de
> <ma...@kippdata.de>> wrote:
>
> On 22.11.2013 23:03, Jim Jagielski wrote:
> > Anyone ever try OpenPA?
> >
> > https://trac.mcs.anl.gov/projects/openpa/
> >
> > It's under MIT, fwiw.
>
> Haven't tried it but the README
>
> http://git.mcs.anl.gov/radix/openpa.git/blob_plain/HEAD:/README
>
> indicates only platform support based on gcc plus Windows (but they say
> they haven't set up a build system for it) plus Solaris (but that seems
> to be Solaris 10+ only, at least my oldest Solaris 8 doesn't seem to
> have the needed implementation).
>
> Regards,
>
> Rainer
>
>
> By the way, Rainer, in your l*m*n test combinations, which compiler(s)
> did you use on Solaris while confirming that 2.4.7 Event works at least
> a little?
I'm only building with gcc. On Solaris 8 with gcc 4.1.2, on Solaris 10
the latest and greatest (4.8.2).
Will post the full 2.4.7 results a bit later (only for the sake of
completeness), but the builds are also broken for event on Linux 32 Bits
even when using gcc:
[Thu Nov 21 11:34:53.149765 2013] [mpm_event:crit] [pid 18492:tid
3080439472] AH02405: atomics not working as expected
Sorry for not having checked progress during the tests, but the last
combinations just finished right now.
Regards,
Rainer
Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number
server/mpm/event/event.c server/mpm/eventopt/eventopt.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Sat, Nov 23, 2013 at 8:04 AM, Rainer Jung <ra...@kippdata.de>wrote:
> On 22.11.2013 23:03, Jim Jagielski wrote:
> > Anyone ever try OpenPA?
> >
> > https://trac.mcs.anl.gov/projects/openpa/
> >
> > It's under MIT, fwiw.
>
> Haven't tried it but the README
>
> http://git.mcs.anl.gov/radix/openpa.git/blob_plain/HEAD:/README
>
> indicates only platform support based on gcc plus Windows (but they say
> they haven't set up a build system for it) plus Solaris (but that seems
> to be Solaris 10+ only, at least my oldest Solaris 8 doesn't seem to
> have the needed implementation).
>
> Regards,
>
> Rainer
>
>
By the way, Rainer, in your l*m*n test combinations, which compiler(s) did
you use on Solaris while confirming that 2.4.7 Event works at least a
little?
--
Born in Roswell... married an alien...
http://emptyhammock.com/
Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number
server/mpm/event/event.c server/mpm/eventopt/eventopt.c
Posted by Rainer Jung <ra...@kippdata.de>.
On 22.11.2013 23:03, Jim Jagielski wrote:
> Anyone ever try OpenPA?
>
> https://trac.mcs.anl.gov/projects/openpa/
>
> It's under MIT, fwiw.
Haven't tried it but the README
http://git.mcs.anl.gov/radix/openpa.git/blob_plain/HEAD:/README
indicates only platform support based on gcc plus Windows (but they say
they haven't set up a build system for it) plus Solaris (but that seems
to be Solaris 10+ only, at least my oldest Solaris 8 doesn't seem to
have the needed implementation).
Regards,
Rainer
Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number server/mpm/event/event.c server/mpm/eventopt/eventopt.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
Anyone ever try OpenPA?
https://trac.mcs.anl.gov/projects/openpa/
It's under MIT, fwiw.
Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number server/mpm/event/event.c server/mpm/eventopt/eventopt.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
Anyone ever try OpenPA?
https://trac.mcs.anl.gov/projects/openpa/
It's under MIT, fwiw.
Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number
server/mpm/event/event.c server/mpm/eventopt/eventopt.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Fri, Nov 22, 2013 at 4:27 PM, Jeff Trawick <tr...@gmail.com> wrote:
> On Fri, Nov 22, 2013 at 3:57 PM, Jeff Trawick <tr...@gmail.com> wrote:
>
>> On Fri, Nov 22, 2013 at 3:40 PM, Jeff Trawick <tr...@gmail.com> wrote:
>>
>>> On Fri, Nov 22, 2013 at 3:24 PM, Jeff Trawick <tr...@gmail.com> wrote:
>>>
>>>> On Fri, Nov 22, 2013 at 2:52 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>>
>>>>> Note, the only think changed in event now (via
>>>>> https://svn.apache.org/viewvc?view=revision&revision=1542560)
>>>>> is that event *checks* that atomics work as required for
>>>>> event... if the check fails, it means that event has
>>>>> been "broken" on that system, assuming it ever hit
>>>>> blocked idlers, for a *long* time...
>>>>>
>>>>
>>>> Got it... fdqueue.c is asking for trouble...
>>>>
>>>> I'm using atomic/unix/ia32.c with icc too.
>>>>
>>>> Need to compare generated code... I hate stuff like "int foo() {
>>>> unsigned char x; ... return x; }"
>>>>
>>>>
>>> APR is documented as returning "zero if the value becomes zero on
>>> decrement, otherwise non-zero".
>>>
>>> With gcc we use get __sync_sub_and_fetch(), which returns the new value
>>> after the decrement.
>>>
>>> With icc we use the assembler implementation in APR. I think that's
>>> returning 1 instead of -1.
>>>
>>
>> It does a decrement long, which sets the zero flag as appropriate. Then
>> it does set-not-equal to set what becomes the return value to 0 if the
>> result of the decrement was 0 and 1 otherwise.
>>
>> It "should be easy" to make it return the new value like the
>> __sync_sub_and_fetch() version does :)
>>
>
> Any Intel assembly gurus out there?
>
> --- apr.orig/atomic/unix/ia32.c 2013-04-21 14:28:47.000000000 -0700
> +++ apr/atomic/unix/ia32.c 2013-11-22 13:16:04.000000000 -0800
> @@ -57,14 +57,14 @@
>
> APR_DECLARE(int) apr_atomic_dec32(volatile apr_uint32_t *mem)
> {
> - unsigned char prev;
> + apr_uint32_t newvalue;
>
> - asm volatile ("lock; decl %0; setnz %1"
> - : "=m" (*mem), "=qm" (prev)
> + asm volatile ("lock; decl %0; movl %0,%1"
> + : "=m" (*mem), "=qm" (newvalue)
> : "m" (*mem)
> : "memory");
>
> - return prev;
> + return newvalue;
> }
>
> APR_DECLARE(apr_uint32_t) apr_atomic_cas32(volatile apr_uint32_t *mem,
> apr_uint32_t with,
>
> It probably uses a zillion more cycles than the previous version. I
> wonder what __sync_sub_and_fetch() does exactly.
>
> Also, maybe stdcxx has an implementation somewhere that matches
> __sync_sub_and_fetch() (I see
> http://svn.apache.org/repos/asf/stdcxx/branches/4.2.x/include/rw/_atomic-sync.h,
> but no time to look now.
>
"Most" (I won't swear exactly which ones ;) ) APR implementations of
apr_atomic_dec32() return the new value. The z/OS code in
atomic/os390/atomic.c needs an obvious fix too. (IIUC, Event MPM should
work there because of recently added pollset support, though I guess a few
lines in configure and event need to be patched to recognize the new
pollset implementation as usable by event.)
>
>>
>>>
>>> Here is where fdqueue needs to be able to see a negative return code:
>>>
>>> apr_status_t ap_queue_info_try_get_idler(fd_queue_info_t * queue_info)
>>> {
>>> int prev_idlers;
>>> prev_idlers = apr_atomic_dec32((apr_uint32_t
>>> *)&(queue_info->idlers));
>>> if (prev_idlers <= 0) {
>>> apr_atomic_inc32((apr_uint32_t *)&(queue_info->idlers)); /*
>>> back out dec */
>>> return APR_EAGAIN;
>>> }
>>> return APR_SUCCESS;
>>> }
>>>
>>> ("prev" in the ia32.c version of apr_atomic_dec32() and "prev_idlers"
>>> here is deceiving.)
>>>
>>>
>>>>
>>>>>
>>>>> You should be seeing it in trunk as well...
>>>>>
>>>>>
>>>>> On Nov 22, 2013, at 2:43 PM, Jeff Trawick <tr...@gmail.com> wrote:
>>>>>
>>>>> > On Fri, Nov 22, 2013 at 2:39 PM, Jim Jagielski <ji...@jagunet.com>
>>>>> wrote:
>>>>> >
>>>>> > On Nov 22, 2013, at 2:22 PM, Jeff Trawick <tr...@gmail.com> wrote:
>>>>> >
>>>>> > > On Sat, Nov 17, 2012 at 6:00 AM, Ruediger Pluem <rp...@apache.org>
>>>>> wrote:
>>>>> > >
>>>>> > >
>>>>> > > jim@apache.org wrote:
>>>>> > > > + i = apr_atomic_dec32(&foo);
>>>>> > > > + if (i >= 0) {
>>>>> > >
>>>>> > > Why can we expect i < 0? apr_atomic_dec32 returns 0 if the dec
>>>>> causes foo to become zero and it returns non zero
>>>>> > > otherwise. Shouldn't this behavior the same across all platforms?
>>>>> And if not should that be fixed in APR?
>>>>> > >
>>>>> > > icc (Intel) builds of httpd 2.4.7 event MPM (with apr-1.5.0) bomb
>>>>> here.
>>>>> > >
>>>>> > > --enable-nonportable-atomics is specified for apr, though I
>>>>> haven't checked what that does with icc.
>>>>> > >
>>>>> >
>>>>> > As noted back with the orig update, this test is due to the
>>>>> > fdqueue code in the new event:
>>>>> >
>>>>> > apr_status_t ap_queue_info_set_idle(fd_queue_info_t * queue_info,
>>>>> > apr_pool_t * pool_to_recycle)
>>>>> > {
>>>>> > apr_status_t rv;
>>>>> > int prev_idlers;
>>>>> >
>>>>> > ap_push_pool(queue_info, pool_to_recycle);
>>>>> >
>>>>> > /* Atomically increment the count of idle workers */
>>>>> > /*
>>>>> > * TODO: The atomics expect unsigned whereas we're using signed.
>>>>> > * Need to double check that they work as expected or else
>>>>> > * rework how we determine blocked.
>>>>> > * UPDATE: Correct operation is performed during open_logs()
>>>>> > */
>>>>> > prev_idlers = apr_atomic_inc32((apr_uint32_t
>>>>> *)&(queue_info->idlers));
>>>>> >
>>>>> > /* If other threads are waiting on a worker, wake one up */
>>>>> > if (prev_idlers < 0) {
>>>>> >
>>>>> >
>>>>> > See the comments ("The atomics expect unsigned whereas...") for
>>>>> > the reason, etc.
>>>>> >
>>>>> > When you say "icc (Intel) builds of httpd 2.4.7 event MPM (with
>>>>> apr-1.5.0) bomb here."
>>>>> > do you mean that you get the 'atomics not working as expected' error
>>>>> > (and the internal server error) or that it core dumps?
>>>>> >
>>>>> >
>>>>> > "atomics not working as expected"
>>>>> >
>>>>> > Let me see what code is used...
>>>>> >
>>>>> > --
>>>>> > Born in Roswell... married an alien...
>>>>> > http://emptyhammock.com/
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Born in Roswell... married an alien...
>>>> http://emptyhammock.com/
>>>>
>>>
>>>
>>>
>>> --
>>> Born in Roswell... married an alien...
>>> http://emptyhammock.com/
>>>
>>
>>
>>
>> --
>> Born in Roswell... married an alien...
>> http://emptyhammock.com/
>>
>
>
>
> --
> Born in Roswell... married an alien...
> http://emptyhammock.com/
>
--
Born in Roswell... married an alien...
http://emptyhammock.com/
Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number
server/mpm/event/event.c server/mpm/eventopt/eventopt.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Fri, Nov 22, 2013 at 4:27 PM, Jeff Trawick <tr...@gmail.com> wrote:
> On Fri, Nov 22, 2013 at 3:57 PM, Jeff Trawick <tr...@gmail.com> wrote:
>
>> On Fri, Nov 22, 2013 at 3:40 PM, Jeff Trawick <tr...@gmail.com> wrote:
>>
>>> On Fri, Nov 22, 2013 at 3:24 PM, Jeff Trawick <tr...@gmail.com> wrote:
>>>
>>>> On Fri, Nov 22, 2013 at 2:52 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>>
>>>>> Note, the only think changed in event now (via
>>>>> https://svn.apache.org/viewvc?view=revision&revision=1542560)
>>>>> is that event *checks* that atomics work as required for
>>>>> event... if the check fails, it means that event has
>>>>> been "broken" on that system, assuming it ever hit
>>>>> blocked idlers, for a *long* time...
>>>>>
>>>>
>>>> Got it... fdqueue.c is asking for trouble...
>>>>
>>>> I'm using atomic/unix/ia32.c with icc too.
>>>>
>>>> Need to compare generated code... I hate stuff like "int foo() {
>>>> unsigned char x; ... return x; }"
>>>>
>>>>
>>> APR is documented as returning "zero if the value becomes zero on
>>> decrement, otherwise non-zero".
>>>
>>> With gcc we use get __sync_sub_and_fetch(), which returns the new value
>>> after the decrement.
>>>
>>> With icc we use the assembler implementation in APR. I think that's
>>> returning 1 instead of -1.
>>>
>>
>> It does a decrement long, which sets the zero flag as appropriate. Then
>> it does set-not-equal to set what becomes the return value to 0 if the
>> result of the decrement was 0 and 1 otherwise.
>>
>> It "should be easy" to make it return the new value like the
>> __sync_sub_and_fetch() version does :)
>>
>
> Any Intel assembly gurus out there?
>
> --- apr.orig/atomic/unix/ia32.c 2013-04-21 14:28:47.000000000 -0700
> +++ apr/atomic/unix/ia32.c 2013-11-22 13:16:04.000000000 -0800
> @@ -57,14 +57,14 @@
>
> APR_DECLARE(int) apr_atomic_dec32(volatile apr_uint32_t *mem)
> {
> - unsigned char prev;
> + apr_uint32_t newvalue;
>
> - asm volatile ("lock; decl %0; setnz %1"
> - : "=m" (*mem), "=qm" (prev)
> + asm volatile ("lock; decl %0; movl %0,%1"
> + : "=m" (*mem), "=qm" (newvalue)
> : "m" (*mem)
> : "memory");
>
> - return prev;
> + return newvalue;
> }
>
> APR_DECLARE(apr_uint32_t) apr_atomic_cas32(volatile apr_uint32_t *mem,
> apr_uint32_t with,
>
> It probably uses a zillion more cycles than the previous version. I
> wonder what __sync_sub_and_fetch() does exactly.
>
> Also, maybe stdcxx has an implementation somewhere that matches
> __sync_sub_and_fetch() (I see
> http://svn.apache.org/repos/asf/stdcxx/branches/4.2.x/include/rw/_atomic-sync.h,
> but no time to look now.
>
"Most" (I won't swear exactly which ones ;) ) APR implementations of
apr_atomic_dec32() return the new value. The z/OS code in
atomic/os390/atomic.c needs an obvious fix too. (IIUC, Event MPM should
work there because of recently added pollset support, though I guess a few
lines in configure and event need to be patched to recognize the new
pollset implementation as usable by event.)
>
>>
>>>
>>> Here is where fdqueue needs to be able to see a negative return code:
>>>
>>> apr_status_t ap_queue_info_try_get_idler(fd_queue_info_t * queue_info)
>>> {
>>> int prev_idlers;
>>> prev_idlers = apr_atomic_dec32((apr_uint32_t
>>> *)&(queue_info->idlers));
>>> if (prev_idlers <= 0) {
>>> apr_atomic_inc32((apr_uint32_t *)&(queue_info->idlers)); /*
>>> back out dec */
>>> return APR_EAGAIN;
>>> }
>>> return APR_SUCCESS;
>>> }
>>>
>>> ("prev" in the ia32.c version of apr_atomic_dec32() and "prev_idlers"
>>> here is deceiving.)
>>>
>>>
>>>>
>>>>>
>>>>> You should be seeing it in trunk as well...
>>>>>
>>>>>
>>>>> On Nov 22, 2013, at 2:43 PM, Jeff Trawick <tr...@gmail.com> wrote:
>>>>>
>>>>> > On Fri, Nov 22, 2013 at 2:39 PM, Jim Jagielski <ji...@jagunet.com>
>>>>> wrote:
>>>>> >
>>>>> > On Nov 22, 2013, at 2:22 PM, Jeff Trawick <tr...@gmail.com> wrote:
>>>>> >
>>>>> > > On Sat, Nov 17, 2012 at 6:00 AM, Ruediger Pluem <rp...@apache.org>
>>>>> wrote:
>>>>> > >
>>>>> > >
>>>>> > > jim@apache.org wrote:
>>>>> > > > + i = apr_atomic_dec32(&foo);
>>>>> > > > + if (i >= 0) {
>>>>> > >
>>>>> > > Why can we expect i < 0? apr_atomic_dec32 returns 0 if the dec
>>>>> causes foo to become zero and it returns non zero
>>>>> > > otherwise. Shouldn't this behavior the same across all platforms?
>>>>> And if not should that be fixed in APR?
>>>>> > >
>>>>> > > icc (Intel) builds of httpd 2.4.7 event MPM (with apr-1.5.0) bomb
>>>>> here.
>>>>> > >
>>>>> > > --enable-nonportable-atomics is specified for apr, though I
>>>>> haven't checked what that does with icc.
>>>>> > >
>>>>> >
>>>>> > As noted back with the orig update, this test is due to the
>>>>> > fdqueue code in the new event:
>>>>> >
>>>>> > apr_status_t ap_queue_info_set_idle(fd_queue_info_t * queue_info,
>>>>> > apr_pool_t * pool_to_recycle)
>>>>> > {
>>>>> > apr_status_t rv;
>>>>> > int prev_idlers;
>>>>> >
>>>>> > ap_push_pool(queue_info, pool_to_recycle);
>>>>> >
>>>>> > /* Atomically increment the count of idle workers */
>>>>> > /*
>>>>> > * TODO: The atomics expect unsigned whereas we're using signed.
>>>>> > * Need to double check that they work as expected or else
>>>>> > * rework how we determine blocked.
>>>>> > * UPDATE: Correct operation is performed during open_logs()
>>>>> > */
>>>>> > prev_idlers = apr_atomic_inc32((apr_uint32_t
>>>>> *)&(queue_info->idlers));
>>>>> >
>>>>> > /* If other threads are waiting on a worker, wake one up */
>>>>> > if (prev_idlers < 0) {
>>>>> >
>>>>> >
>>>>> > See the comments ("The atomics expect unsigned whereas...") for
>>>>> > the reason, etc.
>>>>> >
>>>>> > When you say "icc (Intel) builds of httpd 2.4.7 event MPM (with
>>>>> apr-1.5.0) bomb here."
>>>>> > do you mean that you get the 'atomics not working as expected' error
>>>>> > (and the internal server error) or that it core dumps?
>>>>> >
>>>>> >
>>>>> > "atomics not working as expected"
>>>>> >
>>>>> > Let me see what code is used...
>>>>> >
>>>>> > --
>>>>> > Born in Roswell... married an alien...
>>>>> > http://emptyhammock.com/
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Born in Roswell... married an alien...
>>>> http://emptyhammock.com/
>>>>
>>>
>>>
>>>
>>> --
>>> Born in Roswell... married an alien...
>>> http://emptyhammock.com/
>>>
>>
>>
>>
>> --
>> Born in Roswell... married an alien...
>> http://emptyhammock.com/
>>
>
>
>
> --
> Born in Roswell... married an alien...
> http://emptyhammock.com/
>
--
Born in Roswell... married an alien...
http://emptyhammock.com/
Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number
server/mpm/event/event.c server/mpm/eventopt/eventopt.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Fri, Nov 22, 2013 at 3:57 PM, Jeff Trawick <tr...@gmail.com> wrote:
> On Fri, Nov 22, 2013 at 3:40 PM, Jeff Trawick <tr...@gmail.com> wrote:
>
>> On Fri, Nov 22, 2013 at 3:24 PM, Jeff Trawick <tr...@gmail.com> wrote:
>>
>>> On Fri, Nov 22, 2013 at 2:52 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>
>>>> Note, the only think changed in event now (via
>>>> https://svn.apache.org/viewvc?view=revision&revision=1542560)
>>>> is that event *checks* that atomics work as required for
>>>> event... if the check fails, it means that event has
>>>> been "broken" on that system, assuming it ever hit
>>>> blocked idlers, for a *long* time...
>>>>
>>>
>>> Got it... fdqueue.c is asking for trouble...
>>>
>>> I'm using atomic/unix/ia32.c with icc too.
>>>
>>> Need to compare generated code... I hate stuff like "int foo() {
>>> unsigned char x; ... return x; }"
>>>
>>>
>> APR is documented as returning "zero if the value becomes zero on
>> decrement, otherwise non-zero".
>>
>> With gcc we use get __sync_sub_and_fetch(), which returns the new value
>> after the decrement.
>>
>> With icc we use the assembler implementation in APR. I think that's
>> returning 1 instead of -1.
>>
>
> It does a decrement long, which sets the zero flag as appropriate. Then
> it does set-not-equal to set what becomes the return value to 0 if the
> result of the decrement was 0 and 1 otherwise.
>
> It "should be easy" to make it return the new value like the
> __sync_sub_and_fetch() version does :)
>
Any Intel assembly gurus out there?
--- apr.orig/atomic/unix/ia32.c 2013-04-21 14:28:47.000000000 -0700
+++ apr/atomic/unix/ia32.c 2013-11-22 13:16:04.000000000 -0800
@@ -57,14 +57,14 @@
APR_DECLARE(int) apr_atomic_dec32(volatile apr_uint32_t *mem)
{
- unsigned char prev;
+ apr_uint32_t newvalue;
- asm volatile ("lock; decl %0; setnz %1"
- : "=m" (*mem), "=qm" (prev)
+ asm volatile ("lock; decl %0; movl %0,%1"
+ : "=m" (*mem), "=qm" (newvalue)
: "m" (*mem)
: "memory");
- return prev;
+ return newvalue;
}
APR_DECLARE(apr_uint32_t) apr_atomic_cas32(volatile apr_uint32_t *mem,
apr_uint32_t with,
It probably uses a zillion more cycles than the previous version. I wonder
what __sync_sub_and_fetch() does exactly.
Also, maybe stdcxx has an implementation somewhere that matches
__sync_sub_and_fetch() (I see
http://svn.apache.org/repos/asf/stdcxx/branches/4.2.x/include/rw/_atomic-sync.h,
but no time to look now.
>
>>
>> Here is where fdqueue needs to be able to see a negative return code:
>>
>> apr_status_t ap_queue_info_try_get_idler(fd_queue_info_t * queue_info)
>> {
>> int prev_idlers;
>> prev_idlers = apr_atomic_dec32((apr_uint32_t *)&(queue_info->idlers));
>> if (prev_idlers <= 0) {
>> apr_atomic_inc32((apr_uint32_t *)&(queue_info->idlers)); /*
>> back out dec */
>> return APR_EAGAIN;
>> }
>> return APR_SUCCESS;
>> }
>>
>> ("prev" in the ia32.c version of apr_atomic_dec32() and "prev_idlers"
>> here is deceiving.)
>>
>>
>>>
>>>>
>>>> You should be seeing it in trunk as well...
>>>>
>>>>
>>>> On Nov 22, 2013, at 2:43 PM, Jeff Trawick <tr...@gmail.com> wrote:
>>>>
>>>> > On Fri, Nov 22, 2013 at 2:39 PM, Jim Jagielski <ji...@jagunet.com>
>>>> wrote:
>>>> >
>>>> > On Nov 22, 2013, at 2:22 PM, Jeff Trawick <tr...@gmail.com> wrote:
>>>> >
>>>> > > On Sat, Nov 17, 2012 at 6:00 AM, Ruediger Pluem <rp...@apache.org>
>>>> wrote:
>>>> > >
>>>> > >
>>>> > > jim@apache.org wrote:
>>>> > > > + i = apr_atomic_dec32(&foo);
>>>> > > > + if (i >= 0) {
>>>> > >
>>>> > > Why can we expect i < 0? apr_atomic_dec32 returns 0 if the dec
>>>> causes foo to become zero and it returns non zero
>>>> > > otherwise. Shouldn't this behavior the same across all platforms?
>>>> And if not should that be fixed in APR?
>>>> > >
>>>> > > icc (Intel) builds of httpd 2.4.7 event MPM (with apr-1.5.0) bomb
>>>> here.
>>>> > >
>>>> > > --enable-nonportable-atomics is specified for apr, though I haven't
>>>> checked what that does with icc.
>>>> > >
>>>> >
>>>> > As noted back with the orig update, this test is due to the
>>>> > fdqueue code in the new event:
>>>> >
>>>> > apr_status_t ap_queue_info_set_idle(fd_queue_info_t * queue_info,
>>>> > apr_pool_t * pool_to_recycle)
>>>> > {
>>>> > apr_status_t rv;
>>>> > int prev_idlers;
>>>> >
>>>> > ap_push_pool(queue_info, pool_to_recycle);
>>>> >
>>>> > /* Atomically increment the count of idle workers */
>>>> > /*
>>>> > * TODO: The atomics expect unsigned whereas we're using signed.
>>>> > * Need to double check that they work as expected or else
>>>> > * rework how we determine blocked.
>>>> > * UPDATE: Correct operation is performed during open_logs()
>>>> > */
>>>> > prev_idlers = apr_atomic_inc32((apr_uint32_t
>>>> *)&(queue_info->idlers));
>>>> >
>>>> > /* If other threads are waiting on a worker, wake one up */
>>>> > if (prev_idlers < 0) {
>>>> >
>>>> >
>>>> > See the comments ("The atomics expect unsigned whereas...") for
>>>> > the reason, etc.
>>>> >
>>>> > When you say "icc (Intel) builds of httpd 2.4.7 event MPM (with
>>>> apr-1.5.0) bomb here."
>>>> > do you mean that you get the 'atomics not working as expected' error
>>>> > (and the internal server error) or that it core dumps?
>>>> >
>>>> >
>>>> > "atomics not working as expected"
>>>> >
>>>> > Let me see what code is used...
>>>> >
>>>> > --
>>>> > Born in Roswell... married an alien...
>>>> > http://emptyhammock.com/
>>>>
>>>>
>>>
>>>
>>> --
>>> Born in Roswell... married an alien...
>>> http://emptyhammock.com/
>>>
>>
>>
>>
>> --
>> Born in Roswell... married an alien...
>> http://emptyhammock.com/
>>
>
>
>
> --
> Born in Roswell... married an alien...
> http://emptyhammock.com/
>
--
Born in Roswell... married an alien...
http://emptyhammock.com/
Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number
server/mpm/event/event.c server/mpm/eventopt/eventopt.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Fri, Nov 22, 2013 at 3:57 PM, Jeff Trawick <tr...@gmail.com> wrote:
> On Fri, Nov 22, 2013 at 3:40 PM, Jeff Trawick <tr...@gmail.com> wrote:
>
>> On Fri, Nov 22, 2013 at 3:24 PM, Jeff Trawick <tr...@gmail.com> wrote:
>>
>>> On Fri, Nov 22, 2013 at 2:52 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>
>>>> Note, the only think changed in event now (via
>>>> https://svn.apache.org/viewvc?view=revision&revision=1542560)
>>>> is that event *checks* that atomics work as required for
>>>> event... if the check fails, it means that event has
>>>> been "broken" on that system, assuming it ever hit
>>>> blocked idlers, for a *long* time...
>>>>
>>>
>>> Got it... fdqueue.c is asking for trouble...
>>>
>>> I'm using atomic/unix/ia32.c with icc too.
>>>
>>> Need to compare generated code... I hate stuff like "int foo() {
>>> unsigned char x; ... return x; }"
>>>
>>>
>> APR is documented as returning "zero if the value becomes zero on
>> decrement, otherwise non-zero".
>>
>> With gcc we use get __sync_sub_and_fetch(), which returns the new value
>> after the decrement.
>>
>> With icc we use the assembler implementation in APR. I think that's
>> returning 1 instead of -1.
>>
>
> It does a decrement long, which sets the zero flag as appropriate. Then
> it does set-not-equal to set what becomes the return value to 0 if the
> result of the decrement was 0 and 1 otherwise.
>
> It "should be easy" to make it return the new value like the
> __sync_sub_and_fetch() version does :)
>
Any Intel assembly gurus out there?
--- apr.orig/atomic/unix/ia32.c 2013-04-21 14:28:47.000000000 -0700
+++ apr/atomic/unix/ia32.c 2013-11-22 13:16:04.000000000 -0800
@@ -57,14 +57,14 @@
APR_DECLARE(int) apr_atomic_dec32(volatile apr_uint32_t *mem)
{
- unsigned char prev;
+ apr_uint32_t newvalue;
- asm volatile ("lock; decl %0; setnz %1"
- : "=m" (*mem), "=qm" (prev)
+ asm volatile ("lock; decl %0; movl %0,%1"
+ : "=m" (*mem), "=qm" (newvalue)
: "m" (*mem)
: "memory");
- return prev;
+ return newvalue;
}
APR_DECLARE(apr_uint32_t) apr_atomic_cas32(volatile apr_uint32_t *mem,
apr_uint32_t with,
It probably uses a zillion more cycles than the previous version. I wonder
what __sync_sub_and_fetch() does exactly.
Also, maybe stdcxx has an implementation somewhere that matches
__sync_sub_and_fetch() (I see
http://svn.apache.org/repos/asf/stdcxx/branches/4.2.x/include/rw/_atomic-sync.h,
but no time to look now.
>
>>
>> Here is where fdqueue needs to be able to see a negative return code:
>>
>> apr_status_t ap_queue_info_try_get_idler(fd_queue_info_t * queue_info)
>> {
>> int prev_idlers;
>> prev_idlers = apr_atomic_dec32((apr_uint32_t *)&(queue_info->idlers));
>> if (prev_idlers <= 0) {
>> apr_atomic_inc32((apr_uint32_t *)&(queue_info->idlers)); /*
>> back out dec */
>> return APR_EAGAIN;
>> }
>> return APR_SUCCESS;
>> }
>>
>> ("prev" in the ia32.c version of apr_atomic_dec32() and "prev_idlers"
>> here is deceiving.)
>>
>>
>>>
>>>>
>>>> You should be seeing it in trunk as well...
>>>>
>>>>
>>>> On Nov 22, 2013, at 2:43 PM, Jeff Trawick <tr...@gmail.com> wrote:
>>>>
>>>> > On Fri, Nov 22, 2013 at 2:39 PM, Jim Jagielski <ji...@jagunet.com>
>>>> wrote:
>>>> >
>>>> > On Nov 22, 2013, at 2:22 PM, Jeff Trawick <tr...@gmail.com> wrote:
>>>> >
>>>> > > On Sat, Nov 17, 2012 at 6:00 AM, Ruediger Pluem <rp...@apache.org>
>>>> wrote:
>>>> > >
>>>> > >
>>>> > > jim@apache.org wrote:
>>>> > > > + i = apr_atomic_dec32(&foo);
>>>> > > > + if (i >= 0) {
>>>> > >
>>>> > > Why can we expect i < 0? apr_atomic_dec32 returns 0 if the dec
>>>> causes foo to become zero and it returns non zero
>>>> > > otherwise. Shouldn't this behavior the same across all platforms?
>>>> And if not should that be fixed in APR?
>>>> > >
>>>> > > icc (Intel) builds of httpd 2.4.7 event MPM (with apr-1.5.0) bomb
>>>> here.
>>>> > >
>>>> > > --enable-nonportable-atomics is specified for apr, though I haven't
>>>> checked what that does with icc.
>>>> > >
>>>> >
>>>> > As noted back with the orig update, this test is due to the
>>>> > fdqueue code in the new event:
>>>> >
>>>> > apr_status_t ap_queue_info_set_idle(fd_queue_info_t * queue_info,
>>>> > apr_pool_t * pool_to_recycle)
>>>> > {
>>>> > apr_status_t rv;
>>>> > int prev_idlers;
>>>> >
>>>> > ap_push_pool(queue_info, pool_to_recycle);
>>>> >
>>>> > /* Atomically increment the count of idle workers */
>>>> > /*
>>>> > * TODO: The atomics expect unsigned whereas we're using signed.
>>>> > * Need to double check that they work as expected or else
>>>> > * rework how we determine blocked.
>>>> > * UPDATE: Correct operation is performed during open_logs()
>>>> > */
>>>> > prev_idlers = apr_atomic_inc32((apr_uint32_t
>>>> *)&(queue_info->idlers));
>>>> >
>>>> > /* If other threads are waiting on a worker, wake one up */
>>>> > if (prev_idlers < 0) {
>>>> >
>>>> >
>>>> > See the comments ("The atomics expect unsigned whereas...") for
>>>> > the reason, etc.
>>>> >
>>>> > When you say "icc (Intel) builds of httpd 2.4.7 event MPM (with
>>>> apr-1.5.0) bomb here."
>>>> > do you mean that you get the 'atomics not working as expected' error
>>>> > (and the internal server error) or that it core dumps?
>>>> >
>>>> >
>>>> > "atomics not working as expected"
>>>> >
>>>> > Let me see what code is used...
>>>> >
>>>> > --
>>>> > Born in Roswell... married an alien...
>>>> > http://emptyhammock.com/
>>>>
>>>>
>>>
>>>
>>> --
>>> Born in Roswell... married an alien...
>>> http://emptyhammock.com/
>>>
>>
>>
>>
>> --
>> Born in Roswell... married an alien...
>> http://emptyhammock.com/
>>
>
>
>
> --
> Born in Roswell... married an alien...
> http://emptyhammock.com/
>
--
Born in Roswell... married an alien...
http://emptyhammock.com/