From: Konstantin Taranov <[email protected]>
Intoduce the mana_ib_remove_cq_cb helper to remove cq callbacks.
The helper removes code duplicates.
Signed-off-by: Konstantin Taranov <[email protected]>
---
drivers/infiniband/hw/mana/cq.c | 19 ++++++++++++-------
drivers/infiniband/hw/mana/mana_ib.h | 1 +
drivers/infiniband/hw/mana/qp.c | 26 ++++----------------------
3 files changed, 17 insertions(+), 29 deletions(-)
diff --git a/drivers/infiniband/hw/mana/cq.c b/drivers/infiniband/hw/mana/cq.c
index 0467ee8..6c3bb8c 100644
--- a/drivers/infiniband/hw/mana/cq.c
+++ b/drivers/infiniband/hw/mana/cq.c
@@ -48,16 +48,10 @@ int mana_ib_destroy_cq(struct ib_cq *ibcq, struct ib_udata *udata)
struct mana_ib_cq *cq = container_of(ibcq, struct mana_ib_cq, ibcq);
struct ib_device *ibdev = ibcq->device;
struct mana_ib_dev *mdev;
- struct gdma_context *gc;
mdev = container_of(ibdev, struct mana_ib_dev, ib_dev);
- gc = mdev_to_gc(mdev);
-
- if (cq->queue.id != INVALID_QUEUE_ID) {
- kfree(gc->cq_table[cq->queue.id]);
- gc->cq_table[cq->queue.id] = NULL;
- }
+ mana_ib_remove_cq_cb(mdev, cq);
mana_ib_destroy_queue(mdev, &cq->queue);
return 0;
@@ -89,3 +83,14 @@ int mana_ib_install_cq_cb(struct mana_ib_dev *mdev, struct mana_ib_cq *cq)
gc->cq_table[cq->queue.id] = gdma_cq;
return 0;
}
+
+void mana_ib_remove_cq_cb(struct mana_ib_dev *mdev, struct mana_ib_cq *cq)
+{
+ struct gdma_context *gc = mdev_to_gc(mdev);
+
+ if (cq->queue.id >= gc->max_num_cqs)
+ return;
+
+ kfree(gc->cq_table[cq->queue.id]);
+ gc->cq_table[cq->queue.id] = NULL;
+}
diff --git a/drivers/infiniband/hw/mana/mana_ib.h b/drivers/infiniband/hw/mana/mana_ib.h
index 9c07021..6c19f4f 100644
--- a/drivers/infiniband/hw/mana/mana_ib.h
+++ b/drivers/infiniband/hw/mana/mana_ib.h
@@ -255,6 +255,7 @@ static inline void copy_in_reverse(u8 *dst, const u8 *src, u32 size)
}
int mana_ib_install_cq_cb(struct mana_ib_dev *mdev, struct mana_ib_cq *cq);
+void mana_ib_remove_cq_cb(struct mana_ib_dev *mdev, struct mana_ib_cq *cq);
int mana_ib_create_zero_offset_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
mana_handle_t *gdma_region);
diff --git a/drivers/infiniband/hw/mana/qp.c b/drivers/infiniband/hw/mana/qp.c
index c4fb8b4..169b286 100644
--- a/drivers/infiniband/hw/mana/qp.c
+++ b/drivers/infiniband/hw/mana/qp.c
@@ -95,11 +95,9 @@ static int mana_ib_create_qp_rss(struct ib_qp *ibqp, struct ib_pd *pd,
struct mana_ib_qp *qp = container_of(ibqp, struct mana_ib_qp, ibqp);
struct mana_ib_dev *mdev =
container_of(pd->device, struct mana_ib_dev, ib_dev);
- struct gdma_context *gc = mdev_to_gc(mdev);
struct ib_rwq_ind_table *ind_tbl = attr->rwq_ind_tbl;
struct mana_ib_create_qp_rss_resp resp = {};
struct mana_ib_create_qp_rss ucmd = {};
- struct gdma_queue **gdma_cq_allocated;
mana_handle_t *mana_ind_table;
struct mana_port_context *mpc;
unsigned int ind_tbl_size;
@@ -173,13 +171,6 @@ static int mana_ib_create_qp_rss(struct ib_qp *ibqp, struct ib_pd *pd,
goto fail;
}
- gdma_cq_allocated = kcalloc(ind_tbl_size, sizeof(*gdma_cq_allocated),
- GFP_KERNEL);
- if (!gdma_cq_allocated) {
- ret = -ENOMEM;
- goto fail;
- }
-
qp->port = port;
for (i = 0; i < ind_tbl_size; i++) {
@@ -229,8 +220,6 @@ static int mana_ib_create_qp_rss(struct ib_qp *ibqp, struct ib_pd *pd,
ret = mana_ib_install_cq_cb(mdev, cq);
if (ret)
goto fail;
-
- gdma_cq_allocated[i] = gc->cq_table[cq->queue.id];
}
resp.num_entries = i;
@@ -250,7 +239,6 @@ static int mana_ib_create_qp_rss(struct ib_qp *ibqp, struct ib_pd *pd,
goto fail;
}
- kfree(gdma_cq_allocated);
kfree(mana_ind_table);
return 0;
@@ -262,13 +250,10 @@ fail:
wq = container_of(ibwq, struct mana_ib_wq, ibwq);
cq = container_of(ibcq, struct mana_ib_cq, ibcq);
- gc->cq_table[cq->queue.id] = NULL;
- kfree(gdma_cq_allocated[i]);
-
+ mana_ib_remove_cq_cb(mdev, cq);
mana_destroy_wq_obj(mpc, GDMA_RQ, wq->rx_object);
}
- kfree(gdma_cq_allocated);
kfree(mana_ind_table);
return ret;
@@ -287,10 +272,8 @@ static int mana_ib_create_qp_raw(struct ib_qp *ibqp, struct ib_pd *ibpd,
struct mana_ib_ucontext *mana_ucontext =
rdma_udata_to_drv_context(udata, struct mana_ib_ucontext,
ibucontext);
- struct gdma_context *gc = mdev_to_gc(mdev);
struct mana_ib_create_qp_resp resp = {};
struct mana_ib_create_qp ucmd = {};
- struct gdma_queue *gdma_cq = NULL;
struct mana_obj_spec wq_spec = {};
struct mana_obj_spec cq_spec = {};
struct mana_port_context *mpc;
@@ -395,14 +378,13 @@ static int mana_ib_create_qp_raw(struct ib_qp *ibqp, struct ib_pd *ibpd,
ibdev_dbg(&mdev->ib_dev,
"Failed copy udata for create qp-raw, %d\n",
err);
- goto err_release_gdma_cq;
+ goto err_remove_cq_cb;
}
return 0;
-err_release_gdma_cq:
- kfree(gdma_cq);
- gc->cq_table[send_cq->queue.id] = NULL;
+err_remove_cq_cb:
+ mana_ib_remove_cq_cb(mdev, send_cq);
err_destroy_wq_obj:
mana_destroy_wq_obj(mpc, GDMA_SQ, qp->qp_handle);
--
2.43.0
> Subject: [PATCH rdma-next 4/6] RDMA/mana_ib: introduce a helper to
> remove cq callbacks
>
> From: Konstantin Taranov <[email protected]>
>
> Intoduce the mana_ib_remove_cq_cb helper to remove cq callbacks.
> The helper removes code duplicates.
>
> Signed-off-by: Konstantin Taranov <[email protected]>
> ---
> drivers/infiniband/hw/mana/cq.c | 19 ++++++++++++-------
> drivers/infiniband/hw/mana/mana_ib.h | 1 +
> drivers/infiniband/hw/mana/qp.c | 26 ++++----------------------
> 3 files changed, 17 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/infiniband/hw/mana/cq.c
> b/drivers/infiniband/hw/mana/cq.c index 0467ee8..6c3bb8c 100644
> --- a/drivers/infiniband/hw/mana/cq.c
> +++ b/drivers/infiniband/hw/mana/cq.c
> @@ -48,16 +48,10 @@ int mana_ib_destroy_cq(struct ib_cq *ibcq, struct
> ib_udata *udata)
> struct mana_ib_cq *cq = container_of(ibcq, struct mana_ib_cq, ibcq);
> struct ib_device *ibdev = ibcq->device;
> struct mana_ib_dev *mdev;
> - struct gdma_context *gc;
>
> mdev = container_of(ibdev, struct mana_ib_dev, ib_dev);
> - gc = mdev_to_gc(mdev);
> -
> - if (cq->queue.id != INVALID_QUEUE_ID) {
> - kfree(gc->cq_table[cq->queue.id]);
> - gc->cq_table[cq->queue.id] = NULL;
> - }
>
> + mana_ib_remove_cq_cb(mdev, cq);
> mana_ib_destroy_queue(mdev, &cq->queue);
>
> return 0;
> @@ -89,3 +83,14 @@ int mana_ib_install_cq_cb(struct mana_ib_dev *mdev,
> struct mana_ib_cq *cq)
> gc->cq_table[cq->queue.id] = gdma_cq;
> return 0;
> }
> +
> +void mana_ib_remove_cq_cb(struct mana_ib_dev *mdev, struct
> mana_ib_cq
> +*cq) {
> + struct gdma_context *gc = mdev_to_gc(mdev);
> +
> + if (cq->queue.id >= gc->max_num_cqs)
> + return;
> +
> + kfree(gc->cq_table[cq->queue.id]);
> + gc->cq_table[cq->queue.id] = NULL;
Why the check for (cq->queue.id != INVALID_QUEUE_ID) is removed?
> +}
> diff --git a/drivers/infiniband/hw/mana/mana_ib.h
> b/drivers/infiniband/hw/mana/mana_ib.h
> index 9c07021..6c19f4f 100644
> --- a/drivers/infiniband/hw/mana/mana_ib.h
> +++ b/drivers/infiniband/hw/mana/mana_ib.h
> @@ -255,6 +255,7 @@ static inline void copy_in_reverse(u8 *dst, const u8
> *src, u32 size) }
>
> int mana_ib_install_cq_cb(struct mana_ib_dev *mdev, struct mana_ib_cq
> *cq);
> +void mana_ib_remove_cq_cb(struct mana_ib_dev *mdev, struct
> mana_ib_cq
> +*cq);
>
> int mana_ib_create_zero_offset_dma_region(struct mana_ib_dev *dev,
> struct ib_umem *umem,
> mana_handle_t *gdma_region);
> diff --git a/drivers/infiniband/hw/mana/qp.c
> b/drivers/infiniband/hw/mana/qp.c index c4fb8b4..169b286 100644
> --- a/drivers/infiniband/hw/mana/qp.c
> +++ b/drivers/infiniband/hw/mana/qp.c
> @@ -95,11 +95,9 @@ static int mana_ib_create_qp_rss(struct ib_qp *ibqp,
> struct ib_pd *pd,
> struct mana_ib_qp *qp = container_of(ibqp, struct mana_ib_qp,
> ibqp);
> struct mana_ib_dev *mdev =
> container_of(pd->device, struct mana_ib_dev, ib_dev);
> - struct gdma_context *gc = mdev_to_gc(mdev);
> struct ib_rwq_ind_table *ind_tbl = attr->rwq_ind_tbl;
> struct mana_ib_create_qp_rss_resp resp = {};
> struct mana_ib_create_qp_rss ucmd = {};
> - struct gdma_queue **gdma_cq_allocated;
> mana_handle_t *mana_ind_table;
> struct mana_port_context *mpc;
> unsigned int ind_tbl_size;
> @@ -173,13 +171,6 @@ static int mana_ib_create_qp_rss(struct ib_qp *ibqp,
> struct ib_pd *pd,
> goto fail;
> }
>
> - gdma_cq_allocated = kcalloc(ind_tbl_size,
> sizeof(*gdma_cq_allocated),
> - GFP_KERNEL);
> - if (!gdma_cq_allocated) {
> - ret = -ENOMEM;
> - goto fail;
> - }
> -
Why the allocation for CQs is removed? This is not related to this patch.
> > +void mana_ib_remove_cq_cb(struct mana_ib_dev *mdev, struct
> > mana_ib_cq
> > +*cq) {
> > + struct gdma_context *gc = mdev_to_gc(mdev);
> > +
> > + if (cq->queue.id >= gc->max_num_cqs)
> > + return;
> > +
> > + kfree(gc->cq_table[cq->queue.id]);
> > + gc->cq_table[cq->queue.id] = NULL;
>
> Why the check for (cq->queue.id != INVALID_QUEUE_ID) is removed?
As max_num_cqs is always less than INVALID_QUEUE_ID, it is included in the "if".
I can add " || cq->queue.id == INVALID_QUEUE_ID " to the condition if you want.
> > @@ -173,13 +171,6 @@ static int mana_ib_create_qp_rss(struct ib_qp
> > *ibqp, struct ib_pd *pd,
> > goto fail;
> > }
> >
> > - gdma_cq_allocated = kcalloc(ind_tbl_size,
> > sizeof(*gdma_cq_allocated),
> > - GFP_KERNEL);
> > - if (!gdma_cq_allocated) {
> > - ret = -ENOMEM;
> > - goto fail;
> > - }
> > -
>
> Why the allocation for CQs is removed? This is not related to this patch.
It becomes the dead code if I add the helper. You allocated gdma_cq_allocated to
temporary store gdma_queue to be able to deallocate them. The introduced helper
frees pointers to gdma_queue from kfree(gc->cq_table[cq->queue.id]), making
gdma_cq_allocated unused.
> > > +void mana_ib_remove_cq_cb(struct mana_ib_dev *mdev, struct
> > > mana_ib_cq
> > > +*cq) {
> > > + struct gdma_context *gc = mdev_to_gc(mdev);
> > > +
> > > + if (cq->queue.id >= gc->max_num_cqs)
> > > + return;
> > > +
> > > + kfree(gc->cq_table[cq->queue.id]);
> > > + gc->cq_table[cq->queue.id] = NULL;
> >
> > Why the check for (cq->queue.id != INVALID_QUEUE_ID) is removed?
>
> As max_num_cqs is always less than INVALID_QUEUE_ID, it is included in the "if".
> I can add " || cq->queue.id == INVALID_QUEUE_ID " to the condition if you want.
Okay, can you add a comment before if (cq->queue.id >= gc->max_num_cqs) saying it also works with INVALID_QUEUE_ID?