httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Jagielski <...@jaguNET.com>
Subject Re: svn commit: r1545286 - in /httpd/httpd/trunk/server/mpm/event: event.c fdqueue.c
Date Mon, 25 Nov 2013 15:53:41 GMT

On Nov 25, 2013, at 10:24 AM, Yann Ylavic <ylavic.dev@gmail.com> wrote:

> 
> As Jeff said in the other thread, I think the test here should be :
>    if (prev_idlers <= 1)
> that's because dec32 was returning the new value while add32 now returns the old one
(fetch_and_sub vs sub_and_fetch).
> 

We aren't being consistent.. see below:

> +static const apr_uint32_t zero_pt = ((apr_uint32_t)1 << 31);

Hmmmm... for a 32bit int, shouldn't that be << 29 ??

>  
>  apr_status_t ap_queue_info_try_get_idler(fd_queue_info_t * queue_info)
>  {
> -    int prev_idlers;
> -    prev_idlers = apr_atomic_add32((apr_uint32_t *)&(queue_info->idlers), -1)
- zero_pt;
> -    if (prev_idlers <= 0) {

Note the check here....

> -        apr_atomic_inc32((apr_uint32_t *)&(queue_info->idlers));    /* back out
dec */
> +    apr_int32_t prev_idlers;
> +    prev_idlers = apr_atomic_add32(&queue_info->idlers, -1) - zero_pt;
> +    if (prev_idlers <= 1) {
> +        apr_atomic_inc32(&queue_info->idlers);    /* back out dec */
>          return APR_EAGAIN;
>      }
>      return APR_SUCCESS;
> @@ -140,11 +142,11 @@ apr_status_t ap_queue_info_wait_for_idler(fd_queue
>                                            int *had_to_block)
>  {
>      apr_status_t rv;
> -    int prev_idlers;
> +    apr_int32_t prev_idlers;
>  
>      /* Atomically decrement the idle worker count, saving the old value */
>      /* See TODO in ap_queue_info_set_idle() */
> -    prev_idlers = apr_atomic_add32((apr_uint32_t *)&(queue_info->idlers), -1)
- zero_pt;
> +    prev_idlers = apr_atomic_add32(&queue_info->idlers, -1) - zero_pt;
>  
>      /* Block if there weren't any idle workers */
>      if (prev_idlers <= 0) {

And here....



Mime
View raw message