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

[Kamal Heib <kheib@xxxxxxxxxx>]

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:

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
> > 
> > 
>