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?
> /* Queue Pairs */
> int bnxt_re_destroy_qp(struct ib_qp *ib_qp, struct ib_udata *udata)
> {
> @@ -750,10 +797,18 @@ int bnxt_re_destroy_qp(struct ib_qp *ib_qp, struct ib_udata *udata)
> unsigned int flags;
> int rc;
>
> + mutex_lock(&rdev->qp_lock);
> + list_del(&qp->list);
> + atomic_dec(&rdev->qp_count);
> + mutex_unlock(&rdev->qp_lock);
> bnxt_qplib_flush_cqn_wq(&qp->qplib_qp);
> rc = bnxt_qplib_destroy_qp(&rdev->qplib_res, &qp->qplib_qp);
> if (rc) {
> dev_err(rdev_to_dev(rdev), "Failed to destroy HW QP");
> + mutex_lock(&rdev->qp_lock);
> + list_add_tail(&qp->list, &rdev->qp_list);
> + atomic_inc(&rdev->qp_count);
> + mutex_unlock(&rdev->qp_lock);
> return rc;
> }
More..
Jason
