Re: [PATCH 20/22] IB/iser: Support up to 8MB data transfer in a single command
On Tue, Aug 4, 2015 at 8:10 PM, Sagi Grimberg wrote: > Why SIZE_4K and not PAGE_SIZE? >>> >>> >>> Yes, I'll change that to PAGE_SIZE. >>> >>> Thanks. >> >> >> Would non-4KB pages (e.g. PPC 64KB) be an issue? Would this work between >> hosts with different page sizes? > > > iser was always using 4K segments for reasons I don't perfectly > understand. Maybe Or can comment on this? I do plan to move it to work > with system page size (soon I hope). Yep, back when we integrated the code upstream, we were testing on IA64 too - where the system page size was 16KB but for some reasons SGs provided by the block layer were many times NON FMR aligned if you worked with 16KB FMR chunks. I would recommend to test on PPC64 and if possible on IA64 before/after removing the usage of 4k segments. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 20/22] IB/iser: Support up to 8MB data transfer in a single command
Why SIZE_4K and not PAGE_SIZE? Yes, I'll change that to PAGE_SIZE. Thanks. Would non-4KB pages (e.g. PPC 64KB) be an issue? Would this work between hosts with different page sizes? iser was always using 4K segments for reasons I don't perfectly understand. Maybe Or can comment on this? I do plan to move it to work with system page size (soon I hope). -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 20/22] IB/iser: Support up to 8MB data transfer in a single command
On Aug 2, 2015, at 4:01 AM, Sagi Grimberg wrote:
>>> +static void
>>> +iser_calc_scsi_params(struct iser_conn *iser_conn,
>>> + unsigned int max_sectors)
>>> +{
>>> +struct iser_device *device = iser_conn->ib_conn.device;
>>> +unsigned short sg_tablesize, sup_sg_tablesize;
>>> +
>>> +sg_tablesize = DIV_ROUND_UP(max_sectors * 512, SIZE_4K);
>>> +sup_sg_tablesize = min_t(unsigned, ISCSI_ISER_MAX_SG_TABLESIZE,
>>> + device->dev_attr.max_fast_reg_page_list_len);
>>> +
>>> +if (sg_tablesize > sup_sg_tablesize) {
>>> +sg_tablesize = sup_sg_tablesize;
>>> +iser_conn->scsi_max_sectors = sg_tablesize * SIZE_4K / 512;
>>> +} else {
>>> +iser_conn->scsi_max_sectors = max_sectors;
>>> +}
>>> +
>>
>> Why SIZE_4K and not PAGE_SIZE?
>
> Yes, I'll change that to PAGE_SIZE.
>
> Thanks.
Would non-4KB pages (e.g. PPC 64KB) be an issue? Would this work between hosts
with different page sizes?
Scott
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 20/22] IB/iser: Support up to 8MB data transfer in a single command
+static void
+iser_calc_scsi_params(struct iser_conn *iser_conn,
+ unsigned int max_sectors)
+{
+struct iser_device *device = iser_conn->ib_conn.device;
+unsigned short sg_tablesize, sup_sg_tablesize;
+
+sg_tablesize = DIV_ROUND_UP(max_sectors * 512, SIZE_4K);
+sup_sg_tablesize = min_t(unsigned, ISCSI_ISER_MAX_SG_TABLESIZE,
+ device->dev_attr.max_fast_reg_page_list_len);
+
+if (sg_tablesize > sup_sg_tablesize) {
+sg_tablesize = sup_sg_tablesize;
+iser_conn->scsi_max_sectors = sg_tablesize * SIZE_4K / 512;
+} else {
+iser_conn->scsi_max_sectors = max_sectors;
+}
+
Why SIZE_4K and not PAGE_SIZE?
Yes, I'll change that to PAGE_SIZE.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 20/22] IB/iser: Support up to 8MB data transfer in a single command
On 7/30/2015 3:06 AM, Sagi Grimberg wrote:
iser support up to 512KB data transfer in a single scsi
command. In order to support up to 8MB, iser needs to pre-allocate
larger memory regions and larger page vectors.
Given that a few target implementations don't support data transfers
of more than 512KB by default and the fact that larger IO sizes require
more resources, we introduce a module parameter to determine the
maximum number of 512B sectors in a single scsi command.
Users that are interested in larger transfers can change this value given
that the target supports larger transfers.
IO operations that consists of N pages will need a page vector
of size N+1 in case the first SG element contains an offset. Given
that some devices allocates memory regions in powers of 2, this
means that allocating a region with N+1 pages, will result in
region resources allocation of the next power of 2. Since we don't
want that to happen, in case we are in the limit of IO size supported
and the first SG element has an offset, we align the SG list using a
bounce buffer (which is OK given that this is not likely to happen a lot).
Signed-off-by: Sagi Grimberg
---
drivers/infiniband/ulp/iser/iscsi_iser.c | 19 ---
drivers/infiniband/ulp/iser/iscsi_iser.h | 14 --
drivers/infiniband/ulp/iser/iser_initiator.c | 2 +-
drivers/infiniband/ulp/iser/iser_memory.c| 14 --
drivers/infiniband/ulp/iser/iser_verbs.c | 27 +++
5 files changed, 60 insertions(+), 16 deletions(-)
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c
b/drivers/infiniband/ulp/iser/iscsi_iser.c
index e3cea61..9eeefc8 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -93,6 +93,10 @@ static unsigned int iscsi_max_lun = 512;
module_param_named(max_lun, iscsi_max_lun, uint, S_IRUGO);
MODULE_PARM_DESC(max_lun, "Max LUNs to allow per session (default:512");
+unsigned int iser_max_sectors = ISER_DEF_MAX_SECTORS;
+module_param_named(max_sectors, iser_max_sectors, uint, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(max_sectors, "Max number of sectors in a single scsi command
(default:1024");
+
bool iser_pi_enable = false;
module_param_named(pi_enable, iser_pi_enable, bool, S_IRUGO);
MODULE_PARM_DESC(pi_enable, "Enable T10-PI offload support
(default:disabled)");
@@ -625,6 +629,8 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
if (ep) {
iser_conn = ep->dd_data;
max_cmds = iser_conn->max_cmds;
+ shost->sg_tablesize = iser_conn->scsi_sg_tablesize;
+ shost->max_sectors = iser_conn->scsi_max_sectors;
mutex_lock(&iser_conn->state_mutex);
if (iser_conn->state != ISER_CONN_UP) {
@@ -643,15 +649,6 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
SHOST_DIX_GUARD_CRC);
}
- /*
-* Limit the sg_tablesize and max_sectors based on the device
-* max fastreg page list length.
-*/
- shost->sg_tablesize = min_t(unsigned short, shost->sg_tablesize,
- ib_conn->device->dev_attr.max_fast_reg_page_list_len);
- shost->max_sectors = min_t(unsigned int,
- 1024, (shost->sg_tablesize * PAGE_SIZE) >> 9);
-
if (iscsi_host_add(shost,
ib_conn->device->ib_device->dma_device)) {
mutex_unlock(&iser_conn->state_mutex);
@@ -966,8 +963,8 @@ static struct scsi_host_template iscsi_iser_sht = {
.name = "iSCSI Initiator over iSER",
.queuecommand = iscsi_queuecommand,
.change_queue_depth = scsi_change_queue_depth,
- .sg_tablesize = ISCSI_ISER_SG_TABLESIZE,
- .max_sectors= 1024,
+ .sg_tablesize = ISCSI_ISER_DEF_SG_TABLESIZE,
+ .max_sectors= ISER_DEF_MAX_SECTORS,
.cmd_per_lun= ISER_DEF_CMD_PER_LUN,
.eh_abort_handler = iscsi_eh_abort,
.eh_device_reset_handler= iscsi_eh_device_reset,
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h
b/drivers/infiniband/ulp/iser/iscsi_iser.h
index e9ebe0b..8a32e20 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -98,8 +98,13 @@
#define SHIFT_4K 12
#define SIZE_4K (1ULL << SHIFT_4K)
#define MASK_4K (~(SIZE_4K-1))
- /* support up to 512KB in one RDMA */
-#define ISCSI_ISER_SG_TABLESIZE (0x8 >> SHIFT_4K)
+
+/* Default support is 512KB I/O size */
+#define ISER_DEF_MAX_SECTORS 1024
+#define ISCSI_ISER_DEF_SG_TABLESIZE((ISER_DEF_MAX_SECTORS * 512) >>
SHIFT_4K)
+/* Maximum support is 8MB I/O size */
+#define ISCSI_ISER_MAX_SG_TABLESIZE(16384 * 512 >> SHIFT_
Re: [PATCH 20/22] IB/iser: Support up to 8MB data transfer in a single command
On 7/30/2015 1:22 PM, Or Gerlitz wrote: On Thu, Jul 30, 2015 at 11:06 AM, Sagi Grimberg wrote: iser support up to 512KB data transfer in a single scsi command. In order to support up to 8MB, iser needs to pre-allocate larger memory regions and larger page vectors. We should be doing things for a reason, and we are following that practice, it's missing. I believe we have nice motivation to put here for why we want to do that. Please add it. Sure. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 20/22] IB/iser: Support up to 8MB data transfer in a single command
On Thu, Jul 30, 2015 at 11:06 AM, Sagi Grimberg wrote: > iser support up to 512KB data transfer in a single scsi > command. In order to support up to 8MB, iser needs to pre-allocate > larger memory regions and larger page vectors. We should be doing things for a reason, and we are following that practice, it's missing. I believe we have nice motivation to put here for why we want to do that. Please add it. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
