[RFC][Focal][PATCH 0/2] Add IB peer memory interface

[Tim Gardner]

On 8/30/21 4:50 PM, dann frazier wrote:
> BugLink: https://bugs.launchpad.net/bugs/1923104
> 
> This is a backport of a SAUCE patch we're already carrying in hirsute[*].
> In a conversation w/ Terry, he mentioned that he'd like to see some amount
> of real world functional testing be completed before we'd consider SRU'ing
> to other kernels. That testing will take some scheduling/engineering effort
> so, in order to minimize the risk of respins/retests, I'm submitting this as
> an RFC. My goal here is to try and fish out any aspects of this patch that
> are likely to get NAK'd - even if testing were to pass.
> 
> Note: Going back to 5.4 requires backporting an additional upstream patch,
> which changes the API of an exported symbol (ib_umem_get). The only out
> of tree modules of which I'm aware that use this symbol are the Mellanox
> OFED drivers, but they also bundle their own ib_core module that overrides
> the ib_umem_get interface we provide, so they aren't directly impacted.
> Of course, we can't rule out other users.
> 
> [*] https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/hirsute/commit/?id=e9eb90eb5e4a5aef6f516abbc720038fc0d1a139
> 
> Feras Daoud (1):
>    RDMA/core: Introduce peer memory interface
> 
> Moni Shoua (1):
>    IB: Allow calls to ib_umem_get from kernel ULPs
> 
>   drivers/infiniband/core/Makefile              |   2 +-
>   drivers/infiniband/core/ib_peer_mem.h         |  52 ++
>   drivers/infiniband/core/peer_mem.c            | 484 ++++++++++++++++++
>   drivers/infiniband/core/umem.c                |  69 ++-
>   drivers/infiniband/core/umem_odp.c            |  33 +-
>   drivers/infiniband/hw/bnxt_re/ib_verbs.c      |  12 +-
>   drivers/infiniband/hw/cxgb3/iwch_provider.c   |   2 +-
>   drivers/infiniband/hw/cxgb4/mem.c             |   2 +-
>   drivers/infiniband/hw/efa/efa_verbs.c         |   2 +-
>   drivers/infiniband/hw/hns/hns_roce_cq.c       |   2 +-
>   drivers/infiniband/hw/hns/hns_roce_db.c       |   3 +-
>   drivers/infiniband/hw/hns/hns_roce_mr.c       |   4 +-
>   drivers/infiniband/hw/hns/hns_roce_qp.c       |   2 +-
>   drivers/infiniband/hw/hns/hns_roce_srq.c      |   5 +-
>   drivers/infiniband/hw/i40iw/i40iw_verbs.c     |   2 +-
>   drivers/infiniband/hw/mlx4/cq.c               |   2 +-
>   drivers/infiniband/hw/mlx4/doorbell.c         |   3 +-
>   drivers/infiniband/hw/mlx4/mr.c               |   8 +-
>   drivers/infiniband/hw/mlx4/qp.c               |   5 +-
>   drivers/infiniband/hw/mlx4/srq.c              |   3 +-
>   drivers/infiniband/hw/mlx5/cq.c               |  11 +-
>   drivers/infiniband/hw/mlx5/devx.c             |   2 +-
>   drivers/infiniband/hw/mlx5/doorbell.c         |   3 +-
>   drivers/infiniband/hw/mlx5/mem.c              |  11 +-
>   drivers/infiniband/hw/mlx5/mr.c               |  60 ++-
>   drivers/infiniband/hw/mlx5/odp.c              |   2 +-
>   drivers/infiniband/hw/mlx5/qp.c               |   4 +-
>   drivers/infiniband/hw/mlx5/srq.c              |   2 +-
>   drivers/infiniband/hw/mthca/mthca_provider.c  |   2 +-
>   drivers/infiniband/hw/ocrdma/ocrdma_verbs.c   |   2 +-
>   drivers/infiniband/hw/qedr/verbs.c            |   9 +-
>   drivers/infiniband/hw/vmw_pvrdma/pvrdma_cq.c  |   2 +-
>   drivers/infiniband/hw/vmw_pvrdma/pvrdma_mr.c  |   2 +-
>   drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c  |   7 +-
>   drivers/infiniband/hw/vmw_pvrdma/pvrdma_srq.c |   2 +-
>   drivers/infiniband/sw/rdmavt/mr.c             |   2 +-
>   drivers/infiniband/sw/rxe/rxe_mr.c            |   2 +-
>   include/rdma/ib_umem.h                        |  33 +-
>   include/rdma/ib_umem_odp.h                    |   9 +-
>   include/rdma/peer_mem.h                       | 165 ++++++
>   40 files changed, 908 insertions(+), 121 deletions(-)
>   create mode 100644 drivers/infiniband/core/ib_peer_mem.h
>   create mode 100644 drivers/infiniband/core/peer_mem.c
>   create mode 100644 include/rdma/peer_mem.h
> 
Hi Dann,

If this was being submitted for real then I'd like a bit more 
explanation about what use cases this patch addresses. Is it a ginormous 
performance gain ? Who is requesting this feature ? This is a lot of 
code, so its gonna need some pretty solid justification for a stable 
kernel that we have to maintain for next N years.

As a backport, patch 1 could use some explanation about what were the 
conflicts and how were they resolved. Did it require major sugery ?

Patch 2 is missing provenance. 'Change-Id: 
I1d77f52d56aec2c79e6b9d9ec1096e83a95155cd' doesn't mean much to this 
list. Did this come from Hirsute as well ? Should it be a SAUCE patch ?

rtg
-----------
Tim Gardner
Canonical, Inc