Re: [PATCH 08/25] IB/uverbs: ufile must be freed only when not used anymore

[Jason Gunthorpe <jgg@ziepe.ca>]

On Tue, Jul 16, 2019 at 09:11:43PM +0300, Shamir Rabinovitch wrote:
> From: Shamir Rabinovitch <shamir.rabinovitch@oracle.com>
> 
> ufile (&ucontext) with the process who own them must not be released
> when there are other ufile (&ucontext) that depens at them.

We already have a kref, why do we need more? Especially wrongly done
refcounts with atomics?

Trying to sequence the destroy of the ucontext seems inherently wrong
to me. If the driver has to link the PD/MR to data in the ucontext it
can't support sharing.

> Signed-off-by: Shamir Rabinovitch <shamir.rabinovitch@oracle.com>
> Signed-off-by: Shamir Rabinovitch <srabinov7@gmail.com>
>  drivers/infiniband/core/rdma_core.c   | 29 +++++++++++++++++++++++++++
>  drivers/infiniband/core/uverbs.h      | 22 ++++++++++++++++++++
>  drivers/infiniband/core/uverbs_cmd.c  | 16 +++++++++++++++
>  drivers/infiniband/core/uverbs_main.c |  4 ++++
>  4 files changed, 71 insertions(+)
> 
> diff --git a/drivers/infiniband/core/rdma_core.c b/drivers/infiniband/core/rdma_core.c
> index 651625f632d7..c81ff8e28fc6 100644
> +++ b/drivers/infiniband/core/rdma_core.c
> @@ -841,6 +841,33 @@ static void ufile_destroy_ucontext(struct ib_uverbs_file *ufile,
>  	ufile->ucontext = NULL;
>  }
>  
> +static void __uverbs_ufile_refcount(struct ib_uverbs_file *ufile)
> +{
> +	int wait;
> +
> +	if (ufile->parent) {
> +		pr_debug("%s: release parent ufile. ufile %p parent %p\n",
> +			 __func__, ufile, ufile->parent);
> +		if (atomic_dec_and_test(&ufile->parent->refcount))
> +			complete(&ufile->parent->context_released);
> +	}
> +
> +	if (!atomic_dec_and_test(&ufile->refcount)) {
> +wait:
> +		wait = wait_for_completion_interruptible_timeout(
> +			&ufile->context_released, 3*HZ);
> +		if (wait == -ERESTARTSYS) {
> +			WARN_ONCE(1,
> +			"signal while waiting for context release! ufile %p\n",
> +				ufile);

????

Jason