On Wed, Aug 21, 2019 at 05:21:06PM +0300, Yuval Shaia wrote:
> From: Shamir Rabinovitch <shamir.rabinovitch@xxxxxxxxxx>
>
> This new refcnt will points to the refcnt member of the HW object and will
> behaves as expected by refcnt, i.e. will be increased and decreased as a
> result of usage changes and will destroy the object when reaches to zero.
> For a non-shared object refcnt will remain NULL.
>
> Signed-off-by: Shamir Rabinovitch <shamir.rabinovitch@xxxxxxxxxx>
> Signed-off-by: Shamir Rabinovitch <srabinov7@xxxxxxxxx>
> drivers/infiniband/core/rdma_core.c | 23 +++++++++++++++++++++--
> include/rdma/ib_verbs.h | 7 +++++++
> 2 files changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
> index ccf4d069c25c..651625f632d7 100644
> +++ b/drivers/infiniband/core/rdma_core.c
> @@ -516,7 +516,26 @@ static int __must_check destroy_hw_idr_uobject(struct ib_uobject *uobj,
> const struct uverbs_obj_idr_type *idr_type =
> container_of(uobj->uapi_object->type_attrs,
> struct uverbs_obj_idr_type, type);
> - int ret = idr_type->destroy_object(uobj, why, attrs);
> + static DEFINE_MUTEX(lock);
> + int ret, count;
> +
> + mutex_lock(&lock);
> +
> + if (uobj->refcnt) {
> + count = atomic_dec_return(uobj->refcnt);
> + WARN_ON(count < 0); /* use after free! */
Use a proper refcount_t
> + if (count) {
> + mutex_unlock(&lock);
> + goto skip;
> + }
> + }
> +
> + ret = idr_type->destroy_object(uobj, why, attrs);
> +
> + if (ret)
> + atomic_inc(uobj->refcnt);
> +
> + mutex_unlock(&lock);
>
> /*
> * We can only fail gracefully if the user requested to destroy the
> @@ -525,7 +544,7 @@ static int __must_check destroy_hw_idr_uobject(struct ib_uobject *uobj,
> */
> if (ib_is_destroy_retryable(ret, why, uobj))
> return ret;
> -
> +skip:
This doesn't seem to properly define who owns the rdmacg count - it
should belong to the HW object not to the uobject.
> +
> + /*
> + * ib_X HW object sharing support
> + * - NULL for HW objects that are not shareable
> + * - Pointer to ib_X reference counter for shareable HW objects
> + */
> + atomic_t *refcnt; /* ib_X object ref count */
Gross, shouldn't this actually be in the hw object?
Jason
