Re: [PATCH for-next v6 3/8] RDMA/rxe: Cleanup pool APIs for keyed objects

[Jason Gunthorpe <jgg@nvidia.com>]

On Mon, Dec 06, 2021 at 03:12:38PM -0600, Bob Pearson wrote:
> +/**
> + * rxe_pool_add_key() - lookup or add object with key in pool
> + * @pool: the object pool
> + * @key: the key
> + *
> + * Returns: If object matching key is present in pool return
> + *	    its address and take a reference else allocate a
> + *	    new object to pool with key and return its address
> + *	    with one reference.
> + */
> +void *rxe_pool_add_key(struct rxe_pool *pool, void *key)
> +{
> +	void *obj;
> +
> +	rxe_pool_lock_bh(pool);
> +	obj = __rxe_get_key(pool, key);
> +	if (obj)
> +		goto done;
> +
> +	obj = __rxe_alloc(pool, GFP_ATOMIC);

Really try hard to avoid GFP_ATOMIC:

	rxe_pool_lock_bh(pool);
	obj = __rxe_get_key(pool, key);
        rxe_pool_unlock_bh(pool);
	if (obj)
	   return obj;

	obj = __rxe_alloc(pool, GFP_KERNEL);
	if (!obj)
	   return NULL;

	rxe_pool_lock_bh(pool);
	old_obj = __xa_cmpxchg(&pool->xarray.xa, key, NULL, entry, GFP_KERNEL);
	if (xa_is_err(old_obj))
           kfree(obj)
	   return NULL;

 	if (old_obj) {
	   kfree(obj);
	   obj = old_obj;
        }
        kref_get(old_obj)
        rxe_pool_unlock_bh(pool);
	return obj;       

If it is very rare that this function would collide the index then
forget the first lookup and use the cmpxchg

> +void rxe_elem_release(struct kref *kref)
> +{
> +	struct rxe_pool_elem *elem =
> +		container_of(kref, struct rxe_pool_elem, ref_cnt);
> +	struct rxe_pool *pool = elem->pool;
> +
> +	if (pool->flags & RXE_POOL_INDEX)
> +		__xa_erase(&pool->xarray.xa, elem->index);
> +
> +	if (pool->flags & RXE_POOL_KEY)
> +		rb_erase(&elem->key_node, &pool->key.tree);

It is a bit jarring to see these as not exclusive ?

Jason