[PATCH 00/22] iser patches for 4.3

[PATCH 00/22] iser patches for 4.3

[Sagi Grimberg]

This set is a resend that includes some extra patches that
piled up in the meantime.

I still have some patches in the pipe (including initiator/target
support for remote invalidate) but I'm targeting those to 4.4

This patch set includes:
- Small fixes for bugs encountered in testing
- Small fixes detected by static checkers
- Memory registration code path rework (consolidate to
  a single code path that branches only at the actual registration
  FRWR vs. FMR). This reduces code duplication that exists in current code.
- Larger IO transfer size support (up to 8MB at the moment) depending on
  the device capabilities.
- Optimize Io path by chaining send work requests and posting them
  only once.

Adir Lev (1):
  IB/iser: Maintain connection fmr_pool under a single registration
    descriptor

Jenny Falkovich (1):
  IB/iser: Change some module parameters to be RO

Sagi Grimberg (20):
  IB/iser: Change minor assignments and logging prints
  IB/iser: Remove '.' from log message
  IB/iser: Fix missing return status check in iser_send_data_out
  IB/iser: Get rid of un-maintained counters
  IB/iser: Fix possible bogus DMA unmapping
  IB/iser: Remove a redundant always-false condition
  IB/iser: Remove an unneeded print for unaligned memory
  IB/iser: Introduce struct iser_reg_resources
  IB/iser: Rename struct fast_reg_descriptor -> iser_fr_desc
  IB/iser: Remove dead code in fmr_pool alloc/free
  IB/iser: Introduce iser_reg_ops
  IB/iser: Move fastreg descriptor allocation to
    iser_create_fastreg_desc
  IB/iser: Introduce iser registration pool struct
  IB/iser: Rename iser_reg_page_vec to iser_fast_reg_fmr
  IB/iser: Make reg_desc_get a per device routine
  IB/iser: Unify fast memory registration flows
  IB/iser: Pass registration pool a size parameter
  IB/iser: Support up to 8MB data transfer in a single command
  IB/iser: Add debug prints to the various memory registration methods
  IB/iser: Chain all iser transaction send work requests

 drivers/infiniband/ulp/iser/iscsi_iser.c     |  89 +++--
 drivers/infiniband/ulp/iser/iscsi_iser.h     | 206 ++++++++----
 drivers/infiniband/ulp/iser/iser_initiator.c |  34 +-
 drivers/infiniband/ulp/iser/iser_memory.c    | 480 +++++++++++++++------------
 drivers/infiniband/ulp/iser/iser_verbs.c     | 328 ++++++++++--------
 5 files changed, 645 insertions(+), 492 deletions(-)

-- 
1.8.4.3

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

[PATCH 01/22] IB/iser: Change some module parameters to be RO

[Sagi Grimberg]

From: Jenny Falkovich <jennyf-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

While we're at it, use permission defines instead
of octal values and rearrange a little bit.

Signed-off-by: Jenny Derzhavetz <jennyf-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index c969fc1..c7cea25 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -74,34 +74,33 @@
 
 #include "iscsi_iser.h"
 
+MODULE_DESCRIPTION("iSER (iSCSI Extensions for RDMA) Datamover");
+MODULE_LICENSE("Dual BSD/GPL");
+MODULE_AUTHOR("Alex Nezhinsky, Dan Bar Dov, Or Gerlitz");
+MODULE_VERSION(DRV_VER);
+
 static struct scsi_host_template iscsi_iser_sht;
 static struct iscsi_transport iscsi_iser_transport;
 static struct scsi_transport_template *iscsi_iser_scsi_transport;
+static struct workqueue_struct *release_wq;
+struct iser_global ig;
+
+int iser_debug_level = 0;
+module_param_named(debug_level, iser_debug_level, int, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(debug_level, "Enable debug tracing if > 0 (default:disabled)");
 
 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");
 
-int iser_debug_level = 0;
 bool iser_pi_enable = false;
-int iser_pi_guard = 1;
-
-MODULE_DESCRIPTION("iSER (iSCSI Extensions for RDMA) Datamover");
-MODULE_LICENSE("Dual BSD/GPL");
-MODULE_AUTHOR("Alex Nezhinsky, Dan Bar Dov, Or Gerlitz");
-MODULE_VERSION(DRV_VER);
-
-module_param_named(debug_level, iser_debug_level, int, 0644);
-MODULE_PARM_DESC(debug_level, "Enable debug tracing if > 0 (default:disabled)");
-
-module_param_named(pi_enable, iser_pi_enable, bool, 0644);
+module_param_named(pi_enable, iser_pi_enable, bool, S_IRUGO);
 MODULE_PARM_DESC(pi_enable, "Enable T10-PI offload support (default:disabled)");
 
-module_param_named(pi_guard, iser_pi_guard, int, 0644);
+int iser_pi_guard;
+module_param_named(pi_guard, iser_pi_guard, int, S_IRUGO);
 MODULE_PARM_DESC(pi_guard, "T10-PI guard_type [deprecated]");
 
-static struct workqueue_struct *release_wq;
-struct iser_global ig;
-
 /*
  * iscsi_iser_recv() - Process a successfull recv completion
  * @conn:         iscsi connection
-- 
1.8.4.3

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

[PATCH 02/22] IB/iser: Change minor assignments and logging prints

[Sagi Grimberg]

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index c7cea25..7420062 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -847,10 +847,9 @@ failure:
 static int
 iscsi_iser_ep_poll(struct iscsi_endpoint *ep, int timeout_ms)
 {
-	struct iser_conn *iser_conn;
+	struct iser_conn *iser_conn = ep->dd_data;
 	int rc;
 
-	iser_conn = ep->dd_data;
 	rc = wait_for_completion_interruptible_timeout(&iser_conn->up_completion,
 						       msecs_to_jiffies(timeout_ms));
 	/* if conn establishment failed, return error code to iscsi */
@@ -862,7 +861,7 @@ iscsi_iser_ep_poll(struct iscsi_endpoint *ep, int timeout_ms)
 		mutex_unlock(&iser_conn->state_mutex);
 	}
 
-	iser_info("ib conn %p rc = %d\n", iser_conn, rc);
+	iser_info("iser conn %p rc = %d\n", iser_conn, rc);
 
 	if (rc > 0)
 		return 1; /* success, this is the equivalent of POLLOUT */
@@ -884,11 +883,9 @@ iscsi_iser_ep_poll(struct iscsi_endpoint *ep, int timeout_ms)
 static void
 iscsi_iser_ep_disconnect(struct iscsi_endpoint *ep)
 {
-	struct iser_conn *iser_conn;
+	struct iser_conn *iser_conn = ep->dd_data;
 
-	iser_conn = ep->dd_data;
-	iser_info("ep %p iser conn %p state %d\n",
-		  ep, iser_conn, iser_conn->state);
+	iser_info("ep %p iser conn %p\n", ep, iser_conn);
 
 	mutex_lock(&iser_conn->state_mutex);
 	iser_conn_terminate(iser_conn);
-- 
1.8.4.3

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

[PATCH 03/22] IB/iser: Remove '.' from log message

[Sagi Grimberg]

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 7420062..859d9d9 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -905,6 +905,7 @@ iscsi_iser_ep_disconnect(struct iscsi_endpoint *ep)
 		mutex_unlock(&iser_conn->state_mutex);
 		iser_conn_release(iser_conn);
 	}
+
 	iscsi_destroy_endpoint(ep);
 }
 
@@ -1079,7 +1080,7 @@ static void __exit iser_exit(void)
 
 	if (!connlist_empty) {
 		iser_err("Error cleanup stage completed but we still have iser "
-			 "connections, destroying them anyway.\n");
+			 "connections, destroying them anyway\n");
 		list_for_each_entry_safe(iser_conn, n, &ig.connlist,
 					 conn_list) {
 			iser_conn_release(iser_conn);
-- 
1.8.4.3

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

[PATCH 04/22] IB/iser: Fix missing return status check in iser_send_data_out

[Sagi Grimberg]

iser_initialize_task_headers() might fail.

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iser_initiator.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c
index 2d02f04..174799e 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -454,7 +454,7 @@ int iser_send_data_out(struct iscsi_conn *conn,
 	unsigned long buf_offset;
 	unsigned long data_seg_len;
 	uint32_t itt;
-	int err = 0;
+	int err;
 	struct ib_sge *tx_dsg;
 
 	itt = (__force uint32_t)hdr->itt;
@@ -475,7 +475,9 @@ int iser_send_data_out(struct iscsi_conn *conn,
 	memcpy(&tx_desc->iscsi_header, hdr, sizeof(struct iscsi_hdr));
 
 	/* build the tx desc */
-	iser_initialize_task_headers(task, tx_desc);
+	err = iser_initialize_task_headers(task, tx_desc);
+	if (err)
+		goto send_data_out_error;
 
 	mem_reg = &iser_task->rdma_reg[ISER_DIR_OUT];
 	tx_dsg = &tx_desc->tx_sg[1];
@@ -502,7 +504,7 @@ int iser_send_data_out(struct iscsi_conn *conn,
 
 send_data_out_error:
 	kmem_cache_free(ig.desc_cache, tx_desc);
-	iser_err("conn %p failed err %d\n",conn, err);
+	iser_err("conn %p failed err %d\n", conn, err);
 	return err;
 }
 
-- 
1.8.4.3

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

[PATCH 05/22] IB/iser: Get rid of un-maintained counters

[Sagi Grimberg]

We don't update those anywhere in the code and they
seem pretty useless (no one seem to care about those).

qp_tx_queue_full: We never should get this
fmr_map_not_avail: We can never get to this
eh_abort_cnt: We don't monitor aborts

Go ahead and remove them.

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 859d9d9..92b1020 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -750,15 +750,9 @@ iscsi_iser_conn_get_stats(struct iscsi_cls_conn *cls_conn, struct iscsi_stats *s
 	stats->r2t_pdus = conn->r2t_pdus_cnt; /* always 0 */
 	stats->tmfcmd_pdus = conn->tmfcmd_pdus_cnt;
 	stats->tmfrsp_pdus = conn->tmfrsp_pdus_cnt;
-	stats->custom_length = 4;
-	strcpy(stats->custom[0].desc, "qp_tx_queue_full");
-	stats->custom[0].value = 0; /* TB iser_conn->qp_tx_queue_full; */
-	strcpy(stats->custom[1].desc, "fmr_map_not_avail");
-	stats->custom[1].value = 0; /* TB iser_conn->fmr_map_not_avail */;
-	strcpy(stats->custom[2].desc, "eh_abort_cnt");
-	stats->custom[2].value = conn->eh_abort_cnt;
-	strcpy(stats->custom[3].desc, "fmr_unalign_cnt");
-	stats->custom[3].value = conn->fmr_unalign_cnt;
+	stats->custom_length = 1;
+	strcpy(stats->custom[0].desc, "fmr_unalign_cnt");
+	stats->custom[0].value = conn->fmr_unalign_cnt;
 }
 
 static int iscsi_iser_get_ep_param(struct iscsi_endpoint *ep,
-- 
1.8.4.3

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

[PATCH 06/22] IB/iser: Fix possible bogus DMA unmapping

[Sagi Grimberg]

If iser_initialize_task_headers() routine failed before
dma mapping, we should not attempt to unmap in cleanup_task().

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c | 12 ++++++++----
 drivers/infiniband/ulp/iser/iscsi_iser.h |  2 ++
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 92b1020..e3cea61 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -200,6 +200,7 @@ iser_initialize_task_headers(struct iscsi_task *task,
 		goto out;
 	}
 
+	tx_desc->mapped = true;
 	tx_desc->dma_addr = dma_addr;
 	tx_desc->tx_sg[0].addr   = tx_desc->dma_addr;
 	tx_desc->tx_sg[0].length = ISER_HEADERS_LEN;
@@ -359,16 +360,19 @@ iscsi_iser_task_xmit(struct iscsi_task *task)
 static void iscsi_iser_cleanup_task(struct iscsi_task *task)
 {
 	struct iscsi_iser_task *iser_task = task->dd_data;
-	struct iser_tx_desc    *tx_desc   = &iser_task->desc;
-	struct iser_conn       *iser_conn	  = task->conn->dd_data;
+	struct iser_tx_desc *tx_desc = &iser_task->desc;
+	struct iser_conn *iser_conn = task->conn->dd_data;
 	struct iser_device *device = iser_conn->ib_conn.device;
 
 	/* DEVICE_REMOVAL event might have already released the device */
 	if (!device)
 		return;
 
-	ib_dma_unmap_single(device->ib_device,
-		tx_desc->dma_addr, ISER_HEADERS_LEN, DMA_TO_DEVICE);
+	if (likely(tx_desc->mapped)) {
+		ib_dma_unmap_single(device->ib_device, tx_desc->dma_addr,
+				    ISER_HEADERS_LEN, DMA_TO_DEVICE);
+		tx_desc->mapped = false;
+	}
 
 	/* mgmt tasks do not need special cleanup */
 	if (!task->sc)
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 262ba1f..d2b6caf 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -270,6 +270,7 @@ enum iser_desc_type {
  *                 sg[1] optionally points to either of immediate data
  *                 unsolicited data-out or control
  * @num_sge:       number sges used on this TX task
+ * @mapped:        Is the task header mapped
  */
 struct iser_tx_desc {
 	struct iser_hdr              iser_header;
@@ -278,6 +279,7 @@ struct iser_tx_desc {
 	u64		             dma_addr;
 	struct ib_sge		     tx_sg[2];
 	int                          num_sge;
+	bool			     mapped;
 };
 
 #define ISER_RX_PAD_SIZE	(256 - (ISER_RX_PAYLOAD_SIZE + \
-- 
1.8.4.3

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

[PATCH 07/22] IB/iser: Remove a redundant always-false condition

[Sagi Grimberg]

We always call iser_initialize_task_headers() and set
the header tx_sg.lkey to the device mr lkey, so no
point in checking it in iser_create_send_desc().

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iser_initiator.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c
index 174799e..42d6f42 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -170,13 +170,7 @@ static void iser_create_send_desc(struct iser_conn	*iser_conn,
 
 	memset(&tx_desc->iser_header, 0, sizeof(struct iser_hdr));
 	tx_desc->iser_header.flags = ISER_VER;
-
 	tx_desc->num_sge = 1;
-
-	if (tx_desc->tx_sg[0].lkey != device->pd->local_dma_lkey) {
-		tx_desc->tx_sg[0].lkey = device->pd->local_dma_lkey;
-		iser_dbg("sdesc %p lkey mismatch, fixing\n", tx_desc);
-	}
 }
 
 static void iser_free_login_buf(struct iser_conn *iser_conn)
-- 
1.8.4.3

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

[PATCH 08/22] IB/iser: Remove an unneeded print for unaligned memory

[Sagi Grimberg]

We can do it in iser_aligned_data_len instead and
it will save us an argument that is passed to
fall_to_counce_buf just for the print.

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iser_memory.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index 3129a42..56dd53b 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -330,8 +330,11 @@ static int iser_data_buf_aligned_len(struct iser_data_buf *data,
 			break;
 	}
 	ret_len = (next_sg) ? i : i+1;
-	iser_dbg("Found %d aligned entries out of %d in sg:0x%p\n",
-		 ret_len, data->dma_nents, data);
+
+	if (unlikely(ret_len != data->dma_nents))
+		iser_warn("rdma alignment violation (%d/%d aligned)\n",
+			  ret_len, data->dma_nents);
+
 	return ret_len;
 }
 
@@ -407,15 +410,12 @@ iser_reg_dma(struct iser_device *device, struct iser_data_buf *mem,
 
 static int fall_to_bounce_buf(struct iscsi_iser_task *iser_task,
 			      struct iser_data_buf *mem,
-			      enum iser_data_dir cmd_dir,
-			      int aligned_len)
+			      enum iser_data_dir cmd_dir)
 {
 	struct iscsi_conn *iscsi_conn = iser_task->iser_conn->iscsi_conn;
 	struct iser_device *device = iser_task->iser_conn->ib_conn.device;
 
 	iscsi_conn->fmr_unalign_cnt++;
-	iser_warn("rdma alignment violation (%d/%d aligned) or FMR not supported\n",
-		  aligned_len, mem->size);
 
 	if (iser_debug_level > 0)
 		iser_data_buf_dump(mem, device->ib_device);
@@ -537,8 +537,7 @@ int iser_reg_rdma_mem_fmr(struct iscsi_iser_task *iser_task,
 
 	aligned_len = iser_data_buf_aligned_len(mem, ibdev);
 	if (aligned_len != mem->dma_nents) {
-		err = fall_to_bounce_buf(iser_task, mem,
-					 cmd_dir, aligned_len);
+		err = fall_to_bounce_buf(iser_task, mem, cmd_dir);
 		if (err) {
 			iser_err("failed to allocate bounce buffer\n");
 			return err;
@@ -800,8 +799,7 @@ int iser_reg_rdma_mem_fastreg(struct iscsi_iser_task *iser_task,
 
 	aligned_len = iser_data_buf_aligned_len(mem, ibdev);
 	if (aligned_len != mem->dma_nents) {
-		err = fall_to_bounce_buf(iser_task, mem,
-					 cmd_dir, aligned_len);
+		err = fall_to_bounce_buf(iser_task, mem, cmd_dir);
 		if (err) {
 			iser_err("failed to allocate bounce buffer\n");
 			return err;
@@ -828,7 +826,7 @@ int iser_reg_rdma_mem_fastreg(struct iscsi_iser_task *iser_task,
 			aligned_len = iser_data_buf_aligned_len(mem, ibdev);
 			if (aligned_len != mem->dma_nents) {
 				err = fall_to_bounce_buf(iser_task, mem,
-							 cmd_dir, aligned_len);
+							 cmd_dir);
 				if (err) {
 					iser_err("failed to allocate bounce buffer\n");
 					return err;
-- 
1.8.4.3

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

[PATCH 09/22] IB/iser: Introduce struct iser_reg_resources

[Sagi Grimberg]

Have fast_reg_descriptor hold struct iser_reg_resources
(mr, frpl, valid flag). This will be useful when the
actual buffer registration routines will be passed with
the needed registration resources (i.e. iser_reg_resources)
without being aware of their nature (i.e. data or protection).

In order to achieve this, we remove reg_indicators flags container
and place specific flags (mr_valid) within iser_reg_resources struct.
We also place the sig_mr_valid and sig_protcted flags in iser_pi_context.

This patch also modifies iser_fast_reg_mr to receive the
reg_resources instead of the fast_reg_descriptor and a data/protection
indicator.

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Adir Lev <adirl-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iscsi_iser.h  |  36 ++++++-----
 drivers/infiniband/ulp/iser/iser_memory.c |  35 +++++------
 drivers/infiniband/ulp/iser/iser_verbs.c  | 101 +++++++++++++++++-------------
 3 files changed, 91 insertions(+), 81 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index d2b6caf..9cdfdbd 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -367,41 +367,45 @@ struct iser_device {
 #define ISER_CHECK_REFTAG	0x0f
 #define ISER_CHECK_APPTAG	0x30
 
-enum iser_reg_indicator {
-	ISER_DATA_KEY_VALID	= 1 << 0,
-	ISER_PROT_KEY_VALID	= 1 << 1,
-	ISER_SIG_KEY_VALID	= 1 << 2,
-	ISER_FASTREG_PROTECTED	= 1 << 3,
+/**
+ * struct iser_reg_resources - Fast registration recources
+ *
+ * @mr:         memory region
+ * @frpl:       fast reg page list
+ * @mr_valid:   is mr valid indicator
+ */
+struct iser_reg_resources {
+	struct ib_mr			 *mr;
+	struct ib_fast_reg_page_list     *frpl;
+	u8				  mr_valid:1;
 };
 
 /**
  * struct iser_pi_context - Protection information context
  *
- * @prot_mr:        protection memory region
- * @prot_frpl:      protection fastreg page list
- * @sig_mr:         signature feature enabled memory region
+ * @rsc:             protection buffer registration resources
+ * @sig_mr:          signature enable memory region
+ * @sig_mr_valid:    is sig_mr valid indicator
+ * @sig_protected:   is region protected indicator
  */
 struct iser_pi_context {
-	struct ib_mr                   *prot_mr;
-	struct ib_fast_reg_page_list   *prot_frpl;
+	struct iser_reg_resources	rsc;
 	struct ib_mr                   *sig_mr;
+	u8                              sig_mr_valid:1;
+	u8                              sig_protected:1;
 };
 
 /**
  * struct fast_reg_descriptor - Fast registration descriptor
  *
  * @list:           entry in connection fastreg pool
- * @data_mr:        data memory region
- * @data_frpl:      data fastreg page list
+ * @rsc:            data buffer registration resources
  * @pi_ctx:         protection information context
- * @reg_indicators: fast registration indicators
  */
 struct fast_reg_descriptor {
 	struct list_head		  list;
-	struct ib_mr			 *data_mr;
-	struct ib_fast_reg_page_list     *data_frpl;
+	struct iser_reg_resources	  rsc;
 	struct iser_pi_context		 *pi_ctx;
-	u8				  reg_indicators;
 };
 
 /**
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index 56dd53b..e6516bc 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -647,13 +647,12 @@ iser_inv_rkey(struct ib_send_wr *inv_wr, struct ib_mr *mr)
 
 static int
 iser_reg_sig_mr(struct iscsi_iser_task *iser_task,
-		struct fast_reg_descriptor *desc,
+		struct iser_pi_context *pi_ctx,
 		struct iser_mem_reg *data_reg,
 		struct iser_mem_reg *prot_reg,
 		struct iser_mem_reg *sig_reg)
 {
 	struct ib_conn *ib_conn = &iser_task->iser_conn->ib_conn;
-	struct iser_pi_context *pi_ctx = desc->pi_ctx;
 	struct ib_send_wr sig_wr, inv_wr;
 	struct ib_send_wr *bad_wr, *wr = NULL;
 	struct ib_sig_attrs sig_attrs;
@@ -666,7 +665,7 @@ iser_reg_sig_mr(struct iscsi_iser_task *iser_task,
 
 	iser_set_prot_checks(iser_task->sc, &sig_attrs.check_mask);
 
-	if (!(desc->reg_indicators & ISER_SIG_KEY_VALID)) {
+	if (!pi_ctx->sig_mr_valid) {
 		iser_inv_rkey(&inv_wr, pi_ctx->sig_mr);
 		wr = &inv_wr;
 	}
@@ -694,7 +693,7 @@ iser_reg_sig_mr(struct iscsi_iser_task *iser_task,
 		iser_err("reg_sig_mr failed, ret:%d\n", ret);
 		goto err;
 	}
-	desc->reg_indicators &= ~ISER_SIG_KEY_VALID;
+	pi_ctx->sig_mr_valid = 0;
 
 	sig_reg->sge.lkey = pi_ctx->sig_mr->lkey;
 	sig_reg->rkey = pi_ctx->sig_mr->rkey;
@@ -710,8 +709,7 @@ err:
 
 static int iser_fast_reg_mr(struct iscsi_iser_task *iser_task,
 			    struct iser_data_buf *mem,
-			    struct fast_reg_descriptor *desc,
-			    enum iser_reg_indicator ind,
+			    struct iser_reg_resources *rsc,
 			    struct iser_mem_reg *reg)
 {
 	struct ib_conn *ib_conn = &iser_task->iser_conn->ib_conn;
@@ -726,13 +724,8 @@ static int iser_fast_reg_mr(struct iscsi_iser_task *iser_task,
 	if (mem->dma_nents == 1)
 		return iser_reg_dma(device, mem, reg);
 
-	if (ind == ISER_DATA_KEY_VALID) {
-		mr = desc->data_mr;
-		frpl = desc->data_frpl;
-	} else {
-		mr = desc->pi_ctx->prot_mr;
-		frpl = desc->pi_ctx->prot_frpl;
-	}
+	mr = rsc->mr;
+	frpl = rsc->frpl;
 
 	plen = iser_sg_to_page_vec(mem, device->ib_device, frpl->page_list,
 				   &offset, &size);
@@ -741,7 +734,7 @@ static int iser_fast_reg_mr(struct iscsi_iser_task *iser_task,
 		return -EINVAL;
 	}
 
-	if (!(desc->reg_indicators & ind)) {
+	if (!rsc->mr_valid) {
 		iser_inv_rkey(&inv_wr, mr);
 		wr = &inv_wr;
 	}
@@ -770,7 +763,7 @@ static int iser_fast_reg_mr(struct iscsi_iser_task *iser_task,
 		iser_err("fast registration failed, ret:%d\n", ret);
 		return ret;
 	}
-	desc->reg_indicators &= ~ind;
+	rsc->mr_valid = 0;
 
 	reg->sge.lkey = mr->lkey;
 	reg->rkey = mr->rkey;
@@ -812,8 +805,8 @@ int iser_reg_rdma_mem_fastreg(struct iscsi_iser_task *iser_task,
 		mem_reg->mem_h = desc;
 	}
 
-	err = iser_fast_reg_mr(iser_task, mem, desc,
-			       ISER_DATA_KEY_VALID, mem_reg);
+	err = iser_fast_reg_mr(iser_task, mem,
+			       desc ? &desc->rsc : NULL, mem_reg);
 	if (err)
 		goto err_reg;
 
@@ -833,19 +826,19 @@ int iser_reg_rdma_mem_fastreg(struct iscsi_iser_task *iser_task,
 				}
 			}
 
-			err = iser_fast_reg_mr(iser_task, mem, desc,
-					       ISER_PROT_KEY_VALID, &prot_reg);
+			err = iser_fast_reg_mr(iser_task, mem,
+					       &desc->pi_ctx->rsc, &prot_reg);
 			if (err)
 				goto err_reg;
 		}
 
-		err = iser_reg_sig_mr(iser_task, desc, mem_reg,
+		err = iser_reg_sig_mr(iser_task, desc->pi_ctx, mem_reg,
 				      &prot_reg, mem_reg);
 		if (err) {
 			iser_err("Failed to register signature mr\n");
 			return err;
 		}
-		desc->reg_indicators |= ISER_FASTREG_PROTECTED;
+		desc->pi_ctx->sig_protected = 1;
 	}
 
 	return 0;
diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index 0a7ceb9..46b8875 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -280,6 +280,45 @@ void iser_free_fmr_pool(struct ib_conn *ib_conn)
 }
 
 static int
+iser_alloc_reg_res(struct ib_device *ib_device, struct ib_pd *pd,
+		   struct iser_reg_resources *res)
+{
+	int ret;
+
+	res->frpl = ib_alloc_fast_reg_page_list(ib_device,
+						ISCSI_ISER_SG_TABLESIZE + 1);
+	if (IS_ERR(res->frpl)) {
+		ret = PTR_ERR(res->frpl);
+		iser_err("Failed to allocate ib_fast_reg_page_list err=%d\n",
+			 ret);
+		return PTR_ERR(res->frpl);
+	}
+
+	res->mr = ib_alloc_mr(pd, IB_MR_TYPE_MEM_REG,
+			      ISCSI_ISER_SG_TABLESIZE + 1);
+	if (IS_ERR(res->mr)) {
+		ret = PTR_ERR(res->mr);
+		iser_err("Failed to allocate ib_fast_reg_mr err=%d\n", ret);
+		goto fast_reg_mr_failure;
+	}
+	res->mr_valid = 1;
+
+	return 0;
+
+fast_reg_mr_failure:
+	ib_free_fast_reg_page_list(res->frpl);
+
+	return ret;
+}
+
+static void
+iser_free_reg_res(struct iser_reg_resources *rsc)
+{
+	ib_dereg_mr(rsc->mr);
+	ib_free_fast_reg_page_list(rsc->frpl);
+}
+
+static int
 iser_alloc_pi_ctx(struct ib_device *ib_device, struct ib_pd *pd,
 		  struct fast_reg_descriptor *desc)
 {
@@ -292,36 +331,25 @@ iser_alloc_pi_ctx(struct ib_device *ib_device, struct ib_pd *pd,
 
 	pi_ctx = desc->pi_ctx;
 
-	pi_ctx->prot_frpl = ib_alloc_fast_reg_page_list(ib_device,
-					    ISCSI_ISER_SG_TABLESIZE);
-	if (IS_ERR(pi_ctx->prot_frpl)) {
-		ret = PTR_ERR(pi_ctx->prot_frpl);
-		goto prot_frpl_failure;
-	}
-
-	pi_ctx->prot_mr = ib_alloc_mr(pd, IB_MR_TYPE_MEM_REG,
-				      ISCSI_ISER_SG_TABLESIZE + 1);
-	if (IS_ERR(pi_ctx->prot_mr)) {
-		ret = PTR_ERR(pi_ctx->prot_mr);
-		goto prot_mr_failure;
+	ret = iser_alloc_reg_res(ib_device, pd, &pi_ctx->rsc);
+	if (ret) {
+		iser_err("failed to allocate reg_resources\n");
+		goto alloc_reg_res_err;
 	}
-	desc->reg_indicators |= ISER_PROT_KEY_VALID;
 
 	pi_ctx->sig_mr = ib_alloc_mr(pd, IB_MR_TYPE_SIGNATURE, 2);
 	if (IS_ERR(pi_ctx->sig_mr)) {
 		ret = PTR_ERR(pi_ctx->sig_mr);
 		goto sig_mr_failure;
 	}
-	desc->reg_indicators |= ISER_SIG_KEY_VALID;
-	desc->reg_indicators &= ~ISER_FASTREG_PROTECTED;
+	pi_ctx->sig_mr_valid = 1;
+	desc->pi_ctx->sig_protected = 0;
 
 	return 0;
 
 sig_mr_failure:
-	ib_dereg_mr(desc->pi_ctx->prot_mr);
-prot_mr_failure:
-	ib_free_fast_reg_page_list(desc->pi_ctx->prot_frpl);
-prot_frpl_failure:
+	iser_free_reg_res(&pi_ctx->rsc);
+alloc_reg_res_err:
 	kfree(desc->pi_ctx);
 
 	return ret;
@@ -330,8 +358,7 @@ prot_frpl_failure:
 static void
 iser_free_pi_ctx(struct iser_pi_context *pi_ctx)
 {
-	ib_free_fast_reg_page_list(pi_ctx->prot_frpl);
-	ib_dereg_mr(pi_ctx->prot_mr);
+	iser_free_reg_res(&pi_ctx->rsc);
 	ib_dereg_mr(pi_ctx->sig_mr);
 	kfree(pi_ctx);
 }
@@ -342,23 +369,11 @@ iser_create_fastreg_desc(struct ib_device *ib_device, struct ib_pd *pd,
 {
 	int ret;
 
-	desc->data_frpl = ib_alloc_fast_reg_page_list(ib_device,
-						      ISCSI_ISER_SG_TABLESIZE + 1);
-	if (IS_ERR(desc->data_frpl)) {
-		ret = PTR_ERR(desc->data_frpl);
-		iser_err("Failed to allocate ib_fast_reg_page_list err=%d\n",
-			 ret);
-		return PTR_ERR(desc->data_frpl);
-	}
-
-	desc->data_mr = ib_alloc_mr(pd, IB_MR_TYPE_MEM_REG,
-				    ISCSI_ISER_SG_TABLESIZE + 1);
-	if (IS_ERR(desc->data_mr)) {
-		ret = PTR_ERR(desc->data_mr);
-		iser_err("Failed to allocate ib_fast_reg_mr err=%d\n", ret);
-		goto fast_reg_mr_failure;
+	ret = iser_alloc_reg_res(ib_device, pd, &desc->rsc);
+	if (ret) {
+		iser_err("failed to allocate reg_resources\n");
+		return ret;
 	}
-	desc->reg_indicators |= ISER_DATA_KEY_VALID;
 
 	if (pi_enable) {
 		ret = iser_alloc_pi_ctx(ib_device, pd, desc);
@@ -367,10 +382,9 @@ iser_create_fastreg_desc(struct ib_device *ib_device, struct ib_pd *pd,
 	}
 
 	return 0;
+
 pi_ctx_alloc_failure:
-	ib_dereg_mr(desc->data_mr);
-fast_reg_mr_failure:
-	ib_free_fast_reg_page_list(desc->data_frpl);
+	iser_free_reg_res(&desc->rsc);
 
 	return ret;
 }
@@ -431,8 +445,7 @@ void iser_free_fastreg_pool(struct ib_conn *ib_conn)
 
 	list_for_each_entry_safe(desc, tmp, &ib_conn->fastreg.pool, list) {
 		list_del(&desc->list);
-		ib_free_fast_reg_page_list(desc->data_frpl);
-		ib_dereg_mr(desc->data_mr);
+		iser_free_reg_res(&desc->rsc);
 		if (desc->pi_ctx)
 			iser_free_pi_ctx(desc->pi_ctx);
 		kfree(desc);
@@ -1244,8 +1257,8 @@ u8 iser_check_task_pi_status(struct iscsi_iser_task *iser_task,
 	struct ib_mr_status mr_status;
 	int ret;
 
-	if (desc && desc->reg_indicators & ISER_FASTREG_PROTECTED) {
-		desc->reg_indicators &= ~ISER_FASTREG_PROTECTED;
+	if (desc && desc->pi_ctx->sig_protected) {
+		desc->pi_ctx->sig_protected = 0;
 		ret = ib_check_mr_status(desc->pi_ctx->sig_mr,
 					 IB_MR_CHECK_SIG_STATUS, &mr_status);
 		if (ret) {
-- 
1.8.4.3

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

[PATCH 10/22] IB/iser: Rename struct fast_reg_descriptor -> iser_fr_desc

[Sagi Grimberg]

Avoid struct names without iser_ prefix.

This patch does not change any functionality.

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iscsi_iser.h  |  8 ++++----
 drivers/infiniband/ulp/iser/iser_memory.c | 10 +++++-----
 drivers/infiniband/ulp/iser/iser_verbs.c  | 10 +++++-----
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 9cdfdbd..70bf6e7 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -396,13 +396,13 @@ struct iser_pi_context {
 };
 
 /**
- * struct fast_reg_descriptor - Fast registration descriptor
+ * struct iser_fr_desc - Fast registration descriptor
  *
  * @list:           entry in connection fastreg pool
  * @rsc:            data buffer registration resources
  * @pi_ctx:         protection information context
  */
-struct fast_reg_descriptor {
+struct iser_fr_desc {
 	struct list_head		  list;
 	struct iser_reg_resources	  rsc;
 	struct iser_pi_context		 *pi_ctx;
@@ -642,9 +642,9 @@ int iser_create_fastreg_pool(struct ib_conn *ib_conn, unsigned cmds_max);
 void iser_free_fastreg_pool(struct ib_conn *ib_conn);
 u8 iser_check_task_pi_status(struct iscsi_iser_task *iser_task,
 			     enum iser_data_dir cmd_dir, sector_t *sector);
-struct fast_reg_descriptor *
+struct iser_fr_desc *
 iser_reg_desc_get(struct ib_conn *ib_conn);
 void
 iser_reg_desc_put(struct ib_conn *ib_conn,
-		  struct fast_reg_descriptor *desc);
+		  struct iser_fr_desc *desc);
 #endif
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index e6516bc..4209d73 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -146,15 +146,15 @@ iser_copy_to_bounce(struct iser_data_buf *data)
 	iser_copy_bounce(data, true);
 }
 
-struct fast_reg_descriptor *
+struct iser_fr_desc *
 iser_reg_desc_get(struct ib_conn *ib_conn)
 {
-	struct fast_reg_descriptor *desc;
+	struct iser_fr_desc *desc;
 	unsigned long flags;
 
 	spin_lock_irqsave(&ib_conn->lock, flags);
 	desc = list_first_entry(&ib_conn->fastreg.pool,
-				struct fast_reg_descriptor, list);
+				struct iser_fr_desc, list);
 	list_del(&desc->list);
 	spin_unlock_irqrestore(&ib_conn->lock, flags);
 
@@ -163,7 +163,7 @@ iser_reg_desc_get(struct ib_conn *ib_conn)
 
 void
 iser_reg_desc_put(struct ib_conn *ib_conn,
-		  struct fast_reg_descriptor *desc)
+		  struct iser_fr_desc *desc)
 {
 	unsigned long flags;
 
@@ -787,7 +787,7 @@ int iser_reg_rdma_mem_fastreg(struct iscsi_iser_task *iser_task,
 	struct ib_device *ibdev = device->ib_device;
 	struct iser_data_buf *mem = &iser_task->data[cmd_dir];
 	struct iser_mem_reg *mem_reg = &iser_task->rdma_reg[cmd_dir];
-	struct fast_reg_descriptor *desc = NULL;
+	struct iser_fr_desc *desc = NULL;
 	int err, aligned_len;
 
 	aligned_len = iser_data_buf_aligned_len(mem, ibdev);
diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index 46b8875..f7828e3 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -320,7 +320,7 @@ iser_free_reg_res(struct iser_reg_resources *rsc)
 
 static int
 iser_alloc_pi_ctx(struct ib_device *ib_device, struct ib_pd *pd,
-		  struct fast_reg_descriptor *desc)
+		  struct iser_fr_desc *desc)
 {
 	struct iser_pi_context *pi_ctx = NULL;
 	int ret;
@@ -365,7 +365,7 @@ iser_free_pi_ctx(struct iser_pi_context *pi_ctx)
 
 static int
 iser_create_fastreg_desc(struct ib_device *ib_device, struct ib_pd *pd,
-			 bool pi_enable, struct fast_reg_descriptor *desc)
+			 bool pi_enable, struct iser_fr_desc *desc)
 {
 	int ret;
 
@@ -397,7 +397,7 @@ pi_ctx_alloc_failure:
 int iser_create_fastreg_pool(struct ib_conn *ib_conn, unsigned cmds_max)
 {
 	struct iser_device *device = ib_conn->device;
-	struct fast_reg_descriptor *desc;
+	struct iser_fr_desc *desc;
 	int i, ret;
 
 	INIT_LIST_HEAD(&ib_conn->fastreg.pool);
@@ -435,7 +435,7 @@ err:
  */
 void iser_free_fastreg_pool(struct ib_conn *ib_conn)
 {
-	struct fast_reg_descriptor *desc, *tmp;
+	struct iser_fr_desc *desc, *tmp;
 	int i = 0;
 
 	if (list_empty(&ib_conn->fastreg.pool))
@@ -1252,7 +1252,7 @@ u8 iser_check_task_pi_status(struct iscsi_iser_task *iser_task,
 			     enum iser_data_dir cmd_dir, sector_t *sector)
 {
 	struct iser_mem_reg *reg = &iser_task->rdma_reg[cmd_dir];
-	struct fast_reg_descriptor *desc = reg->mem_h;
+	struct iser_fr_desc *desc = reg->mem_h;
 	unsigned long sector_size = iser_task->sc->device->sector_size;
 	struct ib_mr_status mr_status;
 	int ret;
-- 
1.8.4.3

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

[PATCH 11/22] IB/iser: Remove dead code in fmr_pool alloc/free

[Sagi Grimberg]

In the past the we always tried to allocate an fmr_pool
and if it failed on ENOSYS (not supported) then we continued
with dma mr. This is not the case anymore and if we tried to
allocate an fmr_pool then it is supported and we expect to succeed.

Also, the check if fmr_pool is allocated when free is called is
redundant as well as we are guaranteed it exists.

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iser_verbs.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index f7828e3..2a0cb42 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -244,22 +244,18 @@ int iser_create_fmr_pool(struct ib_conn *ib_conn, unsigned cmds_max)
 				    IB_ACCESS_REMOTE_READ);
 
 	ib_conn->fmr.pool = ib_create_fmr_pool(device->pd, &params);
-	if (!IS_ERR(ib_conn->fmr.pool))
-		return 0;
+	if (IS_ERR(ib_conn->fmr.pool)) {
+		ret = PTR_ERR(ib_conn->fmr.pool);
+		iser_err("FMR allocation failed, err %d\n", ret);
+		goto err;
+	}
+
+	return 0;
 
-	/* no FMR => no need for page_vec */
+err:
 	kfree(ib_conn->fmr.page_vec);
 	ib_conn->fmr.page_vec = NULL;
-
-	ret = PTR_ERR(ib_conn->fmr.pool);
-	ib_conn->fmr.pool = NULL;
-	if (ret != -ENOSYS) {
-		iser_err("FMR allocation failed, err %d\n", ret);
-		return ret;
-	} else {
-		iser_warn("FMRs are not supported, using unaligned mode\n");
-		return 0;
-	}
+	return ret;
 }
 
 /**
@@ -270,9 +266,7 @@ void iser_free_fmr_pool(struct ib_conn *ib_conn)
 	iser_info("freeing conn %p fmr pool %p\n",
 		  ib_conn, ib_conn->fmr.pool);
 
-	if (ib_conn->fmr.pool != NULL)
-		ib_destroy_fmr_pool(ib_conn->fmr.pool);
-
+	ib_destroy_fmr_pool(ib_conn->fmr.pool);
 	ib_conn->fmr.pool = NULL;
 
 	kfree(ib_conn->fmr.page_vec);
-- 
1.8.4.3

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

[PATCH 12/22] IB/iser: Introduce iser_reg_ops

[Sagi Grimberg]

Move all the per-device function pointers to an easy
extensible iser_reg_ops structure that contains all
the iser registration operations.

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iscsi_iser.h     | 39 ++++++++++++++++++----------
 drivers/infiniband/ulp/iser/iser_initiator.c | 16 ++++++------
 drivers/infiniband/ulp/iser/iser_memory.c    | 35 +++++++++++++++++++++++++
 drivers/infiniband/ulp/iser/iser_verbs.c     | 30 +++++----------------
 4 files changed, 75 insertions(+), 45 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 70bf6e7..9ce090c 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -326,6 +326,25 @@ struct iser_comp {
 };
 
 /**
+ * struct iser_device - Memory registration operations
+ *     per-device registration schemes
+ *
+ * @alloc_reg_res:     Allocate registration resources
+ * @free_reg_res:      Free registration resources
+ * @reg_rdma_mem:      Register memory buffers
+ * @unreg_rdma_mem:    Un-register memory buffers
+ */
+struct iser_reg_ops {
+	int            (*alloc_reg_res)(struct ib_conn *ib_conn,
+					unsigned cmds_max);
+	void           (*free_reg_res)(struct ib_conn *ib_conn);
+	int            (*reg_rdma_mem)(struct iscsi_iser_task *iser_task,
+				       enum iser_data_dir cmd_dir);
+	void           (*unreg_rdma_mem)(struct iscsi_iser_task *iser_task,
+					 enum iser_data_dir cmd_dir);
+};
+
+/**
  * struct iser_device - iSER device handle
  *
  * @ib_device:     RDMA device
@@ -338,11 +357,7 @@ struct iser_comp {
  * @comps_used:    Number of completion contexts used, Min between online
  *                 cpus and device max completion vectors
  * @comps:         Dinamically allocated array of completion handlers
- * Memory registration pool Function pointers (FMR or Fastreg):
- *     @iser_alloc_rdma_reg_res: Allocation of memory regions pool
- *     @iser_free_rdma_reg_res:  Free of memory regions pool
- *     @iser_reg_rdma_mem:       Memory registration routine
- *     @iser_unreg_rdma_mem:     Memory deregistration routine
+ * @reg_ops:       Registration ops
  */
 struct iser_device {
 	struct ib_device             *ib_device;
@@ -354,13 +369,7 @@ struct iser_device {
 	int                          refcount;
 	int			     comps_used;
 	struct iser_comp	     *comps;
-	int                          (*iser_alloc_rdma_reg_res)(struct ib_conn *ib_conn,
-								unsigned cmds_max);
-	void                         (*iser_free_rdma_reg_res)(struct ib_conn *ib_conn);
-	int                          (*iser_reg_rdma_mem)(struct iscsi_iser_task *iser_task,
-							  enum iser_data_dir cmd_dir);
-	void                         (*iser_unreg_rdma_mem)(struct iscsi_iser_task *iser_task,
-							    enum iser_data_dir cmd_dir);
+	struct iser_reg_ops          *reg_ops;
 };
 
 #define ISER_CHECK_GUARD	0xc0
@@ -563,6 +572,8 @@ extern int iser_debug_level;
 extern bool iser_pi_enable;
 extern int iser_pi_guard;
 
+int iser_assign_reg_ops(struct iser_device *device);
+
 int iser_send_control(struct iscsi_conn *conn,
 		      struct iscsi_task *task);
 
@@ -636,9 +647,9 @@ int  iser_initialize_task_headers(struct iscsi_task *task,
 			struct iser_tx_desc *tx_desc);
 int iser_alloc_rx_descriptors(struct iser_conn *iser_conn,
 			      struct iscsi_session *session);
-int iser_create_fmr_pool(struct ib_conn *ib_conn, unsigned cmds_max);
+int iser_alloc_fmr_pool(struct ib_conn *ib_conn, unsigned cmds_max);
 void iser_free_fmr_pool(struct ib_conn *ib_conn);
-int iser_create_fastreg_pool(struct ib_conn *ib_conn, unsigned cmds_max);
+int iser_alloc_fastreg_pool(struct ib_conn *ib_conn, unsigned cmds_max);
 void iser_free_fastreg_pool(struct ib_conn *ib_conn);
 u8 iser_check_task_pi_status(struct iscsi_iser_task *iser_task,
 			     enum iser_data_dir cmd_dir, sector_t *sector);
diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c
index 42d6f42..88d8a89 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -73,7 +73,7 @@ static int iser_prepare_read_cmd(struct iscsi_task *task)
 			return err;
 	}
 
-	err = device->iser_reg_rdma_mem(iser_task, ISER_DIR_IN);
+	err = device->reg_ops->reg_rdma_mem(iser_task, ISER_DIR_IN);
 	if (err) {
 		iser_err("Failed to set up Data-IN RDMA\n");
 		return err;
@@ -128,7 +128,7 @@ iser_prepare_write_cmd(struct iscsi_task *task,
 			return err;
 	}
 
-	err = device->iser_reg_rdma_mem(iser_task, ISER_DIR_OUT);
+	err = device->reg_ops->reg_rdma_mem(iser_task, ISER_DIR_OUT);
 	if (err != 0) {
 		iser_err("Failed to register write cmd RDMA mem\n");
 		return err;
@@ -260,7 +260,7 @@ int iser_alloc_rx_descriptors(struct iser_conn *iser_conn,
 	iser_conn->qp_max_recv_dtos_mask = session->cmds_max - 1; /* cmds_max is 2^N */
 	iser_conn->min_posted_rx = iser_conn->qp_max_recv_dtos >> 2;
 
-	if (device->iser_alloc_rdma_reg_res(ib_conn, session->scsi_cmds_max))
+	if (device->reg_ops->alloc_reg_res(ib_conn, session->scsi_cmds_max))
 		goto create_rdma_reg_res_failed;
 
 	if (iser_alloc_login_buf(iser_conn))
@@ -301,7 +301,7 @@ rx_desc_dma_map_failed:
 rx_desc_alloc_fail:
 	iser_free_login_buf(iser_conn);
 alloc_login_buf_fail:
-	device->iser_free_rdma_reg_res(ib_conn);
+	device->reg_ops->free_reg_res(ib_conn);
 create_rdma_reg_res_failed:
 	iser_err("failed allocating rx descriptors / data buffers\n");
 	return -ENOMEM;
@@ -314,8 +314,8 @@ void iser_free_rx_descriptors(struct iser_conn *iser_conn)
 	struct ib_conn *ib_conn = &iser_conn->ib_conn;
 	struct iser_device *device = ib_conn->device;
 
-	if (device->iser_free_rdma_reg_res)
-		device->iser_free_rdma_reg_res(ib_conn);
+	if (device->reg_ops->free_reg_res)
+		device->reg_ops->free_reg_res(ib_conn);
 
 	rx_desc = iser_conn->rx_descs;
 	for (i = 0; i < iser_conn->qp_max_recv_dtos; i++, rx_desc++)
@@ -699,7 +699,7 @@ void iser_task_rdma_finalize(struct iscsi_iser_task *iser_task)
 	}
 
 	if (iser_task->dir[ISER_DIR_IN]) {
-		device->iser_unreg_rdma_mem(iser_task, ISER_DIR_IN);
+		device->reg_ops->unreg_rdma_mem(iser_task, ISER_DIR_IN);
 		if (is_rdma_data_aligned)
 			iser_dma_unmap_task_data(iser_task,
 						 &iser_task->data[ISER_DIR_IN],
@@ -711,7 +711,7 @@ void iser_task_rdma_finalize(struct iscsi_iser_task *iser_task)
 	}
 
 	if (iser_task->dir[ISER_DIR_OUT]) {
-		device->iser_unreg_rdma_mem(iser_task, ISER_DIR_OUT);
+		device->reg_ops->unreg_rdma_mem(iser_task, ISER_DIR_OUT);
 		if (is_rdma_data_aligned)
 			iser_dma_unmap_task_data(iser_task,
 						 &iser_task->data[ISER_DIR_OUT],
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index 4209d73..ff3ec53 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -39,6 +39,41 @@
 
 #include "iscsi_iser.h"
 
+static struct iser_reg_ops fastreg_ops = {
+	.alloc_reg_res	= iser_alloc_fastreg_pool,
+	.free_reg_res	= iser_free_fastreg_pool,
+	.reg_rdma_mem	= iser_reg_rdma_mem_fastreg,
+	.unreg_rdma_mem	= iser_unreg_mem_fastreg,
+};
+
+static struct iser_reg_ops fmr_ops = {
+	.alloc_reg_res	= iser_alloc_fmr_pool,
+	.free_reg_res	= iser_free_fmr_pool,
+	.reg_rdma_mem	= iser_reg_rdma_mem_fmr,
+	.unreg_rdma_mem	= iser_unreg_mem_fmr,
+};
+
+int iser_assign_reg_ops(struct iser_device *device)
+{
+	struct ib_device_attr *dev_attr = &device->dev_attr;
+
+	/* Assign function handles  - based on FMR support */
+	if (device->ib_device->alloc_fmr && device->ib_device->dealloc_fmr &&
+	    device->ib_device->map_phys_fmr && device->ib_device->unmap_fmr) {
+		iser_info("FMR supported, using FMR for registration\n");
+		device->reg_ops = &fmr_ops;
+	} else
+	if (dev_attr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS) {
+		iser_info("FastReg supported, using FastReg for registration\n");
+		device->reg_ops = &fastreg_ops;
+	} else {
+		iser_err("IB device does not support FMRs nor FastRegs, can't register memory\n");
+		return -1;
+	}
+
+	return 0;
+}
+
 static void
 iser_free_bounce_sg(struct iser_data_buf *data)
 {
diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index 2a0cb42..ca0aba3 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -87,25 +87,9 @@ static int iser_create_device_ib_res(struct iser_device *device)
 		return ret;
 	}
 
-	/* Assign function handles  - based on FMR support */
-	if (device->ib_device->alloc_fmr && device->ib_device->dealloc_fmr &&
-	    device->ib_device->map_phys_fmr && device->ib_device->unmap_fmr) {
-		iser_info("FMR supported, using FMR for registration\n");
-		device->iser_alloc_rdma_reg_res = iser_create_fmr_pool;
-		device->iser_free_rdma_reg_res = iser_free_fmr_pool;
-		device->iser_reg_rdma_mem = iser_reg_rdma_mem_fmr;
-		device->iser_unreg_rdma_mem = iser_unreg_mem_fmr;
-	} else
-	if (dev_attr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS) {
-		iser_info("FastReg supported, using FastReg for registration\n");
-		device->iser_alloc_rdma_reg_res = iser_create_fastreg_pool;
-		device->iser_free_rdma_reg_res = iser_free_fastreg_pool;
-		device->iser_reg_rdma_mem = iser_reg_rdma_mem_fastreg;
-		device->iser_unreg_rdma_mem = iser_unreg_mem_fastreg;
-	} else {
-		iser_err("IB device does not support FMRs nor FastRegs, can't register memory\n");
-		return -1;
-	}
+	ret = iser_assign_reg_ops(device);
+	if (ret)
+		return ret;
 
 	device->comps_used = min_t(int, num_online_cpus(),
 				 device->ib_device->num_comp_vectors);
@@ -211,11 +195,11 @@ static void iser_free_device_ib_res(struct iser_device *device)
 }
 
 /**
- * iser_create_fmr_pool - Creates FMR pool and page_vector
+ * iser_alloc_fmr_pool - Creates FMR pool and page_vector
  *
  * returns 0 on success, or errno code on failure
  */
-int iser_create_fmr_pool(struct ib_conn *ib_conn, unsigned cmds_max)
+int iser_alloc_fmr_pool(struct ib_conn *ib_conn, unsigned cmds_max)
 {
 	struct iser_device *device = ib_conn->device;
 	struct ib_fmr_pool_param params;
@@ -384,11 +368,11 @@ pi_ctx_alloc_failure:
 }
 
 /**
- * iser_create_fastreg_pool - Creates pool of fast_reg descriptors
+ * iser_alloc_fastreg_pool - Creates pool of fast_reg descriptors
  * for fast registration work requests.
  * returns 0 on success, or errno code on failure
  */
-int iser_create_fastreg_pool(struct ib_conn *ib_conn, unsigned cmds_max)
+int iser_alloc_fastreg_pool(struct ib_conn *ib_conn, unsigned cmds_max)
 {
 	struct iser_device *device = ib_conn->device;
 	struct iser_fr_desc *desc;
-- 
1.8.4.3

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

[PATCH 13/22] IB/iser: Move fastreg descriptor allocation to iser_create_fastreg_desc

[Sagi Grimberg]

Don't have the caller allocate the structure and worry about
freeing it in case the routine failed.

This patch does not change any functionality.

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Adir Lev <adirl-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iser_verbs.c | 38 ++++++++++++++------------------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index ca0aba3..5f3af96 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -341,17 +341,20 @@ iser_free_pi_ctx(struct iser_pi_context *pi_ctx)
 	kfree(pi_ctx);
 }
 
-static int
+static struct iser_fr_desc *
 iser_create_fastreg_desc(struct ib_device *ib_device, struct ib_pd *pd,
-			 bool pi_enable, struct iser_fr_desc *desc)
+			 bool pi_enable)
 {
+	struct iser_fr_desc *desc;
 	int ret;
 
+	desc = kzalloc(sizeof(*desc), GFP_KERNEL);
+	if (!desc)
+		return ERR_PTR(-ENOMEM);
+
 	ret = iser_alloc_reg_res(ib_device, pd, &desc->rsc);
-	if (ret) {
-		iser_err("failed to allocate reg_resources\n");
-		return ret;
-	}
+	if (ret)
+		goto reg_res_alloc_failure;
 
 	if (pi_enable) {
 		ret = iser_alloc_pi_ctx(ib_device, pd, desc);
@@ -359,12 +362,14 @@ iser_create_fastreg_desc(struct ib_device *ib_device, struct ib_pd *pd,
 			goto pi_ctx_alloc_failure;
 	}
 
-	return 0;
+	return desc;
 
 pi_ctx_alloc_failure:
 	iser_free_reg_res(&desc->rsc);
+reg_res_alloc_failure:
+	kfree(desc);
 
-	return ret;
+	return ERR_PTR(ret);
 }
 
 /**
@@ -381,19 +386,10 @@ int iser_alloc_fastreg_pool(struct ib_conn *ib_conn, unsigned cmds_max)
 	INIT_LIST_HEAD(&ib_conn->fastreg.pool);
 	ib_conn->fastreg.pool_size = 0;
 	for (i = 0; i < cmds_max; i++) {
-		desc = kzalloc(sizeof(*desc), GFP_KERNEL);
-		if (!desc) {
-			iser_err("Failed to allocate a new fast_reg descriptor\n");
-			ret = -ENOMEM;
-			goto err;
-		}
-
-		ret = iser_create_fastreg_desc(device->ib_device, device->pd,
-					       ib_conn->pi_support, desc);
-		if (ret) {
-			iser_err("Failed to create fastreg descriptor err=%d\n",
-				 ret);
-			kfree(desc);
+		desc = iser_create_fastreg_desc(device->ib_device, device->pd,
+						ib_conn->pi_support);
+		if (IS_ERR(desc)) {
+			ret = PTR_ERR(desc);
 			goto err;
 		}
 
-- 
1.8.4.3

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

[PATCH 14/22] IB/iser: Introduce iser registration pool struct

[Sagi Grimberg]

Instead of having it a part of the connection structure,
have it be under a dedicated (embedded) structure in the
connection. A logical separation of the registration pool
and the connection structure.

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Adir Lev <adirl-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iscsi_iser.h  | 49 ++++++++++++++-----------
 drivers/infiniband/ulp/iser/iser_memory.c | 32 +++++++++--------
 drivers/infiniband/ulp/iser/iser_verbs.c  | 60 ++++++++++++++++++-------------
 3 files changed, 82 insertions(+), 59 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 9ce090c..1fc4c23 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -418,6 +418,33 @@ struct iser_fr_desc {
 };
 
 /**
+ * struct iser_fr_pool: connection fast registration pool
+ *
+ * @lock:                protects fmr/fastreg pool
+ * @union.fmr:
+ *     @pool:            FMR pool for fast registrations
+ *     @page_vec:        fast reg page list to hold mapped commands pages
+ *                       used for registration
+ * @union.fastreg:
+ *     @pool:            Fast registration descriptors pool for fast
+ *                       registrations
+ *     @pool_size:       Size of pool
+ */
+struct iser_fr_pool {
+	spinlock_t		     lock;
+	union {
+		struct {
+			struct ib_fmr_pool              *pool;
+			struct iser_page_vec            *page_vec;
+		} fmr;
+		struct {
+			struct list_head	 pool;
+			int			 pool_size;
+		} fastreg;
+	};
+};
+
+/**
  * struct ib_conn - Infiniband related objects
  *
  * @cma_id:              rdma_cm connection maneger handle
@@ -430,15 +457,7 @@ struct iser_fr_desc {
  * @pi_support:          Indicate device T10-PI support
  * @beacon:              beacon send wr to signal all flush errors were drained
  * @flush_comp:          completes when all connection completions consumed
- * @lock:                protects fmr/fastreg pool
- * @union.fmr:
- *     @pool:            FMR pool for fast registrations
- *     @page_vec:        page vector to hold mapped commands pages
- *                       used for registration
- * @union.fastreg:
- *     @pool:            Fast registration descriptors pool for fast
- *                       registrations
- *     @pool_size:       Size of pool
+ * @fr_pool:             connection fast registration poool
  */
 struct ib_conn {
 	struct rdma_cm_id           *cma_id;
@@ -451,17 +470,7 @@ struct ib_conn {
 	bool			     pi_support;
 	struct ib_send_wr	     beacon;
 	struct completion	     flush_comp;
-	spinlock_t		     lock;
-	union {
-		struct {
-			struct ib_fmr_pool      *pool;
-			struct iser_page_vec	*page_vec;
-		} fmr;
-		struct {
-			struct list_head	 pool;
-			int			 pool_size;
-		} fastreg;
-	};
+	struct iser_fr_pool          fr_pool;
 };
 
 /**
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index ff3ec53..65c035d 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -184,14 +184,15 @@ iser_copy_to_bounce(struct iser_data_buf *data)
 struct iser_fr_desc *
 iser_reg_desc_get(struct ib_conn *ib_conn)
 {
+	struct iser_fr_pool *fr_pool = &ib_conn->fr_pool;
 	struct iser_fr_desc *desc;
 	unsigned long flags;
 
-	spin_lock_irqsave(&ib_conn->lock, flags);
-	desc = list_first_entry(&ib_conn->fastreg.pool,
+	spin_lock_irqsave(&fr_pool->lock, flags);
+	desc = list_first_entry(&fr_pool->fastreg.pool,
 				struct iser_fr_desc, list);
 	list_del(&desc->list);
-	spin_unlock_irqrestore(&ib_conn->lock, flags);
+	spin_unlock_irqrestore(&fr_pool->lock, flags);
 
 	return desc;
 }
@@ -200,11 +201,12 @@ void
 iser_reg_desc_put(struct ib_conn *ib_conn,
 		  struct iser_fr_desc *desc)
 {
+	struct iser_fr_pool *fr_pool = &ib_conn->fr_pool;
 	unsigned long flags;
 
-	spin_lock_irqsave(&ib_conn->lock, flags);
-	list_add(&desc->list, &ib_conn->fastreg.pool);
-	spin_unlock_irqrestore(&ib_conn->lock, flags);
+	spin_lock_irqsave(&fr_pool->lock, flags);
+	list_add(&desc->list, &fr_pool->fastreg.pool);
+	spin_unlock_irqrestore(&fr_pool->lock, flags);
 }
 
 /**
@@ -480,6 +482,7 @@ int iser_reg_page_vec(struct iscsi_iser_task *iser_task,
 		      struct iser_mem_reg *mem_reg)
 {
 	struct ib_conn *ib_conn = &iser_task->iser_conn->ib_conn;
+	struct iser_fr_pool *fr_pool = &ib_conn->fr_pool;
 	struct iser_device *device = ib_conn->device;
 	struct ib_pool_fmr *fmr;
 	int ret, plen;
@@ -496,7 +499,7 @@ int iser_reg_page_vec(struct iscsi_iser_task *iser_task,
 		return -EINVAL;
 	}
 
-	fmr  = ib_fmr_pool_map_phys(ib_conn->fmr.pool,
+	fmr  = ib_fmr_pool_map_phys(fr_pool->fmr.pool,
 				    page_vec->pages,
 				    page_vec->length,
 				    page_vec->pages[0]);
@@ -560,6 +563,7 @@ int iser_reg_rdma_mem_fmr(struct iscsi_iser_task *iser_task,
 			  enum iser_data_dir cmd_dir)
 {
 	struct ib_conn *ib_conn = &iser_task->iser_conn->ib_conn;
+	struct iser_fr_pool *fr_pool = &ib_conn->fr_pool;
 	struct iser_device   *device = ib_conn->device;
 	struct ib_device     *ibdev = device->ib_device;
 	struct iser_data_buf *mem = &iser_task->data[cmd_dir];
@@ -583,20 +587,20 @@ int iser_reg_rdma_mem_fmr(struct iscsi_iser_task *iser_task,
 	if (mem->dma_nents == 1) {
 		return iser_reg_dma(device, mem, mem_reg);
 	} else { /* use FMR for multiple dma entries */
-		err = iser_reg_page_vec(iser_task, mem, ib_conn->fmr.page_vec,
-					mem_reg);
+		err = iser_reg_page_vec(iser_task, mem,
+					fr_pool->fmr.page_vec, mem_reg);
 		if (err && err != -EAGAIN) {
 			iser_data_buf_dump(mem, ibdev);
 			iser_err("mem->dma_nents = %d (dlength = 0x%x)\n",
 				 mem->dma_nents,
 				 ntoh24(iser_task->desc.iscsi_header.dlength));
 			iser_err("page_vec: data_size = 0x%x, length = %d, offset = 0x%x\n",
-				 ib_conn->fmr.page_vec->data_size,
-				 ib_conn->fmr.page_vec->length,
-				 ib_conn->fmr.page_vec->offset);
-			for (i = 0; i < ib_conn->fmr.page_vec->length; i++)
+				 fr_pool->fmr.page_vec->data_size,
+				 fr_pool->fmr.page_vec->length,
+				 fr_pool->fmr.page_vec->offset);
+			for (i = 0; i < fr_pool->fmr.page_vec->length; i++)
 				iser_err("page_vec[%d] = 0x%llx\n", i,
-					 (unsigned long long)ib_conn->fmr.page_vec->pages[i]);
+					 (unsigned long long)fr_pool->fmr.page_vec->pages[i]);
 		}
 		if (err)
 			return err;
diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index 5f3af96..d50477c 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -202,16 +202,21 @@ static void iser_free_device_ib_res(struct iser_device *device)
 int iser_alloc_fmr_pool(struct ib_conn *ib_conn, unsigned cmds_max)
 {
 	struct iser_device *device = ib_conn->device;
+	struct iser_fr_pool *fr_pool = &ib_conn->fr_pool;
+	struct iser_page_vec *page_vec;
+	struct ib_fmr_pool *fmr_pool;
 	struct ib_fmr_pool_param params;
 	int ret = -ENOMEM;
 
-	ib_conn->fmr.page_vec = kmalloc(sizeof(*ib_conn->fmr.page_vec) +
-					(sizeof(u64)*(ISCSI_ISER_SG_TABLESIZE + 1)),
-					GFP_KERNEL);
-	if (!ib_conn->fmr.page_vec)
+	spin_lock_init(&fr_pool->lock);
+
+	page_vec = kmalloc(sizeof(*page_vec) +
+			   (sizeof(u64) * (ISCSI_ISER_SG_TABLESIZE + 1)),
+			   GFP_KERNEL);
+	if (!page_vec)
 		return ret;
 
-	ib_conn->fmr.page_vec->pages = (u64 *)(ib_conn->fmr.page_vec + 1);
+	page_vec->pages = (u64 *)(page_vec + 1);
 
 	params.page_shift        = SHIFT_4K;
 	/* when the first/last SG element are not start/end *
@@ -227,18 +232,20 @@ int iser_alloc_fmr_pool(struct ib_conn *ib_conn, unsigned cmds_max)
 				    IB_ACCESS_REMOTE_WRITE |
 				    IB_ACCESS_REMOTE_READ);
 
-	ib_conn->fmr.pool = ib_create_fmr_pool(device->pd, &params);
-	if (IS_ERR(ib_conn->fmr.pool)) {
-		ret = PTR_ERR(ib_conn->fmr.pool);
+	fmr_pool = ib_create_fmr_pool(device->pd, &params);
+	if (IS_ERR(fmr_pool)) {
+		ret = PTR_ERR(fmr_pool);
 		iser_err("FMR allocation failed, err %d\n", ret);
 		goto err;
 	}
 
+	fr_pool->fmr.page_vec = page_vec;
+	fr_pool->fmr.pool = fmr_pool;
+
 	return 0;
 
 err:
-	kfree(ib_conn->fmr.page_vec);
-	ib_conn->fmr.page_vec = NULL;
+	kfree(page_vec);
 	return ret;
 }
 
@@ -247,14 +254,15 @@ err:
  */
 void iser_free_fmr_pool(struct ib_conn *ib_conn)
 {
-	iser_info("freeing conn %p fmr pool %p\n",
-		  ib_conn, ib_conn->fmr.pool);
+	struct iser_fr_pool *fr_pool = &ib_conn->fr_pool;
 
-	ib_destroy_fmr_pool(ib_conn->fmr.pool);
-	ib_conn->fmr.pool = NULL;
+	iser_info("freeing conn %p fmr pool %p\n",
+		  ib_conn, fr_pool->fmr.pool);
 
-	kfree(ib_conn->fmr.page_vec);
-	ib_conn->fmr.page_vec = NULL;
+	ib_destroy_fmr_pool(fr_pool->fmr.pool);
+	fr_pool->fmr.pool = NULL;
+	kfree(fr_pool->fmr.page_vec);
+	fr_pool->fmr.page_vec = NULL;
 }
 
 static int
@@ -380,11 +388,13 @@ reg_res_alloc_failure:
 int iser_alloc_fastreg_pool(struct ib_conn *ib_conn, unsigned cmds_max)
 {
 	struct iser_device *device = ib_conn->device;
+	struct iser_fr_pool *fr_pool = &ib_conn->fr_pool;
 	struct iser_fr_desc *desc;
 	int i, ret;
 
-	INIT_LIST_HEAD(&ib_conn->fastreg.pool);
-	ib_conn->fastreg.pool_size = 0;
+	INIT_LIST_HEAD(&fr_pool->fastreg.pool);
+	spin_lock_init(&fr_pool->lock);
+	fr_pool->fastreg.pool_size = 0;
 	for (i = 0; i < cmds_max; i++) {
 		desc = iser_create_fastreg_desc(device->ib_device, device->pd,
 						ib_conn->pi_support);
@@ -393,8 +403,8 @@ int iser_alloc_fastreg_pool(struct ib_conn *ib_conn, unsigned cmds_max)
 			goto err;
 		}
 
-		list_add_tail(&desc->list, &ib_conn->fastreg.pool);
-		ib_conn->fastreg.pool_size++;
+		list_add_tail(&desc->list, &fr_pool->fastreg.pool);
+		fr_pool->fastreg.pool_size++;
 	}
 
 	return 0;
@@ -409,15 +419,16 @@ err:
  */
 void iser_free_fastreg_pool(struct ib_conn *ib_conn)
 {
+	struct iser_fr_pool *fr_pool = &ib_conn->fr_pool;
 	struct iser_fr_desc *desc, *tmp;
 	int i = 0;
 
-	if (list_empty(&ib_conn->fastreg.pool))
+	if (list_empty(&fr_pool->fastreg.pool))
 		return;
 
 	iser_info("freeing conn %p fr pool\n", ib_conn);
 
-	list_for_each_entry_safe(desc, tmp, &ib_conn->fastreg.pool, list) {
+	list_for_each_entry_safe(desc, tmp, &fr_pool->fastreg.pool, list) {
 		list_del(&desc->list);
 		iser_free_reg_res(&desc->rsc);
 		if (desc->pi_ctx)
@@ -426,9 +437,9 @@ void iser_free_fastreg_pool(struct ib_conn *ib_conn)
 		++i;
 	}
 
-	if (i < ib_conn->fastreg.pool_size)
+	if (i < fr_pool->fastreg.pool_size)
 		iser_warn("pool still has %d regions registered\n",
-			  ib_conn->fastreg.pool_size - i);
+			  fr_pool->fastreg.pool_size - i);
 }
 
 /**
@@ -924,7 +935,6 @@ void iser_conn_init(struct iser_conn *iser_conn)
 	init_completion(&iser_conn->ib_completion);
 	init_completion(&iser_conn->up_completion);
 	INIT_LIST_HEAD(&iser_conn->conn_list);
-	spin_lock_init(&iser_conn->ib_conn.lock);
 	mutex_init(&iser_conn->state_mutex);
 }
 
-- 
1.8.4.3

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

[PATCH 15/22] IB/iser: Maintain connection fmr_pool under a single registration descriptor

[Sagi Grimberg]

From: Adir Lev <adirl-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

This will allow us to unify the memory registration code path between
the various methods which vary by the device capabilities. This change
will make it easier and less intrusive to remove fmr_pools from the
code when we'd want to.

The reason we use a single descriptor is to avoid taking a
redundant spinlock when working with FMRs.

We also change the signature of iser_reg_page_vec to make it match
iser_fast_reg_mr (and the future indirect registration method).

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Adir Lev <adirl-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iscsi_iser.h  | 38 +++++++++------------
 drivers/infiniband/ulp/iser/iser_memory.c | 28 +++++++++-------
 drivers/infiniband/ulp/iser/iser_verbs.c  | 56 ++++++++++++++++++++-----------
 3 files changed, 68 insertions(+), 54 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 1fc4c23..a8c8177 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -380,12 +380,20 @@ struct iser_device {
  * struct iser_reg_resources - Fast registration recources
  *
  * @mr:         memory region
- * @frpl:       fast reg page list
+ * @fmr_pool:   pool of fmrs
+ * @frpl:       fast reg page list used by frwrs
+ * @page_vec:   fast reg page list used by fmr pool
  * @mr_valid:   is mr valid indicator
  */
 struct iser_reg_resources {
-	struct ib_mr			 *mr;
-	struct ib_fast_reg_page_list     *frpl;
+	union {
+		struct ib_mr             *mr;
+		struct ib_fmr_pool       *fmr_pool;
+	};
+	union {
+		struct ib_fast_reg_page_list     *frpl;
+		struct iser_page_vec             *page_vec;
+	};
 	u8				  mr_valid:1;
 };
 
@@ -420,28 +428,14 @@ struct iser_fr_desc {
 /**
  * struct iser_fr_pool: connection fast registration pool
  *
+ * @list:                list of fastreg descriptors
  * @lock:                protects fmr/fastreg pool
- * @union.fmr:
- *     @pool:            FMR pool for fast registrations
- *     @page_vec:        fast reg page list to hold mapped commands pages
- *                       used for registration
- * @union.fastreg:
- *     @pool:            Fast registration descriptors pool for fast
- *                       registrations
- *     @pool_size:       Size of pool
+ * @size:                size of the pool
  */
 struct iser_fr_pool {
-	spinlock_t		     lock;
-	union {
-		struct {
-			struct ib_fmr_pool              *pool;
-			struct iser_page_vec            *page_vec;
-		} fmr;
-		struct {
-			struct list_head	 pool;
-			int			 pool_size;
-		} fastreg;
-	};
+	struct list_head        list;
+	spinlock_t              lock;
+	int                     size;
 };
 
 /**
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index 65c035d..d0c01e1 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -189,7 +189,7 @@ iser_reg_desc_get(struct ib_conn *ib_conn)
 	unsigned long flags;
 
 	spin_lock_irqsave(&fr_pool->lock, flags);
-	desc = list_first_entry(&fr_pool->fastreg.pool,
+	desc = list_first_entry(&fr_pool->list,
 				struct iser_fr_desc, list);
 	list_del(&desc->list);
 	spin_unlock_irqrestore(&fr_pool->lock, flags);
@@ -205,7 +205,7 @@ iser_reg_desc_put(struct ib_conn *ib_conn,
 	unsigned long flags;
 
 	spin_lock_irqsave(&fr_pool->lock, flags);
-	list_add(&desc->list, &fr_pool->fastreg.pool);
+	list_add(&desc->list, &fr_pool->list);
 	spin_unlock_irqrestore(&fr_pool->lock, flags);
 }
 
@@ -478,12 +478,13 @@ static int fall_to_bounce_buf(struct iscsi_iser_task *iser_task,
 static
 int iser_reg_page_vec(struct iscsi_iser_task *iser_task,
 		      struct iser_data_buf *mem,
-		      struct iser_page_vec *page_vec,
+		      struct iser_reg_resources *rsc,
 		      struct iser_mem_reg *mem_reg)
 {
 	struct ib_conn *ib_conn = &iser_task->iser_conn->ib_conn;
-	struct iser_fr_pool *fr_pool = &ib_conn->fr_pool;
 	struct iser_device *device = ib_conn->device;
+	struct iser_page_vec *page_vec = rsc->page_vec;
+	struct ib_fmr_pool *fmr_pool = rsc->fmr_pool;
 	struct ib_pool_fmr *fmr;
 	int ret, plen;
 
@@ -499,7 +500,7 @@ int iser_reg_page_vec(struct iscsi_iser_task *iser_task,
 		return -EINVAL;
 	}
 
-	fmr  = ib_fmr_pool_map_phys(fr_pool->fmr.pool,
+	fmr  = ib_fmr_pool_map_phys(fmr_pool,
 				    page_vec->pages,
 				    page_vec->length,
 				    page_vec->pages[0]);
@@ -587,20 +588,23 @@ int iser_reg_rdma_mem_fmr(struct iscsi_iser_task *iser_task,
 	if (mem->dma_nents == 1) {
 		return iser_reg_dma(device, mem, mem_reg);
 	} else { /* use FMR for multiple dma entries */
-		err = iser_reg_page_vec(iser_task, mem,
-					fr_pool->fmr.page_vec, mem_reg);
+		struct iser_fr_desc *desc;
+
+		desc = list_first_entry(&fr_pool->list,
+					struct iser_fr_desc, list);
+		err = iser_reg_page_vec(iser_task, mem, &desc->rsc, mem_reg);
 		if (err && err != -EAGAIN) {
 			iser_data_buf_dump(mem, ibdev);
 			iser_err("mem->dma_nents = %d (dlength = 0x%x)\n",
 				 mem->dma_nents,
 				 ntoh24(iser_task->desc.iscsi_header.dlength));
 			iser_err("page_vec: data_size = 0x%x, length = %d, offset = 0x%x\n",
-				 fr_pool->fmr.page_vec->data_size,
-				 fr_pool->fmr.page_vec->length,
-				 fr_pool->fmr.page_vec->offset);
-			for (i = 0; i < fr_pool->fmr.page_vec->length; i++)
+				 desc->rsc.page_vec->data_size,
+				 desc->rsc.page_vec->length,
+				 desc->rsc.page_vec->offset);
+			for (i = 0; i < desc->rsc.page_vec->length; i++)
 				iser_err("page_vec[%d] = 0x%llx\n", i,
-					 (unsigned long long)fr_pool->fmr.page_vec->pages[i]);
+					 (unsigned long long)desc->rsc.page_vec->pages[i]);
 		}
 		if (err)
 			return err;
diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index d50477c..6b464e6 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -204,17 +204,25 @@ int iser_alloc_fmr_pool(struct ib_conn *ib_conn, unsigned cmds_max)
 	struct iser_device *device = ib_conn->device;
 	struct iser_fr_pool *fr_pool = &ib_conn->fr_pool;
 	struct iser_page_vec *page_vec;
+	struct iser_fr_desc *desc;
 	struct ib_fmr_pool *fmr_pool;
 	struct ib_fmr_pool_param params;
-	int ret = -ENOMEM;
+	int ret;
 
+	INIT_LIST_HEAD(&fr_pool->list);
 	spin_lock_init(&fr_pool->lock);
 
+	desc = kzalloc(sizeof(*desc), GFP_KERNEL);
+	if (!desc)
+		return -ENOMEM;
+
 	page_vec = kmalloc(sizeof(*page_vec) +
 			   (sizeof(u64) * (ISCSI_ISER_SG_TABLESIZE + 1)),
 			   GFP_KERNEL);
-	if (!page_vec)
-		return ret;
+	if (!page_vec) {
+		ret = -ENOMEM;
+		goto err_frpl;
+	}
 
 	page_vec->pages = (u64 *)(page_vec + 1);
 
@@ -236,16 +244,20 @@ int iser_alloc_fmr_pool(struct ib_conn *ib_conn, unsigned cmds_max)
 	if (IS_ERR(fmr_pool)) {
 		ret = PTR_ERR(fmr_pool);
 		iser_err("FMR allocation failed, err %d\n", ret);
-		goto err;
+		goto err_fmr;
 	}
 
-	fr_pool->fmr.page_vec = page_vec;
-	fr_pool->fmr.pool = fmr_pool;
+	desc->rsc.page_vec = page_vec;
+	desc->rsc.fmr_pool = fmr_pool;
+	list_add(&desc->list, &fr_pool->list);
 
 	return 0;
 
-err:
+err_fmr:
 	kfree(page_vec);
+err_frpl:
+	kfree(desc);
+
 	return ret;
 }
 
@@ -255,14 +267,18 @@ err:
 void iser_free_fmr_pool(struct ib_conn *ib_conn)
 {
 	struct iser_fr_pool *fr_pool = &ib_conn->fr_pool;
+	struct iser_fr_desc *desc;
+
+	desc = list_first_entry(&fr_pool->list,
+				struct iser_fr_desc, list);
+	list_del(&desc->list);
 
 	iser_info("freeing conn %p fmr pool %p\n",
-		  ib_conn, fr_pool->fmr.pool);
+		  ib_conn, desc->rsc.fmr_pool);
 
-	ib_destroy_fmr_pool(fr_pool->fmr.pool);
-	fr_pool->fmr.pool = NULL;
-	kfree(fr_pool->fmr.page_vec);
-	fr_pool->fmr.page_vec = NULL;
+	ib_destroy_fmr_pool(desc->rsc.fmr_pool);
+	kfree(desc->rsc.page_vec);
+	kfree(desc);
 }
 
 static int
@@ -392,9 +408,9 @@ int iser_alloc_fastreg_pool(struct ib_conn *ib_conn, unsigned cmds_max)
 	struct iser_fr_desc *desc;
 	int i, ret;
 
-	INIT_LIST_HEAD(&fr_pool->fastreg.pool);
+	INIT_LIST_HEAD(&fr_pool->list);
 	spin_lock_init(&fr_pool->lock);
-	fr_pool->fastreg.pool_size = 0;
+	fr_pool->size = 0;
 	for (i = 0; i < cmds_max; i++) {
 		desc = iser_create_fastreg_desc(device->ib_device, device->pd,
 						ib_conn->pi_support);
@@ -403,8 +419,8 @@ int iser_alloc_fastreg_pool(struct ib_conn *ib_conn, unsigned cmds_max)
 			goto err;
 		}
 
-		list_add_tail(&desc->list, &fr_pool->fastreg.pool);
-		fr_pool->fastreg.pool_size++;
+		list_add_tail(&desc->list, &fr_pool->list);
+		fr_pool->size++;
 	}
 
 	return 0;
@@ -423,12 +439,12 @@ void iser_free_fastreg_pool(struct ib_conn *ib_conn)
 	struct iser_fr_desc *desc, *tmp;
 	int i = 0;
 
-	if (list_empty(&fr_pool->fastreg.pool))
+	if (list_empty(&fr_pool->list))
 		return;
 
 	iser_info("freeing conn %p fr pool\n", ib_conn);
 
-	list_for_each_entry_safe(desc, tmp, &fr_pool->fastreg.pool, list) {
+	list_for_each_entry_safe(desc, tmp, &fr_pool->list, list) {
 		list_del(&desc->list);
 		iser_free_reg_res(&desc->rsc);
 		if (desc->pi_ctx)
@@ -437,9 +453,9 @@ void iser_free_fastreg_pool(struct ib_conn *ib_conn)
 		++i;
 	}
 
-	if (i < fr_pool->fastreg.pool_size)
+	if (i < fr_pool->size)
 		iser_warn("pool still has %d regions registered\n",
-			  fr_pool->fastreg.pool_size - i);
+			  fr_pool->size - i);
 }
 
 /**
-- 
1.8.4.3

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

[PATCH 16/22] IB/iser: Rename iser_reg_page_vec to iser_fast_reg_fmr

[Sagi Grimberg]

Also, change a name of a local variable.

This patch does not change any functionality.

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Adir Lev <adirl-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iser_memory.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index d0c01e1..11bba5a 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -476,10 +476,10 @@ static int fall_to_bounce_buf(struct iscsi_iser_task *iser_task,
  * returns: 0 on success, errno code on failure
  */
 static
-int iser_reg_page_vec(struct iscsi_iser_task *iser_task,
+int iser_fast_reg_fmr(struct iscsi_iser_task *iser_task,
 		      struct iser_data_buf *mem,
 		      struct iser_reg_resources *rsc,
-		      struct iser_mem_reg *mem_reg)
+		      struct iser_mem_reg *reg)
 {
 	struct ib_conn *ib_conn = &iser_task->iser_conn->ib_conn;
 	struct iser_device *device = ib_conn->device;
@@ -510,11 +510,11 @@ int iser_reg_page_vec(struct iscsi_iser_task *iser_task,
 		return ret;
 	}
 
-	mem_reg->sge.lkey = fmr->fmr->lkey;
-	mem_reg->rkey = fmr->fmr->rkey;
-	mem_reg->sge.addr = page_vec->pages[0] + page_vec->offset;
-	mem_reg->sge.length = page_vec->data_size;
-	mem_reg->mem_h = fmr;
+	reg->sge.lkey = fmr->fmr->lkey;
+	reg->rkey = fmr->fmr->rkey;
+	reg->sge.addr = page_vec->pages[0] + page_vec->offset;
+	reg->sge.length = page_vec->data_size;
+	reg->mem_h = fmr;
 
 	return 0;
 }
@@ -592,7 +592,7 @@ int iser_reg_rdma_mem_fmr(struct iscsi_iser_task *iser_task,
 
 		desc = list_first_entry(&fr_pool->list,
 					struct iser_fr_desc, list);
-		err = iser_reg_page_vec(iser_task, mem, &desc->rsc, mem_reg);
+		err = iser_fast_reg_fmr(iser_task, mem, &desc->rsc, mem_reg);
 		if (err && err != -EAGAIN) {
 			iser_data_buf_dump(mem, ibdev);
 			iser_err("mem->dma_nents = %d (dlength = 0x%x)\n",
-- 
1.8.4.3

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

[PATCH 17/22] IB/iser: Make reg_desc_get a per device routine

[Sagi Grimberg]

As for fmrs we will hold a single registration descriptor
as no need for multiple like in the frwr mode (descriptor
for each task). This change helps unifying the duplicate
registration code paths.

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Adir Lev <adirl-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iscsi_iser.h  | 16 ++++++++++---
 drivers/infiniband/ulp/iser/iser_memory.c | 38 +++++++++++++++++++++++--------
 2 files changed, 41 insertions(+), 13 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index a8c8177..611abaa 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -333,6 +333,8 @@ struct iser_comp {
  * @free_reg_res:      Free registration resources
  * @reg_rdma_mem:      Register memory buffers
  * @unreg_rdma_mem:    Un-register memory buffers
+ * @reg_desc_get:      Get a registration descriptor for pool
+ * @reg_desc_put:      Get a registration descriptor to pool
  */
 struct iser_reg_ops {
 	int            (*alloc_reg_res)(struct ib_conn *ib_conn,
@@ -342,6 +344,9 @@ struct iser_reg_ops {
 				       enum iser_data_dir cmd_dir);
 	void           (*unreg_rdma_mem)(struct iscsi_iser_task *iser_task,
 					 enum iser_data_dir cmd_dir);
+	struct iser_fr_desc * (*reg_desc_get)(struct ib_conn *ib_conn);
+	void           (*reg_desc_put)(struct ib_conn *ib_conn,
+				       struct iser_fr_desc *desc);
 };
 
 /**
@@ -657,8 +662,13 @@ void iser_free_fastreg_pool(struct ib_conn *ib_conn);
 u8 iser_check_task_pi_status(struct iscsi_iser_task *iser_task,
 			     enum iser_data_dir cmd_dir, sector_t *sector);
 struct iser_fr_desc *
-iser_reg_desc_get(struct ib_conn *ib_conn);
+iser_reg_desc_get_fr(struct ib_conn *ib_conn);
 void
-iser_reg_desc_put(struct ib_conn *ib_conn,
-		  struct iser_fr_desc *desc);
+iser_reg_desc_put_fr(struct ib_conn *ib_conn,
+		     struct iser_fr_desc *desc);
+struct iser_fr_desc *
+iser_reg_desc_get_fmr(struct ib_conn *ib_conn);
+void
+iser_reg_desc_put_fmr(struct ib_conn *ib_conn,
+		      struct iser_fr_desc *desc);
 #endif
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index 11bba5a..4e687ee 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -44,6 +44,8 @@ static struct iser_reg_ops fastreg_ops = {
 	.free_reg_res	= iser_free_fastreg_pool,
 	.reg_rdma_mem	= iser_reg_rdma_mem_fastreg,
 	.unreg_rdma_mem	= iser_unreg_mem_fastreg,
+	.reg_desc_get	= iser_reg_desc_get_fr,
+	.reg_desc_put	= iser_reg_desc_put_fr,
 };
 
 static struct iser_reg_ops fmr_ops = {
@@ -51,6 +53,8 @@ static struct iser_reg_ops fmr_ops = {
 	.free_reg_res	= iser_free_fmr_pool,
 	.reg_rdma_mem	= iser_reg_rdma_mem_fmr,
 	.unreg_rdma_mem	= iser_unreg_mem_fmr,
+	.reg_desc_get	= iser_reg_desc_get_fmr,
+	.reg_desc_put	= iser_reg_desc_put_fmr,
 };
 
 int iser_assign_reg_ops(struct iser_device *device)
@@ -182,7 +186,7 @@ iser_copy_to_bounce(struct iser_data_buf *data)
 }
 
 struct iser_fr_desc *
-iser_reg_desc_get(struct ib_conn *ib_conn)
+iser_reg_desc_get_fr(struct ib_conn *ib_conn)
 {
 	struct iser_fr_pool *fr_pool = &ib_conn->fr_pool;
 	struct iser_fr_desc *desc;
@@ -198,8 +202,8 @@ iser_reg_desc_get(struct ib_conn *ib_conn)
 }
 
 void
-iser_reg_desc_put(struct ib_conn *ib_conn,
-		  struct iser_fr_desc *desc)
+iser_reg_desc_put_fr(struct ib_conn *ib_conn,
+		     struct iser_fr_desc *desc)
 {
 	struct iser_fr_pool *fr_pool = &ib_conn->fr_pool;
 	unsigned long flags;
@@ -209,6 +213,21 @@ iser_reg_desc_put(struct ib_conn *ib_conn,
 	spin_unlock_irqrestore(&fr_pool->lock, flags);
 }
 
+struct iser_fr_desc *
+iser_reg_desc_get_fmr(struct ib_conn *ib_conn)
+{
+	struct iser_fr_pool *fr_pool = &ib_conn->fr_pool;
+
+	return list_first_entry(&fr_pool->list,
+				struct iser_fr_desc, list);
+}
+
+void
+iser_reg_desc_put_fmr(struct ib_conn *ib_conn,
+		      struct iser_fr_desc *desc)
+{
+}
+
 /**
  * iser_start_rdma_unaligned_sg
  */
@@ -544,13 +563,14 @@ void iser_unreg_mem_fmr(struct iscsi_iser_task *iser_task,
 void iser_unreg_mem_fastreg(struct iscsi_iser_task *iser_task,
 			    enum iser_data_dir cmd_dir)
 {
+	struct iser_device *device = iser_task->iser_conn->ib_conn.device;
 	struct iser_mem_reg *reg = &iser_task->rdma_reg[cmd_dir];
 
 	if (!reg->mem_h)
 		return;
 
-	iser_reg_desc_put(&iser_task->iser_conn->ib_conn,
-			  reg->mem_h);
+	device->reg_ops->reg_desc_put(&iser_task->iser_conn->ib_conn,
+				     reg->mem_h);
 	reg->mem_h = NULL;
 }
 
@@ -564,7 +584,6 @@ int iser_reg_rdma_mem_fmr(struct iscsi_iser_task *iser_task,
 			  enum iser_data_dir cmd_dir)
 {
 	struct ib_conn *ib_conn = &iser_task->iser_conn->ib_conn;
-	struct iser_fr_pool *fr_pool = &ib_conn->fr_pool;
 	struct iser_device   *device = ib_conn->device;
 	struct ib_device     *ibdev = device->ib_device;
 	struct iser_data_buf *mem = &iser_task->data[cmd_dir];
@@ -590,8 +609,7 @@ int iser_reg_rdma_mem_fmr(struct iscsi_iser_task *iser_task,
 	} else { /* use FMR for multiple dma entries */
 		struct iser_fr_desc *desc;
 
-		desc = list_first_entry(&fr_pool->list,
-					struct iser_fr_desc, list);
+		desc = device->reg_ops->reg_desc_get(ib_conn);
 		err = iser_fast_reg_fmr(iser_task, mem, &desc->rsc, mem_reg);
 		if (err && err != -EAGAIN) {
 			iser_data_buf_dump(mem, ibdev);
@@ -844,7 +862,7 @@ int iser_reg_rdma_mem_fastreg(struct iscsi_iser_task *iser_task,
 
 	if (mem->dma_nents != 1 ||
 	    scsi_get_prot_op(iser_task->sc) != SCSI_PROT_NORMAL) {
-		desc = iser_reg_desc_get(ib_conn);
+		desc = device->reg_ops->reg_desc_get(ib_conn);
 		mem_reg->mem_h = desc;
 	}
 
@@ -887,7 +905,7 @@ int iser_reg_rdma_mem_fastreg(struct iscsi_iser_task *iser_task,
 	return 0;
 err_reg:
 	if (desc)
-		iser_reg_desc_put(ib_conn, desc);
+		device->reg_ops->reg_desc_put(ib_conn, desc);
 
 	return err;
 }
-- 
1.8.4.3

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

[PATCH 18/22] IB/iser: Unify fast memory registration flows

[Sagi Grimberg]

iser_reg_rdma_mem_[fastreg|fmr] share a lot of code, and
logically do the same thing other than the buffer registration
method itself (iser_fast_reg_mr vs. iser_fast_reg_fmr).
The DIF logic is not implemented in the FMR flow as there is no
existing device that supports FMRs and Signature feature.

This patch unifies the flow in a single routine iser_reg_rdma_mem
and just split to fmr/frwr for the buffer registration itself.

Also, for symmetry reasons, unify iser_unreg_rdma_mem (which will
call the relevant device specific unreg routine).

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Adir Lev <adirl-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iscsi_iser.h     |  23 +--
 drivers/infiniband/ulp/iser/iser_initiator.c |  11 +-
 drivers/infiniband/ulp/iser/iser_memory.c    | 210 ++++++++++++---------------
 3 files changed, 113 insertions(+), 131 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 611abaa..e6105d2 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -239,6 +239,7 @@ struct iser_data_buf {
 struct iser_device;
 struct iscsi_iser_task;
 struct iscsi_endpoint;
+struct iser_reg_resources;
 
 /**
  * struct iser_mem_reg - iSER memory registration info
@@ -331,8 +332,8 @@ struct iser_comp {
  *
  * @alloc_reg_res:     Allocate registration resources
  * @free_reg_res:      Free registration resources
- * @reg_rdma_mem:      Register memory buffers
- * @unreg_rdma_mem:    Un-register memory buffers
+ * @fast_reg_mem:      Register memory buffers
+ * @unreg_mem:         Un-register memory buffers
  * @reg_desc_get:      Get a registration descriptor for pool
  * @reg_desc_put:      Get a registration descriptor to pool
  */
@@ -340,10 +341,12 @@ struct iser_reg_ops {
 	int            (*alloc_reg_res)(struct ib_conn *ib_conn,
 					unsigned cmds_max);
 	void           (*free_reg_res)(struct ib_conn *ib_conn);
-	int            (*reg_rdma_mem)(struct iscsi_iser_task *iser_task,
-				       enum iser_data_dir cmd_dir);
-	void           (*unreg_rdma_mem)(struct iscsi_iser_task *iser_task,
-					 enum iser_data_dir cmd_dir);
+	int            (*reg_mem)(struct iscsi_iser_task *iser_task,
+				  struct iser_data_buf *mem,
+				  struct iser_reg_resources *rsc,
+				  struct iser_mem_reg *reg);
+	void           (*unreg_mem)(struct iscsi_iser_task *iser_task,
+				    enum iser_data_dir cmd_dir);
 	struct iser_fr_desc * (*reg_desc_get)(struct ib_conn *ib_conn);
 	void           (*reg_desc_put)(struct ib_conn *ib_conn,
 				       struct iser_fr_desc *desc);
@@ -622,10 +625,10 @@ void iser_finalize_rdma_unaligned_sg(struct iscsi_iser_task *iser_task,
 				     struct iser_data_buf *mem,
 				     enum iser_data_dir cmd_dir);
 
-int  iser_reg_rdma_mem_fmr(struct iscsi_iser_task *task,
-			   enum iser_data_dir cmd_dir);
-int  iser_reg_rdma_mem_fastreg(struct iscsi_iser_task *task,
-			       enum iser_data_dir cmd_dir);
+int iser_reg_rdma_mem(struct iscsi_iser_task *task,
+		      enum iser_data_dir dir);
+void iser_unreg_rdma_mem(struct iscsi_iser_task *task,
+			 enum iser_data_dir dir);
 
 int  iser_connect(struct iser_conn *iser_conn,
 		  struct sockaddr *src_addr,
diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c
index 88d8a89..cef6c82 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -49,7 +49,6 @@ static int iser_prepare_read_cmd(struct iscsi_task *task)
 
 {
 	struct iscsi_iser_task *iser_task = task->dd_data;
-	struct iser_device  *device = iser_task->iser_conn->ib_conn.device;
 	struct iser_mem_reg *mem_reg;
 	int err;
 	struct iser_hdr *hdr = &iser_task->desc.iser_header;
@@ -73,7 +72,7 @@ static int iser_prepare_read_cmd(struct iscsi_task *task)
 			return err;
 	}
 
-	err = device->reg_ops->reg_rdma_mem(iser_task, ISER_DIR_IN);
+	err = iser_reg_rdma_mem(iser_task, ISER_DIR_IN);
 	if (err) {
 		iser_err("Failed to set up Data-IN RDMA\n");
 		return err;
@@ -103,7 +102,6 @@ iser_prepare_write_cmd(struct iscsi_task *task,
 		       unsigned int edtl)
 {
 	struct iscsi_iser_task *iser_task = task->dd_data;
-	struct iser_device  *device = iser_task->iser_conn->ib_conn.device;
 	struct iser_mem_reg *mem_reg;
 	int err;
 	struct iser_hdr *hdr = &iser_task->desc.iser_header;
@@ -128,7 +126,7 @@ iser_prepare_write_cmd(struct iscsi_task *task,
 			return err;
 	}
 
-	err = device->reg_ops->reg_rdma_mem(iser_task, ISER_DIR_OUT);
+	err = iser_reg_rdma_mem(iser_task, ISER_DIR_OUT);
 	if (err != 0) {
 		iser_err("Failed to register write cmd RDMA mem\n");
 		return err;
@@ -662,7 +660,6 @@ void iser_task_rdma_init(struct iscsi_iser_task *iser_task)
 
 void iser_task_rdma_finalize(struct iscsi_iser_task *iser_task)
 {
-	struct iser_device *device = iser_task->iser_conn->ib_conn.device;
 	int is_rdma_data_aligned = 1;
 	int is_rdma_prot_aligned = 1;
 	int prot_count = scsi_prot_sg_count(iser_task->sc);
@@ -699,7 +696,7 @@ void iser_task_rdma_finalize(struct iscsi_iser_task *iser_task)
 	}
 
 	if (iser_task->dir[ISER_DIR_IN]) {
-		device->reg_ops->unreg_rdma_mem(iser_task, ISER_DIR_IN);
+		iser_unreg_rdma_mem(iser_task, ISER_DIR_IN);
 		if (is_rdma_data_aligned)
 			iser_dma_unmap_task_data(iser_task,
 						 &iser_task->data[ISER_DIR_IN],
@@ -711,7 +708,7 @@ void iser_task_rdma_finalize(struct iscsi_iser_task *iser_task)
 	}
 
 	if (iser_task->dir[ISER_DIR_OUT]) {
-		device->reg_ops->unreg_rdma_mem(iser_task, ISER_DIR_OUT);
+		iser_unreg_rdma_mem(iser_task, ISER_DIR_OUT);
 		if (is_rdma_data_aligned)
 			iser_dma_unmap_task_data(iser_task,
 						 &iser_task->data[ISER_DIR_OUT],
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index 4e687ee..5e807ba 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -38,12 +38,22 @@
 #include <linux/scatterlist.h>
 
 #include "iscsi_iser.h"
+static
+int iser_fast_reg_fmr(struct iscsi_iser_task *iser_task,
+		      struct iser_data_buf *mem,
+		      struct iser_reg_resources *rsc,
+		      struct iser_mem_reg *mem_reg);
+static
+int iser_fast_reg_mr(struct iscsi_iser_task *iser_task,
+		     struct iser_data_buf *mem,
+		     struct iser_reg_resources *rsc,
+		     struct iser_mem_reg *mem_reg);
 
 static struct iser_reg_ops fastreg_ops = {
 	.alloc_reg_res	= iser_alloc_fastreg_pool,
 	.free_reg_res	= iser_free_fastreg_pool,
-	.reg_rdma_mem	= iser_reg_rdma_mem_fastreg,
-	.unreg_rdma_mem	= iser_unreg_mem_fastreg,
+	.reg_mem	= iser_fast_reg_mr,
+	.unreg_mem	= iser_unreg_mem_fastreg,
 	.reg_desc_get	= iser_reg_desc_get_fr,
 	.reg_desc_put	= iser_reg_desc_put_fr,
 };
@@ -51,8 +61,8 @@ static struct iser_reg_ops fastreg_ops = {
 static struct iser_reg_ops fmr_ops = {
 	.alloc_reg_res	= iser_alloc_fmr_pool,
 	.free_reg_res	= iser_free_fmr_pool,
-	.reg_rdma_mem	= iser_reg_rdma_mem_fmr,
-	.unreg_rdma_mem	= iser_unreg_mem_fmr,
+	.reg_mem	= iser_fast_reg_fmr,
+	.unreg_mem	= iser_unreg_mem_fmr,
 	.reg_desc_get	= iser_reg_desc_get_fmr,
 	.reg_desc_put	= iser_reg_desc_put_fmr,
 };
@@ -574,62 +584,6 @@ void iser_unreg_mem_fastreg(struct iscsi_iser_task *iser_task,
 	reg->mem_h = NULL;
 }
 
-/**
- * iser_reg_rdma_mem_fmr - Registers memory intended for RDMA,
- * using FMR (if possible) obtaining rkey and va
- *
- * returns 0 on success, errno code on failure
- */
-int iser_reg_rdma_mem_fmr(struct iscsi_iser_task *iser_task,
-			  enum iser_data_dir cmd_dir)
-{
-	struct ib_conn *ib_conn = &iser_task->iser_conn->ib_conn;
-	struct iser_device   *device = ib_conn->device;
-	struct ib_device     *ibdev = device->ib_device;
-	struct iser_data_buf *mem = &iser_task->data[cmd_dir];
-	struct iser_mem_reg *mem_reg;
-	int aligned_len;
-	int err;
-	int i;
-
-	mem_reg = &iser_task->rdma_reg[cmd_dir];
-
-	aligned_len = iser_data_buf_aligned_len(mem, ibdev);
-	if (aligned_len != mem->dma_nents) {
-		err = fall_to_bounce_buf(iser_task, mem, cmd_dir);
-		if (err) {
-			iser_err("failed to allocate bounce buffer\n");
-			return err;
-		}
-	}
-
-	/* if there a single dma entry, FMR is not needed */
-	if (mem->dma_nents == 1) {
-		return iser_reg_dma(device, mem, mem_reg);
-	} else { /* use FMR for multiple dma entries */
-		struct iser_fr_desc *desc;
-
-		desc = device->reg_ops->reg_desc_get(ib_conn);
-		err = iser_fast_reg_fmr(iser_task, mem, &desc->rsc, mem_reg);
-		if (err && err != -EAGAIN) {
-			iser_data_buf_dump(mem, ibdev);
-			iser_err("mem->dma_nents = %d (dlength = 0x%x)\n",
-				 mem->dma_nents,
-				 ntoh24(iser_task->desc.iscsi_header.dlength));
-			iser_err("page_vec: data_size = 0x%x, length = %d, offset = 0x%x\n",
-				 desc->rsc.page_vec->data_size,
-				 desc->rsc.page_vec->length,
-				 desc->rsc.page_vec->offset);
-			for (i = 0; i < desc->rsc.page_vec->length; i++)
-				iser_err("page_vec[%d] = 0x%llx\n", i,
-					 (unsigned long long)desc->rsc.page_vec->pages[i]);
-		}
-		if (err)
-			return err;
-	}
-	return 0;
-}
-
 static void
 iser_set_dif_domain(struct scsi_cmnd *sc, struct ib_sig_attrs *sig_attrs,
 		    struct ib_sig_domain *domain)
@@ -775,19 +729,12 @@ static int iser_fast_reg_mr(struct iscsi_iser_task *iser_task,
 {
 	struct ib_conn *ib_conn = &iser_task->iser_conn->ib_conn;
 	struct iser_device *device = ib_conn->device;
-	struct ib_mr *mr;
-	struct ib_fast_reg_page_list *frpl;
+	struct ib_mr *mr = rsc->mr;
+	struct ib_fast_reg_page_list *frpl = rsc->frpl;
 	struct ib_send_wr fastreg_wr, inv_wr;
 	struct ib_send_wr *bad_wr, *wr = NULL;
 	int ret, offset, size, plen;
 
-	/* if there a single dma entry, dma mr suffices */
-	if (mem->dma_nents == 1)
-		return iser_reg_dma(device, mem, reg);
-
-	mr = rsc->mr;
-	frpl = rsc->frpl;
-
 	plen = iser_sg_to_page_vec(mem, device->ib_device, frpl->page_list,
 				   &offset, &size);
 	if (plen * SIZE_4K < size) {
@@ -834,78 +781,113 @@ static int iser_fast_reg_mr(struct iscsi_iser_task *iser_task,
 	return ret;
 }
 
-/**
- * iser_reg_rdma_mem_fastreg - Registers memory intended for RDMA,
- * using Fast Registration WR (if possible) obtaining rkey and va
- *
- * returns 0 on success, errno code on failure
- */
-int iser_reg_rdma_mem_fastreg(struct iscsi_iser_task *iser_task,
-			      enum iser_data_dir cmd_dir)
+static int
+iser_handle_unaligned_buf(struct iscsi_iser_task *task,
+			  struct iser_data_buf *mem,
+			  enum iser_data_dir dir)
 {
-	struct ib_conn *ib_conn = &iser_task->iser_conn->ib_conn;
-	struct iser_device *device = ib_conn->device;
-	struct ib_device *ibdev = device->ib_device;
-	struct iser_data_buf *mem = &iser_task->data[cmd_dir];
-	struct iser_mem_reg *mem_reg = &iser_task->rdma_reg[cmd_dir];
-	struct iser_fr_desc *desc = NULL;
+	struct iser_conn *iser_conn = task->iser_conn;
+	struct iser_device *device = iser_conn->ib_conn.device;
 	int err, aligned_len;
 
-	aligned_len = iser_data_buf_aligned_len(mem, ibdev);
+	aligned_len = iser_data_buf_aligned_len(mem, device->ib_device);
 	if (aligned_len != mem->dma_nents) {
-		err = fall_to_bounce_buf(iser_task, mem, cmd_dir);
-		if (err) {
-			iser_err("failed to allocate bounce buffer\n");
+		err = fall_to_bounce_buf(task, mem, dir);
+		if (err)
 			return err;
-		}
 	}
 
+	return 0;
+}
+
+static int
+iser_reg_prot_sg(struct iscsi_iser_task *task,
+		 struct iser_data_buf *mem,
+		 struct iser_fr_desc *desc,
+		 struct iser_mem_reg *reg)
+{
+	struct iser_device *device = task->iser_conn->ib_conn.device;
+
+	if (mem->dma_nents == 1)
+		return iser_reg_dma(device, mem, reg);
+
+	return device->reg_ops->reg_mem(task, mem, &desc->pi_ctx->rsc, reg);
+}
+
+static int
+iser_reg_data_sg(struct iscsi_iser_task *task,
+		 struct iser_data_buf *mem,
+		 struct iser_fr_desc *desc,
+		 struct iser_mem_reg *reg)
+{
+	struct iser_device *device = task->iser_conn->ib_conn.device;
+
+	if (mem->dma_nents == 1)
+		return iser_reg_dma(device, mem, reg);
+
+	return device->reg_ops->reg_mem(task, mem, &desc->rsc, reg);
+}
+
+int iser_reg_rdma_mem(struct iscsi_iser_task *task,
+		      enum iser_data_dir dir)
+{
+	struct ib_conn *ib_conn = &task->iser_conn->ib_conn;
+	struct iser_device *device = ib_conn->device;
+	struct iser_data_buf *mem = &task->data[dir];
+	struct iser_mem_reg *reg = &task->rdma_reg[dir];
+	struct iser_fr_desc *desc = NULL;
+	int err;
+
+	err = iser_handle_unaligned_buf(task, mem, dir);
+	if (unlikely(err))
+		return err;
+
 	if (mem->dma_nents != 1 ||
-	    scsi_get_prot_op(iser_task->sc) != SCSI_PROT_NORMAL) {
+	    scsi_get_prot_op(task->sc) != SCSI_PROT_NORMAL) {
 		desc = device->reg_ops->reg_desc_get(ib_conn);
-		mem_reg->mem_h = desc;
+		reg->mem_h = desc;
 	}
 
-	err = iser_fast_reg_mr(iser_task, mem,
-			       desc ? &desc->rsc : NULL, mem_reg);
-	if (err)
+	err = iser_reg_data_sg(task, mem, desc, reg);
+	if (unlikely(err))
 		goto err_reg;
 
-	if (scsi_get_prot_op(iser_task->sc) != SCSI_PROT_NORMAL) {
+	if (scsi_get_prot_op(task->sc) != SCSI_PROT_NORMAL) {
 		struct iser_mem_reg prot_reg;
 
 		memset(&prot_reg, 0, sizeof(prot_reg));
-		if (scsi_prot_sg_count(iser_task->sc)) {
-			mem = &iser_task->prot[cmd_dir];
-			aligned_len = iser_data_buf_aligned_len(mem, ibdev);
-			if (aligned_len != mem->dma_nents) {
-				err = fall_to_bounce_buf(iser_task, mem,
-							 cmd_dir);
-				if (err) {
-					iser_err("failed to allocate bounce buffer\n");
-					return err;
-				}
-			}
+		if (scsi_prot_sg_count(task->sc)) {
+			mem = &task->prot[dir];
+			err = iser_handle_unaligned_buf(task, mem, dir);
+			if (unlikely(err))
+				goto err_reg;
 
-			err = iser_fast_reg_mr(iser_task, mem,
-					       &desc->pi_ctx->rsc, &prot_reg);
-			if (err)
+			err = iser_reg_prot_sg(task, mem, desc, &prot_reg);
+			if (unlikely(err))
 				goto err_reg;
 		}
 
-		err = iser_reg_sig_mr(iser_task, desc->pi_ctx, mem_reg,
-				      &prot_reg, mem_reg);
-		if (err) {
-			iser_err("Failed to register signature mr\n");
-			return err;
-		}
+		err = iser_reg_sig_mr(task, desc->pi_ctx, reg,
+				      &prot_reg, reg);
+		if (unlikely(err))
+			goto err_reg;
+
 		desc->pi_ctx->sig_protected = 1;
 	}
 
 	return 0;
+
 err_reg:
 	if (desc)
 		device->reg_ops->reg_desc_put(ib_conn, desc);
 
 	return err;
 }
+
+void iser_unreg_rdma_mem(struct iscsi_iser_task *task,
+			 enum iser_data_dir dir)
+{
+	struct iser_device *device = task->iser_conn->ib_conn.device;
+
+	device->reg_ops->unreg_mem(task, dir);
+}
-- 
1.8.4.3

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

[PATCH 19/22] IB/iser: Pass registration pool a size parameter

[Sagi Grimberg]

Hard coded for now. This will allow to allocate different
sized MRs depending on the IO size needed (and device
capabilities).

This patch does not change any functionality.

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iscsi_iser.h     | 11 +++++--
 drivers/infiniband/ulp/iser/iser_initiator.c |  3 +-
 drivers/infiniband/ulp/iser/iser_verbs.c     | 47 +++++++++++++++-------------
 3 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index e6105d2..e9ebe0b 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -339,7 +339,8 @@ struct iser_comp {
  */
 struct iser_reg_ops {
 	int            (*alloc_reg_res)(struct ib_conn *ib_conn,
-					unsigned cmds_max);
+					unsigned cmds_max,
+					unsigned int size);
 	void           (*free_reg_res)(struct ib_conn *ib_conn);
 	int            (*reg_mem)(struct iscsi_iser_task *iser_task,
 				  struct iser_data_buf *mem,
@@ -658,9 +659,13 @@ int  iser_initialize_task_headers(struct iscsi_task *task,
 			struct iser_tx_desc *tx_desc);
 int iser_alloc_rx_descriptors(struct iser_conn *iser_conn,
 			      struct iscsi_session *session);
-int iser_alloc_fmr_pool(struct ib_conn *ib_conn, unsigned cmds_max);
+int iser_alloc_fmr_pool(struct ib_conn *ib_conn,
+			unsigned cmds_max,
+			unsigned int size);
 void iser_free_fmr_pool(struct ib_conn *ib_conn);
-int iser_alloc_fastreg_pool(struct ib_conn *ib_conn, unsigned cmds_max);
+int iser_alloc_fastreg_pool(struct ib_conn *ib_conn,
+			    unsigned cmds_max,
+			    unsigned int size);
 void iser_free_fastreg_pool(struct ib_conn *ib_conn);
 u8 iser_check_task_pi_status(struct iscsi_iser_task *iser_task,
 			     enum iser_data_dir cmd_dir, sector_t *sector);
diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c
index cef6c82..268a3d6 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -258,7 +258,8 @@ int iser_alloc_rx_descriptors(struct iser_conn *iser_conn,
 	iser_conn->qp_max_recv_dtos_mask = session->cmds_max - 1; /* cmds_max is 2^N */
 	iser_conn->min_posted_rx = iser_conn->qp_max_recv_dtos >> 2;
 
-	if (device->reg_ops->alloc_reg_res(ib_conn, session->scsi_cmds_max))
+	if (device->reg_ops->alloc_reg_res(ib_conn, session->scsi_cmds_max,
+					   ISCSI_ISER_SG_TABLESIZE + 1))
 		goto create_rdma_reg_res_failed;
 
 	if (iser_alloc_login_buf(iser_conn))
diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index 6b464e6..fa778ba 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -199,7 +199,9 @@ static void iser_free_device_ib_res(struct iser_device *device)
  *
  * returns 0 on success, or errno code on failure
  */
-int iser_alloc_fmr_pool(struct ib_conn *ib_conn, unsigned cmds_max)
+int iser_alloc_fmr_pool(struct ib_conn *ib_conn,
+			unsigned cmds_max,
+			unsigned int size)
 {
 	struct iser_device *device = ib_conn->device;
 	struct iser_fr_pool *fr_pool = &ib_conn->fr_pool;
@@ -216,8 +218,7 @@ int iser_alloc_fmr_pool(struct ib_conn *ib_conn, unsigned cmds_max)
 	if (!desc)
 		return -ENOMEM;
 
-	page_vec = kmalloc(sizeof(*page_vec) +
-			   (sizeof(u64) * (ISCSI_ISER_SG_TABLESIZE + 1)),
+	page_vec = kmalloc(sizeof(*page_vec) + (sizeof(u64) * size),
 			   GFP_KERNEL);
 	if (!page_vec) {
 		ret = -ENOMEM;
@@ -227,9 +228,7 @@ int iser_alloc_fmr_pool(struct ib_conn *ib_conn, unsigned cmds_max)
 	page_vec->pages = (u64 *)(page_vec + 1);
 
 	params.page_shift        = SHIFT_4K;
-	/* when the first/last SG element are not start/end *
-	 * page aligned, the map whould be of N+1 pages     */
-	params.max_pages_per_fmr = ISCSI_ISER_SG_TABLESIZE + 1;
+	params.max_pages_per_fmr = size;
 	/* make the pool size twice the max number of SCSI commands *
 	 * the ML is expected to queue, watermark for unmap at 50%  */
 	params.pool_size	 = cmds_max * 2;
@@ -282,13 +281,14 @@ void iser_free_fmr_pool(struct ib_conn *ib_conn)
 }
 
 static int
-iser_alloc_reg_res(struct ib_device *ib_device, struct ib_pd *pd,
-		   struct iser_reg_resources *res)
+iser_alloc_reg_res(struct ib_device *ib_device,
+		   struct ib_pd *pd,
+		   struct iser_reg_resources *res,
+		   unsigned int size)
 {
 	int ret;
 
-	res->frpl = ib_alloc_fast_reg_page_list(ib_device,
-						ISCSI_ISER_SG_TABLESIZE + 1);
+	res->frpl = ib_alloc_fast_reg_page_list(ib_device, size);
 	if (IS_ERR(res->frpl)) {
 		ret = PTR_ERR(res->frpl);
 		iser_err("Failed to allocate ib_fast_reg_page_list err=%d\n",
@@ -296,8 +296,7 @@ iser_alloc_reg_res(struct ib_device *ib_device, struct ib_pd *pd,
 		return PTR_ERR(res->frpl);
 	}
 
-	res->mr = ib_alloc_mr(pd, IB_MR_TYPE_MEM_REG,
-			      ISCSI_ISER_SG_TABLESIZE + 1);
+	res->mr = ib_alloc_mr(pd, IB_MR_TYPE_MEM_REG, size);
 	if (IS_ERR(res->mr)) {
 		ret = PTR_ERR(res->mr);
 		iser_err("Failed to allocate ib_fast_reg_mr err=%d\n", ret);
@@ -321,8 +320,10 @@ iser_free_reg_res(struct iser_reg_resources *rsc)
 }
 
 static int
-iser_alloc_pi_ctx(struct ib_device *ib_device, struct ib_pd *pd,
-		  struct iser_fr_desc *desc)
+iser_alloc_pi_ctx(struct ib_device *ib_device,
+		  struct ib_pd *pd,
+		  struct iser_fr_desc *desc,
+		  unsigned int size)
 {
 	struct iser_pi_context *pi_ctx = NULL;
 	int ret;
@@ -333,7 +334,7 @@ iser_alloc_pi_ctx(struct ib_device *ib_device, struct ib_pd *pd,
 
 	pi_ctx = desc->pi_ctx;
 
-	ret = iser_alloc_reg_res(ib_device, pd, &pi_ctx->rsc);
+	ret = iser_alloc_reg_res(ib_device, pd, &pi_ctx->rsc, size);
 	if (ret) {
 		iser_err("failed to allocate reg_resources\n");
 		goto alloc_reg_res_err;
@@ -366,8 +367,10 @@ iser_free_pi_ctx(struct iser_pi_context *pi_ctx)
 }
 
 static struct iser_fr_desc *
-iser_create_fastreg_desc(struct ib_device *ib_device, struct ib_pd *pd,
-			 bool pi_enable)
+iser_create_fastreg_desc(struct ib_device *ib_device,
+			 struct ib_pd *pd,
+			 bool pi_enable,
+			 unsigned int size)
 {
 	struct iser_fr_desc *desc;
 	int ret;
@@ -376,12 +379,12 @@ iser_create_fastreg_desc(struct ib_device *ib_device, struct ib_pd *pd,
 	if (!desc)
 		return ERR_PTR(-ENOMEM);
 
-	ret = iser_alloc_reg_res(ib_device, pd, &desc->rsc);
+	ret = iser_alloc_reg_res(ib_device, pd, &desc->rsc, size);
 	if (ret)
 		goto reg_res_alloc_failure;
 
 	if (pi_enable) {
-		ret = iser_alloc_pi_ctx(ib_device, pd, desc);
+		ret = iser_alloc_pi_ctx(ib_device, pd, desc, size);
 		if (ret)
 			goto pi_ctx_alloc_failure;
 	}
@@ -401,7 +404,9 @@ reg_res_alloc_failure:
  * for fast registration work requests.
  * returns 0 on success, or errno code on failure
  */
-int iser_alloc_fastreg_pool(struct ib_conn *ib_conn, unsigned cmds_max)
+int iser_alloc_fastreg_pool(struct ib_conn *ib_conn,
+			    unsigned cmds_max,
+			    unsigned int size)
 {
 	struct iser_device *device = ib_conn->device;
 	struct iser_fr_pool *fr_pool = &ib_conn->fr_pool;
@@ -413,7 +418,7 @@ int iser_alloc_fastreg_pool(struct ib_conn *ib_conn, unsigned cmds_max)
 	fr_pool->size = 0;
 	for (i = 0; i < cmds_max; i++) {
 		desc = iser_create_fastreg_desc(device->ib_device, device->pd,
-						ib_conn->pi_support);
+						ib_conn->pi_support, size);
 		if (IS_ERR(desc)) {
 			ret = PTR_ERR(desc);
 			goto err;
-- 
1.8.4.3

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

[PATCH 20/22] IB/iser: Support up to 8MB data transfer in a single command

[Sagi Grimberg]

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 <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 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         (0x80000 >> 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_4K)
+
 #define ISER_DEF_XMIT_CMDS_DEFAULT		512
 #if ISCSI_DEF_XMIT_CMDS_MAX > ISER_DEF_XMIT_CMDS_DEFAULT
 	#define ISER_DEF_XMIT_CMDS_MAX		ISCSI_DEF_XMIT_CMDS_MAX
@@ -504,6 +509,8 @@ struct ib_conn {
  * @rx_desc_head:     head of rx_descs cyclic buffer
  * @rx_descs:         rx buffers array (cyclic buffer)
  * @num_rx_descs:     number of rx descriptors
+ * @scsi_sg_tablesize: scsi host sg_tablesize
+ * @scsi_max_sectors: scsi host max sectors
  */
 struct iser_conn {
 	struct ib_conn		     ib_conn;
@@ -528,6 +535,8 @@ struct iser_conn {
 	unsigned int 		     rx_desc_head;
 	struct iser_rx_desc	     *rx_descs;
 	u32                          num_rx_descs;
+	unsigned short               scsi_sg_tablesize;
+	unsigned int                 scsi_max_sectors;
 };
 
 /**
@@ -583,6 +592,7 @@ extern struct iser_global ig;
 extern int iser_debug_level;
 extern bool iser_pi_enable;
 extern int iser_pi_guard;
+extern unsigned int iser_max_sectors;
 
 int iser_assign_reg_ops(struct iser_device *device);
 
diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c
index 268a3d6..d511879 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -259,7 +259,7 @@ int iser_alloc_rx_descriptors(struct iser_conn *iser_conn,
 	iser_conn->min_posted_rx = iser_conn->qp_max_recv_dtos >> 2;
 
 	if (device->reg_ops->alloc_reg_res(ib_conn, session->scsi_cmds_max,
-					   ISCSI_ISER_SG_TABLESIZE + 1))
+					   iser_conn->scsi_sg_tablesize))
 		goto create_rdma_reg_res_failed;
 
 	if (iser_alloc_login_buf(iser_conn))
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index 5e807ba..cbf6152 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -363,7 +363,8 @@ static int iser_sg_to_page_vec(struct iser_data_buf *data,
  * consecutive SG elements are actually fragments of the same physcial page.
  */
 static int iser_data_buf_aligned_len(struct iser_data_buf *data,
-				      struct ib_device *ibdev)
+				     struct ib_device *ibdev,
+				     unsigned sg_tablesize)
 {
 	struct scatterlist *sg, *sgl, *next_sg = NULL;
 	u64 start_addr, end_addr;
@@ -375,6 +376,14 @@ static int iser_data_buf_aligned_len(struct iser_data_buf *data,
 	sgl = data->sg;
 	start_addr  = ib_sg_dma_address(ibdev, sgl);
 
+	if (unlikely(sgl[0].offset &&
+		     data->data_len >= sg_tablesize * PAGE_SIZE)) {
+		iser_dbg("can't register length %lx with offset %x "
+			 "fall to bounce buffer\n", data->data_len,
+			 sgl[0].offset);
+		return 0;
+	}
+
 	for_each_sg(sgl, sg, data->dma_nents, i) {
 		if (start_check && !IS_4K_ALIGNED(start_addr))
 			break;
@@ -790,7 +799,8 @@ iser_handle_unaligned_buf(struct iscsi_iser_task *task,
 	struct iser_device *device = iser_conn->ib_conn.device;
 	int err, aligned_len;
 
-	aligned_len = iser_data_buf_aligned_len(mem, device->ib_device);
+	aligned_len = iser_data_buf_aligned_len(mem, device->ib_device,
+						iser_conn->scsi_sg_tablesize);
 	if (aligned_len != mem->dma_nents) {
 		err = fall_to_bounce_buf(task, mem, dir);
 		if (err)
diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index fa778ba..f69cee7 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -756,6 +756,31 @@ static void iser_connect_error(struct rdma_cm_id *cma_id)
 	iser_conn->state = ISER_CONN_TERMINATING;
 }
 
+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;
+	}
+
+	iser_conn->scsi_sg_tablesize = sg_tablesize;
+
+	iser_dbg("iser_conn %p, sg_tablesize %u, max_sectors %u\n",
+		 iser_conn, iser_conn->scsi_sg_tablesize,
+		 iser_conn->scsi_max_sectors);
+}
+
 /**
  * Called with state mutex held
  **/
@@ -794,6 +819,8 @@ static void iser_addr_handler(struct rdma_cm_id *cma_id)
 		}
 	}
 
+	iser_calc_scsi_params(iser_conn, iser_max_sectors);
+
 	ret = rdma_resolve_route(cma_id, 1000);
 	if (ret) {
 		iser_err("resolve route failed: %d\n", ret);
-- 
1.8.4.3

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

[PATCH 21/22] IB/iser: Add debug prints to the various memory registration methods

[Sagi Grimberg]

Easier to debug when we have the registration details.

This patch does not change any functionality.

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Adir Lev <adirl-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iser_memory.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index cbf6152..750f03f 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -554,6 +554,10 @@ int iser_fast_reg_fmr(struct iscsi_iser_task *iser_task,
 	reg->sge.length = page_vec->data_size;
 	reg->mem_h = fmr;
 
+	iser_dbg("fmr reg: lkey=0x%x, rkey=0x%x, addr=0x%llx,"
+		 " length=0x%x\n", reg->sge.lkey, reg->rkey,
+		 reg->sge.addr, reg->sge.length);
+
 	return 0;
 }
 
@@ -724,7 +728,7 @@ iser_reg_sig_mr(struct iscsi_iser_task *iser_task,
 	sig_reg->sge.addr = 0;
 	sig_reg->sge.length = scsi_transfer_length(iser_task->sc);
 
-	iser_dbg("sig_sge: lkey: 0x%x, rkey: 0x%x, addr: 0x%llx, length: %u\n",
+	iser_dbg("sig reg: lkey: 0x%x, rkey: 0x%x, addr: 0x%llx, length: %u\n",
 		 sig_reg->sge.lkey, sig_reg->rkey, sig_reg->sge.addr,
 		 sig_reg->sge.length);
 err:
@@ -787,6 +791,10 @@ static int iser_fast_reg_mr(struct iscsi_iser_task *iser_task,
 	reg->sge.addr = frpl->page_list[0] + offset;
 	reg->sge.length = size;
 
+	iser_dbg("fast reg: lkey=0x%x, rkey=0x%x, addr=0x%llx,"
+		 " length=0x%x\n", reg->sge.lkey, reg->rkey,
+		 reg->sge.addr, reg->sge.length);
+
 	return ret;
 }
 
-- 
1.8.4.3

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

[PATCH 22/22] IB/iser: Chain all iser transaction send work requests

[Sagi Grimberg]

Concatination of send work requests benefits performance
by reducing the send queue lock contention (acquired in
ib_post_send) and saves us HW doorbells which is posted
only once.

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c  |   1 +
 drivers/infiniband/ulp/iser/iscsi_iser.h  |  34 +++++++++
 drivers/infiniband/ulp/iser/iser_memory.c | 120 +++++++++++++-----------------
 drivers/infiniband/ulp/iser/iser_verbs.c  |  21 +++---
 4 files changed, 99 insertions(+), 77 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 9eeefc8..ec87ce1 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -204,6 +204,7 @@ iser_initialize_task_headers(struct iscsi_task *task,
 		goto out;
 	}
 
+	tx_desc->wr_idx = 0;
 	tx_desc->mapped = true;
 	tx_desc->dma_addr = dma_addr;
 	tx_desc->tx_sg[0].addr   = tx_desc->dma_addr;
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 8a32e20..4af1916 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -265,6 +265,14 @@ enum iser_desc_type {
 	ISCSI_TX_DATAOUT
 };
 
+/* Maximum number of work requests per task:
+ * Data memory region local invalidate + fast registration
+ * Protection memory region local invalidate + fast registration
+ * Signature memory region local invalidate + fast registration
+ * PDU send
+ */
+#define ISER_MAX_WRS 7
+
 /**
  * struct iser_tx_desc - iSER TX descriptor (for send wr_id)
  *
@@ -277,6 +285,11 @@ enum iser_desc_type {
  *                 unsolicited data-out or control
  * @num_sge:       number sges used on this TX task
  * @mapped:        Is the task header mapped
+ * @wr_idx:        Current WR index
+ * @wrs:           Array of WRs per task
+ * @data_reg:      Data buffer registration details
+ * @prot_reg:      Protection buffer registration details
+ * @sig_attrs:     Signature attributes
  */
 struct iser_tx_desc {
 	struct iser_hdr              iser_header;
@@ -286,6 +299,11 @@ struct iser_tx_desc {
 	struct ib_sge		     tx_sg[2];
 	int                          num_sge;
 	bool			     mapped;
+	u8                           wr_idx;
+	struct ib_send_wr            wrs[ISER_MAX_WRS];
+	struct iser_mem_reg          data_reg;
+	struct iser_mem_reg          prot_reg;
+	struct ib_sig_attrs          sig_attrs;
 };
 
 #define ISER_RX_PAD_SIZE	(256 - (ISER_RX_PAYLOAD_SIZE + \
@@ -689,4 +707,20 @@ iser_reg_desc_get_fmr(struct ib_conn *ib_conn);
 void
 iser_reg_desc_put_fmr(struct ib_conn *ib_conn,
 		      struct iser_fr_desc *desc);
+
+static inline struct ib_send_wr *
+iser_tx_next_wr(struct iser_tx_desc *tx_desc)
+{
+	struct ib_send_wr *cur_wr = &tx_desc->wrs[tx_desc->wr_idx];
+	struct ib_send_wr *last_wr;
+
+	if (tx_desc->wr_idx) {
+		last_wr = &tx_desc->wrs[tx_desc->wr_idx - 1];
+		last_wr->next = cur_wr;
+	}
+	tx_desc->wr_idx++;
+
+	return cur_wr;
+}
+
 #endif
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index 750f03f..2493cc7 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -664,10 +664,11 @@ iser_inv_rkey(struct ib_send_wr *inv_wr, struct ib_mr *mr)
 {
 	u32 rkey;
 
-	memset(inv_wr, 0, sizeof(*inv_wr));
 	inv_wr->opcode = IB_WR_LOCAL_INV;
 	inv_wr->wr_id = ISER_FASTREG_LI_WRID;
 	inv_wr->ex.invalidate_rkey = mr->rkey;
+	inv_wr->send_flags = 0;
+	inv_wr->num_sge = 0;
 
 	rkey = ib_inc_rkey(mr->rkey);
 	ib_update_fast_reg_key(mr, rkey);
@@ -680,47 +681,38 @@ iser_reg_sig_mr(struct iscsi_iser_task *iser_task,
 		struct iser_mem_reg *prot_reg,
 		struct iser_mem_reg *sig_reg)
 {
-	struct ib_conn *ib_conn = &iser_task->iser_conn->ib_conn;
-	struct ib_send_wr sig_wr, inv_wr;
-	struct ib_send_wr *bad_wr, *wr = NULL;
-	struct ib_sig_attrs sig_attrs;
+	struct iser_tx_desc *tx_desc = &iser_task->desc;
+	struct ib_sig_attrs *sig_attrs = &tx_desc->sig_attrs;
+	struct ib_send_wr *wr;
 	int ret;
 
-	memset(&sig_attrs, 0, sizeof(sig_attrs));
-	ret = iser_set_sig_attrs(iser_task->sc, &sig_attrs);
+	memset(sig_attrs, 0, sizeof(*sig_attrs));
+	ret = iser_set_sig_attrs(iser_task->sc, sig_attrs);
 	if (ret)
 		goto err;
 
-	iser_set_prot_checks(iser_task->sc, &sig_attrs.check_mask);
+	iser_set_prot_checks(iser_task->sc, &sig_attrs->check_mask);
 
 	if (!pi_ctx->sig_mr_valid) {
-		iser_inv_rkey(&inv_wr, pi_ctx->sig_mr);
-		wr = &inv_wr;
+		wr = iser_tx_next_wr(tx_desc);
+		iser_inv_rkey(wr, pi_ctx->sig_mr);
 	}
 
-	memset(&sig_wr, 0, sizeof(sig_wr));
-	sig_wr.opcode = IB_WR_REG_SIG_MR;
-	sig_wr.wr_id = ISER_FASTREG_LI_WRID;
-	sig_wr.sg_list = &data_reg->sge;
-	sig_wr.num_sge = 1;
-	sig_wr.wr.sig_handover.sig_attrs = &sig_attrs;
-	sig_wr.wr.sig_handover.sig_mr = pi_ctx->sig_mr;
+	wr = iser_tx_next_wr(tx_desc);
+	wr->opcode = IB_WR_REG_SIG_MR;
+	wr->wr_id = ISER_FASTREG_LI_WRID;
+	wr->sg_list = &data_reg->sge;
+	wr->num_sge = 1;
+	wr->send_flags = 0;
+	wr->wr.sig_handover.sig_attrs = sig_attrs;
+	wr->wr.sig_handover.sig_mr = pi_ctx->sig_mr;
 	if (scsi_prot_sg_count(iser_task->sc))
-		sig_wr.wr.sig_handover.prot = &prot_reg->sge;
-	sig_wr.wr.sig_handover.access_flags = IB_ACCESS_LOCAL_WRITE |
-					      IB_ACCESS_REMOTE_READ |
-					      IB_ACCESS_REMOTE_WRITE;
-
-	if (!wr)
-		wr = &sig_wr;
+		wr->wr.sig_handover.prot = &prot_reg->sge;
 	else
-		wr->next = &sig_wr;
-
-	ret = ib_post_send(ib_conn->qp, wr, &bad_wr);
-	if (ret) {
-		iser_err("reg_sig_mr failed, ret:%d\n", ret);
-		goto err;
-	}
+		wr->wr.sig_handover.prot = NULL;
+	wr->wr.sig_handover.access_flags = IB_ACCESS_LOCAL_WRITE |
+					   IB_ACCESS_REMOTE_READ |
+					   IB_ACCESS_REMOTE_WRITE;
 	pi_ctx->sig_mr_valid = 0;
 
 	sig_reg->sge.lkey = pi_ctx->sig_mr->lkey;
@@ -744,9 +736,9 @@ static int iser_fast_reg_mr(struct iscsi_iser_task *iser_task,
 	struct iser_device *device = ib_conn->device;
 	struct ib_mr *mr = rsc->mr;
 	struct ib_fast_reg_page_list *frpl = rsc->frpl;
-	struct ib_send_wr fastreg_wr, inv_wr;
-	struct ib_send_wr *bad_wr, *wr = NULL;
-	int ret, offset, size, plen;
+	struct iser_tx_desc *tx_desc = &iser_task->desc;
+	struct ib_send_wr *wr;
+	int offset, size, plen;
 
 	plen = iser_sg_to_page_vec(mem, device->ib_device, frpl->page_list,
 				   &offset, &size);
@@ -756,34 +748,23 @@ static int iser_fast_reg_mr(struct iscsi_iser_task *iser_task,
 	}
 
 	if (!rsc->mr_valid) {
-		iser_inv_rkey(&inv_wr, mr);
-		wr = &inv_wr;
+		wr = iser_tx_next_wr(tx_desc);
+		iser_inv_rkey(wr, mr);
 	}
 
-	/* Prepare FASTREG WR */
-	memset(&fastreg_wr, 0, sizeof(fastreg_wr));
-	fastreg_wr.wr_id = ISER_FASTREG_LI_WRID;
-	fastreg_wr.opcode = IB_WR_FAST_REG_MR;
-	fastreg_wr.wr.fast_reg.iova_start = frpl->page_list[0] + offset;
-	fastreg_wr.wr.fast_reg.page_list = frpl;
-	fastreg_wr.wr.fast_reg.page_list_len = plen;
-	fastreg_wr.wr.fast_reg.page_shift = SHIFT_4K;
-	fastreg_wr.wr.fast_reg.length = size;
-	fastreg_wr.wr.fast_reg.rkey = mr->rkey;
-	fastreg_wr.wr.fast_reg.access_flags = (IB_ACCESS_LOCAL_WRITE  |
-					       IB_ACCESS_REMOTE_WRITE |
-					       IB_ACCESS_REMOTE_READ);
-
-	if (!wr)
-		wr = &fastreg_wr;
-	else
-		wr->next = &fastreg_wr;
-
-	ret = ib_post_send(ib_conn->qp, wr, &bad_wr);
-	if (ret) {
-		iser_err("fast registration failed, ret:%d\n", ret);
-		return ret;
-	}
+	wr = iser_tx_next_wr(tx_desc);
+	wr->opcode = IB_WR_FAST_REG_MR;
+	wr->wr_id = ISER_FASTREG_LI_WRID;
+	wr->send_flags = 0;
+	wr->wr.fast_reg.iova_start = frpl->page_list[0] + offset;
+	wr->wr.fast_reg.page_list = frpl;
+	wr->wr.fast_reg.page_list_len = plen;
+	wr->wr.fast_reg.page_shift = SHIFT_4K;
+	wr->wr.fast_reg.length = size;
+	wr->wr.fast_reg.rkey = mr->rkey;
+	wr->wr.fast_reg.access_flags = (IB_ACCESS_LOCAL_WRITE  |
+					IB_ACCESS_REMOTE_WRITE |
+					IB_ACCESS_REMOTE_READ);
 	rsc->mr_valid = 0;
 
 	reg->sge.lkey = mr->lkey;
@@ -795,7 +776,7 @@ static int iser_fast_reg_mr(struct iscsi_iser_task *iser_task,
 		 " length=0x%x\n", reg->sge.lkey, reg->rkey,
 		 reg->sge.addr, reg->sge.length);
 
-	return ret;
+	return 0;
 }
 
 static int
@@ -853,6 +834,7 @@ int iser_reg_rdma_mem(struct iscsi_iser_task *task,
 	struct iser_device *device = ib_conn->device;
 	struct iser_data_buf *mem = &task->data[dir];
 	struct iser_mem_reg *reg = &task->rdma_reg[dir];
+	struct iser_mem_reg *data_reg;
 	struct iser_fr_desc *desc = NULL;
 	int err;
 
@@ -866,27 +848,31 @@ int iser_reg_rdma_mem(struct iscsi_iser_task *task,
 		reg->mem_h = desc;
 	}
 
-	err = iser_reg_data_sg(task, mem, desc, reg);
+	if (scsi_get_prot_op(task->sc) == SCSI_PROT_NORMAL)
+		data_reg = reg;
+	else
+		data_reg = &task->desc.data_reg;
+
+	err = iser_reg_data_sg(task, mem, desc, data_reg);
 	if (unlikely(err))
 		goto err_reg;
 
 	if (scsi_get_prot_op(task->sc) != SCSI_PROT_NORMAL) {
-		struct iser_mem_reg prot_reg;
+		struct iser_mem_reg *prot_reg = &task->desc.prot_reg;
 
-		memset(&prot_reg, 0, sizeof(prot_reg));
 		if (scsi_prot_sg_count(task->sc)) {
 			mem = &task->prot[dir];
 			err = iser_handle_unaligned_buf(task, mem, dir);
 			if (unlikely(err))
 				goto err_reg;
 
-			err = iser_reg_prot_sg(task, mem, desc, &prot_reg);
+			err = iser_reg_prot_sg(task, mem, desc, prot_reg);
 			if (unlikely(err))
 				goto err_reg;
 		}
 
-		err = iser_reg_sig_mr(task, desc->pi_ctx, reg,
-				      &prot_reg, reg);
+		err = iser_reg_sig_mr(task, desc->pi_ctx, data_reg,
+				      prot_reg, reg);
 		if (unlikely(err))
 			goto err_reg;
 
diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index f69cee7..ad2d2b5 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -1116,23 +1116,24 @@ int iser_post_recvm(struct iser_conn *iser_conn, int count)
 int iser_post_send(struct ib_conn *ib_conn, struct iser_tx_desc *tx_desc,
 		   bool signal)
 {
-	int		  ib_ret;
-	struct ib_send_wr send_wr, *send_wr_failed;
+	struct ib_send_wr *bad_wr, *wr = iser_tx_next_wr(tx_desc);
+	int ib_ret;
 
 	ib_dma_sync_single_for_device(ib_conn->device->ib_device,
 				      tx_desc->dma_addr, ISER_HEADERS_LEN,
 				      DMA_TO_DEVICE);
 
-	send_wr.next	   = NULL;
-	send_wr.wr_id	   = (uintptr_t)tx_desc;
-	send_wr.sg_list	   = tx_desc->tx_sg;
-	send_wr.num_sge	   = tx_desc->num_sge;
-	send_wr.opcode	   = IB_WR_SEND;
-	send_wr.send_flags = signal ? IB_SEND_SIGNALED : 0;
+	wr->next = NULL;
+	wr->wr_id = (uintptr_t)tx_desc;
+	wr->sg_list = tx_desc->tx_sg;
+	wr->num_sge = tx_desc->num_sge;
+	wr->opcode = IB_WR_SEND;
+	wr->send_flags = signal ? IB_SEND_SIGNALED : 0;
 
-	ib_ret = ib_post_send(ib_conn->qp, &send_wr, &send_wr_failed);
+	ib_ret = ib_post_send(ib_conn->qp, &tx_desc->wrs[0], &bad_wr);
 	if (ib_ret)
-		iser_err("ib_post_send failed, ret:%d\n", ib_ret);
+		iser_err("ib_post_send failed, ret:%d opcode:%d\n",
+			 ib_ret, bad_wr->opcode);
 
 	return ib_ret;
 }
-- 
1.8.4.3

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

Re: [PATCH 05/22] IB/iser: Get rid of un-maintained counters

[Or Gerlitz]

On Thu, Jul 30, 2015 at 11:06 AM, Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> We don't update those anywhere in the code and they
> seem pretty useless (no one seem to care about those).
>
> qp_tx_queue_full: We never should get this

why?

> fmr_map_not_avail: We can never get to this

why? if for some reason fmr pool thread which does unmapping on the
background is slowed/delayed? or the
device firmware/hardware is in troubles?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.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

[Or Gerlitz]

On Thu, Jul 30, 2015 at 11:06 AM, Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/22] IB/iser: Fix possible bogus DMA unmapping

[Or Gerlitz]

On Thu, Jul 30, 2015 at 11:06 AM, Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> If iser_initialize_task_headers() routine failed before
> dma mapping, we should not attempt to unmap in cleanup_task().

This fixes some specific commit? if yes, add Fixes: line, if not, is
that from day-0?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 22/22] IB/iser: Chain all iser transaction send work requests

[Or Gerlitz]

On Thu, Jul 30, 2015 at 11:06 AM, Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> Concatination of send work requests benefits performance
> by reducing the send queue lock contention (acquired in
> ib_post_send) and saves us HW doorbells which is posted
> only once.

s/Concatination/Concatenation/

AFAIK,  do we today! isn't that the case? if partially, please specify
in the change-logs
what flows were not fully optimized in that respect and are such after
the patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 11/22] IB/iser: Remove dead code in fmr_pool alloc/free

[Or Gerlitz]

On Thu, Jul 30, 2015 at 11:06 AM, Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> In the past the we always tried to allocate an fmr_pool
> and if it failed on ENOSYS (not supported) then we continued
> with dma mr. This is not the case anymore and if we tried to
> allocate an fmr_pool then it is supported and we expect to succeed.

AFAIK, the ENOSYS flow was something that came into play when working
e.g over VF drivers such as mlx4 that don't support fmr-ing but we still wanted
an optimal performance. What does "this is not the case anymore" means? these
VF drivers are still out there.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/22] IB/iser: Get rid of un-maintained counters

[Sagi Grimberg]

On 7/30/2015 1:20 PM, Or Gerlitz wrote:
> On Thu, Jul 30, 2015 at 11:06 AM, Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>> We don't update those anywhere in the code and they
>> seem pretty useless (no one seem to care about those).
>>
>> qp_tx_queue_full: We never should get this
>
> why?

We reserve slots in our queue-pair based on the maximum inflight IOs.
And we lower the user's cmds_max to our queue-pair size.

So we never get there - also the code never touches this counter.

>
>> fmr_map_not_avail: We can never get to this
>
> why? if for some reason fmr pool thread which does unmapping on the
> background is slowed/delayed? or the
> device firmware/hardware is in troubles?

For FMRs we allocate x2 FMRs to absorb the background unmapping.
Besides, We don't increment this in any place in the code...

I don't see a point in keeping parameters that are not maintained. 
Actually I never noticed they were there... a quick git searched didn't
show any usage of those ever.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/22] IB/iser: Fix possible bogus DMA unmapping

[Sagi Grimberg]

On 7/30/2015 1:23 PM, Or Gerlitz wrote:
> On Thu, Jul 30, 2015 at 11:06 AM, Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>> If iser_initialize_task_headers() routine failed before
>> dma mapping, we should not attempt to unmap in cleanup_task().
>
> This fixes some specific commit? if yes, add Fixes: line, if not, is
> that from day-0?

This was actually caught in code stare :)

This got in commit: IB/iser: Fix race between iser connection teardown 
and scsi TMFs

Which made iser_initialize_task_headers() possibly fail if the device
handle is not accessible in certain wired device catastrophic error 
conditions.

I'll add the Fixes tag.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 11/22] IB/iser: Remove dead code in fmr_pool alloc/free

[Sagi Grimberg]

On 7/30/2015 1:31 PM, Or Gerlitz wrote:
> On Thu, Jul 30, 2015 at 11:06 AM, Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>> In the past the we always tried to allocate an fmr_pool
>> and if it failed on ENOSYS (not supported) then we continued
>> with dma mr. This is not the case anymore and if we tried to
>> allocate an fmr_pool then it is supported and we expect to succeed.
>
> AFAIK, the ENOSYS flow was something that came into play when working
> e.g over VF drivers such as mlx4 that don't support fmr-ing but we still wanted
> an optimal performance. What does "this is not the case anymore" means? these
> VF drivers are still out there.

Today, iser is not usable with no FRWR and no FMR support. (it once was
when we bounced to higher-order allocations but we don't do that
anymore). Memory registration is a requirement support for iser today.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.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

[Sagi Grimberg]

On 7/30/2015 1:22 PM, Or Gerlitz wrote:
> On Thu, Jul 30, 2015 at 11:06 AM, Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 22/22] IB/iser: Chain all iser transaction send work requests

[Sagi Grimberg]

On 7/30/2015 1:27 PM, Or Gerlitz wrote:
> On Thu, Jul 30, 2015 at 11:06 AM, Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>> Concatination of send work requests benefits performance
>> by reducing the send queue lock contention (acquired in
>> ib_post_send) and saves us HW doorbells which is posted
>> only once.
>
> s/Concatination/Concatenation/
>
> AFAIK,  do we today! isn't that the case? if partially, please specify
> in the change-logs
> what flows were not fully optimized in that respect and are such after
> the patch.

I'll add which current work requests are not chained.

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

Re: COMMERCIAL: Re: [PATCH 06/22] IB/iser: Fix possible bogus DMA unmapping

[Or Gerlitz]

On 7/30/2015 3:09 PM, Sagi Grimberg wrote:
> I'll add the Fixes tag. 

don't forget to use --abbrev=12 for the Fixes: tag
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 11/22] IB/iser: Remove dead code in fmr_pool alloc/free

[Or Gerlitz]

On Thu, Jul 30, 2015 at 3:23 PM, Sagi Grimberg <sagig-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote:
> Today, iser is not usable with no FRWR and no FMR support. (it once was
> when we bounced to higher-order allocations but we don't do that
> anymore). Memory registration is a requirement support for iser today.

OK, sure, we now have FRWR support, back then we didn't
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 12/22] IB/iser: Introduce iser_reg_ops

[Steve Wise]

On 7/30/2015 3:06 AM, Sagi Grimberg wrote:
> Move all the per-device function pointers to an easy
> extensible iser_reg_ops structure that contains all
> the iser registration operations.
>
> Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>   drivers/infiniband/ulp/iser/iscsi_iser.h     | 39 ++++++++++++++++++----------
>   drivers/infiniband/ulp/iser/iser_initiator.c | 16 ++++++------
>   drivers/infiniband/ulp/iser/iser_memory.c    | 35 +++++++++++++++++++++++++
>   drivers/infiniband/ulp/iser/iser_verbs.c     | 30 +++++----------------
>   4 files changed, 75 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
> index 70bf6e7..9ce090c 100644
> --- a/drivers/infiniband/ulp/iser/iscsi_iser.h
> +++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
> @@ -326,6 +326,25 @@ struct iser_comp {
>   };
>   
>   /**
> + * struct iser_device - Memory registration operations
> + *     per-device registration schemes
> + *
> + * @alloc_reg_res:     Allocate registration resources
> + * @free_reg_res:      Free registration resources
> + * @reg_rdma_mem:      Register memory buffers
> + * @unreg_rdma_mem:    Un-register memory buffers
> + */
> +struct iser_reg_ops {
> +	int            (*alloc_reg_res)(struct ib_conn *ib_conn,
> +					unsigned cmds_max);
> +	void           (*free_reg_res)(struct ib_conn *ib_conn);
> +	int            (*reg_rdma_mem)(struct iscsi_iser_task *iser_task,
> +				       enum iser_data_dir cmd_dir);
> +	void           (*unreg_rdma_mem)(struct iscsi_iser_task *iser_task,
> +					 enum iser_data_dir cmd_dir);
> +};
> +
> +/**
>    * struct iser_device - iSER device handle
>    *
>    * @ib_device:     RDMA device
> @@ -338,11 +357,7 @@ struct iser_comp {
>    * @comps_used:    Number of completion contexts used, Min between online
>    *                 cpus and device max completion vectors
>    * @comps:         Dinamically allocated array of completion handlers
> - * Memory registration pool Function pointers (FMR or Fastreg):
> - *     @iser_alloc_rdma_reg_res: Allocation of memory regions pool
> - *     @iser_free_rdma_reg_res:  Free of memory regions pool
> - *     @iser_reg_rdma_mem:       Memory registration routine
> - *     @iser_unreg_rdma_mem:     Memory deregistration routine
> + * @reg_ops:       Registration ops
>    */
>   struct iser_device {
>   	struct ib_device             *ib_device;
> @@ -354,13 +369,7 @@ struct iser_device {
>   	int                          refcount;
>   	int			     comps_used;
>   	struct iser_comp	     *comps;
> -	int                          (*iser_alloc_rdma_reg_res)(struct ib_conn *ib_conn,
> -								unsigned cmds_max);
> -	void                         (*iser_free_rdma_reg_res)(struct ib_conn *ib_conn);
> -	int                          (*iser_reg_rdma_mem)(struct iscsi_iser_task *iser_task,
> -							  enum iser_data_dir cmd_dir);
> -	void                         (*iser_unreg_rdma_mem)(struct iscsi_iser_task *iser_task,
> -							    enum iser_data_dir cmd_dir);
> +	struct iser_reg_ops          *reg_ops;
>   };
>   
>   #define ISER_CHECK_GUARD	0xc0
> @@ -563,6 +572,8 @@ extern int iser_debug_level;
>   extern bool iser_pi_enable;
>   extern int iser_pi_guard;
>   
> +int iser_assign_reg_ops(struct iser_device *device);
> +
>   int iser_send_control(struct iscsi_conn *conn,
>   		      struct iscsi_task *task);
>   
> @@ -636,9 +647,9 @@ int  iser_initialize_task_headers(struct iscsi_task *task,
>   			struct iser_tx_desc *tx_desc);
>   int iser_alloc_rx_descriptors(struct iser_conn *iser_conn,
>   			      struct iscsi_session *session);
> -int iser_create_fmr_pool(struct ib_conn *ib_conn, unsigned cmds_max);
> +int iser_alloc_fmr_pool(struct ib_conn *ib_conn, unsigned cmds_max);
>   void iser_free_fmr_pool(struct ib_conn *ib_conn);
> -int iser_create_fastreg_pool(struct ib_conn *ib_conn, unsigned cmds_max);
> +int iser_alloc_fastreg_pool(struct ib_conn *ib_conn, unsigned cmds_max);
>   void iser_free_fastreg_pool(struct ib_conn *ib_conn);
>   u8 iser_check_task_pi_status(struct iscsi_iser_task *iser_task,
>   			     enum iser_data_dir cmd_dir, sector_t *sector);
> diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c
> index 42d6f42..88d8a89 100644
> --- a/drivers/infiniband/ulp/iser/iser_initiator.c
> +++ b/drivers/infiniband/ulp/iser/iser_initiator.c
> @@ -73,7 +73,7 @@ static int iser_prepare_read_cmd(struct iscsi_task *task)
>   			return err;
>   	}
>   
> -	err = device->iser_reg_rdma_mem(iser_task, ISER_DIR_IN);
> +	err = device->reg_ops->reg_rdma_mem(iser_task, ISER_DIR_IN);
>   	if (err) {
>   		iser_err("Failed to set up Data-IN RDMA\n");
>   		return err;
> @@ -128,7 +128,7 @@ iser_prepare_write_cmd(struct iscsi_task *task,
>   			return err;
>   	}
>   
> -	err = device->iser_reg_rdma_mem(iser_task, ISER_DIR_OUT);
> +	err = device->reg_ops->reg_rdma_mem(iser_task, ISER_DIR_OUT);
>   	if (err != 0) {
>   		iser_err("Failed to register write cmd RDMA mem\n");
>   		return err;
> @@ -260,7 +260,7 @@ int iser_alloc_rx_descriptors(struct iser_conn *iser_conn,
>   	iser_conn->qp_max_recv_dtos_mask = session->cmds_max - 1; /* cmds_max is 2^N */
>   	iser_conn->min_posted_rx = iser_conn->qp_max_recv_dtos >> 2;
>   
> -	if (device->iser_alloc_rdma_reg_res(ib_conn, session->scsi_cmds_max))
> +	if (device->reg_ops->alloc_reg_res(ib_conn, session->scsi_cmds_max))
>   		goto create_rdma_reg_res_failed;
>   
>   	if (iser_alloc_login_buf(iser_conn))
> @@ -301,7 +301,7 @@ rx_desc_dma_map_failed:
>   rx_desc_alloc_fail:
>   	iser_free_login_buf(iser_conn);
>   alloc_login_buf_fail:
> -	device->iser_free_rdma_reg_res(ib_conn);
> +	device->reg_ops->free_reg_res(ib_conn);
>   create_rdma_reg_res_failed:
>   	iser_err("failed allocating rx descriptors / data buffers\n");
>   	return -ENOMEM;
> @@ -314,8 +314,8 @@ void iser_free_rx_descriptors(struct iser_conn *iser_conn)
>   	struct ib_conn *ib_conn = &iser_conn->ib_conn;
>   	struct iser_device *device = ib_conn->device;
>   
> -	if (device->iser_free_rdma_reg_res)
> -		device->iser_free_rdma_reg_res(ib_conn);
> +	if (device->reg_ops->free_reg_res)
> +		device->reg_ops->free_reg_res(ib_conn);
>   
>   	rx_desc = iser_conn->rx_descs;
>   	for (i = 0; i < iser_conn->qp_max_recv_dtos; i++, rx_desc++)
> @@ -699,7 +699,7 @@ void iser_task_rdma_finalize(struct iscsi_iser_task *iser_task)
>   	}
>   
>   	if (iser_task->dir[ISER_DIR_IN]) {
> -		device->iser_unreg_rdma_mem(iser_task, ISER_DIR_IN);
> +		device->reg_ops->unreg_rdma_mem(iser_task, ISER_DIR_IN);
>   		if (is_rdma_data_aligned)
>   			iser_dma_unmap_task_data(iser_task,
>   						 &iser_task->data[ISER_DIR_IN],
> @@ -711,7 +711,7 @@ void iser_task_rdma_finalize(struct iscsi_iser_task *iser_task)
>   	}
>   
>   	if (iser_task->dir[ISER_DIR_OUT]) {
> -		device->iser_unreg_rdma_mem(iser_task, ISER_DIR_OUT);
> +		device->reg_ops->unreg_rdma_mem(iser_task, ISER_DIR_OUT);
>   		if (is_rdma_data_aligned)
>   			iser_dma_unmap_task_data(iser_task,
>   						 &iser_task->data[ISER_DIR_OUT],
> diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
> index 4209d73..ff3ec53 100644
> --- a/drivers/infiniband/ulp/iser/iser_memory.c
> +++ b/drivers/infiniband/ulp/iser/iser_memory.c
> @@ -39,6 +39,41 @@
>   
>   #include "iscsi_iser.h"
>   
> +static struct iser_reg_ops fastreg_ops = {
> +	.alloc_reg_res	= iser_alloc_fastreg_pool,
> +	.free_reg_res	= iser_free_fastreg_pool,
> +	.reg_rdma_mem	= iser_reg_rdma_mem_fastreg,
> +	.unreg_rdma_mem	= iser_unreg_mem_fastreg,
> +};
> +
> +static struct iser_reg_ops fmr_ops = {
> +	.alloc_reg_res	= iser_alloc_fmr_pool,
> +	.free_reg_res	= iser_free_fmr_pool,
> +	.reg_rdma_mem	= iser_reg_rdma_mem_fmr,
> +	.unreg_rdma_mem	= iser_unreg_mem_fmr,
> +};
> +
> +int iser_assign_reg_ops(struct iser_device *device)
> +{
> +	struct ib_device_attr *dev_attr = &device->dev_attr;
> +
> +	/* Assign function handles  - based on FMR support */
> +	if (device->ib_device->alloc_fmr && device->ib_device->dealloc_fmr &&
> +	    device->ib_device->map_phys_fmr && device->ib_device->unmap_fmr) {
> +		iser_info("FMR supported, using FMR for registration\n");
> +		device->reg_ops = &fmr_ops;
> +	} else
> +	if (dev_attr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS) {
> +		iser_info("FastReg supported, using FastReg for registration\n");
> +		device->reg_ops = &fastreg_ops;
> +	} else {
> +		iser_err("IB device does not support FMRs nor FastRegs, can't register memory\n");
> +		return -1;
> +	}
> +

Perhaps no device supports both FMR and FRMR, but the above code would 
choose FMR over FRMR.  Shouldn't it be the other way around?

> +	return 0;
> +}
> +
>   static void
>   iser_free_bounce_sg(struct iser_data_buf *data)
>   {
> diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
> index 2a0cb42..ca0aba3 100644
> --- a/drivers/infiniband/ulp/iser/iser_verbs.c
> +++ b/drivers/infiniband/ulp/iser/iser_verbs.c
> @@ -87,25 +87,9 @@ static int iser_create_device_ib_res(struct iser_device *device)
>   		return ret;
>   	}
>   
> -	/* Assign function handles  - based on FMR support */
> -	if (device->ib_device->alloc_fmr && device->ib_device->dealloc_fmr &&
> -	    device->ib_device->map_phys_fmr && device->ib_device->unmap_fmr) {
> -		iser_info("FMR supported, using FMR for registration\n");
> -		device->iser_alloc_rdma_reg_res = iser_create_fmr_pool;
> -		device->iser_free_rdma_reg_res = iser_free_fmr_pool;
> -		device->iser_reg_rdma_mem = iser_reg_rdma_mem_fmr;
> -		device->iser_unreg_rdma_mem = iser_unreg_mem_fmr;
> -	} else
> -	if (dev_attr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS) {
> -		iser_info("FastReg supported, using FastReg for registration\n");
> -		device->iser_alloc_rdma_reg_res = iser_create_fastreg_pool;
> -		device->iser_free_rdma_reg_res = iser_free_fastreg_pool;
> -		device->iser_reg_rdma_mem = iser_reg_rdma_mem_fastreg;
> -		device->iser_unreg_rdma_mem = iser_unreg_mem_fastreg;
> -	} else {
> -		iser_err("IB device does not support FMRs nor FastRegs, can't register memory\n");
> -		return -1;
> -	}
> +	ret = iser_assign_reg_ops(device);
> +	if (ret)
> +		return ret;
>   
>   	device->comps_used = min_t(int, num_online_cpus(),
>   				 device->ib_device->num_comp_vectors);
> @@ -211,11 +195,11 @@ static void iser_free_device_ib_res(struct iser_device *device)
>   }
>   
>   /**
> - * iser_create_fmr_pool - Creates FMR pool and page_vector
> + * iser_alloc_fmr_pool - Creates FMR pool and page_vector
>    *
>    * returns 0 on success, or errno code on failure
>    */
> -int iser_create_fmr_pool(struct ib_conn *ib_conn, unsigned cmds_max)
> +int iser_alloc_fmr_pool(struct ib_conn *ib_conn, unsigned cmds_max)
>   {
>   	struct iser_device *device = ib_conn->device;
>   	struct ib_fmr_pool_param params;
> @@ -384,11 +368,11 @@ pi_ctx_alloc_failure:
>   }
>   
>   /**
> - * iser_create_fastreg_pool - Creates pool of fast_reg descriptors
> + * iser_alloc_fastreg_pool - Creates pool of fast_reg descriptors
>    * for fast registration work requests.
>    * returns 0 on success, or errno code on failure
>    */
> -int iser_create_fastreg_pool(struct ib_conn *ib_conn, unsigned cmds_max)
> +int iser_alloc_fastreg_pool(struct ib_conn *ib_conn, unsigned cmds_max)
>   {
>   	struct iser_device *device = ib_conn->device;
>   	struct iser_fr_desc *desc;

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.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

[Steve Wise]

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 <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>   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         (0x80000 >> 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_4K)
> +
>   #define ISER_DEF_XMIT_CMDS_DEFAULT		512
>   #if ISCSI_DEF_XMIT_CMDS_MAX > ISER_DEF_XMIT_CMDS_DEFAULT
>   	#define ISER_DEF_XMIT_CMDS_MAX		ISCSI_DEF_XMIT_CMDS_MAX
> @@ -504,6 +509,8 @@ struct ib_conn {
>    * @rx_desc_head:     head of rx_descs cyclic buffer
>    * @rx_descs:         rx buffers array (cyclic buffer)
>    * @num_rx_descs:     number of rx descriptors
> + * @scsi_sg_tablesize: scsi host sg_tablesize
> + * @scsi_max_sectors: scsi host max sectors
>    */
>   struct iser_conn {
>   	struct ib_conn		     ib_conn;
> @@ -528,6 +535,8 @@ struct iser_conn {
>   	unsigned int 		     rx_desc_head;
>   	struct iser_rx_desc	     *rx_descs;
>   	u32                          num_rx_descs;
> +	unsigned short               scsi_sg_tablesize;
> +	unsigned int                 scsi_max_sectors;
>   };
>   
>   /**
> @@ -583,6 +592,7 @@ extern struct iser_global ig;
>   extern int iser_debug_level;
>   extern bool iser_pi_enable;
>   extern int iser_pi_guard;
> +extern unsigned int iser_max_sectors;
>   
>   int iser_assign_reg_ops(struct iser_device *device);
>   
> diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c
> index 268a3d6..d511879 100644
> --- a/drivers/infiniband/ulp/iser/iser_initiator.c
> +++ b/drivers/infiniband/ulp/iser/iser_initiator.c
> @@ -259,7 +259,7 @@ int iser_alloc_rx_descriptors(struct iser_conn *iser_conn,
>   	iser_conn->min_posted_rx = iser_conn->qp_max_recv_dtos >> 2;
>   
>   	if (device->reg_ops->alloc_reg_res(ib_conn, session->scsi_cmds_max,
> -					   ISCSI_ISER_SG_TABLESIZE + 1))
> +					   iser_conn->scsi_sg_tablesize))
>   		goto create_rdma_reg_res_failed;
>   
>   	if (iser_alloc_login_buf(iser_conn))
> diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
> index 5e807ba..cbf6152 100644
> --- a/drivers/infiniband/ulp/iser/iser_memory.c
> +++ b/drivers/infiniband/ulp/iser/iser_memory.c
> @@ -363,7 +363,8 @@ static int iser_sg_to_page_vec(struct iser_data_buf *data,
>    * consecutive SG elements are actually fragments of the same physcial page.
>    */
>   static int iser_data_buf_aligned_len(struct iser_data_buf *data,
> -				      struct ib_device *ibdev)
> +				     struct ib_device *ibdev,
> +				     unsigned sg_tablesize)
>   {
>   	struct scatterlist *sg, *sgl, *next_sg = NULL;
>   	u64 start_addr, end_addr;
> @@ -375,6 +376,14 @@ static int iser_data_buf_aligned_len(struct iser_data_buf *data,
>   	sgl = data->sg;
>   	start_addr  = ib_sg_dma_address(ibdev, sgl);
>   
> +	if (unlikely(sgl[0].offset &&
> +		     data->data_len >= sg_tablesize * PAGE_SIZE)) {
> +		iser_dbg("can't register length %lx with offset %x "
> +			 "fall to bounce buffer\n", data->data_len,
> +			 sgl[0].offset);
> +		return 0;
> +	}
> +
>   	for_each_sg(sgl, sg, data->dma_nents, i) {
>   		if (start_check && !IS_4K_ALIGNED(start_addr))
>   			break;
> @@ -790,7 +799,8 @@ iser_handle_unaligned_buf(struct iscsi_iser_task *task,
>   	struct iser_device *device = iser_conn->ib_conn.device;
>   	int err, aligned_len;
>   
> -	aligned_len = iser_data_buf_aligned_len(mem, device->ib_device);
> +	aligned_len = iser_data_buf_aligned_len(mem, device->ib_device,
> +						iser_conn->scsi_sg_tablesize);
>   	if (aligned_len != mem->dma_nents) {
>   		err = fall_to_bounce_buf(task, mem, dir);
>   		if (err)
> diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
> index fa778ba..f69cee7 100644
> --- a/drivers/infiniband/ulp/iser/iser_verbs.c
> +++ b/drivers/infiniband/ulp/iser/iser_verbs.c
> @@ -756,6 +756,31 @@ static void iser_connect_error(struct rdma_cm_id *cma_id)
>   	iser_conn->state = ISER_CONN_TERMINATING;
>   }
>   
> +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?

> +	iser_conn->scsi_sg_tablesize = sg_tablesize;
> +
> +	iser_dbg("iser_conn %p, sg_tablesize %u, max_sectors %u\n",
> +		 iser_conn, iser_conn->scsi_sg_tablesize,
> +		 iser_conn->scsi_max_sectors);
> +}
> +
>   /**
>    * Called with state mutex held
>    **/
> @@ -794,6 +819,8 @@ static void iser_addr_handler(struct rdma_cm_id *cma_id)
>   		}
>   	}
>   
> +	iser_calc_scsi_params(iser_conn, iser_max_sectors);
> +
>   	ret = rdma_resolve_route(cma_id, 1000);
>   	if (ret) {
>   		iser_err("resolve route failed: %d\n", ret);

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

Re: [PATCH 12/22] IB/iser: Introduce iser_reg_ops

[Jason Gunthorpe]

On Thu, Jul 30, 2015 at 10:05:53AM -0500, Steve Wise wrote:
> >+int iser_assign_reg_ops(struct iser_device *device)
> >+{
> >+	struct ib_device_attr *dev_attr = &device->dev_attr;
> >+
> >+	/* Assign function handles  - based on FMR support */
> >+	if (device->ib_device->alloc_fmr && device->ib_device->dealloc_fmr &&
> >+	    device->ib_device->map_phys_fmr && device->ib_device->unmap_fmr) {
> >+		iser_info("FMR supported, using FMR for registration\n");
> >+		device->reg_ops = &fmr_ops;
> >+	} else
> >+	if (dev_attr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS) {
> >+		iser_info("FastReg supported, using FastReg for registration\n");
> >+		device->reg_ops = &fastreg_ops;
> >+	} else {
> >+		iser_err("IB device does not support FMRs nor FastRegs, can't register memory\n");
> >+		return -1;
> >+	}
> >+
> 
> Perhaps no device supports both FMR and FRMR, but the above code would
> choose FMR over FRMR.  Shouldn't it be the other way around?

Agree. We should always prefer FRMR. Many of the IB drivers support
both..

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

Re: [PATCH 12/22] IB/iser: Introduce iser_reg_ops

[Jason Gunthorpe]

On Thu, Jul 30, 2015 at 11:06:25AM +0300, Sagi Grimberg wrote:

>  /**
> + * struct iser_device - Memory registration operations
> + *     per-device registration schemes
> + *
> + * @alloc_reg_res:     Allocate registration resources
> + * @free_reg_res:      Free registration resources
> + * @reg_rdma_mem:      Register memory buffers
> + * @unreg_rdma_mem:    Un-register memory buffers
> + */
> +struct iser_reg_ops {
> +	int            (*alloc_reg_res)(struct ib_conn *ib_conn,
> +					unsigned cmds_max);
> +	void           (*free_reg_res)(struct ib_conn *ib_conn);
> +	int            (*reg_rdma_mem)(struct iscsi_iser_task *iser_task,
> +				       enum iser_data_dir cmd_dir);
> +	void           (*unreg_rdma_mem)(struct iscsi_iser_task *iser_task,
> +					 enum iser_data_dir cmd_dir);
> +};

It sucks we need every ULP to have function pointer swap outs just to
support MRs.. NFS has the same...
>  
>  	if (iser_task->dir[ISER_DIR_IN]) {
> -		device->iser_unreg_rdma_mem(iser_task, ISER_DIR_IN);
> +		device->reg_ops->unreg_rdma_mem(iser_task, ISER_DIR_IN);
>  		if (is_rdma_data_aligned)
>  			iser_dma_unmap_task_data(iser_task,
>  						 &iser_task->data[ISER_DIR_IN],

Is this the same wrong ordering that NFS had?

DMA unmap (and CPU access) for ACCESS_REMOTE_WRITE rkeys should happen
after invalidate completes, not before.

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

Re: [PATCH 12/22] IB/iser: Introduce iser_reg_ops

[Sagi Grimberg]

On 7/30/2015 8:25 PM, Jason Gunthorpe wrote:
> On Thu, Jul 30, 2015 at 10:05:53AM -0500, Steve Wise wrote:
>>> +int iser_assign_reg_ops(struct iser_device *device)
>>> +{
>>> +	struct ib_device_attr *dev_attr = &device->dev_attr;
>>> +
>>> +	/* Assign function handles  - based on FMR support */
>>> +	if (device->ib_device->alloc_fmr && device->ib_device->dealloc_fmr &&
>>> +	    device->ib_device->map_phys_fmr && device->ib_device->unmap_fmr) {
>>> +		iser_info("FMR supported, using FMR for registration\n");
>>> +		device->reg_ops = &fmr_ops;
>>> +	} else
>>> +	if (dev_attr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS) {
>>> +		iser_info("FastReg supported, using FastReg for registration\n");
>>> +		device->reg_ops = &fastreg_ops;
>>> +	} else {
>>> +		iser_err("IB device does not support FMRs nor FastRegs, can't register memory\n");
>>> +		return -1;
>>> +	}
>>> +
>>
>> Perhaps no device supports both FMR and FRMR, but the above code would
>> choose FMR over FRMR.  Shouldn't it be the other way around?
>
> Agree. We should always prefer FRMR. Many of the IB drivers support
> both..

I agree,

I planned to change that as part of my remote invalidate support
patches. Would it be acceptable to hold that off for the next cycle?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.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

[Sagi Grimberg]

>> +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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.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

[Atchley, Scott]

On Aug 2, 2015, at 4:01 AM, Sagi Grimberg <sagig-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.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

[Sagi Grimberg]

>>> 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.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

[Or Gerlitz]

On Tue, Aug 4, 2015 at 8:10 PM, Sagi Grimberg <sagig-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html