* [PATCH 2/3] IB/isert: use new shared CQ mechanism
2020-07-22 13:56 [PATCH 1/3] IB/iser: use new shared CQ mechanism Max Gurtovoy
@ 2020-07-22 13:56 ` Max Gurtovoy
2020-07-29 9:39 ` Sagi Grimberg
2020-07-22 13:56 ` [PATCH 3/3] IB/srpt: " Max Gurtovoy
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Max Gurtovoy @ 2020-07-22 13:56 UTC (permalink / raw)
To: sagi, yaminf, dledford, linux-rdma, leon, bvanassche
Cc: israelr, oren, jgg, idanb, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/3] IB/srpt: use new shared CQ mechanism
2020-07-22 13:56 [PATCH 1/3] IB/iser: use new shared CQ mechanism Max Gurtovoy
2020-07-22 13:56 ` [PATCH 2/3] IB/isert: " Max Gurtovoy
@ 2020-07-22 13:56 ` Max Gurtovoy
2020-07-22 15:02 ` Bart Van Assche
2020-07-29 9:40 ` Sagi Grimberg
2020-07-29 8:34 ` [PATCH 1/3] IB/iser: " Max Gurtovoy
` (2 subsequent siblings)
4 siblings, 2 replies; 10+ messages in thread
From: Max Gurtovoy @ 2020-07-22 13:56 UTC (permalink / raw)
To: sagi, yaminf, dledford, linux-rdma, leon, bvanassche
Cc: israelr, oren, jgg, idanb, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 3/3] IB/srpt: use new shared CQ mechanism
2020-07-22 13:56 ` [PATCH 3/3] IB/srpt: " Max Gurtovoy
@ 2020-07-22 15:02 ` Bart Van Assche
2020-07-29 9:40 ` Sagi Grimberg
1 sibling, 0 replies; 10+ messages in thread
From: Bart Van Assche @ 2020-07-22 15:02 UTC (permalink / raw)
To: Max Gurtovoy, sagi, yaminf, dledford, linux-rdma, leon
Cc: israelr, oren, jgg, idanb
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>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 3/3] IB/srpt: use new shared CQ mechanism
2020-07-22 13:56 ` [PATCH 3/3] IB/srpt: " Max Gurtovoy
2020-07-22 15:02 ` Bart Van Assche
@ 2020-07-29 9:40 ` Sagi Grimberg
1 sibling, 0 replies; 10+ messages in thread
From: Sagi Grimberg @ 2020-07-29 9:40 UTC (permalink / raw)
To: Max Gurtovoy, yaminf, dledford, linux-rdma, leon, bvanassche
Cc: israelr, oren, jgg, idanb
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] IB/iser: use new shared CQ mechanism
2020-07-22 13:56 [PATCH 1/3] IB/iser: use new shared CQ mechanism Max Gurtovoy
2020-07-22 13:56 ` [PATCH 2/3] IB/isert: " Max Gurtovoy
2020-07-22 13:56 ` [PATCH 3/3] IB/srpt: " Max Gurtovoy
@ 2020-07-29 8:34 ` Max Gurtovoy
2020-07-29 11:34 ` Leon Romanovsky
2020-07-29 9:39 ` Sagi Grimberg
2020-07-29 12:23 ` Jason Gunthorpe
4 siblings, 1 reply; 10+ messages in thread
From: Max Gurtovoy @ 2020-07-29 8:34 UTC (permalink / raw)
To: sagi, yaminf, dledford, linux-rdma, leon, bvanassche
Cc: israelr, oren, jgg, idanb
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;
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/3] IB/iser: use new shared CQ mechanism
2020-07-29 8:34 ` [PATCH 1/3] IB/iser: " Max Gurtovoy
@ 2020-07-29 11:34 ` Leon Romanovsky
0 siblings, 0 replies; 10+ messages in thread
From: Leon Romanovsky @ 2020-07-29 11:34 UTC (permalink / raw)
To: Max Gurtovoy
Cc: sagi, yaminf, dledford, linux-rdma, bvanassche, israelr, oren,
jgg, idanb
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] IB/iser: use new shared CQ mechanism
2020-07-22 13:56 [PATCH 1/3] IB/iser: use new shared CQ mechanism Max Gurtovoy
` (2 preceding siblings ...)
2020-07-29 8:34 ` [PATCH 1/3] IB/iser: " Max Gurtovoy
@ 2020-07-29 9:39 ` Sagi Grimberg
2020-07-29 12:23 ` Jason Gunthorpe
4 siblings, 0 replies; 10+ messages in thread
From: Sagi Grimberg @ 2020-07-29 9:39 UTC (permalink / raw)
To: Max Gurtovoy, yaminf, dledford, linux-rdma, leon, bvanassche
Cc: israelr, oren, jgg, idanb
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/3] IB/iser: use new shared CQ mechanism
2020-07-22 13:56 [PATCH 1/3] IB/iser: use new shared CQ mechanism Max Gurtovoy
` (3 preceding siblings ...)
2020-07-29 9:39 ` Sagi Grimberg
@ 2020-07-29 12:23 ` Jason Gunthorpe
4 siblings, 0 replies; 10+ messages in thread
From: Jason Gunthorpe @ 2020-07-29 12:23 UTC (permalink / raw)
To: Max Gurtovoy
Cc: sagi, yaminf, dledford, linux-rdma, leon, bvanassche, israelr,
oren, idanb
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
^ permalink raw reply [flat|nested] 10+ messages in thread