[PATCH 1/3] IB/iser: use new shared CQ mechanism

[PATCH 1/3] IB/iser: use new shared CQ mechanism

[Max Gurtovoy]

From: Yamin Friedman <yaminf@mellanox.com>

Has the driver use shared CQs provided by the rdma core driver.
Because this provides similar functionality to iser_comp it has been
removed. Now there is no reason to allocate very large CQs when the driver
is loaded while gaining the advantage of shared CQs.

Signed-off-by: Yamin Friedman <yaminf@mellanox.com>
Acked-by: Max Gurtovoy <maxg@mellanox.com>
---
 drivers/infiniband/ulp/iser/iscsi_iser.h |  23 ++-----
 drivers/infiniband/ulp/iser/iser_verbs.c | 112 +++++++------------------------
 2 files changed, 29 insertions(+), 106 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 1d77c7f..fddcb88 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -300,18 +300,6 @@ struct iser_login_desc {
 struct iscsi_iser_task;
 
 /**
- * struct iser_comp - iSER completion context
- *
- * @cq:         completion queue
- * @active_qps: Number of active QPs attached
- *              to completion context
- */
-struct iser_comp {
-	struct ib_cq		*cq;
-	int                      active_qps;
-};
-
-/**
  * struct iser_device - iSER device handle
  *
  * @ib_device:     RDMA device
@@ -320,9 +308,6 @@ struct iser_comp {
  * @event_handler: IB events handle routine
  * @ig_list:	   entry in devices list
  * @refcount:      Reference counter, dominated by open iser connections
- * @comps_used:    Number of completion contexts used, Min between online
- *                 cpus and device max completion vectors
- * @comps:         Dinamically allocated array of completion handlers
  */
 struct iser_device {
 	struct ib_device             *ib_device;
@@ -330,8 +315,6 @@ struct iser_device {
 	struct ib_event_handler      event_handler;
 	struct list_head             ig_list;
 	int                          refcount;
-	int			     comps_used;
-	struct iser_comp	     *comps;
 };
 
 /**
@@ -380,11 +363,12 @@ struct iser_fr_pool {
  *
  * @cma_id:              rdma_cm connection maneger handle
  * @qp:                  Connection Queue-pair
+ * @cq:                  Connection completion queue
+ * @cq_size:             The number of max outstanding completions
  * @post_recv_buf_count: post receive counter
  * @sig_count:           send work request signal count
  * @rx_wr:               receive work request for batch posts
  * @device:              reference to iser device
- * @comp:                iser completion context
  * @fr_pool:             connection fast registration poool
  * @pi_support:          Indicate device T10-PI support
  * @reg_cqe:             completion handler
@@ -392,11 +376,12 @@ struct iser_fr_pool {
 struct ib_conn {
 	struct rdma_cm_id           *cma_id;
 	struct ib_qp	            *qp;
+	struct ib_cq		    *cq;
+	u32			    cq_size;
 	int                          post_recv_buf_count;
 	u8                           sig_count;
 	struct ib_recv_wr	     rx_wr[ISER_MIN_POSTED_RX];
 	struct iser_device          *device;
-	struct iser_comp	    *comp;
 	struct iser_fr_pool          fr_pool;
 	bool			     pi_support;
 	struct ib_cqe		     reg_cqe;
diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index c1f44c4..699e075 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -68,59 +68,23 @@ static void iser_event_handler(struct ib_event_handler *handler,
 static int iser_create_device_ib_res(struct iser_device *device)
 {
 	struct ib_device *ib_dev = device->ib_device;
-	int i, max_cqe;
 
 	if (!(ib_dev->attrs.device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS)) {
 		iser_err("IB device does not support memory registrations\n");
 		return -1;
 	}
 
-	device->comps_used = min_t(int, num_online_cpus(),
-				 ib_dev->num_comp_vectors);
-
-	device->comps = kcalloc(device->comps_used, sizeof(*device->comps),
-				GFP_KERNEL);
-	if (!device->comps)
-		goto comps_err;
-
-	max_cqe = min(ISER_MAX_CQ_LEN, ib_dev->attrs.max_cqe);
-
-	iser_info("using %d CQs, device %s supports %d vectors max_cqe %d\n",
-		  device->comps_used, dev_name(&ib_dev->dev),
-		  ib_dev->num_comp_vectors, max_cqe);
-
 	device->pd = ib_alloc_pd(ib_dev,
 		iser_always_reg ? 0 : IB_PD_UNSAFE_GLOBAL_RKEY);
 	if (IS_ERR(device->pd))
 		goto pd_err;
 
-	for (i = 0; i < device->comps_used; i++) {
-		struct iser_comp *comp = &device->comps[i];
-
-		comp->cq = ib_alloc_cq(ib_dev, comp, max_cqe, i,
-				       IB_POLL_SOFTIRQ);
-		if (IS_ERR(comp->cq)) {
-			comp->cq = NULL;
-			goto cq_err;
-		}
-	}
-
 	INIT_IB_EVENT_HANDLER(&device->event_handler, ib_dev,
 			      iser_event_handler);
 	ib_register_event_handler(&device->event_handler);
 	return 0;
 
-cq_err:
-	for (i = 0; i < device->comps_used; i++) {
-		struct iser_comp *comp = &device->comps[i];
-
-		if (comp->cq)
-			ib_free_cq(comp->cq);
-	}
-	ib_dealloc_pd(device->pd);
 pd_err:
-	kfree(device->comps);
-comps_err:
 	iser_err("failed to allocate an IB resource\n");
 	return -1;
 }
@@ -131,20 +95,9 @@ static int iser_create_device_ib_res(struct iser_device *device)
  */
 static void iser_free_device_ib_res(struct iser_device *device)
 {
-	int i;
-
-	for (i = 0; i < device->comps_used; i++) {
-		struct iser_comp *comp = &device->comps[i];
-
-		ib_free_cq(comp->cq);
-		comp->cq = NULL;
-	}
-
 	ib_unregister_event_handler(&device->event_handler);
 	ib_dealloc_pd(device->pd);
 
-	kfree(device->comps);
-	device->comps = NULL;
 	device->pd = NULL;
 }
 
@@ -287,70 +240,57 @@ static int iser_create_ib_conn_res(struct ib_conn *ib_conn)
 	struct ib_device	*ib_dev;
 	struct ib_qp_init_attr	init_attr;
 	int			ret = -ENOMEM;
-	int index, min_index = 0;
+	unsigned int max_send_wr, cq_size;
 
 	BUG_ON(ib_conn->device == NULL);
 
 	device = ib_conn->device;
 	ib_dev = device->ib_device;
 
-	memset(&init_attr, 0, sizeof init_attr);
+	if (ib_conn->pi_support)
+		max_send_wr = ISER_QP_SIG_MAX_REQ_DTOS + 1;
+	else
+		max_send_wr = ISER_QP_MAX_REQ_DTOS + 1;
+	max_send_wr = min_t(unsigned int, max_send_wr,
+			    (unsigned int)ib_dev->attrs.max_qp_wr);
 
-	mutex_lock(&ig.connlist_mutex);
-	/* select the CQ with the minimal number of usages */
-	for (index = 0; index < device->comps_used; index++) {
-		if (device->comps[index].active_qps <
-		    device->comps[min_index].active_qps)
-			min_index = index;
+	cq_size = max_send_wr + ISER_QP_MAX_RECV_DTOS;
+	ib_conn->cq = ib_cq_pool_get(ib_dev, cq_size, -1, IB_POLL_SOFTIRQ);
+	if (IS_ERR(ib_conn->cq)) {
+		ret = PTR_ERR(ib_conn->cq);
+		goto cq_err;
 	}
-	ib_conn->comp = &device->comps[min_index];
-	ib_conn->comp->active_qps++;
-	mutex_unlock(&ig.connlist_mutex);
-	iser_info("cq index %d used for ib_conn %p\n", min_index, ib_conn);
+	ib_conn->cq_size = cq_size;
+
+	memset(&init_attr, 0, sizeof(init_attr));
 
 	init_attr.event_handler = iser_qp_event_callback;
 	init_attr.qp_context	= (void *)ib_conn;
-	init_attr.send_cq	= ib_conn->comp->cq;
-	init_attr.recv_cq	= ib_conn->comp->cq;
+	init_attr.send_cq	= ib_conn->cq;
+	init_attr.recv_cq	= ib_conn->cq;
 	init_attr.cap.max_recv_wr  = ISER_QP_MAX_RECV_DTOS;
 	init_attr.cap.max_send_sge = 2;
 	init_attr.cap.max_recv_sge = 1;
 	init_attr.sq_sig_type	= IB_SIGNAL_REQ_WR;
 	init_attr.qp_type	= IB_QPT_RC;
-	if (ib_conn->pi_support) {
-		init_attr.cap.max_send_wr = ISER_QP_SIG_MAX_REQ_DTOS + 1;
+	init_attr.cap.max_send_wr = max_send_wr;
+	if (ib_conn->pi_support)
 		init_attr.create_flags |= IB_QP_CREATE_INTEGRITY_EN;
-		iser_conn->max_cmds =
-			ISER_GET_MAX_XMIT_CMDS(ISER_QP_SIG_MAX_REQ_DTOS);
-	} else {
-		if (ib_dev->attrs.max_qp_wr > ISER_QP_MAX_REQ_DTOS) {
-			init_attr.cap.max_send_wr  = ISER_QP_MAX_REQ_DTOS + 1;
-			iser_conn->max_cmds =
-				ISER_GET_MAX_XMIT_CMDS(ISER_QP_MAX_REQ_DTOS);
-		} else {
-			init_attr.cap.max_send_wr = ib_dev->attrs.max_qp_wr;
-			iser_conn->max_cmds =
-				ISER_GET_MAX_XMIT_CMDS(ib_dev->attrs.max_qp_wr);
-			iser_dbg("device %s supports max_send_wr %d\n",
-				 dev_name(&device->ib_device->dev),
-				 ib_dev->attrs.max_qp_wr);
-		}
-	}
+	iser_conn->max_cmds = ISER_GET_MAX_XMIT_CMDS(max_send_wr - 1);
 
 	ret = rdma_create_qp(ib_conn->cma_id, device->pd, &init_attr);
 	if (ret)
 		goto out_err;
 
 	ib_conn->qp = ib_conn->cma_id->qp;
-	iser_info("setting conn %p cma_id %p qp %p\n",
+	iser_info("setting conn %p cma_id %p qp %p max_send_wr %d\n",
 		  ib_conn, ib_conn->cma_id,
-		  ib_conn->cma_id->qp);
+		  ib_conn->cma_id->qp, max_send_wr);
 	return ret;
 
 out_err:
-	mutex_lock(&ig.connlist_mutex);
-	ib_conn->comp->active_qps--;
-	mutex_unlock(&ig.connlist_mutex);
+	ib_cq_pool_put(ib_conn->cq, ib_conn->cq_size);
+cq_err:
 	iser_err("unable to alloc mem or create resource, err %d\n", ret);
 
 	return ret;
@@ -462,10 +402,8 @@ static void iser_free_ib_conn_res(struct iser_conn *iser_conn,
 		  iser_conn, ib_conn->cma_id, ib_conn->qp);
 
 	if (ib_conn->qp != NULL) {
-		mutex_lock(&ig.connlist_mutex);
-		ib_conn->comp->active_qps--;
-		mutex_unlock(&ig.connlist_mutex);
 		rdma_destroy_qp(ib_conn->cma_id);
+		ib_cq_pool_put(ib_conn->cq, ib_conn->cq_size);
 		ib_conn->qp = NULL;
 	}
 
-- 
1.8.3.1

[PATCH 2/3] IB/isert: use new shared CQ mechanism

[Max Gurtovoy]

From: Yamin Friedman <yaminf@mellanox.com>

Has the driver use shared CQs provided by the rdma core driver.
Because this provides similar functionality to iser_comp it has been
removed. Now there is no reason to allocate very large CQs when the driver
is loaded while gaining the advantage of shared CQs. Previously when a
single connection was opened a CQ was opened for every core with enough
space for eight connections, this is a very large overhead that in most
cases will not be utilized.

Signed-off-by: Yamin Friedman <yaminf@mellanox.com>
Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
---
 drivers/infiniband/ulp/isert/ib_isert.c | 169 ++++++--------------------------
 drivers/infiniband/ulp/isert/ib_isert.h |  18 +---
 2 files changed, 34 insertions(+), 153 deletions(-)

diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index b7df38e..4e9307b 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -24,13 +24,6 @@
 
 #include "ib_isert.h"
 
-#define	ISERT_MAX_CONN		8
-#define ISER_MAX_RX_CQ_LEN	(ISERT_QP_MAX_RECV_DTOS * ISERT_MAX_CONN)
-#define ISER_MAX_TX_CQ_LEN \
-	((ISERT_QP_MAX_REQ_DTOS + ISCSI_DEF_XMIT_CMDS_MAX) * ISERT_MAX_CONN)
-#define ISER_MAX_CQ_LEN		(ISER_MAX_RX_CQ_LEN + ISER_MAX_TX_CQ_LEN + \
-				 ISERT_MAX_CONN)
-
 static int isert_debug_level;
 module_param_named(debug_level, isert_debug_level, int, 0644);
 MODULE_PARM_DESC(debug_level, "Enable debug tracing if > 0 (default:0)");
@@ -82,50 +75,29 @@
 	}
 }
 
-static struct isert_comp *
-isert_comp_get(struct isert_conn *isert_conn)
-{
-	struct isert_device *device = isert_conn->device;
-	struct isert_comp *comp;
-	int i, min = 0;
-
-	mutex_lock(&device_list_mutex);
-	for (i = 0; i < device->comps_used; i++)
-		if (device->comps[i].active_qps <
-		    device->comps[min].active_qps)
-			min = i;
-	comp = &device->comps[min];
-	comp->active_qps++;
-	mutex_unlock(&device_list_mutex);
-
-	isert_info("conn %p, using comp %p min_index: %d\n",
-		   isert_conn, comp, min);
-
-	return comp;
-}
-
-static void
-isert_comp_put(struct isert_comp *comp)
-{
-	mutex_lock(&device_list_mutex);
-	comp->active_qps--;
-	mutex_unlock(&device_list_mutex);
-}
-
 static struct ib_qp *
 isert_create_qp(struct isert_conn *isert_conn,
-		struct isert_comp *comp,
 		struct rdma_cm_id *cma_id)
 {
+	u32 cq_size = ISERT_QP_MAX_REQ_DTOS + ISERT_QP_MAX_RECV_DTOS + 2;
 	struct isert_device *device = isert_conn->device;
+	struct ib_device *ib_dev = device->ib_device;
 	struct ib_qp_init_attr attr;
 	int ret;
 
+	isert_conn->cq = ib_cq_pool_get(ib_dev, cq_size, -1, IB_POLL_WORKQUEUE);
+	if (IS_ERR(isert_conn->cq)) {
+		isert_err("Unable to allocate cq\n");
+		ret = PTR_ERR(isert_conn->cq);
+		return ERR_PTR(ret);
+	}
+	isert_conn->cq_size = cq_size;
+
 	memset(&attr, 0, sizeof(struct ib_qp_init_attr));
 	attr.event_handler = isert_qp_event_callback;
 	attr.qp_context = isert_conn;
-	attr.send_cq = comp->cq;
-	attr.recv_cq = comp->cq;
+	attr.send_cq = isert_conn->cq;
+	attr.recv_cq = isert_conn->cq;
 	attr.cap.max_send_wr = ISERT_QP_MAX_REQ_DTOS + 1;
 	attr.cap.max_recv_wr = ISERT_QP_MAX_RECV_DTOS + 1;
 	attr.cap.max_rdma_ctxs = ISCSI_DEF_XMIT_CMDS_MAX;
@@ -139,6 +111,8 @@
 	ret = rdma_create_qp(cma_id, device->pd, &attr);
 	if (ret) {
 		isert_err("rdma_create_qp failed for cma_id %d\n", ret);
+		ib_cq_pool_put(isert_conn->cq, isert_conn->cq_size);
+
 		return ERR_PTR(ret);
 	}
 
@@ -146,25 +120,6 @@
 }
 
 static int
-isert_conn_setup_qp(struct isert_conn *isert_conn, struct rdma_cm_id *cma_id)
-{
-	struct isert_comp *comp;
-	int ret;
-
-	comp = isert_comp_get(isert_conn);
-	isert_conn->qp = isert_create_qp(isert_conn, comp, cma_id);
-	if (IS_ERR(isert_conn->qp)) {
-		ret = PTR_ERR(isert_conn->qp);
-		goto err;
-	}
-
-	return 0;
-err:
-	isert_comp_put(comp);
-	return ret;
-}
-
-static int
 isert_alloc_rx_descriptors(struct isert_conn *isert_conn)
 {
 	struct isert_device *device = isert_conn->device;
@@ -231,61 +186,6 @@
 	isert_conn->rx_descs = NULL;
 }
 
-static void
-isert_free_comps(struct isert_device *device)
-{
-	int i;
-
-	for (i = 0; i < device->comps_used; i++) {
-		struct isert_comp *comp = &device->comps[i];
-
-		if (comp->cq)
-			ib_free_cq(comp->cq);
-	}
-	kfree(device->comps);
-}
-
-static int
-isert_alloc_comps(struct isert_device *device)
-{
-	int i, max_cqe, ret = 0;
-
-	device->comps_used = min(ISERT_MAX_CQ, min_t(int, num_online_cpus(),
-				 device->ib_device->num_comp_vectors));
-
-	isert_info("Using %d CQs, %s supports %d vectors support "
-		   "pi_capable %d\n",
-		   device->comps_used, dev_name(&device->ib_device->dev),
-		   device->ib_device->num_comp_vectors,
-		   device->pi_capable);
-
-	device->comps = kcalloc(device->comps_used, sizeof(struct isert_comp),
-				GFP_KERNEL);
-	if (!device->comps)
-		return -ENOMEM;
-
-	max_cqe = min(ISER_MAX_CQ_LEN, device->ib_device->attrs.max_cqe);
-
-	for (i = 0; i < device->comps_used; i++) {
-		struct isert_comp *comp = &device->comps[i];
-
-		comp->device = device;
-		comp->cq = ib_alloc_cq(device->ib_device, comp, max_cqe, i,
-				IB_POLL_WORKQUEUE);
-		if (IS_ERR(comp->cq)) {
-			isert_err("Unable to allocate cq\n");
-			ret = PTR_ERR(comp->cq);
-			comp->cq = NULL;
-			goto out_cq;
-		}
-	}
-
-	return 0;
-out_cq:
-	isert_free_comps(device);
-	return ret;
-}
-
 static int
 isert_create_device_ib_res(struct isert_device *device)
 {
@@ -296,16 +196,12 @@
 		  ib_dev->attrs.max_send_sge, ib_dev->attrs.max_recv_sge);
 	isert_dbg("devattr->max_sge_rd: %d\n", ib_dev->attrs.max_sge_rd);
 
-	ret = isert_alloc_comps(device);
-	if (ret)
-		goto out;
-
 	device->pd = ib_alloc_pd(ib_dev, 0);
 	if (IS_ERR(device->pd)) {
 		ret = PTR_ERR(device->pd);
 		isert_err("failed to allocate pd, device %p, ret=%d\n",
 			  device, ret);
-		goto out_cq;
+		return ret;
 	}
 
 	/* Check signature cap */
@@ -313,13 +209,6 @@
 			     IB_DEVICE_INTEGRITY_HANDOVER ? true : false;
 
 	return 0;
-
-out_cq:
-	isert_free_comps(device);
-out:
-	if (ret > 0)
-		ret = -EINVAL;
-	return ret;
 }
 
 static void
@@ -328,7 +217,6 @@
 	isert_info("device %p\n", device);
 
 	ib_dealloc_pd(device->pd);
-	isert_free_comps(device);
 }
 
 static void
@@ -490,6 +378,13 @@
 	}
 }
 
+static void
+isert_destroy_qp(struct isert_conn *isert_conn)
+{
+	ib_destroy_qp(isert_conn->qp);
+	ib_cq_pool_put(isert_conn->cq, isert_conn->cq_size);
+}
+
 static int
 isert_connect_request(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
 {
@@ -530,17 +425,19 @@
 
 	isert_set_nego_params(isert_conn, &event->param.conn);
 
-	ret = isert_conn_setup_qp(isert_conn, cma_id);
-	if (ret)
+	isert_conn->qp = isert_create_qp(isert_conn, cma_id);
+	if (IS_ERR(isert_conn->qp)) {
+		ret = PTR_ERR(isert_conn->qp);
 		goto out_conn_dev;
+	}
 
 	ret = isert_login_post_recv(isert_conn);
 	if (ret)
-		goto out_conn_dev;
+		goto out_destroy_qp;
 
 	ret = isert_rdma_accept(isert_conn);
 	if (ret)
-		goto out_conn_dev;
+		goto out_destroy_qp;
 
 	mutex_lock(&isert_np->mutex);
 	list_add_tail(&isert_conn->node, &isert_np->accepted);
@@ -548,6 +445,8 @@
 
 	return 0;
 
+out_destroy_qp:
+	isert_destroy_qp(isert_conn);
 out_conn_dev:
 	isert_device_put(device);
 out_rsp_dma_map:
@@ -572,12 +471,8 @@
 	    !isert_conn->dev_removed)
 		rdma_destroy_id(isert_conn->cm_id);
 
-	if (isert_conn->qp) {
-		struct isert_comp *comp = isert_conn->qp->recv_cq->cq_context;
-
-		isert_comp_put(comp);
-		ib_destroy_qp(isert_conn->qp);
-	}
+	if (isert_conn->qp)
+		isert_destroy_qp(isert_conn);
 
 	if (isert_conn->login_req_buf)
 		isert_free_login_buf(isert_conn);
diff --git a/drivers/infiniband/ulp/isert/ib_isert.h b/drivers/infiniband/ulp/isert/ib_isert.h
index 3b296ba..efe755f 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.h
+++ b/drivers/infiniband/ulp/isert/ib_isert.h
@@ -155,6 +155,8 @@ struct isert_conn {
 	struct iser_tx_desc	login_tx_desc;
 	struct rdma_cm_id	*cm_id;
 	struct ib_qp		*qp;
+	struct ib_cq		*cq;
+	u32			cq_size;
 	struct isert_device	*device;
 	struct mutex		mutex;
 	struct kref		kref;
@@ -165,22 +167,6 @@ struct isert_conn {
 	bool			dev_removed;
 };
 
-#define ISERT_MAX_CQ 64
-
-/**
- * struct isert_comp - iSER completion context
- *
- * @device:     pointer to device handle
- * @cq:         completion queue
- * @active_qps: Number of active QPs attached
- *              to completion context
- */
-struct isert_comp {
-	struct isert_device     *device;
-	struct ib_cq		*cq;
-	int                      active_qps;
-};
-
 struct isert_device {
 	bool			pi_capable;
 	int			refcount;
-- 
1.8.3.1

[PATCH 3/3] IB/srpt: use new shared CQ mechanism

[Max Gurtovoy]

From: Yamin Friedman <yaminf@mellanox.com>

Has the driver use shared CQs provided by the rdma core driver.
This provides an advantage of improved efficiency handling interrupts.

Signed-off-by: Yamin Friedman <yaminf@mellanox.com>
Reviewed-by: Max Gurtovoy <maxg@mellanox.com>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 17 +++++++++--------
 drivers/infiniband/ulp/srpt/ib_srpt.h |  1 +
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index ef7fcd3..7a4884c 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -869,7 +869,7 @@ static int srpt_zerolength_write(struct srpt_rdma_ch *ch)
 
 static void srpt_zerolength_write_done(struct ib_cq *cq, struct ib_wc *wc)
 {
-	struct srpt_rdma_ch *ch = cq->cq_context;
+	struct srpt_rdma_ch *ch = wc->qp->qp_context;
 
 	pr_debug("%s-%d wc->status %d\n", ch->sess_name, ch->qp->qp_num,
 		 wc->status);
@@ -1322,7 +1322,7 @@ static int srpt_abort_cmd(struct srpt_send_ioctx *ioctx)
  */
 static void srpt_rdma_read_done(struct ib_cq *cq, struct ib_wc *wc)
 {
-	struct srpt_rdma_ch *ch = cq->cq_context;
+	struct srpt_rdma_ch *ch = wc->qp->qp_context;
 	struct srpt_send_ioctx *ioctx =
 		container_of(wc->wr_cqe, struct srpt_send_ioctx, rdma_cqe);
 
@@ -1683,7 +1683,7 @@ static void srpt_handle_tsk_mgmt(struct srpt_rdma_ch *ch,
 
 static void srpt_recv_done(struct ib_cq *cq, struct ib_wc *wc)
 {
-	struct srpt_rdma_ch *ch = cq->cq_context;
+	struct srpt_rdma_ch *ch = wc->qp->qp_context;
 	struct srpt_recv_ioctx *ioctx =
 		container_of(wc->wr_cqe, struct srpt_recv_ioctx, ioctx.cqe);
 
@@ -1744,7 +1744,7 @@ static void srpt_process_wait_list(struct srpt_rdma_ch *ch)
  */
 static void srpt_send_done(struct ib_cq *cq, struct ib_wc *wc)
 {
-	struct srpt_rdma_ch *ch = cq->cq_context;
+	struct srpt_rdma_ch *ch = wc->qp->qp_context;
 	struct srpt_send_ioctx *ioctx =
 		container_of(wc->wr_cqe, struct srpt_send_ioctx, ioctx.cqe);
 	enum srpt_command_state state;
@@ -1791,7 +1791,7 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)
 		goto out;
 
 retry:
-	ch->cq = ib_alloc_cq_any(sdev->device, ch, ch->rq_size + sq_size,
+	ch->cq = ib_cq_pool_get(sdev->device, ch->rq_size + sq_size, -1,
 				 IB_POLL_WORKQUEUE);
 	if (IS_ERR(ch->cq)) {
 		ret = PTR_ERR(ch->cq);
@@ -1799,6 +1799,7 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)
 		       ch->rq_size + sq_size, ret);
 		goto out;
 	}
+	ch->cq_size = ch->rq_size + sq_size;
 
 	qp_init->qp_context = (void *)ch;
 	qp_init->event_handler
@@ -1843,7 +1844,7 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)
 		if (retry) {
 			pr_debug("failed to create queue pair with sq_size = %d (%d) - retrying\n",
 				 sq_size, ret);
-			ib_free_cq(ch->cq);
+			ib_cq_pool_put(ch->cq, ch->cq_size);
 			sq_size = max(sq_size / 2, MIN_SRPT_SQ_SIZE);
 			goto retry;
 		} else {
@@ -1869,14 +1870,14 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)
 
 err_destroy_cq:
 	ch->qp = NULL;
-	ib_free_cq(ch->cq);
+	ib_cq_pool_put(ch->cq, ch->cq_size);
 	goto out;
 }
 
 static void srpt_destroy_ch_ib(struct srpt_rdma_ch *ch)
 {
 	ib_destroy_qp(ch->qp);
-	ib_free_cq(ch->cq);
+	ib_cq_pool_put(ch->cq, ch->cq_size);
 }
 
 /**
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h
index f31c349..41435a6 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.h
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.h
@@ -300,6 +300,7 @@ struct srpt_rdma_ch {
 		} rdma_cm;
 	};
 	struct ib_cq		*cq;
+	u32			cq_size;
 	struct ib_cqe		zw_cqe;
 	struct rcu_head		rcu;
 	struct kref		kref;
-- 
1.8.3.1

Re: [PATCH 3/3] IB/srpt: use new shared CQ mechanism

[Bart Van Assche]

On 2020-07-22 06:56, Max Gurtovoy wrote:
> Has the driver use shared CQs provided by the rdma core driver.
  ^^^
  Make?
> This provides an advantage of improved efficiency handling interrupts.
                ^^                                ^^^
                the?                              of?

Anyway:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

Re: [PATCH 1/3] IB/iser: use new shared CQ mechanism

[Max Gurtovoy]

Jason/Leon,

can you please pull this to "for-kernel-5.9" branch please ?

Or do we need more reviews on this series ?

On 7/22/2020 4:56 PM, Max Gurtovoy wrote:
> From: Yamin Friedman <yaminf@mellanox.com>
>
> Has the driver use shared CQs provided by the rdma core driver.
> Because this provides similar functionality to iser_comp it has been
> removed. Now there is no reason to allocate very large CQs when the driver
> is loaded while gaining the advantage of shared CQs.
>
> Signed-off-by: Yamin Friedman <yaminf@mellanox.com>
> Acked-by: Max Gurtovoy <maxg@mellanox.com>
> ---
>   drivers/infiniband/ulp/iser/iscsi_iser.h |  23 ++-----
>   drivers/infiniband/ulp/iser/iser_verbs.c | 112 +++++++------------------------
>   2 files changed, 29 insertions(+), 106 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
> index 1d77c7f..fddcb88 100644
> --- a/drivers/infiniband/ulp/iser/iscsi_iser.h
> +++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
> @@ -300,18 +300,6 @@ struct iser_login_desc {
>   struct iscsi_iser_task;
>   
>   /**
> - * struct iser_comp - iSER completion context
> - *
> - * @cq:         completion queue
> - * @active_qps: Number of active QPs attached
> - *              to completion context
> - */
> -struct iser_comp {
> -	struct ib_cq		*cq;
> -	int                      active_qps;
> -};
> -
> -/**
>    * struct iser_device - iSER device handle
>    *
>    * @ib_device:     RDMA device
> @@ -320,9 +308,6 @@ struct iser_comp {
>    * @event_handler: IB events handle routine
>    * @ig_list:	   entry in devices list
>    * @refcount:      Reference counter, dominated by open iser connections
> - * @comps_used:    Number of completion contexts used, Min between online
> - *                 cpus and device max completion vectors
> - * @comps:         Dinamically allocated array of completion handlers
>    */
>   struct iser_device {
>   	struct ib_device             *ib_device;
> @@ -330,8 +315,6 @@ struct iser_device {
>   	struct ib_event_handler      event_handler;
>   	struct list_head             ig_list;
>   	int                          refcount;
> -	int			     comps_used;
> -	struct iser_comp	     *comps;
>   };
>   
>   /**
> @@ -380,11 +363,12 @@ struct iser_fr_pool {
>    *
>    * @cma_id:              rdma_cm connection maneger handle
>    * @qp:                  Connection Queue-pair
> + * @cq:                  Connection completion queue
> + * @cq_size:             The number of max outstanding completions
>    * @post_recv_buf_count: post receive counter
>    * @sig_count:           send work request signal count
>    * @rx_wr:               receive work request for batch posts
>    * @device:              reference to iser device
> - * @comp:                iser completion context
>    * @fr_pool:             connection fast registration poool
>    * @pi_support:          Indicate device T10-PI support
>    * @reg_cqe:             completion handler
> @@ -392,11 +376,12 @@ struct iser_fr_pool {
>   struct ib_conn {
>   	struct rdma_cm_id           *cma_id;
>   	struct ib_qp	            *qp;
> +	struct ib_cq		    *cq;
> +	u32			    cq_size;
>   	int                          post_recv_buf_count;
>   	u8                           sig_count;
>   	struct ib_recv_wr	     rx_wr[ISER_MIN_POSTED_RX];
>   	struct iser_device          *device;
> -	struct iser_comp	    *comp;
>   	struct iser_fr_pool          fr_pool;
>   	bool			     pi_support;
>   	struct ib_cqe		     reg_cqe;
> diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
> index c1f44c4..699e075 100644
> --- a/drivers/infiniband/ulp/iser/iser_verbs.c
> +++ b/drivers/infiniband/ulp/iser/iser_verbs.c
> @@ -68,59 +68,23 @@ static void iser_event_handler(struct ib_event_handler *handler,
>   static int iser_create_device_ib_res(struct iser_device *device)
>   {
>   	struct ib_device *ib_dev = device->ib_device;
> -	int i, max_cqe;
>   
>   	if (!(ib_dev->attrs.device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS)) {
>   		iser_err("IB device does not support memory registrations\n");
>   		return -1;
>   	}
>   
> -	device->comps_used = min_t(int, num_online_cpus(),
> -				 ib_dev->num_comp_vectors);
> -
> -	device->comps = kcalloc(device->comps_used, sizeof(*device->comps),
> -				GFP_KERNEL);
> -	if (!device->comps)
> -		goto comps_err;
> -
> -	max_cqe = min(ISER_MAX_CQ_LEN, ib_dev->attrs.max_cqe);
> -
> -	iser_info("using %d CQs, device %s supports %d vectors max_cqe %d\n",
> -		  device->comps_used, dev_name(&ib_dev->dev),
> -		  ib_dev->num_comp_vectors, max_cqe);
> -
>   	device->pd = ib_alloc_pd(ib_dev,
>   		iser_always_reg ? 0 : IB_PD_UNSAFE_GLOBAL_RKEY);
>   	if (IS_ERR(device->pd))
>   		goto pd_err;
>   
> -	for (i = 0; i < device->comps_used; i++) {
> -		struct iser_comp *comp = &device->comps[i];
> -
> -		comp->cq = ib_alloc_cq(ib_dev, comp, max_cqe, i,
> -				       IB_POLL_SOFTIRQ);
> -		if (IS_ERR(comp->cq)) {
> -			comp->cq = NULL;
> -			goto cq_err;
> -		}
> -	}
> -
>   	INIT_IB_EVENT_HANDLER(&device->event_handler, ib_dev,
>   			      iser_event_handler);
>   	ib_register_event_handler(&device->event_handler);
>   	return 0;
>   
> -cq_err:
> -	for (i = 0; i < device->comps_used; i++) {
> -		struct iser_comp *comp = &device->comps[i];
> -
> -		if (comp->cq)
> -			ib_free_cq(comp->cq);
> -	}
> -	ib_dealloc_pd(device->pd);
>   pd_err:
> -	kfree(device->comps);
> -comps_err:
>   	iser_err("failed to allocate an IB resource\n");
>   	return -1;
>   }
> @@ -131,20 +95,9 @@ static int iser_create_device_ib_res(struct iser_device *device)
>    */
>   static void iser_free_device_ib_res(struct iser_device *device)
>   {
> -	int i;
> -
> -	for (i = 0; i < device->comps_used; i++) {
> -		struct iser_comp *comp = &device->comps[i];
> -
> -		ib_free_cq(comp->cq);
> -		comp->cq = NULL;
> -	}
> -
>   	ib_unregister_event_handler(&device->event_handler);
>   	ib_dealloc_pd(device->pd);
>   
> -	kfree(device->comps);
> -	device->comps = NULL;
>   	device->pd = NULL;
>   }
>   
> @@ -287,70 +240,57 @@ static int iser_create_ib_conn_res(struct ib_conn *ib_conn)
>   	struct ib_device	*ib_dev;
>   	struct ib_qp_init_attr	init_attr;
>   	int			ret = -ENOMEM;
> -	int index, min_index = 0;
> +	unsigned int max_send_wr, cq_size;
>   
>   	BUG_ON(ib_conn->device == NULL);
>   
>   	device = ib_conn->device;
>   	ib_dev = device->ib_device;
>   
> -	memset(&init_attr, 0, sizeof init_attr);
> +	if (ib_conn->pi_support)
> +		max_send_wr = ISER_QP_SIG_MAX_REQ_DTOS + 1;
> +	else
> +		max_send_wr = ISER_QP_MAX_REQ_DTOS + 1;
> +	max_send_wr = min_t(unsigned int, max_send_wr,
> +			    (unsigned int)ib_dev->attrs.max_qp_wr);
>   
> -	mutex_lock(&ig.connlist_mutex);
> -	/* select the CQ with the minimal number of usages */
> -	for (index = 0; index < device->comps_used; index++) {
> -		if (device->comps[index].active_qps <
> -		    device->comps[min_index].active_qps)
> -			min_index = index;
> +	cq_size = max_send_wr + ISER_QP_MAX_RECV_DTOS;
> +	ib_conn->cq = ib_cq_pool_get(ib_dev, cq_size, -1, IB_POLL_SOFTIRQ);
> +	if (IS_ERR(ib_conn->cq)) {
> +		ret = PTR_ERR(ib_conn->cq);
> +		goto cq_err;
>   	}
> -	ib_conn->comp = &device->comps[min_index];
> -	ib_conn->comp->active_qps++;
> -	mutex_unlock(&ig.connlist_mutex);
> -	iser_info("cq index %d used for ib_conn %p\n", min_index, ib_conn);
> +	ib_conn->cq_size = cq_size;
> +
> +	memset(&init_attr, 0, sizeof(init_attr));
>   
>   	init_attr.event_handler = iser_qp_event_callback;
>   	init_attr.qp_context	= (void *)ib_conn;
> -	init_attr.send_cq	= ib_conn->comp->cq;
> -	init_attr.recv_cq	= ib_conn->comp->cq;
> +	init_attr.send_cq	= ib_conn->cq;
> +	init_attr.recv_cq	= ib_conn->cq;
>   	init_attr.cap.max_recv_wr  = ISER_QP_MAX_RECV_DTOS;
>   	init_attr.cap.max_send_sge = 2;
>   	init_attr.cap.max_recv_sge = 1;
>   	init_attr.sq_sig_type	= IB_SIGNAL_REQ_WR;
>   	init_attr.qp_type	= IB_QPT_RC;
> -	if (ib_conn->pi_support) {
> -		init_attr.cap.max_send_wr = ISER_QP_SIG_MAX_REQ_DTOS + 1;
> +	init_attr.cap.max_send_wr = max_send_wr;
> +	if (ib_conn->pi_support)
>   		init_attr.create_flags |= IB_QP_CREATE_INTEGRITY_EN;
> -		iser_conn->max_cmds =
> -			ISER_GET_MAX_XMIT_CMDS(ISER_QP_SIG_MAX_REQ_DTOS);
> -	} else {
> -		if (ib_dev->attrs.max_qp_wr > ISER_QP_MAX_REQ_DTOS) {
> -			init_attr.cap.max_send_wr  = ISER_QP_MAX_REQ_DTOS + 1;
> -			iser_conn->max_cmds =
> -				ISER_GET_MAX_XMIT_CMDS(ISER_QP_MAX_REQ_DTOS);
> -		} else {
> -			init_attr.cap.max_send_wr = ib_dev->attrs.max_qp_wr;
> -			iser_conn->max_cmds =
> -				ISER_GET_MAX_XMIT_CMDS(ib_dev->attrs.max_qp_wr);
> -			iser_dbg("device %s supports max_send_wr %d\n",
> -				 dev_name(&device->ib_device->dev),
> -				 ib_dev->attrs.max_qp_wr);
> -		}
> -	}
> +	iser_conn->max_cmds = ISER_GET_MAX_XMIT_CMDS(max_send_wr - 1);
>   
>   	ret = rdma_create_qp(ib_conn->cma_id, device->pd, &init_attr);
>   	if (ret)
>   		goto out_err;
>   
>   	ib_conn->qp = ib_conn->cma_id->qp;
> -	iser_info("setting conn %p cma_id %p qp %p\n",
> +	iser_info("setting conn %p cma_id %p qp %p max_send_wr %d\n",
>   		  ib_conn, ib_conn->cma_id,
> -		  ib_conn->cma_id->qp);
> +		  ib_conn->cma_id->qp, max_send_wr);
>   	return ret;
>   
>   out_err:
> -	mutex_lock(&ig.connlist_mutex);
> -	ib_conn->comp->active_qps--;
> -	mutex_unlock(&ig.connlist_mutex);
> +	ib_cq_pool_put(ib_conn->cq, ib_conn->cq_size);
> +cq_err:
>   	iser_err("unable to alloc mem or create resource, err %d\n", ret);
>   
>   	return ret;
> @@ -462,10 +402,8 @@ static void iser_free_ib_conn_res(struct iser_conn *iser_conn,
>   		  iser_conn, ib_conn->cma_id, ib_conn->qp);
>   
>   	if (ib_conn->qp != NULL) {
> -		mutex_lock(&ig.connlist_mutex);
> -		ib_conn->comp->active_qps--;
> -		mutex_unlock(&ig.connlist_mutex);
>   		rdma_destroy_qp(ib_conn->cma_id);
> +		ib_cq_pool_put(ib_conn->cq, ib_conn->cq_size);
>   		ib_conn->qp = NULL;
>   	}
>   

Re: [PATCH 1/3] IB/iser: use new shared CQ mechanism

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH 2/3] IB/isert: use new shared CQ mechanism

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH 3/3] IB/srpt: use new shared CQ mechanism

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH 1/3] IB/iser: use new shared CQ mechanism

[Leon Romanovsky]

On Wed, Jul 29, 2020 at 11:34:11AM +0300, Max Gurtovoy wrote:
> Jason/Leon,
>
> can you please pull this to "for-kernel-5.9" branch please ?
>
> Or do we need more reviews on this series ?

I don't think so, Sagi and Bart are more than enough.

Thanks

Re: [PATCH 1/3] IB/iser: use new shared CQ mechanism

[Jason Gunthorpe]

On Wed, Jul 22, 2020 at 04:56:27PM +0300, Max Gurtovoy wrote:
> From: Yamin Friedman <yaminf@mellanox.com>
> 
> Has the driver use shared CQs provided by the rdma core driver.
> Because this provides similar functionality to iser_comp it has been
> removed. Now there is no reason to allocate very large CQs when the driver
> is loaded while gaining the advantage of shared CQs.
> 
> Signed-off-by: Yamin Friedman <yaminf@mellanox.com>
> Acked-by: Max Gurtovoy <maxg@mellanox.com>
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
> ---
>  drivers/infiniband/ulp/iser/iscsi_iser.h |  23 ++-----
>  drivers/infiniband/ulp/iser/iser_verbs.c | 112 +++++++------------------------
>  2 files changed, 29 insertions(+), 106 deletions(-)

No cover letter?

Applied to for-next with Bart's typo fixes

Jason