From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoph Hellwig Subject: Re: Kernel fast memory registration API proposal [RFC] Date: Wed, 15 Jul 2015 00:32:33 -0700 Message-ID: <20150715073233.GA11535@infradead.org> References: <559F8BD1.9080308@dev.mellanox.co.il> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <559F8BD1.9080308-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> Sender: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Sagi Grimberg Cc: "linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Christoph Hellwig , Jason Gunthorpe , Steve Wise , Or Gerlitz , Oren Duer , Chuck Lever , Bart Van Assche , Liran Liss , "Hefty, Sean" , Doug Ledford , Tom Talpey List-Id: linux-rdma@vger.kernel.org Hi Sagi, I went over your proposal based on reviewing the ongoing MR threads and my implementation of a similar in-driver abstraction, so here are some proposed updates. > struct provider_mr { > u64 *page_list; // or what ever the HW uses > ... ... > struct ib_mr ibmr; > }; Call this rdma_mr to fit the scheme we use for "generic" APIs in the RDMA stack? Also let's hash out the API for allocating it, you suggest the existing but currently almost unused ib_create_mr API, which isn't quite suitable as it doesn't transparently allocate the page list or other MR-specific data. Another odd bit in ib_create_mr is that it doesnt actually say which kind of MR to allocate. I'd also get rid of the horrible style of using structs even for simple attributes. so how about: int rdma_create_mr(struct ib_pd *pd, enum rdma_mr_type mr, u32 max_pages, int flags); > * array from a SG list > * @mr: memory region > * @sg: sg list > * @sg_nents: number of elements in the sg > * > * Can fail if the HW is not able to register this > * sg list. In case of failure - caller is responsible > * to handle it (bounce-buffer, multiple registrations...) > */ > int ib_mr_set_sg(struct ib_mr *mr, > struct scatterlist *sg, > unsigned short sg_nents); Call this rdma_map_sg? > /* register the MR */ > frwr.opcode = IB_WR_FAST_REG_MR; > frwr.wrid = my_wrid; > frwr.wr.fast_reg.mr = mr; > frwr.wr.fast_reg.iova = ib_sg_dma_adress(&sg[0]); > frwr.wr.fast_reg.length = length; > frwr.wr.fast_reg.access_flags = my_flags; Provide a helper to hide all this behind the scenes please: void rdma_init_mr_wr(struct ib_send_wr *wr, struct rdma_mr *mr, u64 wr_id, int mr_access_flags); Or if we got with Jason's suggestion split "int mr_access_flags" into "bool remote, bool is_write". To support FRMs if we care enough we'd create a purely in-memory MR in rdma_create_mr and then map it to ib_fmr_pool_map_phys with a helper that the driver can call instead of rdma_init_mr_wr. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html