> -----Original Message-----
> From: linux-rdma-owner@xxxxxxxxxxxxxxx <linux-rdma-
> owner@xxxxxxxxxxxxxxx> On Behalf Of Jason Gunthorpe
> Sent: Wednesday, October 31, 2018 12:24 PM
> To: Shamir Rabinovitch <shamir.rabinovitch@xxxxxxxxxx>; Mark Bloch
> <markb@xxxxxxxxxxxx>
> Cc: linux-rdma@xxxxxxxxxxxxxxx; dledford@xxxxxxxxxx; leon@xxxxxxxxxx;
> santosh.shilimkar@xxxxxxxxxx
> Subject: Re: [PATCH v3 1/5] IB/mlx5: device resources must be created from
> ib_core
>
> On Wed, Oct 31, 2018 at 02:42:38PM +0200, Shamir Rabinovitch wrote:
> > Bypassing ib_core leave the restrack and other fields uninitialized.
> >
> > Signed-off-by: Shamir Rabinovitch <shamir.rabinovitch@xxxxxxxxxx>
> > drivers/infiniband/hw/mlx5/main.c | 30 +++++++++++++++---------------
> > 1 file changed, 15 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/mlx5/main.c
> > b/drivers/infiniband/hw/mlx5/main.c
> > index be701d4..97b44e9 100644
> > +++ b/drivers/infiniband/hw/mlx5/main.c
> > @@ -4565,26 +4565,19 @@ static int create_dev_resources(struct
> > mlx5_ib_resources *devr)
> >
> > mutex_init(&devr->mutex);
> >
> > - devr->p0 = mlx5_ib_alloc_pd(&dev->ib_dev, NULL, NULL);
> > + /* must call ib_core here to initialize restrack! */
> > + devr->p0 = ib_alloc_pd(&dev->ib_dev, 0);
> > if (IS_ERR(devr->p0)) {
> > ret = PTR_ERR(devr->p0);
> > goto error0;
> > }
> > - devr->p0->device = &dev->ib_dev;
> > - devr->p0->uobject = NULL;
> > - atomic_set(&devr->p0->usecnt, 0);
> >
> > - devr->c0 = mlx5_ib_create_cq(&dev->ib_dev, &cq_attr, NULL, NULL);
> > + /* must call ib_core here to initialize restrack! */
> > + devr->c0 = ib_create_cq(&dev->ib_dev, NULL, NULL, NULL, &cq_attr);
> > if (IS_ERR(devr->c0)) {
> > ret = PTR_ERR(devr->c0);
> > goto error1;
> > }
>
> I think the issue here is that the device is not yet registered at this point.
>
> > @@ -4662,14 +4656,16 @@ static int create_dev_resources(struct
> > mlx5_ib_resources *devr)
> >
> > error5:
> > mlx5_ib_destroy_srq(devr->s0);
> > + atomic_dec(&devr->p0->usecnt);
> > + atomic_dec(&devr->s0->ext.cq->usecnt);
> > error4:
> > mlx5_ib_dealloc_xrcd(devr->x1);
> > error3:
> > mlx5_ib_dealloc_xrcd(devr->x0);
> > error2:
> > - mlx5_ib_destroy_cq(devr->c0);
> > + ib_destroy_cq(devr->c0);
> > error1:
> > - mlx5_ib_dealloc_pd(devr->p0);
> > + ib_dealloc_pd(devr->p0);
> > error0:
> > return ret;
> > }
> > @@ -4681,11 +4677,15 @@ static void destroy_dev_resources(struct
> mlx5_ib_resources *devr)
> > int port;
> >
> > mlx5_ib_destroy_srq(devr->s1);
> > + atomic_dec(&devr->p0->usecnt);
> > + atomic_dec(&devr->s1->ext.cq->usecnt);
> > mlx5_ib_destroy_srq(devr->s0);
> > + atomic_dec(&devr->p0->usecnt);
> > + atomic_dec(&devr->s0->ext.cq->usecnt);
> > mlx5_ib_dealloc_xrcd(devr->x0);
> > mlx5_ib_dealloc_xrcd(devr->x1);
> > - mlx5_ib_destroy_cq(devr->c0);
> > - mlx5_ib_dealloc_pd(devr->p0);
> > + ib_destroy_cq(devr->c0);
> > + ib_dealloc_pd(devr->p0);
>
> and has been de-registered by this point.
>
Yes. This also conflicts with the other series that I am doing to unregister-register device in ib core.
So existing code is more appropriate.
> Calling ib_* functions on an unregistered device is not a good idea.
>
> Mark?
>
> Jason
