On Mon, Jan 27, 2020 at 01:43:51PM +0530, Devesh Sharma wrote:
> On Sat, Jan 25, 2020 at 11:16 PM Jason Gunthorpe <jgg@xxxxxxxxxxxx> wrote:
> >
> > On Fri, Jan 24, 2020 at 12:52:39AM -0500, Devesh Sharma wrote:
> > > +static int bnxt_re_destroy_gsi_sqp(struct bnxt_re_qp *qp)
> > > +{
> > > + struct bnxt_re_qp *gsi_sqp;
> > > + struct bnxt_re_ah *gsi_sah;
> > > + struct bnxt_re_dev *rdev;
> > > + int rc = 0;
> > > +
> > > + rdev = qp->rdev;
> > > + gsi_sqp = rdev->gsi_ctx.gsi_sqp;
> > > + gsi_sah = rdev->gsi_ctx.gsi_sah;
> > > +
> > > + /* remove from active qp list */
> > > + mutex_lock(&rdev->qp_lock);
> > > + list_del(&gsi_sqp->list);
> > > + atomic_dec(&rdev->qp_count);
> > > + mutex_unlock(&rdev->qp_lock);
> > > +
> > > + dev_dbg(rdev_to_dev(rdev), "Destroy the shadow AH\n");
> > > + bnxt_qplib_destroy_ah(&rdev->qplib_res,
> > > + &gsi_sah->qplib_ah,
> > > + true);
> > > + bnxt_qplib_clean_qp(&qp->qplib_qp);
> > > +
> > > + dev_dbg(rdev_to_dev(rdev), "Destroy the shadow QP\n");
> > > + rc = bnxt_qplib_destroy_qp(&rdev->qplib_res, &gsi_sqp->qplib_qp);
> > > + if (rc) {
> > > + dev_err(rdev_to_dev(rdev), "Destroy Shadow QP failed");
> > > + goto fail;
> > > + }
> > > + bnxt_qplib_free_qp_res(&rdev->qplib_res, &gsi_sqp->qplib_qp);
> > > +
> > > + kfree(rdev->gsi_ctx.sqp_tbl);
> > > + kfree(gsi_sah);
> > > + kfree(gsi_sqp);
> > > + rdev->gsi_ctx.gsi_sqp = NULL;
> > > + rdev->gsi_ctx.gsi_sah = NULL;
> > > + rdev->gsi_ctx.sqp_tbl = NULL;
> > > +
> > > + return 0;
> > > +fail:
> > > + mutex_lock(&rdev->qp_lock);
> > > + list_add_tail(&gsi_sqp->list, &rdev->qp_list);
> > > + atomic_inc(&rdev->qp_count);
> > > + mutex_unlock(&rdev->qp_lock);
> > > + return rc;
> >
> > This error unwind approach looks racy. destroy is not allowed to
> > fail, so why all this mess?
> True, the unwind is not required, even if the driver wants to keep it
> for debugging purpose, the zombie resource would give rise to
> confusion.
Jason doesn't suggest to leave zombie resources, but to clean everything
without any relation to returned status from bnxt_qplib_destroy_qp().
Thanks
