Re: [PATCHv2 1/1] IB: rxe: remove pool state

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Dec 20, 2018 at 08:41:47AM -0500, Yanjun Zhu wrote:
> The pool state is the duplicate of pool ref_cnt. If ref_cnt > 0,
> it indicates that state is valid. If ref_cnt = 0, it indicates
> that state is invalid.
> 
> Signed-off-by: Zhu Yanjun <yanjun.zhu@xxxxxxxxxx>
> ---
> V1->V2: Follow Jason's advice, the state is replaced with ref_cnt.

Is it v2 the patch "IB/rxe: Remove duplicate pool invalidation"?

> ---
>  drivers/infiniband/sw/rxe/rxe_pool.c | 20 +++-----------------
>  drivers/infiniband/sw/rxe/rxe_pool.h |  6 ------
>  2 files changed, 3 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_pool.c b/drivers/infiniband/sw/rxe/rxe_pool.c
> index 36b53fb..d8f969d 100644
> --- a/drivers/infiniband/sw/rxe/rxe_pool.c
> +++ b/drivers/infiniband/sw/rxe/rxe_pool.c
> @@ -222,8 +222,6 @@ int rxe_pool_init(
>  		pool->key_size = rxe_type_info[type].key_size;
>  	}
>  
> -	pool->state = RXE_POOL_STATE_VALID;
> -
>  out:
>  	return err;
>  }
> @@ -232,7 +230,6 @@ static void rxe_pool_release(struct kref *kref)
>  {
>  	struct rxe_pool *pool = container_of(kref, struct rxe_pool, ref_cnt);
>  
> -	pool->state = RXE_POOL_STATE_INVALID;
>  	kfree(pool->table);
>  }
>  
> @@ -243,14 +240,9 @@ static void rxe_pool_put(struct rxe_pool *pool)
>  
>  int rxe_pool_cleanup(struct rxe_pool *pool)
>  {
> -	unsigned long flags;
> -
> -	write_lock_irqsave(&pool->pool_lock, flags);
> -	pool->state = RXE_POOL_STATE_INVALID;
>  	if (atomic_read(&pool->num_elem) > 0)
>  		pr_warn("%s pool destroyed with unfree'd elem\n",
>  			pool_name(pool));
> -	write_unlock_irqrestore(&pool->pool_lock, flags);

I wonder why it was surrounding the atomic_read call in the first place.

>  
>  	rxe_pool_put(pool);
>  
> @@ -380,17 +372,11 @@ void rxe_drop_index(void *arg)
>  void *rxe_alloc(struct rxe_pool *pool)
>  {
>  	struct rxe_pool_entry *elem;
> -	unsigned long flags;
>  
>  	might_sleep_if(!(pool->flags & RXE_POOL_ATOMIC));
>  
> -	read_lock_irqsave(&pool->pool_lock, flags);
> -	if (pool->state != RXE_POOL_STATE_VALID) {
> -		read_unlock_irqrestore(&pool->pool_lock, flags);
> +	if (!kref_get_unless_zero(&pool->ref_cnt))
>  		return NULL;
> -	}
> -	kref_get(&pool->ref_cnt);
> -	read_unlock_irqrestore(&pool->pool_lock, flags);
>  
>  	kref_get(&pool->rxe->ref_cnt);
>  
> @@ -438,7 +424,7 @@ void *rxe_pool_get_index(struct rxe_pool *pool, u32 index)
>  
>  	read_lock_irqsave(&pool->pool_lock, flags);
>  
> -	if (pool->state != RXE_POOL_STATE_VALID)
> +	if (!kref_read(&pool->ref_cnt))

So ref_cnt cannot go below 0,right?

Will it be safer to check <= 0, just to protect against incorrect calls to
rxe_pool_cleanup?

>  		goto out;
>  
>  	node = pool->tree.rb_node;
> @@ -470,7 +456,7 @@ void *rxe_pool_get_key(struct rxe_pool *pool, void *key)
>  
>  	read_lock_irqsave(&pool->pool_lock, flags);
>  
> -	if (pool->state != RXE_POOL_STATE_VALID)
> +	if (!kref_read(&pool->ref_cnt))

Ditto.

>  		goto out;
>  
>  	node = pool->tree.rb_node;
> diff --git a/drivers/infiniband/sw/rxe/rxe_pool.h b/drivers/infiniband/sw/rxe/rxe_pool.h
> index aa4ba30..eeca733 100644
> --- a/drivers/infiniband/sw/rxe/rxe_pool.h
> +++ b/drivers/infiniband/sw/rxe/rxe_pool.h
> @@ -73,11 +73,6 @@ struct rxe_type_info {
>  
>  extern struct rxe_type_info rxe_type_info[];
>  
> -enum rxe_pool_state {
> -	RXE_POOL_STATE_INVALID,
> -	RXE_POOL_STATE_VALID,
> -};
> -
>  struct rxe_pool_entry {
>  	struct rxe_pool		*pool;
>  	struct kref		ref_cnt;
> @@ -94,7 +89,6 @@ struct rxe_pool {
>  	size_t			elem_size;
>  	struct kref		ref_cnt;
>  	void			(*cleanup)(struct rxe_pool_entry *obj);
> -	enum rxe_pool_state	state;
>  	enum rxe_pool_flags	flags;
>  	enum rxe_elem_type	type;
>  
> -- 
> 2.7.4
> 



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux