Re: [PATCH v9 04/25] RDMA/rtrs: core: lib functions shared between client and server modules

[Leon Romanovsky <leon@kernel.org>]

On Fri, Feb 21, 2020 at 11:47:00AM +0100, Jack Wang wrote:
> From: Jack Wang <jinpu.wang@cloud.ionos.com>
>
> This is a set of library functions existing as a rtrs-core module,
> used by client and server modules.
>
> Mainly these functions wrap IB and RDMA calls and provide a bit higher
> abstraction for implementing of RTRS protocol on client or server
> sides.
>
> Signed-off-by: Danil Kipnis <danil.kipnis@cloud.ionos.com>
> Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
> ---
>  drivers/infiniband/ulp/rtrs/rtrs.c | 594 +++++++++++++++++++++++++++++
>  1 file changed, 594 insertions(+)
>  create mode 100644 drivers/infiniband/ulp/rtrs/rtrs.c

<...>

> +
> +static void dev_free(struct kref *ref)
> +{
> +	struct rtrs_rdma_dev_pd *pool;
> +	struct rtrs_ib_dev *dev;
> +
> +	dev = container_of(ref, typeof(*dev), ref);
> +	pool = dev->pool;
> +
> +	mutex_lock(&pool->mutex);
> +	list_del(&dev->entry);
> +	mutex_unlock(&pool->mutex);
> +
> +	if (pool->ops && pool->ops->deinit)
> +		pool->ops->deinit(dev);
> +
> +	ib_dealloc_pd(dev->ib_pd);
> +
> +	if (pool->ops && pool->ops->free)
> +		pool->ops->free(dev);
> +	else
> +		kfree(dev);
> +}
> +
> +int rtrs_ib_dev_put(struct rtrs_ib_dev *dev)
> +{
> +	return kref_put(&dev->ref, dev_free);
> +}
> +EXPORT_SYMBOL(rtrs_ib_dev_put);
> +
> +static int rtrs_ib_dev_get(struct rtrs_ib_dev *dev)
> +{
> +	return kref_get_unless_zero(&dev->ref);
> +}
> +
> +struct rtrs_ib_dev *
> +rtrs_ib_dev_find_or_add(struct ib_device *ib_dev,
> +			 struct rtrs_rdma_dev_pd *pool)
> +{
> +	struct rtrs_ib_dev *dev;
> +
> +	mutex_lock(&pool->mutex);

The scope of this mutex is unclear, you protected everything here with
this mutex, but in dev_free() you guarded list_del() only.

> +	list_for_each_entry(dev, &pool->list, entry) {
> +		if (dev->ib_dev->node_guid == ib_dev->node_guid &&
> +		    rtrs_ib_dev_get(dev))
> +			goto out_unlock;
> +	}
> +	if (pool->ops && pool->ops->alloc)
> +		dev = pool->ops->alloc();
> +	else
> +		dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> +	if (IS_ERR_OR_NULL(dev))
> +		goto out_err;
> +
> +	kref_init(&dev->ref);
> +	dev->pool = pool;
> +	dev->ib_dev = ib_dev;
> +	dev->ib_pd = ib_alloc_pd(ib_dev, pool->pd_flags);
> +	if (IS_ERR(dev->ib_pd))
> +		goto out_free_dev;
> +
> +	if (pool->ops && pool->ops->init && pool->ops->init(dev))
> +		goto out_free_pd;
> +
> +	list_add(&dev->entry, &pool->list);
> +out_unlock:
> +	mutex_unlock(&pool->mutex);
> +	return dev;
> +
> +out_free_pd:
> +	ib_dealloc_pd(dev->ib_pd);
> +out_free_dev:
> +	if (pool->ops && pool->ops->free)
> +		pool->ops->free(dev);
> +	else
> +		kfree(dev);
> +out_err:
> +	mutex_unlock(&pool->mutex);
> +	return NULL;
> +}
> +EXPORT_SYMBOL(rtrs_ib_dev_find_or_add);
> --
> 2.17.1
>