All of lore.kernel.org
 help / color / mirror / Atom feed
From: Leon Romanovsky <leon@kernel.org>
To: Jack Wang <jinpu.wang@cloud.ionos.com>
Cc: linux-rdma@vger.kernel.org, bvanassche@acm.org,
	dledford@redhat.com, jgg@ziepe.ca, danil.kipnis@cloud.ionos.com
Subject: Re: [PATCH 1/2] RDMA/rtrs: Use new shared CQ mechanism
Date: Sun, 21 Feb 2021 08:23:46 +0200	[thread overview]
Message-ID: <YDH8ckNx/sEPlePt@unreal> (raw)
In-Reply-To: <20210219115019.38981-1-jinpu.wang@cloud.ionos.com>

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>

  parent reply	other threads:[~2021-02-21  6:24 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 ` Leon Romanovsky [this message]
2021-02-22 10:31   ` [PATCH 1/2] RDMA/rtrs: Use new shared CQ mechanism Jinpu Wang
2021-02-22 13:16     ` Leon Romanovsky
2021-02-22 14:13       ` Jinpu Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YDH8ckNx/sEPlePt@unreal \
    --to=leon@kernel.org \
    --cc=bvanassche@acm.org \
    --cc=danil.kipnis@cloud.ionos.com \
    --cc=dledford@redhat.com \
    --cc=jgg@ziepe.ca \
    --cc=jinpu.wang@cloud.ionos.com \
    --cc=linux-rdma@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.