* [PATCH 1/2] RDMA/rtrs: Use new shared CQ mechanism
@ 2021-02-19 11:50 Jack Wang
2021-02-19 11:50 ` [PATCH 2/2] RDMA/rtrs-clt: Use rdma_event_msg in log Jack Wang
2021-02-21 6:23 ` [PATCH 1/2] RDMA/rtrs: Use new shared CQ mechanism Leon Romanovsky
0 siblings, 2 replies; 8+ messages in thread
From: Jack Wang @ 2021-02-19 11:50 UTC (permalink / raw)
To: linux-rdma; +Cc: bvanassche, leon, dledford, jgg, danil.kipnis, jinpu.wang
Has the driver use shared CQs providing ~10%-20% improvement during
test.
Instead of opening a CQ for each QP per connection, a CQ for each QP
will be provided by the RDMA core driver that will be shared between
the QPs on that core reducing interrupt overhead.
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-clt.c | 10 +++++-----
drivers/infiniband/ulp/rtrs/rtrs-pri.h | 1 +
drivers/infiniband/ulp/rtrs/rtrs-srv.c | 10 +++++-----
drivers/infiniband/ulp/rtrs/rtrs.c | 11 +++++++----
4 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 0a08b4b742a3..4e9cf06cc17a 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -325,7 +325,7 @@ static void rtrs_rdma_error_recovery(struct rtrs_clt_con *con)
static void rtrs_clt_fast_reg_done(struct ib_cq *cq, struct ib_wc *wc)
{
- struct rtrs_clt_con *con = cq->cq_context;
+ struct rtrs_clt_con *con = to_clt_con(wc->qp->qp_context);
if (unlikely(wc->status != IB_WC_SUCCESS)) {
rtrs_err(con->c.sess, "Failed IB_WR_REG_MR: %s\n",
@@ -345,7 +345,7 @@ static void rtrs_clt_inv_rkey_done(struct ib_cq *cq, struct ib_wc *wc)
{
struct rtrs_clt_io_req *req =
container_of(wc->wr_cqe, typeof(*req), inv_cqe);
- struct rtrs_clt_con *con = cq->cq_context;
+ struct rtrs_clt_con *con = to_clt_con(wc->qp->qp_context);
if (unlikely(wc->status != IB_WC_SUCCESS)) {
rtrs_err(con->c.sess, "Failed IB_WR_LOCAL_INV: %s\n",
@@ -586,7 +586,7 @@ static int rtrs_post_recv_empty_x2(struct rtrs_con *con, struct ib_cqe *cqe)
static void rtrs_clt_rdma_done(struct ib_cq *cq, struct ib_wc *wc)
{
- struct rtrs_clt_con *con = cq->cq_context;
+ struct rtrs_clt_con *con = to_clt_con(wc->qp->qp_context);
struct rtrs_clt_sess *sess = to_clt_sess(con->c.sess);
u32 imm_type, imm_payload;
bool w_inval = false;
@@ -2241,7 +2241,7 @@ static int init_conns(struct rtrs_clt_sess *sess)
static void rtrs_clt_info_req_done(struct ib_cq *cq, struct ib_wc *wc)
{
- struct rtrs_clt_con *con = cq->cq_context;
+ struct rtrs_clt_con *con = to_clt_con(wc->qp->qp_context);
struct rtrs_clt_sess *sess = to_clt_sess(con->c.sess);
struct rtrs_iu *iu;
@@ -2323,7 +2323,7 @@ static int process_info_rsp(struct rtrs_clt_sess *sess,
static void rtrs_clt_info_rsp_done(struct ib_cq *cq, struct ib_wc *wc)
{
- struct rtrs_clt_con *con = cq->cq_context;
+ struct rtrs_clt_con *con = to_clt_con(wc->qp->qp_context);
struct rtrs_clt_sess *sess = to_clt_sess(con->c.sess);
struct rtrs_msg_info_rsp *msg;
enum rtrs_clt_state state;
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-pri.h b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
index 8caad0a2322b..1b31bda9ca78 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-pri.h
+++ b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
@@ -91,6 +91,7 @@ struct rtrs_con {
struct ib_cq *cq;
struct rdma_cm_id *cm_id;
unsigned int cid;
+ u16 cq_size;
};
struct rtrs_sess {
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index d071809e3ed2..37ba121564a2 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -199,7 +199,7 @@ static void rtrs_srv_wait_ops_ids(struct rtrs_srv_sess *sess)
static void rtrs_srv_reg_mr_done(struct ib_cq *cq, struct ib_wc *wc)
{
- struct rtrs_srv_con *con = cq->cq_context;
+ struct rtrs_srv_con *con = to_srv_con(wc->qp->qp_context);
struct rtrs_sess *s = con->c.sess;
struct rtrs_srv_sess *sess = to_srv_sess(s);
@@ -720,7 +720,7 @@ static void rtrs_srv_stop_hb(struct rtrs_srv_sess *sess)
static void rtrs_srv_info_rsp_done(struct ib_cq *cq, struct ib_wc *wc)
{
- struct rtrs_srv_con *con = cq->cq_context;
+ struct rtrs_srv_con *con = to_srv_con(wc->qp->qp_context);
struct rtrs_sess *s = con->c.sess;
struct rtrs_srv_sess *sess = to_srv_sess(s);
struct rtrs_iu *iu;
@@ -862,7 +862,7 @@ static int process_info_req(struct rtrs_srv_con *con,
static void rtrs_srv_info_req_done(struct ib_cq *cq, struct ib_wc *wc)
{
- struct rtrs_srv_con *con = cq->cq_context;
+ struct rtrs_srv_con *con = to_srv_con(wc->qp->qp_context);
struct rtrs_sess *s = con->c.sess;
struct rtrs_srv_sess *sess = to_srv_sess(s);
struct rtrs_msg_info_req *msg;
@@ -1110,7 +1110,7 @@ static void rtrs_srv_inv_rkey_done(struct ib_cq *cq, struct ib_wc *wc)
{
struct rtrs_srv_mr *mr =
container_of(wc->wr_cqe, typeof(*mr), inv_cqe);
- struct rtrs_srv_con *con = cq->cq_context;
+ struct rtrs_srv_con *con = to_srv_con(wc->qp->qp_context);
struct rtrs_sess *s = con->c.sess;
struct rtrs_srv_sess *sess = to_srv_sess(s);
struct rtrs_srv *srv = sess->srv;
@@ -1167,7 +1167,7 @@ static void rtrs_rdma_process_wr_wait_list(struct rtrs_srv_con *con)
static void rtrs_srv_rdma_done(struct ib_cq *cq, struct ib_wc *wc)
{
- struct rtrs_srv_con *con = cq->cq_context;
+ struct rtrs_srv_con *con = to_srv_con(wc->qp->qp_context);
struct rtrs_sess *s = con->c.sess;
struct rtrs_srv_sess *sess = to_srv_sess(s);
struct rtrs_srv *srv = sess->srv;
diff --git a/drivers/infiniband/ulp/rtrs/rtrs.c b/drivers/infiniband/ulp/rtrs/rtrs.c
index d13aff0aa816..d5ec78280937 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs.c
@@ -218,14 +218,15 @@ static int create_cq(struct rtrs_con *con, int cq_vector, u16 cq_size,
struct rdma_cm_id *cm_id = con->cm_id;
struct ib_cq *cq;
- cq = ib_alloc_cq(cm_id->device, con, cq_size,
- cq_vector, poll_ctx);
+ cq = ib_cq_pool_get(cm_id->device, cq_size,
+ cq_vector, poll_ctx);
if (IS_ERR(cq)) {
rtrs_err(con->sess, "Creating completion queue failed, errno: %ld\n",
PTR_ERR(cq));
return PTR_ERR(cq);
}
con->cq = cq;
+ con->cq_size = cq_size;
return 0;
}
@@ -273,8 +274,9 @@ int rtrs_cq_qp_create(struct rtrs_sess *sess, struct rtrs_con *con,
err = create_qp(con, sess->dev->ib_pd, max_send_wr, max_recv_wr,
max_send_sge);
if (err) {
- ib_free_cq(con->cq);
+ ib_cq_pool_put(con->cq, con->cq_size);
con->cq = NULL;
+ con->cq_size = 0;
return err;
}
con->sess = sess;
@@ -290,8 +292,9 @@ void rtrs_cq_qp_destroy(struct rtrs_con *con)
con->qp = NULL;
}
if (con->cq) {
- ib_free_cq(con->cq);
+ ib_cq_pool_put(con->cq, con->cq_size);
con->cq = NULL;
+ con->cq_size = 0;
}
}
EXPORT_SYMBOL_GPL(rtrs_cq_qp_destroy);
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] RDMA/rtrs-clt: Use rdma_event_msg in log
2021-02-19 11:50 [PATCH 1/2] RDMA/rtrs: Use new shared CQ mechanism Jack Wang
@ 2021-02-19 11:50 ` Jack Wang
2021-02-21 6:24 ` Leon Romanovsky
2021-02-21 6:23 ` [PATCH 1/2] RDMA/rtrs: Use new shared CQ mechanism Leon Romanovsky
1 sibling, 1 reply; 8+ messages in thread
From: Jack Wang @ 2021-02-19 11:50 UTC (permalink / raw)
To: linux-rdma; +Cc: bvanassche, leon, dledford, jgg, danil.kipnis, jinpu.wang
It's easier to understand string instead of enum.
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-clt.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index 4e9cf06cc17a..f95955fc2992 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -1853,12 +1853,14 @@ static int rtrs_clt_rdma_cm_handler(struct rdma_cm_id *cm_id,
case RDMA_CM_EVENT_UNREACHABLE:
case RDMA_CM_EVENT_ADDR_CHANGE:
case RDMA_CM_EVENT_TIMEWAIT_EXIT:
- rtrs_wrn(s, "CM error event %d\n", ev->event);
+ rtrs_wrn(s, "CM error (CM event: %s, err: %d)\n",
+ rdma_event_msg(ev->event), ev->status);
cm_err = -ECONNRESET;
break;
case RDMA_CM_EVENT_ADDR_ERROR:
case RDMA_CM_EVENT_ROUTE_ERROR:
- rtrs_wrn(s, "CM error event %d\n", ev->event);
+ rtrs_wrn(s, "CM error (CM event: %s, err: %d)\n",
+ rdma_event_msg(ev->event), ev->status);
cm_err = -EHOSTUNREACH;
break;
case RDMA_CM_EVENT_DEVICE_REMOVAL:
@@ -1868,7 +1870,8 @@ static int rtrs_clt_rdma_cm_handler(struct rdma_cm_id *cm_id,
rtrs_clt_close_conns(sess, false);
return 0;
default:
- rtrs_err(s, "Unexpected RDMA CM event (%d)\n", ev->event);
+ rtrs_err(s, "Unexpected RDMA CM error (CM event: %s, err: %d)\n",
+ rdma_event_msg(ev->event), ev->status);
cm_err = -ECONNRESET;
break;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] RDMA/rtrs: Use new shared CQ mechanism
2021-02-19 11:50 [PATCH 1/2] RDMA/rtrs: Use new shared CQ mechanism Jack Wang
2021-02-19 11:50 ` [PATCH 2/2] RDMA/rtrs-clt: Use rdma_event_msg in log Jack Wang
@ 2021-02-21 6:23 ` Leon Romanovsky
2021-02-22 10:31 ` Jinpu Wang
1 sibling, 1 reply; 8+ messages in thread
From: Leon Romanovsky @ 2021-02-21 6:23 UTC (permalink / raw)
To: Jack Wang; +Cc: linux-rdma, bvanassche, dledford, jgg, danil.kipnis
On Fri, Feb 19, 2021 at 12:50:18PM +0100, Jack Wang wrote:
> Has the driver use shared CQs providing ~10%-20% improvement during
> test.
> Instead of opening a CQ for each QP per connection, a CQ for each QP
> will be provided by the RDMA core driver that will be shared between
> the QPs on that core reducing interrupt overhead.
>
> Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
> ---
> drivers/infiniband/ulp/rtrs/rtrs-clt.c | 10 +++++-----
> drivers/infiniband/ulp/rtrs/rtrs-pri.h | 1 +
> drivers/infiniband/ulp/rtrs/rtrs-srv.c | 10 +++++-----
> drivers/infiniband/ulp/rtrs/rtrs.c | 11 +++++++----
> 4 files changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> index 0a08b4b742a3..4e9cf06cc17a 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> @@ -325,7 +325,7 @@ static void rtrs_rdma_error_recovery(struct rtrs_clt_con *con)
>
> static void rtrs_clt_fast_reg_done(struct ib_cq *cq, struct ib_wc *wc)
> {
> - struct rtrs_clt_con *con = cq->cq_context;
> + struct rtrs_clt_con *con = to_clt_con(wc->qp->qp_context);
>
> if (unlikely(wc->status != IB_WC_SUCCESS)) {
> rtrs_err(con->c.sess, "Failed IB_WR_REG_MR: %s\n",
> @@ -345,7 +345,7 @@ static void rtrs_clt_inv_rkey_done(struct ib_cq *cq, struct ib_wc *wc)
> {
> struct rtrs_clt_io_req *req =
> container_of(wc->wr_cqe, typeof(*req), inv_cqe);
> - struct rtrs_clt_con *con = cq->cq_context;
> + struct rtrs_clt_con *con = to_clt_con(wc->qp->qp_context);
>
> if (unlikely(wc->status != IB_WC_SUCCESS)) {
> rtrs_err(con->c.sess, "Failed IB_WR_LOCAL_INV: %s\n",
> @@ -586,7 +586,7 @@ static int rtrs_post_recv_empty_x2(struct rtrs_con *con, struct ib_cqe *cqe)
>
> static void rtrs_clt_rdma_done(struct ib_cq *cq, struct ib_wc *wc)
> {
> - struct rtrs_clt_con *con = cq->cq_context;
> + struct rtrs_clt_con *con = to_clt_con(wc->qp->qp_context);
> struct rtrs_clt_sess *sess = to_clt_sess(con->c.sess);
> u32 imm_type, imm_payload;
> bool w_inval = false;
> @@ -2241,7 +2241,7 @@ static int init_conns(struct rtrs_clt_sess *sess)
>
> static void rtrs_clt_info_req_done(struct ib_cq *cq, struct ib_wc *wc)
> {
> - struct rtrs_clt_con *con = cq->cq_context;
> + struct rtrs_clt_con *con = to_clt_con(wc->qp->qp_context);
> struct rtrs_clt_sess *sess = to_clt_sess(con->c.sess);
> struct rtrs_iu *iu;
>
> @@ -2323,7 +2323,7 @@ static int process_info_rsp(struct rtrs_clt_sess *sess,
>
> static void rtrs_clt_info_rsp_done(struct ib_cq *cq, struct ib_wc *wc)
> {
> - struct rtrs_clt_con *con = cq->cq_context;
> + struct rtrs_clt_con *con = to_clt_con(wc->qp->qp_context);
> struct rtrs_clt_sess *sess = to_clt_sess(con->c.sess);
> struct rtrs_msg_info_rsp *msg;
> enum rtrs_clt_state state;
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-pri.h b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> index 8caad0a2322b..1b31bda9ca78 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> @@ -91,6 +91,7 @@ struct rtrs_con {
> struct ib_cq *cq;
> struct rdma_cm_id *cm_id;
> unsigned int cid;
> + u16 cq_size;
> };
>
> struct rtrs_sess {
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> index d071809e3ed2..37ba121564a2 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> @@ -199,7 +199,7 @@ static void rtrs_srv_wait_ops_ids(struct rtrs_srv_sess *sess)
>
> static void rtrs_srv_reg_mr_done(struct ib_cq *cq, struct ib_wc *wc)
> {
> - struct rtrs_srv_con *con = cq->cq_context;
> + struct rtrs_srv_con *con = to_srv_con(wc->qp->qp_context);
> struct rtrs_sess *s = con->c.sess;
> struct rtrs_srv_sess *sess = to_srv_sess(s);
>
> @@ -720,7 +720,7 @@ static void rtrs_srv_stop_hb(struct rtrs_srv_sess *sess)
>
> static void rtrs_srv_info_rsp_done(struct ib_cq *cq, struct ib_wc *wc)
> {
> - struct rtrs_srv_con *con = cq->cq_context;
> + struct rtrs_srv_con *con = to_srv_con(wc->qp->qp_context);
> struct rtrs_sess *s = con->c.sess;
> struct rtrs_srv_sess *sess = to_srv_sess(s);
> struct rtrs_iu *iu;
> @@ -862,7 +862,7 @@ static int process_info_req(struct rtrs_srv_con *con,
>
> static void rtrs_srv_info_req_done(struct ib_cq *cq, struct ib_wc *wc)
> {
> - struct rtrs_srv_con *con = cq->cq_context;
> + struct rtrs_srv_con *con = to_srv_con(wc->qp->qp_context);
> struct rtrs_sess *s = con->c.sess;
> struct rtrs_srv_sess *sess = to_srv_sess(s);
> struct rtrs_msg_info_req *msg;
> @@ -1110,7 +1110,7 @@ static void rtrs_srv_inv_rkey_done(struct ib_cq *cq, struct ib_wc *wc)
> {
> struct rtrs_srv_mr *mr =
> container_of(wc->wr_cqe, typeof(*mr), inv_cqe);
> - struct rtrs_srv_con *con = cq->cq_context;
> + struct rtrs_srv_con *con = to_srv_con(wc->qp->qp_context);
> struct rtrs_sess *s = con->c.sess;
> struct rtrs_srv_sess *sess = to_srv_sess(s);
> struct rtrs_srv *srv = sess->srv;
> @@ -1167,7 +1167,7 @@ static void rtrs_rdma_process_wr_wait_list(struct rtrs_srv_con *con)
>
> static void rtrs_srv_rdma_done(struct ib_cq *cq, struct ib_wc *wc)
> {
> - struct rtrs_srv_con *con = cq->cq_context;
> + struct rtrs_srv_con *con = to_srv_con(wc->qp->qp_context);
> struct rtrs_sess *s = con->c.sess;
> struct rtrs_srv_sess *sess = to_srv_sess(s);
> struct rtrs_srv *srv = sess->srv;
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs.c b/drivers/infiniband/ulp/rtrs/rtrs.c
> index d13aff0aa816..d5ec78280937 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs.c
> @@ -218,14 +218,15 @@ static int create_cq(struct rtrs_con *con, int cq_vector, u16 cq_size,
> struct rdma_cm_id *cm_id = con->cm_id;
> struct ib_cq *cq;
>
> - cq = ib_alloc_cq(cm_id->device, con, cq_size,
> - cq_vector, poll_ctx);
> + cq = ib_cq_pool_get(cm_id->device, cq_size,
> + cq_vector, poll_ctx);
> if (IS_ERR(cq)) {
> rtrs_err(con->sess, "Creating completion queue failed, errno: %ld\n",
> PTR_ERR(cq));
> return PTR_ERR(cq);
> }
> con->cq = cq;
> + con->cq_size = cq_size;
>
> return 0;
> }
> @@ -273,8 +274,9 @@ int rtrs_cq_qp_create(struct rtrs_sess *sess, struct rtrs_con *con,
> err = create_qp(con, sess->dev->ib_pd, max_send_wr, max_recv_wr,
> max_send_sge);
> if (err) {
> - ib_free_cq(con->cq);
> + ib_cq_pool_put(con->cq, con->cq_size);
> con->cq = NULL;
> + con->cq_size = 0;
It is better do not clear fields that not used, it hides bugs.
Other than that.
Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] RDMA/rtrs-clt: Use rdma_event_msg in log
2021-02-19 11:50 ` [PATCH 2/2] RDMA/rtrs-clt: Use rdma_event_msg in log Jack Wang
@ 2021-02-21 6:24 ` Leon Romanovsky
2021-02-22 10:32 ` Jinpu Wang
0 siblings, 1 reply; 8+ messages in thread
From: Leon Romanovsky @ 2021-02-21 6:24 UTC (permalink / raw)
To: Jack Wang; +Cc: linux-rdma, bvanassche, dledford, jgg, danil.kipnis
On Fri, Feb 19, 2021 at 12:50:19PM +0100, Jack Wang wrote:
> It's easier to understand string instead of enum.
>
> Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
> ---
> drivers/infiniband/ulp/rtrs/rtrs-clt.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] RDMA/rtrs: Use new shared CQ mechanism
2021-02-21 6:23 ` [PATCH 1/2] RDMA/rtrs: Use new shared CQ mechanism Leon Romanovsky
@ 2021-02-22 10:31 ` Jinpu Wang
2021-02-22 13:16 ` Leon Romanovsky
0 siblings, 1 reply; 8+ messages in thread
From: Jinpu Wang @ 2021-02-22 10:31 UTC (permalink / raw)
To: Leon Romanovsky
Cc: linux-rdma, Bart Van Assche, Doug Ledford, Jason Gunthorpe, Danil Kipnis
On Sun, Feb 21, 2021 at 7:23 AM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Fri, Feb 19, 2021 at 12:50:18PM +0100, Jack Wang wrote:
> > Has the driver use shared CQs providing ~10%-20% improvement during
> > test.
> > Instead of opening a CQ for each QP per connection, a CQ for each QP
> > will be provided by the RDMA core driver that will be shared between
> > the QPs on that core reducing interrupt overhead.
> >
> > Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
> > ---
> > drivers/infiniband/ulp/rtrs/rtrs-clt.c | 10 +++++-----
> > drivers/infiniband/ulp/rtrs/rtrs-pri.h | 1 +
> > drivers/infiniband/ulp/rtrs/rtrs-srv.c | 10 +++++-----
> > drivers/infiniband/ulp/rtrs/rtrs.c | 11 +++++++----
> > 4 files changed, 18 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> > index 0a08b4b742a3..4e9cf06cc17a 100644
> > --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> > +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> > @@ -325,7 +325,7 @@ static void rtrs_rdma_error_recovery(struct rtrs_clt_con *con)
> >
> > static void rtrs_clt_fast_reg_done(struct ib_cq *cq, struct ib_wc *wc)
> > {
> > - struct rtrs_clt_con *con = cq->cq_context;
> > + struct rtrs_clt_con *con = to_clt_con(wc->qp->qp_context);
> >
> > if (unlikely(wc->status != IB_WC_SUCCESS)) {
> > rtrs_err(con->c.sess, "Failed IB_WR_REG_MR: %s\n",
> > @@ -345,7 +345,7 @@ static void rtrs_clt_inv_rkey_done(struct ib_cq *cq, struct ib_wc *wc)
> > {
> > struct rtrs_clt_io_req *req =
> > container_of(wc->wr_cqe, typeof(*req), inv_cqe);
> > - struct rtrs_clt_con *con = cq->cq_context;
> > + struct rtrs_clt_con *con = to_clt_con(wc->qp->qp_context);
> >
> > if (unlikely(wc->status != IB_WC_SUCCESS)) {
> > rtrs_err(con->c.sess, "Failed IB_WR_LOCAL_INV: %s\n",
> > @@ -586,7 +586,7 @@ static int rtrs_post_recv_empty_x2(struct rtrs_con *con, struct ib_cqe *cqe)
> >
> > static void rtrs_clt_rdma_done(struct ib_cq *cq, struct ib_wc *wc)
> > {
> > - struct rtrs_clt_con *con = cq->cq_context;
> > + struct rtrs_clt_con *con = to_clt_con(wc->qp->qp_context);
> > struct rtrs_clt_sess *sess = to_clt_sess(con->c.sess);
> > u32 imm_type, imm_payload;
> > bool w_inval = false;
> > @@ -2241,7 +2241,7 @@ static int init_conns(struct rtrs_clt_sess *sess)
> >
> > static void rtrs_clt_info_req_done(struct ib_cq *cq, struct ib_wc *wc)
> > {
> > - struct rtrs_clt_con *con = cq->cq_context;
> > + struct rtrs_clt_con *con = to_clt_con(wc->qp->qp_context);
> > struct rtrs_clt_sess *sess = to_clt_sess(con->c.sess);
> > struct rtrs_iu *iu;
> >
> > @@ -2323,7 +2323,7 @@ static int process_info_rsp(struct rtrs_clt_sess *sess,
> >
> > static void rtrs_clt_info_rsp_done(struct ib_cq *cq, struct ib_wc *wc)
> > {
> > - struct rtrs_clt_con *con = cq->cq_context;
> > + struct rtrs_clt_con *con = to_clt_con(wc->qp->qp_context);
> > struct rtrs_clt_sess *sess = to_clt_sess(con->c.sess);
> > struct rtrs_msg_info_rsp *msg;
> > enum rtrs_clt_state state;
> > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-pri.h b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> > index 8caad0a2322b..1b31bda9ca78 100644
> > --- a/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> > +++ b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> > @@ -91,6 +91,7 @@ struct rtrs_con {
> > struct ib_cq *cq;
> > struct rdma_cm_id *cm_id;
> > unsigned int cid;
> > + u16 cq_size;
> > };
> >
> > struct rtrs_sess {
> > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > index d071809e3ed2..37ba121564a2 100644
> > --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> > @@ -199,7 +199,7 @@ static void rtrs_srv_wait_ops_ids(struct rtrs_srv_sess *sess)
> >
> > static void rtrs_srv_reg_mr_done(struct ib_cq *cq, struct ib_wc *wc)
> > {
> > - struct rtrs_srv_con *con = cq->cq_context;
> > + struct rtrs_srv_con *con = to_srv_con(wc->qp->qp_context);
> > struct rtrs_sess *s = con->c.sess;
> > struct rtrs_srv_sess *sess = to_srv_sess(s);
> >
> > @@ -720,7 +720,7 @@ static void rtrs_srv_stop_hb(struct rtrs_srv_sess *sess)
> >
> > static void rtrs_srv_info_rsp_done(struct ib_cq *cq, struct ib_wc *wc)
> > {
> > - struct rtrs_srv_con *con = cq->cq_context;
> > + struct rtrs_srv_con *con = to_srv_con(wc->qp->qp_context);
> > struct rtrs_sess *s = con->c.sess;
> > struct rtrs_srv_sess *sess = to_srv_sess(s);
> > struct rtrs_iu *iu;
> > @@ -862,7 +862,7 @@ static int process_info_req(struct rtrs_srv_con *con,
> >
> > static void rtrs_srv_info_req_done(struct ib_cq *cq, struct ib_wc *wc)
> > {
> > - struct rtrs_srv_con *con = cq->cq_context;
> > + struct rtrs_srv_con *con = to_srv_con(wc->qp->qp_context);
> > struct rtrs_sess *s = con->c.sess;
> > struct rtrs_srv_sess *sess = to_srv_sess(s);
> > struct rtrs_msg_info_req *msg;
> > @@ -1110,7 +1110,7 @@ static void rtrs_srv_inv_rkey_done(struct ib_cq *cq, struct ib_wc *wc)
> > {
> > struct rtrs_srv_mr *mr =
> > container_of(wc->wr_cqe, typeof(*mr), inv_cqe);
> > - struct rtrs_srv_con *con = cq->cq_context;
> > + struct rtrs_srv_con *con = to_srv_con(wc->qp->qp_context);
> > struct rtrs_sess *s = con->c.sess;
> > struct rtrs_srv_sess *sess = to_srv_sess(s);
> > struct rtrs_srv *srv = sess->srv;
> > @@ -1167,7 +1167,7 @@ static void rtrs_rdma_process_wr_wait_list(struct rtrs_srv_con *con)
> >
> > static void rtrs_srv_rdma_done(struct ib_cq *cq, struct ib_wc *wc)
> > {
> > - struct rtrs_srv_con *con = cq->cq_context;
> > + struct rtrs_srv_con *con = to_srv_con(wc->qp->qp_context);
> > struct rtrs_sess *s = con->c.sess;
> > struct rtrs_srv_sess *sess = to_srv_sess(s);
> > struct rtrs_srv *srv = sess->srv;
> > diff --git a/drivers/infiniband/ulp/rtrs/rtrs.c b/drivers/infiniband/ulp/rtrs/rtrs.c
> > index d13aff0aa816..d5ec78280937 100644
> > --- a/drivers/infiniband/ulp/rtrs/rtrs.c
> > +++ b/drivers/infiniband/ulp/rtrs/rtrs.c
> > @@ -218,14 +218,15 @@ static int create_cq(struct rtrs_con *con, int cq_vector, u16 cq_size,
> > struct rdma_cm_id *cm_id = con->cm_id;
> > struct ib_cq *cq;
> >
> > - cq = ib_alloc_cq(cm_id->device, con, cq_size,
> > - cq_vector, poll_ctx);
> > + cq = ib_cq_pool_get(cm_id->device, cq_size,
> > + cq_vector, poll_ctx);
> > if (IS_ERR(cq)) {
> > rtrs_err(con->sess, "Creating completion queue failed, errno: %ld\n",
> > PTR_ERR(cq));
> > return PTR_ERR(cq);
> > }
> > con->cq = cq;
> > + con->cq_size = cq_size;
> >
> > return 0;
> > }
> > @@ -273,8 +274,9 @@ int rtrs_cq_qp_create(struct rtrs_sess *sess, struct rtrs_con *con,
> > err = create_qp(con, sess->dev->ib_pd, max_send_wr, max_recv_wr,
> > max_send_sge);
> > if (err) {
> > - ib_free_cq(con->cq);
> > + ib_cq_pool_put(con->cq, con->cq_size);
> > con->cq = NULL;
> > + con->cq_size = 0;
>
> It is better do not clear fields that not used, it hides bugs.
> Other than that.
I feel rewinding on the error path by resetting the cq_size is the
right thing to do.
>
> Thanks,
> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Thanks for the review!
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] RDMA/rtrs-clt: Use rdma_event_msg in log
2021-02-21 6:24 ` Leon Romanovsky
@ 2021-02-22 10:32 ` Jinpu Wang
0 siblings, 0 replies; 8+ messages in thread
From: Jinpu Wang @ 2021-02-22 10:32 UTC (permalink / raw)
To: Leon Romanovsky
Cc: linux-rdma, Bart Van Assche, Doug Ledford, Jason Gunthorpe, Danil Kipnis
On Sun, Feb 21, 2021 at 7:24 AM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Fri, Feb 19, 2021 at 12:50:19PM +0100, Jack Wang wrote:
> > It's easier to understand string instead of enum.
> >
> > Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
> > ---
> > drivers/infiniband/ulp/rtrs/rtrs-clt.c | 9 ++++++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
>
> Thanks,
> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Thanks & Regards
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] RDMA/rtrs: Use new shared CQ mechanism
2021-02-22 10:31 ` Jinpu Wang
@ 2021-02-22 13:16 ` Leon Romanovsky
2021-02-22 14:13 ` Jinpu Wang
0 siblings, 1 reply; 8+ messages in thread
From: Leon Romanovsky @ 2021-02-22 13:16 UTC (permalink / raw)
To: Jinpu Wang
Cc: linux-rdma, Bart Van Assche, Doug Ledford, Jason Gunthorpe, Danil Kipnis
On Mon, Feb 22, 2021 at 11:31:55AM +0100, Jinpu Wang wrote:
> On Sun, Feb 21, 2021 at 7:23 AM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > On Fri, Feb 19, 2021 at 12:50:18PM +0100, Jack Wang wrote:
> > > Has the driver use shared CQs providing ~10%-20% improvement during
> > > test.
> > > Instead of opening a CQ for each QP per connection, a CQ for each QP
> > > will be provided by the RDMA core driver that will be shared between
> > > the QPs on that core reducing interrupt overhead.
> > >
> > > Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
> > > ---
> > > drivers/infiniband/ulp/rtrs/rtrs-clt.c | 10 +++++-----
> > > drivers/infiniband/ulp/rtrs/rtrs-pri.h | 1 +
> > > drivers/infiniband/ulp/rtrs/rtrs-srv.c | 10 +++++-----
> > > drivers/infiniband/ulp/rtrs/rtrs.c | 11 +++++++----
> > > 4 files changed, 18 insertions(+), 14 deletions(-)
<...>
> > > err = create_qp(con, sess->dev->ib_pd, max_send_wr, max_recv_wr,
> > > max_send_sge);
> > > if (err) {
> > > - ib_free_cq(con->cq);
> > > + ib_cq_pool_put(con->cq, con->cq_size);
> > > con->cq = NULL;
> > > + con->cq_size = 0;
> >
> > It is better do not clear fields that not used, it hides bugs.
> > Other than that.
> I feel rewinding on the error path by resetting the cq_size is the
> right thing to do.
It is the right thing to do if down the road you have an access to
cq_size with if (..) check. Other than that, it is not right thing to
do.
Thanks
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] RDMA/rtrs: Use new shared CQ mechanism
2021-02-22 13:16 ` Leon Romanovsky
@ 2021-02-22 14:13 ` Jinpu Wang
0 siblings, 0 replies; 8+ messages in thread
From: Jinpu Wang @ 2021-02-22 14:13 UTC (permalink / raw)
To: Leon Romanovsky
Cc: linux-rdma, Bart Van Assche, Doug Ledford, Jason Gunthorpe, Danil Kipnis
On Mon, Feb 22, 2021 at 2:17 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Mon, Feb 22, 2021 at 11:31:55AM +0100, Jinpu Wang wrote:
> > On Sun, Feb 21, 2021 at 7:23 AM Leon Romanovsky <leon@kernel.org> wrote:
> > >
> > > On Fri, Feb 19, 2021 at 12:50:18PM +0100, Jack Wang wrote:
> > > > Has the driver use shared CQs providing ~10%-20% improvement during
> > > > test.
> > > > Instead of opening a CQ for each QP per connection, a CQ for each QP
> > > > will be provided by the RDMA core driver that will be shared between
> > > > the QPs on that core reducing interrupt overhead.
> > > >
> > > > Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
> > > > ---
> > > > drivers/infiniband/ulp/rtrs/rtrs-clt.c | 10 +++++-----
> > > > drivers/infiniband/ulp/rtrs/rtrs-pri.h | 1 +
> > > > drivers/infiniband/ulp/rtrs/rtrs-srv.c | 10 +++++-----
> > > > drivers/infiniband/ulp/rtrs/rtrs.c | 11 +++++++----
> > > > 4 files changed, 18 insertions(+), 14 deletions(-)
>
> <...>
>
> > > > err = create_qp(con, sess->dev->ib_pd, max_send_wr, max_recv_wr,
> > > > max_send_sge);
> > > > if (err) {
> > > > - ib_free_cq(con->cq);
> > > > + ib_cq_pool_put(con->cq, con->cq_size);
> > > > con->cq = NULL;
> > > > + con->cq_size = 0;
> > >
> > > It is better do not clear fields that not used, it hides bugs.
> > > Other than that.
> > I feel rewinding on the error path by resetting the cq_size is the
> > right thing to do.
>
> It is the right thing to do if down the road you have an access to
> cq_size with if (..) check. Other than that, it is not right thing to
> do.
>
> Thanks
Double checked, will remove the cq_size clear lines in v2.
Thanks!
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-02-22 14:16 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-19 11:50 [PATCH 1/2] RDMA/rtrs: Use new shared CQ mechanism Jack Wang
2021-02-19 11:50 ` [PATCH 2/2] RDMA/rtrs-clt: Use rdma_event_msg in log Jack Wang
2021-02-21 6:24 ` Leon Romanovsky
2021-02-22 10:32 ` Jinpu Wang
2021-02-21 6:23 ` [PATCH 1/2] RDMA/rtrs: Use new shared CQ mechanism Leon Romanovsky
2021-02-22 10:31 ` Jinpu Wang
2021-02-22 13:16 ` Leon Romanovsky
2021-02-22 14:13 ` Jinpu Wang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.