Re: [PATCH rdma-next] RDMA/rdmavt: Decouple QP and SGE lists allocations

[Leon Romanovsky <leon@kernel.org>]

On Tue, May 11, 2021 at 10:59:52AM +0000, Haakon Bugge wrote:
> 
> 
> > On 11 May 2021, at 12:36, Leon Romanovsky <leon@kernel.org> wrote:
> > 
> > From: Leon Romanovsky <leonro@nvidia.com>
> > 
> > The rdmavt QP has fields that are both needed for the control and data
> > path. Such mixed declaration caused to the very specific allocation flow
> > with kzalloc_node and SGE list embedded into the struct rvt_qp.
> > 
> > This patch separates QP creation to two: regular memory allocation for
> > the control path and specific code for the SGE list, while the access to
> > the later is performed through derefenced pointer.
> > 
> > Such pointer and its context are expected to be in the cache, so
> > performance difference is expected to be negligible, if any exists.
> > 
> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> > ---
> > Hi,
> > 
> > This change is needed to convert QP to core allocation scheme. In that
> > scheme QP is allocated outside of the driver and size of such allocation
> > is constant and can be calculated at the compile time.
> > 
> > Thanks
> > ---
> > drivers/infiniband/sw/rdmavt/qp.c | 13 ++++++++-----
> > include/rdma/rdmavt_qp.h          |  2 +-
> > 2 files changed, 9 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/infiniband/sw/rdmavt/qp.c b/drivers/infiniband/sw/rdmavt/qp.c
> > index 9d13db68283c..4522071fc220 100644
> > --- a/drivers/infiniband/sw/rdmavt/qp.c
> > +++ b/drivers/infiniband/sw/rdmavt/qp.c
> > @@ -1077,7 +1077,7 @@ struct ib_qp *rvt_create_qp(struct ib_pd *ibpd,
> > 	int err;
> > 	struct rvt_swqe *swq = NULL;
> > 	size_t sz;
> > -	size_t sg_list_sz;
> > +	size_t sg_list_sz = 0;
> > 	struct ib_qp *ret = ERR_PTR(-ENOMEM);
> > 	struct rvt_dev_info *rdi = ib_to_rvt(ibpd->device);
> > 	void *priv = NULL;
> > @@ -1125,8 +1125,6 @@ struct ib_qp *rvt_create_qp(struct ib_pd *ibpd,
> > 		if (!swq)
> > 			return ERR_PTR(-ENOMEM);
> > 
> > -		sz = sizeof(*qp);
> > -		sg_list_sz = 0;
> > 		if (init_attr->srq) {
> > 			struct rvt_srq *srq = ibsrq_to_rvtsrq(init_attr->srq);
> > 
> > @@ -1136,10 +1134,13 @@ struct ib_qp *rvt_create_qp(struct ib_pd *ibpd,
> > 		} else if (init_attr->cap.max_recv_sge > 1)
> > 			sg_list_sz = sizeof(*qp->r_sg_list) *
> > 				(init_attr->cap.max_recv_sge - 1);
> > -		qp = kzalloc_node(sz + sg_list_sz, GFP_KERNEL,
> > -				  rdi->dparms.node);
> > +		qp = kzalloc(sizeof(*qp), GFP_KERNEL);
> 
> Why not kzalloc_node() here?

The idea is to delete this kzalloc later in next patch, because all
drivers are doing same thing "qp = kzalloc(sizeof(*qp), GFP_KERNEL);".

Thanks