-----"Leon Romanovsky" <leon@xxxxxxxxxx> wrote: ----- >To: "Bernard Metzler" <BMT@xxxxxxxxxxxxxx> >From: "Leon Romanovsky" <leon@xxxxxxxxxx> >Date: 10/27/2019 06:21AM >Cc: "Jason Gunthorpe" <jgg@xxxxxxxx>, linux-rdma@xxxxxxxxxxxxxxx, >bharat@xxxxxxxxxxx, nirranjan@xxxxxxxxxxx, krishna2@xxxxxxxxxxx, >bvanassche@xxxxxxx >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@xxxxxxxx> wrote: ----- >> >> >To: "Bernard Metzler" <BMT@xxxxxxxxxxxxxx> >> >From: "Jason Gunthorpe" <jgg@xxxxxxxx> >> >Date: 10/04/2019 07:48PM >> >Cc: "Leon Romanovsky" <leon@xxxxxxxxxx>, >linux-rdma@xxxxxxxxxxxxxxx, >> >bharat@xxxxxxxxxxx, nirranjan@xxxxxxxxxxx, krishna2@xxxxxxxxxxx, >> >bvanassche@xxxxxxx >> >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 >> > >