> From: Bernard Metzler <BMT@xxxxxxxxxxxxxx> > Sent: Monday, September 2, 2019 1:55 PM > > -----"Michal Kalderon" <mkalderon@xxxxxxxxxxx> wrote: ----- > > >To: "Bernard Metzler" <BMT@xxxxxxxxxxxxxx> > >From: "Michal Kalderon" <mkalderon@xxxxxxxxxxx> > >Date: 09/02/2019 10:01AM > >Cc: "Ariel Elior" <aelior@xxxxxxxxxxx>, "jgg@xxxxxxxx" > ><jgg@xxxxxxxx>, "dledford@xxxxxxxxxx" <dledford@xxxxxxxxxx>, > >"galpress@xxxxxxxxxx" <galpress@xxxxxxxxxx>, > "sleybo@xxxxxxxxxx" > ><sleybo@xxxxxxxxxx>, "leon@xxxxxxxxxx" <leon@xxxxxxxxxx>, > >"linux-rdma@xxxxxxxxxxxxxxx" <linux-rdma@xxxxxxxxxxxxxxx> > >Subject: [EXTERNAL] RE: RE: [EXT] Re: [PATCH v8 rdma-next 4/7] > >RDMA/siw: Use the common mmap_xa helpers > > > >> From: linux-rdma-owner@xxxxxxxxxxxxxxx <linux-rdma- > >> owner@xxxxxxxxxxxxxxx> On Behalf Of Bernard Metzler > >> > >> -----"Michal Kalderon" <mkalderon@xxxxxxxxxxx> wrote: ----- > >> > >> >To: "Bernard Metzler" <BMT@xxxxxxxxxxxxxx> > >> >From: "Michal Kalderon" <mkalderon@xxxxxxxxxxx> > >> >Date: 08/30/2019 02:42PM > >> >Cc: "Ariel Elior" <aelior@xxxxxxxxxxx>, "jgg@xxxxxxxx" > >> ><jgg@xxxxxxxx>, "dledford@xxxxxxxxxx" <dledford@xxxxxxxxxx>, > >> >"galpress@xxxxxxxxxx" <galpress@xxxxxxxxxx>, > >> "sleybo@xxxxxxxxxx" > >> ><sleybo@xxxxxxxxxx>, "leon@xxxxxxxxxx" <leon@xxxxxxxxxx>, > >> >"linux-rdma@xxxxxxxxxxxxxxx" <linux-rdma@xxxxxxxxxxxxxxx>, "Ariel > >> >Elior" <aelior@xxxxxxxxxxx> > >> >Subject: [EXTERNAL] RE: [EXT] Re: [PATCH v8 rdma-next 4/7] > >RDMA/siw: > >> >Use the common mmap_xa helpers > >> > > >> >> From: Bernard Metzler <BMT@xxxxxxxxxxxxxx> > >> >> Sent: Friday, August 30, 2019 3:08 PM > >> >> > >> >> External Email > >> >> > >> >> > >> > >>-------------------------------------------------------------------- > >- > >> >- > >> >> -----"Michal Kalderon" <michal.kalderon@xxxxxxxxxxx> 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@xxxxxxxxxxx>, <aelior@xxxxxxxxxxx>, > >> <jgg@xxxxxxxx>, > >> >> ><dledford@xxxxxxxxxx>, <bmt@xxxxxxxxxxxxxx>, > >> >> <galpress@xxxxxxxxxx>, > >> >> ><sleybo@xxxxxxxxxx>, <leon@xxxxxxxxxx> > >> >> >From: "Michal Kalderon" <michal.kalderon@xxxxxxxxxxx> > >> >> >Date: 08/27/2019 03:31PM > >> >> >Cc: <linux-rdma@xxxxxxxxxxxxxxx>, "Michal Kalderon" > >> >> ><michal.kalderon@xxxxxxxxxxx>, "Ariel Elior" > >> >> ><ariel.elior@xxxxxxxxxxx> > >> >> >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@xxxxxxxxxxx> > >> >> >Signed-off-by: Michal Kalderon <michal.kalderon@xxxxxxxxxxx> > >> >> >--- > >> >> > 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 > > > Michael, Michal (not Michael) thanks 😊 > I even think there is an underlying design decision I am questioning. The > provider allocates the resource to be mmapped, and there is no reason to > have the mmapping service freeing that resource at any point in time. I > simply don't understand why the mmap service wants to free the mmapped > resource at all, if it did not allocate it. The aim of the service should be limited > to provide mmapping bookkeeping. > > The new mmap helper framework already defines a callback for unmapping. > The provider may use it to free the resource, if appropriate. E.g., siw will not > destroy a currently used CQ, only because the user land application calls > munmap on it. The function that frees the memory is the siw_mmap_free in the provider , not the Mapping service. This is called when provider calls mmap_remove and ALL maps to the address were unmapped or Zapped, we don't want to free the memory before the mapping was removed. Once the provider calls mmap_entry_insert the refcnt is set to '1' and can Only reach zero after provider calls mmap_entry_remove. Thanks, Michal > > Best regards, > Bernard.
