httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@gmail.com>
Subject Re: svn commit: r1410459 - in /httpd/httpd/trunk: docs/log-message-tags/next-number server/mpm/event/event.c server/mpm/eventopt/eventopt.c
Date Mon, 25 Nov 2013 18:54:11 GMT
On Mon, Nov 25, 2013 at 1:27 PM, Jim Jagielski <jim@jagunet.com> wrote:

>
> On Nov 24, 2013, at 7:18 PM, Jeff Trawick <trawick@gmail.com> wrote:
>
> > On Sat, Nov 23, 2013 at 5:39 PM, Yann Ylavic <ylavic.dev@gmail.com>
> wrote:
> > Couldn't ap_queue_info_try_get_idler() and the event_pre_config() check
> use :
> >
> >
> >     prev_idlers = apr_atomic_add32((apr_uint32_t
> *)&(queue_info->idlers), -1);
> >
> > like ap_queue_info_wait_for_idler() does ?
> >
> > I think that's correct...
> >
> > What the code really wants is new_idlers, so edit that slightly to
> >
> > new_idlers =  apr_atomic_add32((apr_uint32_t *)&(queue_info->idlers),
> -1) - 1;
> > if (new_idlers <= 0) {
> >     ...
> >     return APR_EAGAIN;
> > }
>
> It just wants to see if there are any idle ones... If there wasn't
> before the dec, then we need to return APR_EAGAIN. Recall that
> a 0 means "no idlers" and neg means 'this many blocking', and
> the logic is the same in both cases, minus the "wake up" scenario.
>
>
I think this is a safe progression from the original code, which was:

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) {

We know now that ALMOST every apr_atomic_dec32() returned the new idlers
value, and none returned the previous value, so fix the variable name to
reflect reality where it worked:

apr_status_t ap_queue_info_try_get_idler(fd_queue_info_t * queue_info)
{
    int new_idlers;
    new_idlers = apr_atomic_dec32((apr_uint32_t *)&(queue_info->idlers));
    if (new_idlers <= 0) {

You can't simply replace apr_atomic_dec32(foo) with apr_atomic_add32(foo,
-1) since the latter returns the old value instead of the new value, so you
have to subtract 1 from the return code of apr_atomic_dec32() to set
new_idlers to the same result as before.

-- 
Born in Roswell... married an alien...
http://emptyhammock.com/

Mime
View raw message