diff mbox series

[rdma-next,v2,4/4] RDMA/mana_ib: Use struct mana_ib_queue for RAW QPs

Message ID 1710867613-4798-5-git-send-email-kotaranov@linux.microsoft.com (mailing list archive)
State Superseded
Headers show
Series Define and use mana queues for CQs and WQs | expand

Commit Message

Konstantin Taranov March 19, 2024, 5 p.m. UTC 
From: Konstantin Taranov <kotaranov@microsoft.com>

Use struct mana_ib_queue and its helpers for RAW QPs

Signed-off-by: Konstantin Taranov <kotaranov@microsoft.com>
---
 drivers/infiniband/hw/mana/mana_ib.h | 13 +++----
 drivers/infiniband/hw/mana/qp.c      | 54 ++++++++--------------------
 2 files changed, 22 insertions(+), 45 deletions(-)

Comments

Long Li March 25, 2024, 7:40 p.m. UTC | #1 
>  struct mana_ib_qp {
>  	struct ib_qp ibqp;
> 
> -	/* Work queue info */
> -	struct ib_umem *sq_umem;
> -	int sqe;
> -	u64 sq_gdma_region;
> -	u64 sq_id;
> -	mana_handle_t tx_object;
> +	struct mana_ib_raw_sq sq;

Are you planning to add another type of sq for RC here?

If yes, use raw_sq and rc_sq in struct fields.

Long
Konstantin Taranov March 25, 2024, 8:57 p.m. UTC | #2 
> From: Long Li <longli@microsoft.com>
> Subject: RE: [PATCH rdma-next v2 4/4] RDMA/mana_ib: Use struct
> mana_ib_queue for RAW QPs
> 
> >  struct mana_ib_qp {
> >  	struct ib_qp ibqp;
> >
> > -	/* Work queue info */
> > -	struct ib_umem *sq_umem;
> > -	int sqe;
> > -	u64 sq_gdma_region;
> > -	u64 sq_id;
> > -	mana_handle_t tx_object;
> > +	struct mana_ib_raw_sq sq;
> 
> Are you planning to add another type of sq for RC here?
> 

There will be no more SQs. There will be rc_qp, ud_qp, uc_qp, which have several queues inside.
Should I still rename "struct mana_ib_raw_sq sq;" to "struct mana_ib_raw_sq raw_sq;" ?

Konstantin
> If yes, use raw_sq and rc_sq in struct fields.
> 
> Long
Long Li March 25, 2024, 9:07 p.m. UTC | #3 
> > >  struct mana_ib_qp {
> > >  	struct ib_qp ibqp;
> > >
> > > -	/* Work queue info */
> > > -	struct ib_umem *sq_umem;
> > > -	int sqe;
> > > -	u64 sq_gdma_region;
> > > -	u64 sq_id;
> > > -	mana_handle_t tx_object;
> > > +	struct mana_ib_raw_sq sq;
> >
> > Are you planning to add another type of sq for RC here?
> >
> 
> There will be no more SQs. There will be rc_qp, ud_qp, uc_qp, which have several
> queues inside.

Are you going to put rc_qp inside struct mana_ib_qp? Or is it another struct containing mana_ib_qp?
Konstantin Taranov March 25, 2024, 11:33 p.m. UTC | #4 
> > > >  struct mana_ib_qp {
> > > >  	struct ib_qp ibqp;
> > > >
> > > > -	/* Work queue info */
> > > > -	struct ib_umem *sq_umem;
> > > > -	int sqe;
> > > > -	u64 sq_gdma_region;
> > > > -	u64 sq_id;
> > > > -	mana_handle_t tx_object;
> > > > +	struct mana_ib_raw_sq sq;
> > >
> > > Are you planning to add another type of sq for RC here?
> > >
> >
> > There will be no more SQs. There will be rc_qp, ud_qp, uc_qp, which have
> several
> > queues inside.
> 
> Are you going to put rc_qp inside struct mana_ib_qp? Or is it another struct
> containing mana_ib_qp?

It will be like that: 
struct mana_ib_qp {
struct ib_qp ibqp;

union { 
struct mana_ib_raw_sq sq;
struct mana_ib_rc_qp rc_qp;
struct mana_ib_ud_qp ud_qp;
};
};
diff mbox series

Patch

diff --git a/drivers/infiniband/hw/mana/mana_ib.h b/drivers/infiniband/hw/mana/mana_ib.h
index a8953ee80..5d9a6e7cb 100644
--- a/drivers/infiniband/hw/mana/mana_ib.h
+++ b/drivers/infiniband/hw/mana/mana_ib.h
@@ -91,15 +91,16 @@  struct mana_ib_cq {
 	u32 comp_vector;
 };
 
+struct mana_ib_raw_sq {
+	/* Work queue info */
+	struct mana_ib_queue queue;
+	mana_handle_t tx_object;
+};
+
 struct mana_ib_qp {
 	struct ib_qp ibqp;
 
-	/* Work queue info */
-	struct ib_umem *sq_umem;
-	int sqe;
-	u64 sq_gdma_region;
-	u64 sq_id;
-	mana_handle_t tx_object;
+	struct mana_ib_raw_sq sq;
 
 	/* The port on the IB device, starting with 1 */
 	u32 port;
diff --git a/drivers/infiniband/hw/mana/qp.c b/drivers/infiniband/hw/mana/qp.c
index f606caa75..5818b665e 100644
--- a/drivers/infiniband/hw/mana/qp.c
+++ b/drivers/infiniband/hw/mana/qp.c
@@ -297,7 +297,6 @@  static int mana_ib_create_qp_raw(struct ib_qp *ibqp, struct ib_pd *ibpd,
 	struct mana_obj_spec cq_spec = {};
 	struct mana_port_context *mpc;
 	struct net_device *ndev;
-	struct ib_umem *umem;
 	struct mana_eq *eq;
 	int eq_vec;
 	u32 port;
@@ -346,32 +345,15 @@  static int mana_ib_create_qp_raw(struct ib_qp *ibqp, struct ib_pd *ibpd,
 	ibdev_dbg(&mdev->ib_dev, "ucmd sq_buf_addr 0x%llx port %u\n",
 		  ucmd.sq_buf_addr, ucmd.port);
 
-	umem = ib_umem_get(ibpd->device, ucmd.sq_buf_addr, ucmd.sq_buf_size,
-			   IB_ACCESS_LOCAL_WRITE);
-	if (IS_ERR(umem)) {
-		err = PTR_ERR(umem);
-		ibdev_dbg(&mdev->ib_dev,
-			  "Failed to get umem for create qp-raw, err %d\n",
-			  err);
-		goto err_free_vport;
-	}
-	qp->sq_umem = umem;
-
-	err = mana_ib_create_zero_offset_dma_region(mdev, qp->sq_umem,
-						    &qp->sq_gdma_region);
+	err = mana_ib_create_queue(mdev, ucmd.sq_buf_addr, ucmd.sq_buf_size, &qp->sq.queue);
 	if (err) {
 		ibdev_dbg(&mdev->ib_dev,
-			  "Failed to create dma region for create qp-raw, %d\n",
-			  err);
-		goto err_release_umem;
+			  "Failed to create queue for create qp-raw, err %d\n", err);
+		goto err_free_vport;
 	}
 
-	ibdev_dbg(&mdev->ib_dev,
-		  "create_dma_region ret %d gdma_region 0x%llx\n",
-		  err, qp->sq_gdma_region);
-
 	/* Create a WQ on the same port handle used by the Ethernet */
-	wq_spec.gdma_region = qp->sq_gdma_region;
+	wq_spec.gdma_region = qp->sq.queue.gdma_region;
 	wq_spec.queue_size = ucmd.sq_buf_size;
 
 	cq_spec.gdma_region = send_cq->queue.gdma_region;
@@ -382,19 +364,19 @@  static int mana_ib_create_qp_raw(struct ib_qp *ibqp, struct ib_pd *ibpd,
 	cq_spec.attached_eq = eq->eq->id;
 
 	err = mana_create_wq_obj(mpc, mpc->port_handle, GDMA_SQ, &wq_spec,
-				 &cq_spec, &qp->tx_object);
+				 &cq_spec, &qp->sq.tx_object);
 	if (err) {
 		ibdev_dbg(&mdev->ib_dev,
 			  "Failed to create wq for create raw-qp, err %d\n",
 			  err);
-		goto err_destroy_dma_region;
+		goto err_destroy_queue;
 	}
 
 	/* The GDMA regions are now owned by the WQ object */
-	qp->sq_gdma_region = GDMA_INVALID_DMA_REGION;
+	qp->sq.queue.gdma_region = GDMA_INVALID_DMA_REGION;
 	send_cq->queue.gdma_region = GDMA_INVALID_DMA_REGION;
 
-	qp->sq_id = wq_spec.queue_index;
+	qp->sq.queue.id = wq_spec.queue_index;
 	send_cq->queue.id = cq_spec.queue_index;
 
 	/* Create CQ table entry */
@@ -404,9 +386,9 @@  static int mana_ib_create_qp_raw(struct ib_qp *ibqp, struct ib_pd *ibpd,
 
 	ibdev_dbg(&mdev->ib_dev,
 		  "ret %d qp->tx_object 0x%llx sq id %llu cq id %llu\n", err,
-		  qp->tx_object, qp->sq_id, send_cq->queue.id);
+		  qp->sq.tx_object, qp->sq.queue.id, send_cq->queue.id);
 
-	resp.sqid = qp->sq_id;
+	resp.sqid = qp->sq.queue.id;
 	resp.cqid = send_cq->queue.id;
 	resp.tx_vp_offset = pd->tx_vp_offset;
 
@@ -425,13 +407,10 @@  static int mana_ib_create_qp_raw(struct ib_qp *ibqp, struct ib_pd *ibpd,
 	gc->cq_table[send_cq->queue.id] = NULL;
 
 err_destroy_wq_obj:
-	mana_destroy_wq_obj(mpc, GDMA_SQ, qp->tx_object);
+	mana_destroy_wq_obj(mpc, GDMA_SQ, qp->sq.tx_object);
 
-err_destroy_dma_region:
-	mana_ib_gd_destroy_dma_region(mdev, qp->sq_gdma_region);
-
-err_release_umem:
-	ib_umem_release(umem);
+err_destroy_queue:
+	mana_ib_destroy_queue(mdev, &qp->sq.queue);
 
 err_free_vport:
 	mana_ib_uncfg_vport(mdev, pd, port);
@@ -505,12 +484,9 @@  static int mana_ib_destroy_qp_raw(struct mana_ib_qp *qp, struct ib_udata *udata)
 	mpc = netdev_priv(ndev);
 	pd = container_of(ibpd, struct mana_ib_pd, ibpd);
 
-	mana_destroy_wq_obj(mpc, GDMA_SQ, qp->tx_object);
+	mana_destroy_wq_obj(mpc, GDMA_SQ, qp->sq.tx_object);
 
-	if (qp->sq_umem) {
-		mana_ib_gd_destroy_dma_region(mdev, qp->sq_gdma_region);
-		ib_umem_release(qp->sq_umem);
-	}
+	mana_ib_destroy_queue(mdev, &qp->sq.queue);
 
 	mana_ib_uncfg_vport(mdev, pd, qp->port);