Commit efa968ee authored by Leon Romanovsky's avatar Leon Romanovsky Committed by Jason Gunthorpe
Browse files

RDMA/core: Postpone uobject cleanup on failure till FD close 

Remove the ib_is_destroyable_retryable() concept.

The idea here was to allow the drivers to forcibly clean the HW object
even if they otherwise didn't want to (eg because of usecnt). This was an
attempt to clean up in a world where drivers were not allowed to fail HW
object destruction.

Now that we are going back to allowing HW objects to fail destroy this
doesn't make sense. Instead if a uobject's HW object can't be destroyed it
is left on the uobject list and it is up to uverbs_destroy_ufile_hw() to
clean it. Multiple passes over the uobject list allow hidden dependencies
to be resolved. If that fails the HW driver is broken, throw a WARN_ON and
leak the HW object memory.

All the other tricky failure paths (eg on creation error unwind) have
already been updated to this new model.

Link: https://lore.kernel.org/r/20201104144556.3809085-2-leon@kernel.org


Signed-off-by: default avatarLeon Romanovsky <leonro@nvidia.com>
Signed-off-by: default avatarJason Gunthorpe <jgg@nvidia.com>
parent f7a95c90

drivers/infiniband/core/rdma_core.c

+17 −34
Original line number Diff line number Diff line
@@ -137,16 +137,10 @@ static int uverbs_destroy_uobject(struct ib_uobject *uobj, 
	} else if (uobj->object) {

		ret = uobj->uapi_object->type_class->destroy_hw(uobj, reason,

								attrs);

		if (ret) {

			if (ib_is_destroy_retryable(ret, reason, uobj))

		if (ret)

			/* Nothing to be done, wait till ucontext will clean it */

			return ret;



			/* Nothing to be done, dangle the memory and move on */

			WARN(true,

			     "ib_uverbs: failed to remove uobject id %d, driver err=%d",

			     uobj->id, ret);

		}



		uobj->object = NULL;

	}
@@ -543,12 +537,7 @@ 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)
@@ -581,12 +570,8 @@ static int __must_check destroy_hw_fd_uobject(struct ib_uobject *uobj, 
{

	const struct uverbs_obj_fd_type *fd_type = container_of(

		uobj->uapi_object->type_attrs, struct uverbs_obj_fd_type, type);

	int ret = fd_type->destroy_object(uobj, why);



	if (ib_is_destroy_retryable(ret, why, uobj))

		return ret;



	return 0;

	return fd_type->destroy_object(uobj, why);

}



static void remove_handle_fd_uobject(struct ib_uobject *uobj)
@@ -863,11 +848,18 @@ static int __uverbs_cleanup_ufile(struct ib_uverbs_file *ufile, 
		 * racing with a lookup_get.

		 */

		WARN_ON(uverbs_try_lock_object(obj, UVERBS_LOOKUP_WRITE));

		if (reason == RDMA_REMOVE_DRIVER_FAILURE)

			obj->object = NULL;

		if (!uverbs_destroy_uobject(obj, reason, &attrs))

			ret = 0;

		else

			atomic_set(&obj->usecnt, 0);

	}



	if (reason == RDMA_REMOVE_DRIVER_FAILURE) {

		WARN_ON(!list_empty(&ufile->uobjects));

		return 0;

	}

	return ret;

}
@@ -889,21 +881,12 @@ void uverbs_destroy_ufile_hw(struct ib_uverbs_file *ufile, 
	if (!ufile->ucontext)

		goto done;



	ufile->ucontext->cleanup_retryable = true;

	while (!list_empty(&ufile->uobjects))

		if (__uverbs_cleanup_ufile(ufile, reason)) {

			/*

			 * No entry was cleaned-up successfully during this

			 * iteration. It is a driver bug to fail destruction.

			 */

			WARN_ON(!list_empty(&ufile->uobjects));

			break;

	while (!list_empty(&ufile->uobjects) &&

	       !__uverbs_cleanup_ufile(ufile, reason)) {

	}



	ufile->ucontext->cleanup_retryable = false;

	if (!list_empty(&ufile->uobjects))

		__uverbs_cleanup_ufile(ufile, reason);



	if (WARN_ON(!list_empty(&ufile->uobjects)))

		__uverbs_cleanup_ufile(ufile, RDMA_REMOVE_DRIVER_FAILURE);

	ufile_destroy_ucontext(ufile, reason);



done:

drivers/infiniband/core/uverbs_cmd.c

+2 −3
Original line number Diff line number Diff line
@@ -681,8 +681,7 @@ int ib_uverbs_dealloc_xrcd(struct ib_uobject *uobject, struct ib_xrcd *xrcd, 
		return 0;



	ret = ib_dealloc_xrcd_user(xrcd, &attrs->driver_udata);



	if (ib_is_destroy_retryable(ret, why, uobject)) {

	if (ret) {

		atomic_inc(&xrcd->usecnt);

		return ret;

	}
@@ -690,7 +689,7 @@ int ib_uverbs_dealloc_xrcd(struct ib_uobject *uobject, struct ib_xrcd *xrcd, 
	if (inode)

		xrcd_table_delete(dev, inode);



	return ret;

	return 0;

}



static int ib_uverbs_reg_mr(struct uverbs_attr_bundle *attrs)

drivers/infiniband/core/uverbs_std_types.c

+6 −9
Original line number Diff line number Diff line
@@ -88,7 +88,7 @@ static int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject, 
		return -EBUSY;



	ret = rwq_ind_tbl->device->ops.destroy_rwq_ind_table(rwq_ind_tbl);

	if (ib_is_destroy_retryable(ret, why, uobject))

	if (ret)

		return ret;



	for (i = 0; i < table_size; i++)
@@ -96,7 +96,7 @@ static int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject, 


	kfree(rwq_ind_tbl);

	kfree(ind_tbl);

	return ret;

	return 0;

}



static int uverbs_free_xrcd(struct ib_uobject *uobject,
@@ -108,9 +108,8 @@ static int uverbs_free_xrcd(struct ib_uobject *uobject, 
		container_of(uobject, struct ib_uxrcd_object, uobject);

	int ret;



	ret = ib_destroy_usecnt(&uxrcd->refcnt, why, uobject);

	if (ret)

		return ret;

	if (atomic_read(&uxrcd->refcnt))

		return -EBUSY;



	mutex_lock(&attrs->ufile->device->xrcd_tree_mutex);

	ret = ib_uverbs_dealloc_xrcd(uobject, xrcd, why, attrs);
@@ -124,11 +123,9 @@ static int uverbs_free_pd(struct ib_uobject *uobject, 
			  struct uverbs_attr_bundle *attrs)

{

	struct ib_pd *pd = uobject->object;

	int ret;



	ret = ib_destroy_usecnt(&pd->usecnt, why, uobject);

	if (ret)

		return ret;

	if (atomic_read(&pd->usecnt))

		return -EBUSY;



	return ib_dealloc_pd_user(pd, &attrs->driver_udata);

}

drivers/infiniband/core/uverbs_std_types_counters.c

+2 −3
Original line number Diff line number Diff line
@@ -42,9 +42,8 @@ static int uverbs_free_counters(struct ib_uobject *uobject, 
	struct ib_counters *counters = uobject->object;

	int ret;



	ret = ib_destroy_usecnt(&counters->usecnt, why, uobject);

	if (ret)

		return ret;

	if (atomic_read(&counters->usecnt))

		return -EBUSY;



	ret = counters->device->ops.destroy_counters(counters);

	if (ret)

drivers/infiniband/core/uverbs_std_types_cq.c

+2 −2
Original line number Diff line number Diff line
@@ -46,7 +46,7 @@ static int uverbs_free_cq(struct ib_uobject *uobject, 
	int ret;



	ret = ib_destroy_cq_user(cq, &attrs->driver_udata);

	if (ib_is_destroy_retryable(ret, why, uobject))

	if (ret)

		return ret;



	ib_uverbs_release_ucq(
@@ -55,7 +55,7 @@ static int uverbs_free_cq(struct ib_uobject *uobject, 
					ev_queue) :

			   NULL,

		ucq);

	return ret;

	return 0;

}



static int UVERBS_HANDLER(UVERBS_METHOD_CQ_CREATE)(