Re: [PATCH for-rc] RDMA/qedr: Fix qedr_create_user_qp error flow

[Leon Romanovsky <leon@kernel.org>]

On Wed, Feb 07, 2024 at 07:01:53PM -0500, Kamal Heib wrote:
> On Wed, Feb 07, 2024 at 09:31:14AM +0200, Leon Romanovsky wrote:
> > On Tue, Feb 06, 2024 at 12:54:49PM -0500, Kamal Heib wrote:
> > > Avoid the following warning by making sure to call qedr_cleanup_user()
> > > in case qedr_init_user_queue() failed.
> > 
> > <...>
> > 
> > > diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
> > > index 7887a6786ed4..0943abd4de27 100644
> > > --- a/drivers/infiniband/hw/qedr/verbs.c
> > > +++ b/drivers/infiniband/hw/qedr/verbs.c
> > > @@ -1880,7 +1880,7 @@ static int qedr_create_user_qp(struct qedr_dev *dev,
> > >  		rc = qedr_init_user_queue(udata, dev, &qp->urq, ureq.rq_addr,
> > >  					  ureq.rq_len, true, 0, alloc_and_init);
> > >  		if (rc)
> > > -			return rc;
> > > +			goto err1;
> > 
> > "goto err1" will cause to call to qedr_cleanup_user() which will call to qedr_free_pbl()
> > with qp->urq.pbl_tbl) which can be NULL.
> > 
> > Thanks
> >
> 
> I see, what about something like the following:

It will work.

> 
> diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
> index 7887a6786ed4..f118ce0a9a61 100644
> --- a/drivers/infiniband/hw/qedr/verbs.c
> +++ b/drivers/infiniband/hw/qedr/verbs.c
> @@ -1879,8 +1879,17 @@ static int qedr_create_user_qp(struct qedr_dev *dev,
>                 /* RQ - read access only (0) */
>                 rc = qedr_init_user_queue(udata, dev, &qp->urq, ureq.rq_addr,
>                                           ureq.rq_len, true, 0, alloc_and_init);
> -               if (rc)
> +               if (rc) {
> +                       ib_umem_release(qp->usq.umem);
> +                       qp->usq.umem = NULL;
> +                       if (rdma_protocol_roce(&dev->ibdev, 1)) {
> +                               qedr_free_pbl(dev, &qp->usq.pbl_info,
> +                                             qp->usq.pbl_tbl);
> +                       } else {
> +                               kfree(qp->usq.pbl_tbl);
> +                       }
>                         return rc;
> +               }
>         }
>  
>         memset(&in_params, 0, sizeof(in_params));
> 
> 
> Thanks!
> 
>  
> > >  	}
> > >  
> > >  	memset(&in_params, 0, sizeof(in_params));
> > > -- 
> > > 2.43.0
> > > 
> > > 
> > 
>