On Fri, Jan 18, 2019 at 09:59:31PM +0000, Jason Gunthorpe wrote:
> On Thu, Jan 17, 2019 at 08:04:45PM +0200, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> >
> > Adapt mlx5 to share restrack ID instead of local variable.
> >
> > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > drivers/infiniband/hw/mlx5/main.c | 22 +++++++++++++++-------
> > drivers/infiniband/hw/mlx5/mlx5_ib.h | 1 -
> > drivers/infiniband/hw/mlx5/mr.c | 16 ++++++++--------
> > drivers/infiniband/hw/mlx5/qp.c | 22 +++++++++++++---------
> > drivers/infiniband/hw/mlx5/srq.c | 2 +-
> > 5 files changed, 37 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
> > index b32b5c7f1cfd..9db041465266 100644
> > +++ b/drivers/infiniband/hw/mlx5/main.c
> > @@ -2262,11 +2262,14 @@ static struct ib_pd *mlx5_ib_alloc_pd(struct ib_device *ibdev,
> > u32 out[MLX5_ST_SZ_DW(alloc_pd_out)] = {};
> > u32 in[MLX5_ST_SZ_DW(alloc_pd_in)] = {};
> > u16 uid = 0;
> > + u32 pdn;
> >
> > pd = kzalloc(sizeof(*pd), GFP_KERNEL);
> > if (!pd)
> > return ERR_PTR(-ENOMEM);
> >
> > + pd->ibpd.device = ibdev;
>
> This needs to be re-ordered now? Don't like..
I need "device" to know where to store restrack objects.
Because we don't have "priv" for those ib_* objects, we can't
preallocate them before drivers (we don't know the allocation
size). What about if we introduce new functions in IB/core for
all objects to allocate structure and set device?
Something like this:
void *alloc_object(struct ib_device *dev, size_t s, uint64_t offset_to_device_pointer_in_driver)
{
void *a;
a = kzalloc(s, GFP_KERNEL);
if (!a)
return ERR_PTR(-ENOMEM);
*(a + offset_to_device_pointer_in_driver) = dev;
return a;
}
>
> > uid = context ? to_mucontext(context)->devx_uid : 0;
> > MLX5_SET(alloc_pd_in, in, opcode, MLX5_CMD_OP_ALLOC_PD);
> > MLX5_SET(alloc_pd_in, in, uid, uid);
> > @@ -2277,12 +2280,15 @@ static struct ib_pd *mlx5_ib_alloc_pd(struct ib_device *ibdev,
> > return ERR_PTR(err);
> > }
> >
> > - pd->pdn = MLX5_GET(alloc_pd_out, out, pd);
> > + pdn = MLX5_GET(alloc_pd_out, out, pd);
> > pd->uid = uid;
> > - if (context) {
> > - resp.pdn = pd->pdn;
> > + rdma_rt_set_type(&pd->ibpd.res, RDMA_RESTRACK_PD);
> > + rdma_rt_set_id(&pd->ibpd.res, pdn);
> > + err = rdma_restrack_add(&pd->ibpd.res);
> > + if (context || err) {
>
> ?? That needs a goto error unwind now..
>
> > + resp.pdn = pdn;
> > if (ib_copy_to_udata(udata, &resp, sizeof(resp))) {
> > - mlx5_cmd_dealloc_pd(to_mdev(ibdev)->mdev, pd->pdn, uid);
> > + mlx5_cmd_dealloc_pd(to_mdev(ibdev)->mdev, pdn, uid);
> > kfree(pd);
>
> How does the restrack_add get cleaned up here?
>
> Jason
Attachment:
signature.asc
Description: PGP signature
