On Fri, Feb 10, 2017 at 7:53 PM, Jason Gunthorpe
<jgunthorpe@xxxxxxxxxxxxxxxxxxxx> wrote:
> 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.
>
Agreed
>>
>> 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...
>
Yeah, that could come as a later cleanup.
> Otherwise the patch, in context, seems fine to me..
>
May I add your Reviewed-by to later submissions of this patch?
> Jason
Thanks for reviewing.
Matan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
