RE: RE: [EXT] Re: [PATCH v8 rdma-next 4/7] RDMA/siw: Use the common mmap_xa helpers

[Michal Kalderon <mkalderon@marvell.com>]

> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> owner@vger.kernel.org> On Behalf Of Bernard Metzler
> 
> -----"Michal Kalderon" <mkalderon@marvell.com> wrote: -----
> 
> >To: "Bernard Metzler" <BMT@zurich.ibm.com>
> >From: "Michal Kalderon" <mkalderon@marvell.com>
> >Date: 08/30/2019 02:42PM
> >Cc: "Ariel Elior" <aelior@marvell.com>, "jgg@ziepe.ca"
> ><jgg@ziepe.ca>, "dledford@redhat.com" <dledford@redhat.com>,
> >"galpress@amazon.com" <galpress@amazon.com>,
> "sleybo@amazon.com"
> ><sleybo@amazon.com>, "leon@kernel.org" <leon@kernel.org>,
> >"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>, "Ariel
> >Elior" <aelior@marvell.com>
> >Subject: [EXTERNAL] RE: [EXT] Re: [PATCH v8 rdma-next 4/7] RDMA/siw:
> >Use the common mmap_xa helpers
> >
> >> From: Bernard Metzler <BMT@zurich.ibm.com>
> >> Sent: Friday, August 30, 2019 3:08 PM
> >>
> >> External Email
> >>
> >>
> >---------------------------------------------------------------------
> >-
> >> -----"Michal Kalderon" <michal.kalderon@marvell.com> wrote: -----
> >>
> >> Hi Michael,
> >>
> >> I tried this patch. It unfortunately panics immediately when siw
> >gets used. I'll
> >> investigate further. Some comments in line.
> >Thanks for testing,
> >
> >>
> >> Thanks
> >> Bernard.
> >>
> >> >To: <mkalderon@marvell.com>, <aelior@marvell.com>,
> <jgg@ziepe.ca>,
> >> ><dledford@redhat.com>, <bmt@zurich.ibm.com>,
> >> <galpress@amazon.com>,
> >> ><sleybo@amazon.com>, <leon@kernel.org>
> >> >From: "Michal Kalderon" <michal.kalderon@marvell.com>
> >> >Date: 08/27/2019 03:31PM
> >> >Cc: <linux-rdma@vger.kernel.org>, "Michal Kalderon"
> >> ><michal.kalderon@marvell.com>, "Ariel Elior"
> >> ><ariel.elior@marvell.com>
> >> >Subject: [EXTERNAL] [PATCH v8 rdma-next 4/7] RDMA/siw: Use the
> >> common
> >> >mmap_xa helpers
> >> >
> >> >Remove the functions related to managing the mmap_xa database.
> >> >This code is now common in ib_core. Use the common API's instead.
> >> >
> >> >Signed-off-by: Ariel Elior <ariel.elior@marvell.com>
> >> >Signed-off-by: Michal Kalderon <michal.kalderon@marvell.com>
> >> >---
> >> > drivers/infiniband/sw/siw/siw.h       |  20 ++-
> >> > drivers/infiniband/sw/siw/siw_main.c  |   1 +
> >> > drivers/infiniband/sw/siw/siw_verbs.c | 223
> >> >+++++++++++++++++++---------------
> >> > drivers/infiniband/sw/siw/siw_verbs.h |   1 +
> >> > 4 files changed, 144 insertions(+), 101 deletions(-)
> >> >
> >> >+			/* If entry was inserted successfully, qp->sendq
> >> >+			 * will be freed by siw_mmap_free
> >> >+			 */
> >> >+			qp->sendq = NULL;
> >>
> >> qp->sendq points to the SQ array. Zeroing this pointer will leave
> >> siw with no idea where the WQE's are. It will panic de-referencing
> >[NULL +
> >> current position in ring buffer]. Same for RQ, SRQ and CQ.
> >Qp->sendq is only used in kernel mode, and only set to NULL is
> >user-space mode
> >Where it is allocated and mapped in user, so the user will be the one
> >accessing the rings And not kernel, unless I'm missing something.
> 
> These pointers are pointing to the allocated work queues.
> These queues/arrays are holding the actual send/recv/complete/srq work
> queue elements. It is a shared array. That is why we need mmap at all.
> 
> e.g.,
> 
> struct siw_sqe *sqe = &qp->sendq[qp->sq_get % qp->attrs.sq_size];
> 
> 
Ok got it, I'm HW oriented... so user chains and kernel are totally separated. 
Will add a flag whether to free or not and remove setting to NULL. 
Thanks,
Michal