Re: [PATCH rdma-next v6 4/4] RDMA/bnxt_re: Direct Verbs: Support CQ and QP verbs

Jason Gunthorpe
2026-01-09T19:37:58Z

On Wed, Dec 24, 2025 at 09:56:02AM +0530, Sriharsha Basavapatna wrote:
> +static int bnxt_re_dv_create_qplib_cq(struct bnxt_re_dev *rdev,
> + struct bnxt_re_ucontext *re_uctx,
> + struct bnxt_re_cq *cq,
> + struct bnxt_re_cq_req *req)
> +{
> + struct bnxt_qplib_dev_attr *dev_attr = rdev->dev_attr;
> + struct bnxt_qplib_cq *qplcq;
> + struct ib_umem *umem;
> + u32 cqe = req->ncqe;
> + u32 max_active_cqs;
> + int rc = -EINVAL;
> +
> + if (atomic_read(&rdev->stats.res.cq_count) >= dev_attr->max_cq) {
> + ibdev_dbg(&rdev->ibdev, "Create CQ failed - max exceeded(CQs)");
> + return rc;
> + }

This is a racy way to use atomics, this should be an
atomic_inc_return, then check the return code, doing a dec on all
error paths.

> +static void bnxt_re_dv_init_ib_cq(struct bnxt_re_dev *rdev,
> + struct bnxt_re_cq *re_cq)
> +{
> + struct ib_cq *ib_cq;
> +
> + ib_cq = &re_cq->ib_cq;
> + ib_cq->device = &rdev->ibdev;

> + ib_cq->uobject = NULL;
> + ib_cq->comp_handler = NULL;
> + ib_cq->event_handler = NULL;
> + atomic_set(&ib_cq->usecnt, 0);
> +}

All these should already be 0 since this was freshly zallocated, no?

> +int bnxt_re_dv_create_cq(struct bnxt_re_dev *rdev, struct ib_udata *udata,
> + struct bnxt_re_cq *re_cq, struct bnxt_re_cq_req *req)
> +{
> + struct bnxt_re_ucontext *re_uctx =
> + rdma_udata_to_drv_context(udata, struct bnxt_re_ucontext, ib_uctx);
> + struct bnxt_re_cq_resp resp = {};
> + int ret;
> +
> + ret = bnxt_re_dv_create_qplib_cq(rdev, re_uctx, re_cq, req);
> + if (ret)
> + return ret;
> +
> + ret = bnxt_re_dv_create_cq_resp(rdev, re_cq, &resp);
> + if (ret)
> + goto fail_resp;
> +
> + ret = ib_copy_to_udata(udata, &resp, min(sizeof(resp), udata->outlen));
> + if (ret)
> + goto fail_resp;
> +
> + bnxt_re_dv_init_ib_cq(rdev, re_cq);
> + re_cq->is_dv_cq = true;
> + atomic_inc(&rdev->dv_cq_count);
> + return 0;
> +
> +fail_resp:
> + bnxt_qplib_destroy_cq(&rdev->qplib_res, &re_cq->qplib_cq);
> + bnxt_re_put_nq(rdev, re_cq->qplib_cq.nq);
> + ib_umem_release(re_cq->umem);

This seems really weird error unwinding, I expect to see functions
with error unwinds that match the calls within the function.

So why doesn't bnxt_qplib_destroy_cq() do the umem release and
req_put_nq?

Jason