-----"Leon Romanovsky" <leon@xxxxxxxxxx> wrote: ----- >To: "Bernard Metzler" <BMT@xxxxxxxxxxxxxx> >From: "Leon Romanovsky" <leon@xxxxxxxxxx> >Date: 10/05/2019 10:26AM >Cc: linux-rdma@xxxxxxxxxxxxxxx, bharat@xxxxxxxxxxx, jgg@xxxxxxxx, >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: >> -----"Leon Romanovsky" <leon@xxxxxxxxxx> 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. Let me >> see if I can compact QP objects to contain the >> ib_qp. I'd like to avoid following pointers >> potentially causing cache misses on the >> fast path. This is why I still have that >> little boolean within the siw private >> structure. > >You have this variable in CQ and SRQ too. > >I have serious doubts that this value gives any performance >advantages. >In both flows, you will need to fetch ib_qp pointer, so you don't >save >here anything by looking on kernel_verbs value. > Yes, I see you are right for both CQ and SRQ. For the CQ, we have a nested structure where siw_cq contains ib_cq. So it is not far away. For SRQ it is the same. For QP's we have a split between siw_qp and ib_qp. I will look into how to get that one solved. I will prepare an extra patch for that whole kernel_verbs thing, but let's not have it gating acceptance of this unrelated patch. Thanks very much, Bernard.