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

[Jason Gunthorpe <jgg@ziepe.ca>]

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