Re: [PATCHv1 1/1] IB/iser: Fix sg_tablesize calculation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 1/16/2017 11:03 PM, Sagi Grimberg wrote:

For devices that can register page list that is bigger than
USHRT_MAX, we actually take the wrong value for sg_tablesize.
E.g: for CX4 max_fast_reg_page_list_len is 65536 (bigger than USHRT_MAX)
so we set sg_tablesize to 0 by mistake. Therefore, each IO that is
bigger than 4k splitted to "< 4k" chunks that cause performance
degredation.

Fixes: 7854550ae6d8 ("RDMA/iser: Limit sgs to the device fastreg depth")
Signed-off-by: Max Gurtovoy <maxg@xxxxxxxxxxxx>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c
b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 9104e6b..ff37f19 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -656,7 +656,8 @@ static void iscsi_iser_cleanup_task(struct
iscsi_task *task)
          * max fastreg page list length.
          */

Hi Max,

         shost->sg_tablesize = min_t(unsigned short, shost->sg_tablesize,
-
ib_conn->device->ib_device->attrs.max_fast_reg_page_list_len);
+                      min_t(unsigned int, USHRT_MAX,
+
ib_conn->device->ib_device->attrs.max_fast_reg_page_list_len));

Hmm, This looks broken since I fixed up mlx5 capability at:
911f4331bc87 ("IB/mlx5: Expose correct max_fast_reg_page_list_len")

However, I suspect this assignment should be removed altogether because
we already take that into account in the address resolution handler,
there the min statement is for unsigned and casted back to ushrt.

Hi Sagi,
I can send v2 with assignment removal (there is some work to do in iser_calc_scsi_params too) but I think that your commit 911f4331bc87 broke it accidently and not logically. Think of other providers that also may support max_fast_reg_page_list_len > USHRT_MAX (there was no such providers till that point in time).

Altought it's not optimal but it's the best solution in competability means since there are 4 commits involved in that area (from first to last):
1. 7854550ae6 ("RDMA/iser: Limit sgs to the device fastreg depth")
2. df749cdc45 ("IB/iser: Support up to 8MB data transfer in a single command")
3. 911f4331bc ("IB/mlx5: Expose correct max_fast_reg_page_list_len")
4. 9c674815d3 ("IB/iser: Fix max_sectors calculation")

and the last one fixes all the loop.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux