On 6/1/23 6:54 PM, Jason Gunthorpe wrote: > On Fri, May 19, 2023 at 12:54:14PM -0400, Dennis Dalessandro wrote: >> Here are 3 more patches related/spawned out of the work to scale back the >> page pinning re-work. This series depends on [1] which was submitted for RC >> recently. >> >> [1] https://patchwork.kernel.org/project/linux-rdma/patch/168451393605.3700681.13493776139032178861.stgit@xxxxxxxxxxxxxxxxxxxxxxxxxxxx/ >> >> --- >> >> Brendan Cunningham (2): >> IB/hfi1: Add mmu_rb_node refcount to hfi1_mmu_rb_template tracepoints >> IB/hfi1: Remove unused struct mmu_rb_ops fields .insert, .invalidate > > I took these two > >> Patrick Kelsey (1): >> IB/hfi1: Separate user SDMA page-pinning from memory type > > Don't know what to do with this one I'm not seeing why it's a problem. It improves our existing page pinning by making the code easier to follow. It makes the code easier to maintain as well centralizing the pinning code. > Maybe send a complete feature. It is a complete feature. It's just a refactoring really of an existing feature. It makes it more flexible to extend in the future and is the current interface or psm2 and libfabric. We are clearly not throwing some uapi changes over the fence that have no user backing. We have existing use cases already. Now all of this being said, this is starting to concern me. We plan to start sending patches for supporting new HW soon. We were going to do this incrementally. Is that going to be considered an incomplete feature? Should we wait until it's all done and ship it all at once? I was envisioning doing things the way we did for rdmavt, showing our work so to speak, making incremental changes overtime as opposed to how we submitted the original hfi1 code in a giant blob. For instance, we are going to add a field to a data structure to hold parameters that differ between multiple chips. The first patch was going to be adding that infrastructure and moving WFR (the existing chip) values into it. In my opinion that's basically the same thing we are doing here with the pinning interface. This is all in effort to be open and involve the community as much as possible along the way so we avoid a fiasco like we had years ago with hfi1's introduction. -Denny