[PATCH 1/1] IB/iser: Remove hard coded values for cqe and send_wr

[Jay Kallickal]

From: Jayamohan Kallickal <jayamohank-***@public.gmane.org>

This patch allows the underlying hardware to choose
values other than hard coded max values for cqe and send_wr
while preventing them from exceeding max supported values.

Signed-off-by: Minh Tran <minhduc.tran-laKkSmNT4hbQT0dZR+***@public.gmane.org>
Signed-off-by: Jayamohan Kallickal <jayamohan.kallickal-laKkSmNT4hbQT0dZR+***@public.gmane.org>
---
drivers/infiniband/ulp/iser/iser_verbs.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index 32849f2..7cdb297 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -73,7 +73,7 @@ static int iser_create_device_ib_res(struct iser_device *device)
{
struct iser_cq_desc *cq_desc;
struct ib_device_attr *dev_attr = &device->dev_attr;
- int ret, i, j;
+ int ret, i, j, max_cqe;

ret = ib_query_device(device->ib_device, dev_attr);
if (ret) {
@@ -120,18 +120,24 @@ static int iser_create_device_ib_res(struct iser_device *device)
cq_desc[i].device = device;
cq_desc[i].cq_index = i;

+ max_cqe = (dev_attr->max_cqe < ISER_MAX_RX_CQ_LEN) ?
+ dev_attr->max_cqe : ISER_MAX_RX_CQ_LEN;
+
device->rx_cq[i] = ib_create_cq(device->ib_device,
iser_cq_callback,
iser_cq_event_callback,
(void *)&cq_desc[i],
- ISER_MAX_RX_CQ_LEN, i);
+ max_cqe, i);
if (IS_ERR(device->rx_cq[i]))
goto cq_err;

+ max_cqe = (dev_attr->max_cqe < ISER_MAX_TX_CQ_LEN) ?
+ dev_attr->max_cqe : ISER_MAX_TX_CQ_LEN;
+
device->tx_cq[i] = ib_create_cq(device->ib_device,
NULL, iser_cq_event_callback,
(void *)&cq_desc[i],
- ISER_MAX_TX_CQ_LEN, i);
+ max_cqe, i);

if (IS_ERR(device->tx_cq[i]))
goto cq_err;
@@ -439,6 +445,7 @@ void iser_free_fastreg_pool(struct iser_conn *ib_conn)
static int iser_create_ib_conn_res(struct iser_conn *ib_conn)
{
struct iser_device *device;
+ struct ib_device_attr *dev_attr;
struct ib_qp_init_attr init_attr;
int ret = -ENOMEM;
int index, min_index = 0;
@@ -459,6 +466,7 @@ static int iser_create_ib_conn_res(struct iser_conn *ib_conn)
mutex_unlock(&ig.connlist_mutex);
iser_info("cq index %d used for ib_conn %p\n", min_index, ib_conn);

+ dev_attr = &device->dev_attr;
init_attr.event_handler = iser_qp_event_callback;
init_attr.qp_context = (void *)ib_conn;
init_attr.send_cq = device->tx_cq[min_index];
@@ -472,7 +480,9 @@ static int iser_create_ib_conn_res(struct iser_conn *ib_conn)
init_attr.cap.max_send_wr = ISER_QP_SIG_MAX_REQ_DTOS;
init_attr.create_flags |= IB_QP_CREATE_SIGNATURE_EN;
} else {
- init_attr.cap.max_send_wr = ISER_QP_MAX_REQ_DTOS;
+ init_attr.cap.max_send_wr =
+ (dev_attr->max_qp_wr < ISER_QP_MAX_REQ_DTOS) ?
+ dev_attr->max_qp_wr : ISER_QP_MAX_REQ_DTOS;
}

ret = rdma_create_qp(ib_conn->cma_id, device->pd, &init_attr);

[Sagi Grimberg]

Post by Jay Kallickal
This patch allows the underlying hardware to choose
values other than hard coded max values for cqe and send_wr
while preventing them from exceeding max supported values.

Hi Minh and Jayamohan,

So I agree that we would want to take device capabilities into account
here, but we need to be able to adjust scsi_cmds_max (can_queue) in case
the max wqe supported is lower than scsi_cmds_max * num_posts_per_cmd.

So generally I agree with this approach, but we need to take care of
stuff later when the session is created.

One more thing, this is not rebased on the latest iser patches please
send v1 on top of: http://marc.info/?l=linux-rdma&m=141216135013146&w=2

P.S.
What device did you test with (that supports less than iSER needs)?

Thanks!
Sagi.

Post by Jay Kallickal
---
drivers/infiniband/ulp/iser/iser_verbs.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Sagi Grimberg]

On 10/9/2014 8:14 AM, Jayamohan.K wrote:
<SNIP>

Post by Sagi Grimberg
Hi Minh and Jayamohan,
So I agree that we would want to take device capabilities into account
here, but we need to be able to adjust scsi_cmds_max (can_queue) in case
the max wqe supported is lower than scsi_cmds_max * num_posts_per_cmd.
I feel we should be fine as long as we support the max_cmds of 128
supported by open-iscsi layer.

The cmds_max can be modified by the user, so we should be fine in this
case as well.

Post by Sagi Grimberg
I see the iser layer uses a value of 512 though it only serves as a
limit check.

So if iser supports less than 512 (due to device capability) the
boundary check should be modified as well.

Post by Sagi Grimberg
So generally I agree with this approach, but we need to take care of
stuff later when the session is created.
One more thing, this is not rebased on the latest iser patches please
http://marc.info/?l=linux-__rdma&m=141216135013146&w=2
<http://marc.info/?l=linux-rdma&m=141216135013146&w=2>
Yes, I will recreate the patch and send on top of this

Great!

Post by Sagi Grimberg
P.S.
What device did you test with (that supports less than iSER needs)?

This question still holds.

Sagi.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Jayamohan Kallickal]

Post by Sagi Grimberg

Post by Jay Kallickal
This patch allows the underlying hardware to choose
values other than hard coded max values for cqe and send_wr
while preventing them from exceeding max supported values.

Hi Minh and Jayamohan,
So I agree that we would want to take device capabilities into account
here, but we need to be able to adjust scsi_cmds_max (can_queue) in case
the max wqe supported is lower than scsi_cmds_max * num_posts_per_cmd.

I feel we should be fine as long as we support the max_cmds of 128 supported by open-iscsi layer.

I see the iser layer uses a value of 512 though it only serves as a limit check.

Post by Sagi Grimberg
So generally I agree with this approach, but we need to take care of
stuff later when the session is created.
One more thing, this is not rebased on the latest iser patches please
send v1 on top of: http://marc.info/?l=linux-rdma&m=141216135013146&w=2

Yes, I will recreate the patch and send on top of this


Resending:
Looks like my response got dropped by the filters.

Am travelling . Minh is working on the patches and would respond
-Jay
--
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html