[sheepdog] [PATCH 2/2] sockfd_cache: guard fds_in_grow with atomic operations

[Liu Yuan]

On 11/01/2012 01:07 PM, MORITA Kazutaka wrote:
> check_idx can be called by multiple threads at the same time.
> 
> Signed-off-by: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp>
> ---
>  sheep/sockfd_cache.c |    7 +++----
>  1 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/sheep/sockfd_cache.c b/sheep/sockfd_cache.c
> index cdb71d7..ae23ae6 100644
> --- a/sheep/sockfd_cache.c
> +++ b/sheep/sockfd_cache.c
> @@ -305,15 +305,15 @@ static void do_grow_fds(struct work *work)
>  	pthread_rwlock_unlock(&sockfd_cache.lock);
>  }
>  
> -static bool fds_in_grow;
> +static int fds_in_grow;
>  static int fds_high_watermark = DEFAULT_FDS_WATERMARK;
>  
>  static void grow_fds_done(struct work *work)
>  {
> -	fds_in_grow = false;
>  	fds_count *= 2;
>  	fds_high_watermark = fds_count * 3 / 4;
>  	dprintf("fd count has been grown into %d\n", fds_count);
> +	uatomic_unlock(&fds_in_grow);
>  	free(work);
>  }
>  
> @@ -323,13 +323,12 @@ static inline void check_idx(int idx)
>  
>  	if (idx <= fds_high_watermark)
>  		return;
> -	if (fds_in_grow)
> +	if (!uatomic_trylock(&fds_in_grow))
>  		return;
>  
>  	w = xmalloc(sizeof(*w));
>  	w->fn = do_grow_fds;
>  	w->done = grow_fds_done;
> -	fds_in_grow = true;
>  	queue_work(sys->sockfd_wqueue, w);
>  }
>  
> 

It looks to me that trylock & unlock pair doesn't fit into true/false
semantics. This patch confuses the code a lot. I think uaotmic_read is
better than trylock for this function.

Thanks,
Yuan