Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c561c288 authored by Jason Gunthorpe's avatar Jason Gunthorpe
Browse files

IB/uverbs: Clarify the kref'ing ordering for alloc_commit 



The alloc_commit callback makes the uobj visible to other threads,
and it does so using a 'move' semantic of the uobj kref on the stack
into the public storage (eg the IDR, uobject list and file_private_data)

Once this is done another thread could start up and trigger deletion
of the kref. Fortunately cleanup_rwsem happens to prevent this from
being a bug, but that is a fantastically unclear side effect.

Re-organize things so that alloc_commit is that last thing to touch
the uobj, get rid of the sneaky implicit dependency on cleanup_rwsem,
and add a comment reminding that uobj is no longer kref'd after
alloc_commit.

Signed-off-by: default avatarJason Gunthorpe <jgg@mellanox.com>
parent 1250c304

drivers/infiniband/core/rdma_core.c

+22 −4
Original line number Diff line number Diff line
@@ -498,24 +498,41 @@ int rdma_explicit_destroy(struct ib_uobject *uobject) 


static void alloc_commit_idr_uobject(struct ib_uobject *uobj)

{

	spin_lock(&uobj->ufile->idr_lock);

	struct ib_uverbs_file *ufile = uobj->ufile;



	spin_lock(&ufile->idr_lock);

	/*

	 * We already allocated this IDR with a NULL object, so

	 * this shouldn't fail.

	 *

	 * NOTE: Once we set the IDR we loose ownership of our kref on uobj.

	 * It will be put by remove_commit_idr_uobject()

	 */

	WARN_ON(idr_replace(&uobj->ufile->idr, uobj, uobj->id));

	spin_unlock(&uobj->ufile->idr_lock);

	WARN_ON(idr_replace(&ufile->idr, uobj, uobj->id));

	spin_unlock(&ufile->idr_lock);

}



static void alloc_commit_fd_uobject(struct ib_uobject *uobj)

{

	fd_install(uobj->id, uobj->object);

	int fd = uobj->id;



	/* This shouldn't be used anymore. Use the file object instead */

	uobj->id = 0;



	/* Get another reference as we export this to the fops */

	uverbs_uobject_get(uobj);



	/*

	 * NOTE: Once we install the file we loose ownership of our kref on

	 * uobj. It will be put by uverbs_close_fd()

	 */

	fd_install(fd, uobj->object);

}



/*

 * In all cases rdma_alloc_commit_uobject() consumes the kref to uobj and the

 * caller can no longer assume uobj is valid.

 */

int rdma_alloc_commit_uobject(struct ib_uobject *uobj)

{

	struct ib_uverbs_file *ufile = uobj->ufile;
@@ -541,6 +558,7 @@ int rdma_alloc_commit_uobject(struct ib_uobject *uobj) 
	list_add(&uobj->list, &ufile->uobjects);

	mutex_unlock(&ufile->uobjects_lock);



	/* alloc_commit consumes the uobj kref */

	uobj->type->type_class->alloc_commit(uobj);

	up_read(&ufile->cleanup_rwsem);