Re: [PATCH rdma-core 4/6] rdmacm: Use C11 stdatomic for all atomics

[Bart Van Assche <Bart.VanAssche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2098 bytes --]

On Wed, 2017-03-15 at 17:18 -0600, Jason Gunthorpe wrote:
> Indeed, you are right, I will sqush in this patch, thanks
> 
> diff --git a/librdmacm/cma.h b/librdmacm/cma.h
> index 645d1e43576279..2f5f86cf498918 100644
> --- a/librdmacm/cma.h
> +++ b/librdmacm/cma.h
> @@ -69,12 +69,12 @@ static inline void fastlock_destroy(fastlock_t *lock)
>  }
>  static inline void fastlock_acquire(fastlock_t *lock)
>  {
> -	if (atomic_fetch_add(&lock->cnt, 1) > 1)
> +	if (atomic_fetch_add(&lock->cnt, 1) > 0)
>  		sem_wait(&lock->sem);
>  }
>  static inline void fastlock_release(fastlock_t *lock)
>  {
> -	if (atomic_fetch_sub(&lock->cnt, 1) > 0)
> +	if (atomic_fetch_sub(&lock->cnt, 1) > 1)
>  		sem_post(&lock->sem);
>  }
>  
> diff --git a/librdmacm/preload.c b/librdmacm/preload.c
> index bd1bcb1d701015..82cb0c03850b05 100644
> --- a/librdmacm/preload.c
> +++ b/librdmacm/preload.c
> @@ -1012,7 +1012,7 @@ int close(int socket)
>  			return ret;
>  	}
>  
> -	if (atomic_fetch_sub(&fdi->refcnt, 1))
> +	if (atomic_fetch_sub(&fdi->refcnt, 1) != 1)
>  		return 0;
>  
>  	idm_clear(&idm, socket);
> diff --git a/librdmacm/rsocket.c b/librdmacm/rsocket.c
> index 4fbe7aaa938610..9a71035fbeed8b 100644
> --- a/librdmacm/rsocket.c
> +++ b/librdmacm/rsocket.c
> @@ -898,7 +898,7 @@ static int rs_create_ep(struct rsocket *rs)
>  
>  static void rs_release_iomap_mr(struct rs_iomap_mr *iomr)
>  {
> -	if (atomic_fetch_sub(&iomr->refcnt, 1))
> +	if (atomic_fetch_sub(&iomr->refcnt, 1) != 1)
>  		return;
>  
>  	dlist_remove(&iomr->entry);

Hello Jason,

Have you considered to introduce something like the function below? I think
that would make the changes smaller and hence easier to review.

static inline int atomic_add_fetch(_Atomic(int) *a, int arg)
{
	return atomic_fetch_add(a, arg) + arg;
}

static inline int atomic_sub_fetch(_Atomic(int) *a, int arg)
{
	return atomic_fetch_sub(a, arg) - arg;
}

Bart.
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš