Re: Re: Re: Re: [[PATCH v2 for-next]] RDMA/siw: Fix SQ/RQ drain logic

["Bernard Metzler" <BMT@zurich.ibm.com>]

-----"Leon Romanovsky" <leon@kernel.org> wrote: -----

>To: "Bernard Metzler" <BMT@zurich.ibm.com>
>From: "Leon Romanovsky" <leon@kernel.org>
>Date: 10/27/2019 06:21AM
>Cc: "Jason Gunthorpe" <jgg@ziepe.ca>, linux-rdma@vger.kernel.org,
>bharat@chelsio.com, nirranjan@chelsio.com, krishna2@chelsio.com,
>bvanassche@acm.org
>Subject: [EXTERNAL] Re: Re: Re: [[PATCH v2 for-next]] RDMA/siw: Fix
>SQ/RQ drain logic
>
>On Fri, Oct 25, 2019 at 12:11:16PM +0000, Bernard Metzler wrote:
>> -----"Jason Gunthorpe" <jgg@ziepe.ca> wrote: -----
>>
>> >To: "Bernard Metzler" <BMT@zurich.ibm.com>
>> >From: "Jason Gunthorpe" <jgg@ziepe.ca>
>> >Date: 10/04/2019 07:48PM
>> >Cc: "Leon Romanovsky" <leon@kernel.org>,
>linux-rdma@vger.kernel.org,
>> >bharat@chelsio.com, nirranjan@chelsio.com, krishna2@chelsio.com,
>> >bvanassche@acm.org
>> >Subject: [EXTERNAL] Re: Re: [[PATCH v2 for-next]] RDMA/siw: Fix
>SQ/RQ
>> >drain logic
>> >
>> >On Fri, Oct 04, 2019 at 02:09:57PM +0000, Bernard Metzler wrote:
>> >> <...>
>> >>
>> >> >>   *
>> >> >> @@ -705,6 +746,12 @@ int siw_post_send(struct ib_qp *base_qp,
>> >const
>> >> >struct ib_send_wr *wr,
>> >> >>  	unsigned long flags;
>> >> >>  	int rv = 0;
>> >> >>
>> >> >> +	if (wr && !qp->kernel_verbs) {
>> >> >
>> >> >It is not related to this specific patch, but all siw
>> >"kernel_verbs"
>> >> >should go, we have standard way to distinguish between kernel
>and
>> >> >user
>> >> >verbs.
>> >> >
>> >> >Thanks
>> >> >
>> >> Understood. I think we touched on that already.
>> >> rdma core objects have a uobject pointer which
>> >> is valid only if it belongs to a user land
>> >> application. We might better use that.
>> >
>> >No, the uobject pointer is not to be touched by drivers
>> >
>> Now what would be the appropriate way of remembering/
>> detecting user level nature of endpoint resources?
>> I see drivers _are_ doing 'if (!ibqp->uobject)' ...
>
>IMHO, you should rely in "udata" to distinguish user/kernel objects.
>

right, we already do that at resource creation time, when
'udata' is available.  But there is no such parameter
around during resource access (post_send/post_recv/poll_cq/...),
when user land or kernel land application specific
code might be required.
That's why siw currently saves that info to a resource
(QP/CQ/SRQ) specific parameter 'kernel_verbs'. I agree
this parameter is redundant, if the rdma core object
provides that information as well. The only way I see
it provided is the validity of the uobject pointer of
all those resources.
Either (1) we use that uobject pointer as an indication
of application location, or (2) we remember it from
resource creation time when udata was available, or
(3) we have the rdma core exporting that information
explicitly.
siw, and other drivers, are currently implementing (2).
Some drivers implement (1). I'd be happy to change siw
to implement (1) - to get rid of 'kernel_verbs'.

Thanks and best regards,
Bernard.




>>
>> Other drivers keep it with the private state, like iw40,
>> but I learned we shall get rid of it.
>>
>> We may export an inline query from RDMA core, or simply
>> #define is_usermode(ib_obj *) (ib_obj->uobject != NULL)
>> ?
>>
>> Thanks and best regards,
>> Bernard
>>
>
>