On Mon, Oct 12, 2020 at 07:55:58AM +0300, Leon Romanovsky wrote:
> @@ -543,17 +537,9 @@ static int __must_check destroy_hw_idr_uobject(struct ib_uobject *uobj,
> struct uverbs_obj_idr_type, type);
> int ret = idr_type->destroy_object(uobj, why, attrs);
>
> - /*
> - * We can only fail gracefully if the user requested to destroy the
> - * object or when a retry may be called upon an error.
> - * In the rest of the cases, just remove whatever you can.
> - */
> - if (ib_is_destroy_retryable(ret, why, uobj))
> + if (ret)
> return ret;
>
> - if (why == RDMA_REMOVE_ABORT)
> - return 0;
This shouldn't be deleted..
There are also a few too many WARN_ONs if this path triggers, I came up
with this:
diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
index 3d366cb79cef42..3ae878f3d173d3 100644
--- a/drivers/infiniband/core/rdma_core.c
+++ b/drivers/infiniband/core/rdma_core.c
@@ -540,6 +540,9 @@ static int __must_check destroy_hw_idr_uobject(struct ib_uobject *uobj,
if (ret)
return ret;
+ if (why == RDMA_REMOVE_ABORT)
+ return 0;
+
ib_rdmacg_uncharge(&uobj->cg_obj, uobj->context->device,
RDMACG_RESOURCE_HCA_OBJECT);
@@ -727,10 +730,8 @@ void release_ufile_idr_uobject(struct ib_uverbs_file *ufile)
*
* This is an optimized equivalent to remove_handle_idr_uobject
*/
- xa_for_each(&ufile->idr, id, entry) {
- WARN_ON(entry->object);
+ xa_for_each(&ufile->idr, id, entry)
uverbs_uobject_put(entry);
- }
xa_destroy(&ufile->idr);
}
@@ -875,25 +876,31 @@ void uverbs_destroy_ufile_hw(struct ib_uverbs_file *ufile,
goto done;
while (!list_empty(&ufile->uobjects))
- if (__uverbs_cleanup_ufile(ufile, reason)) {
+ if (__uverbs_cleanup_ufile(ufile, reason))
+ break;
+
+ /*
+ * In case destruction failed try to free as much memory as we can,
+ * and leak the HW objects.
+ */
+ if (!list_empty(&ufile->uobjects)) {
+ WARN(true, "RDMA driver did not destroy all HW objects, leaking memory");
+ list_for_each_entry_safe (obj, next_obj, &ufile->uobjects,
+ list) {
+ spin_lock_irqsave(&ufile->uobjects_lock, flags);
+ list_del_init(&obj->list);
+ spin_unlock_irqrestore(&ufile->uobjects_lock, flags);
/*
- * No entry was cleaned-up successfully during this
- * iteration. It is a driver bug to fail destruction.
+ * Pairs with the get in rdma_alloc_commit_uobject(),
+ * could destroy uobj.
*/
- WARN_ON(!list_empty(&ufile->uobjects));
- break;
+ uverbs_uobject_put(obj);
}
-
- list_for_each_entry_safe (obj, next_obj, &ufile->uobjects, list) {
- spin_lock_irqsave(&ufile->uobjects_lock, flags);
- list_del_init(&obj->list);
- spin_unlock_irqrestore(&ufile->uobjects_lock, flags);
- /*
- * Pairs with the get in rdma_alloc_commit_uobject(), could
- * destroy uobj.
- */
- uverbs_uobject_put(obj);
+ /* release_ufile_idr_uobject() will clean up the IDR */
+ } else {
+ WARN_ON(!xa_empty(&ufile->idr));
}
+
ufile_destroy_ucontext(ufile, reason);
done:
