Re: [PATCH V1 for-next 1/7] IB/core: Refactor idr to be per uverbs_file

[Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]

On Wed, Feb 01, 2017 at 02:38:59PM +0200, Matan Barak wrote:
>  static struct ib_pd *idr_read_pd(int pd_handle, struct ib_ucontext *context)
>  {
> -	return idr_read_obj(&ib_uverbs_pd_idr, pd_handle, context, 0);
> +	return idr_read_obj(pd_handle, context, 0);
>  }

As a note: At this point in the series we loose the type checking on
the IDR handles. Eg at this moment idr_read_obj() could return
something that is not a PD and resulting ugliness..

Since that is ultimately fixed as we go through the series I'm not
sure it is worth addressing.

>  
>  	ucontext->device = ib_dev;
> +	/* ufile is required when some objects are released */
> +	ucontext->ufile = file;

Hurm. This reads weirdly, but it is OK..

Overall this is starting to really look weird. ucontext has a pointer
to ufile and ufile has a pointer to ucontext. That tells me they
should be in the same allocation. To make this saner you'd have to
embed ib_ucontext into ib_uverbs_file and use a ->priv for the driver
data instead of sticking at the end of the ib_ucontext allocation.

Probably too big for this patch series but keep it in mind...

Otherwise the patch, in context, seems fine to me..

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html