[PATCH rdma-next 29/50] RDMA/rxe: Split user and kernel CQ creation paths

Leon Romanovsky posted 50 patches 1 month, 2 weeks ago
[PATCH rdma-next 29/50] RDMA/rxe: Split user and kernel CQ creation paths
Posted by Leon Romanovsky 1 month, 2 weeks ago
From: Leon Romanovsky <leonro@nvidia.com>

Separate the CQ creation logic into distinct kernel and user flows.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/infiniband/sw/rxe/rxe_verbs.c | 81 ++++++++++++++++++++---------------
 1 file changed, 47 insertions(+), 34 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
index 38d8c408320f..1e651bdd8622 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
@@ -1072,58 +1072,70 @@ static int rxe_post_recv(struct ib_qp *ibqp, const struct ib_recv_wr *wr,
 }
 
 /* cq */
-static int rxe_create_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr,
-			 struct uverbs_attr_bundle *attrs)
+static int rxe_create_user_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr,
+			      struct uverbs_attr_bundle *attrs)
 {
 	struct ib_udata *udata = &attrs->driver_udata;
 	struct ib_device *dev = ibcq->device;
 	struct rxe_dev *rxe = to_rdev(dev);
 	struct rxe_cq *cq = to_rcq(ibcq);
-	struct rxe_create_cq_resp __user *uresp = NULL;
-	int err, cleanup_err;
+	struct rxe_create_cq_resp __user *uresp;
+	int err;
 
-	if (udata) {
-		if (udata->outlen < sizeof(*uresp)) {
-			err = -EINVAL;
-			rxe_dbg_dev(rxe, "malformed udata, err = %d\n", err);
-			goto err_out;
-		}
-		uresp = udata->outbuf;
-	}
+	if (udata->outlen < sizeof(*uresp))
+		return -EINVAL;
 
-	if (attr->flags) {
-		err = -EOPNOTSUPP;
-		rxe_dbg_dev(rxe, "bad attr->flags, err = %d\n", err);
-		goto err_out;
-	}
+	uresp = udata->outbuf;
 
-	err = rxe_cq_chk_attr(rxe, NULL, attr->cqe, attr->comp_vector);
-	if (err) {
-		rxe_dbg_dev(rxe, "bad init attributes, err = %d\n", err);
-		goto err_out;
-	}
+	if (attr->flags || ibcq->umem)
+		return -EOPNOTSUPP;
+
+	if (attr->cqe > rxe->attr.max_cqe)
+		return -EINVAL;
 
 	err = rxe_add_to_pool(&rxe->cq_pool, cq);
-	if (err) {
-		rxe_dbg_dev(rxe, "unable to create cq, err = %d\n", err);
-		goto err_out;
-	}
+	if (err)
+		return err;
 
 	err = rxe_cq_from_init(rxe, cq, attr->cqe, attr->comp_vector, udata,
 			       uresp);
-	if (err) {
-		rxe_dbg_cq(cq, "create cq failed, err = %d\n", err);
+	if (err)
 		goto err_cleanup;
-	}
 
 	return 0;
 
 err_cleanup:
-	cleanup_err = rxe_cleanup(cq);
-	if (cleanup_err)
-		rxe_err_cq(cq, "cleanup failed, err = %d\n", cleanup_err);
-err_out:
-	rxe_err_dev(rxe, "returned err = %d\n", err);
+	rxe_cleanup(cq);
+	return err;
+}
+
+static int rxe_create_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr,
+			 struct uverbs_attr_bundle *attrs)
+{
+	struct ib_device *dev = ibcq->device;
+	struct rxe_dev *rxe = to_rdev(dev);
+	struct rxe_cq *cq = to_rcq(ibcq);
+	int err;
+
+	if (attr->flags)
+		return -EOPNOTSUPP;
+
+	if (attr->cqe > rxe->attr.max_cqe)
+		return -EINVAL;
+
+	err = rxe_add_to_pool(&rxe->cq_pool, cq);
+	if (err)
+		return err;
+
+	err = rxe_cq_from_init(rxe, cq, attr->cqe, attr->comp_vector, NULL,
+			       NULL);
+	if (err)
+		goto err_cleanup;
+
+	return 0;
+
+err_cleanup:
+	rxe_cleanup(cq);
 	return err;
 }
 
@@ -1478,6 +1490,7 @@ static const struct ib_device_ops rxe_dev_ops = {
 	.attach_mcast = rxe_attach_mcast,
 	.create_ah = rxe_create_ah,
 	.create_cq = rxe_create_cq,
+	.create_user_cq = rxe_create_user_cq,
 	.create_qp = rxe_create_qp,
 	.create_srq = rxe_create_srq,
 	.create_user_ah = rxe_create_ah,

-- 
2.52.0
Re: [PATCH rdma-next 29/50] RDMA/rxe: Split user and kernel CQ creation paths
Posted by yanjun.zhu 1 month, 2 weeks ago
On 2/13/26 2:58 AM, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@nvidia.com>
> 
> Separate the CQ creation logic into distinct kernel and user flows.
> 
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> ---
>   drivers/infiniband/sw/rxe/rxe_verbs.c | 81 ++++++++++++++++++++---------------
>   1 file changed, 47 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
> index 38d8c408320f..1e651bdd8622 100644
> --- a/drivers/infiniband/sw/rxe/rxe_verbs.c
> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
> @@ -1072,58 +1072,70 @@ static int rxe_post_recv(struct ib_qp *ibqp, const struct ib_recv_wr *wr,
>   }
>   
>   /* cq */
> -static int rxe_create_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr,
> -			 struct uverbs_attr_bundle *attrs)
> +static int rxe_create_user_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr,
> +			      struct uverbs_attr_bundle *attrs)
>   {
>   	struct ib_udata *udata = &attrs->driver_udata;
>   	struct ib_device *dev = ibcq->device;
>   	struct rxe_dev *rxe = to_rdev(dev);
>   	struct rxe_cq *cq = to_rcq(ibcq);
> -	struct rxe_create_cq_resp __user *uresp = NULL;
> -	int err, cleanup_err;
> +	struct rxe_create_cq_resp __user *uresp;
> +	int err;
>   
> -	if (udata) {
> -		if (udata->outlen < sizeof(*uresp)) {
> -			err = -EINVAL;
> -			rxe_dbg_dev(rxe, "malformed udata, err = %d\n", err);
> -			goto err_out;
> -		}
> -		uresp = udata->outbuf;
> -	}
> +	if (udata->outlen < sizeof(*uresp))
> +		return -EINVAL;
>   
> -	if (attr->flags) {
> -		err = -EOPNOTSUPP;
> -		rxe_dbg_dev(rxe, "bad attr->flags, err = %d\n", err);
> -		goto err_out;
> -	}
> +	uresp = udata->outbuf;
>   
> -	err = rxe_cq_chk_attr(rxe, NULL, attr->cqe, attr->comp_vector);
> -	if (err) {
> -		rxe_dbg_dev(rxe, "bad init attributes, err = %d\n", err);
> -		goto err_out;
> -	}
> +	if (attr->flags || ibcq->umem)
> +		return -EOPNOTSUPP;
> +
> +	if (attr->cqe > rxe->attr.max_cqe)
> +		return -EINVAL;
>   
>   	err = rxe_add_to_pool(&rxe->cq_pool, cq);
> -	if (err) {
> -		rxe_dbg_dev(rxe, "unable to create cq, err = %d\n", err);
> -		goto err_out;
> -	}
> +	if (err)
> +		return err;
>   
>   	err = rxe_cq_from_init(rxe, cq, attr->cqe, attr->comp_vector, udata,
>   			       uresp);

Neither rxe_create_user_cq() nor rxe_create_cq() explicitly validates 
attr->comp_vector. Is this guaranteed to be validated by the core before 
reaching the driver, or should rxe still enforce device-specific limits?

> -	if (err) {
> -		rxe_dbg_cq(cq, "create cq failed, err = %d\n", err);
> +	if (err)
>   		goto err_cleanup;

The err_cleanup label is only used for this specific error path. It may 
improve readability to inline the cleanup logic at this site and remove 
the label altogether.

> -	}
>   
>   	return 0;
>   
>   err_cleanup:
> -	cleanup_err = rxe_cleanup(cq);
> -	if (cleanup_err)
> -		rxe_err_cq(cq, "cleanup failed, err = %d\n", cleanup_err);
> -err_out:
> -	rxe_err_dev(rxe, "returned err = %d\n", err);
> +	rxe_cleanup(cq);
> +	return err;
> +}
> +
> +static int rxe_create_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr,
> +			 struct uverbs_attr_bundle *attrs)
> +{
> +	struct ib_device *dev = ibcq->device;
> +	struct rxe_dev *rxe = to_rdev(dev);
> +	struct rxe_cq *cq = to_rcq(ibcq);
> +	int err;
> +
> +	if (attr->flags)
> +		return -EOPNOTSUPP;
> +
> +	if (attr->cqe > rxe->attr.max_cqe)
> +		return -EINVAL;
> +
> +	err = rxe_add_to_pool(&rxe->cq_pool, cq);
> +	if (err)
> +		return err;
> +
> +	err = rxe_cq_from_init(rxe, cq, attr->cqe, attr->comp_vector, NULL,
> +			       NULL);
> +	if (err)
> +		goto err_cleanup;

ditto

Thanks a lot.

Zhu Yanjun

> +
> +	return 0;
> +
> +err_cleanup:
> +	rxe_cleanup(cq);
>   	return err;
>   }
>   
> @@ -1478,6 +1490,7 @@ static const struct ib_device_ops rxe_dev_ops = {
>   	.attach_mcast = rxe_attach_mcast,
>   	.create_ah = rxe_create_ah,
>   	.create_cq = rxe_create_cq,
> +	.create_user_cq = rxe_create_user_cq,
>   	.create_qp = rxe_create_qp,
>   	.create_srq = rxe_create_srq,
>   	.create_user_ah = rxe_create_ah,
>
Re: [PATCH rdma-next 29/50] RDMA/rxe: Split user and kernel CQ creation paths
Posted by Leon Romanovsky 1 month, 2 weeks ago
On Fri, Feb 13, 2026 at 03:22:13PM -0800, yanjun.zhu wrote:
> On 2/13/26 2:58 AM, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@nvidia.com>
> > 
> > Separate the CQ creation logic into distinct kernel and user flows.
> > 
> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> > ---
> >   drivers/infiniband/sw/rxe/rxe_verbs.c | 81 ++++++++++++++++++++---------------
> >   1 file changed, 47 insertions(+), 34 deletions(-)

<...>

> > +	if (err)
> > +		return err;
> >   	err = rxe_cq_from_init(rxe, cq, attr->cqe, attr->comp_vector, udata,
> >   			       uresp);
> 
> Neither rxe_create_user_cq() nor rxe_create_cq() explicitly validates
> attr->comp_vector. Is this guaranteed to be validated by the core before
> reaching the driver, or should rxe still enforce device-specific limits?

We should validate it in IB/core level.
https://github.com/linux-rdma/rdma-core/blob/8b9cdb7c6bd2b6e4e64e08888c10124b0d1873f2/libibverbs/man/ibv_create_cq.3#L32
.I comp_vector
for signaling completion events; it must be at least zero and less than
.I context\fR->num_comp_vectors.

> 
> > -	if (err) {
> > -		rxe_dbg_cq(cq, "create cq failed, err = %d\n", err);
> > +	if (err)
> >   		goto err_cleanup;
> 
> The err_cleanup label is only used for this specific error path. It may
> improve readability to inline the cleanup logic at this site and remove the
> label altogether.

Ill delete. Thanks