diff mbox

[for-next,08/10] iser-target: Support the remote invalidation exception

Message ID 1447691861-3796-9-git-send-email-sagig@mellanox.com (mailing list archive)
State Superseded
Headers show

Commit Message

Sagi Grimberg Nov. 16, 2015, 4:37 p.m. UTC 
From: Jenny Derzhavetz <jennyf@mellanox.com>

We'll use remote invalidate, according to negotiation result
during connection establishment. If the initiator declared that
it supports the remote invalidate exception then the target will
use IB_WR_SEND_WITH_INV with the correct rkey for the response.

Signed-off-by: Jenny Derzhavetz <jennyf@mellanox.com>
Signed-off-by: Sagi Grimberg <sagig@mellanox.com>
---
 drivers/infiniband/ulp/isert/ib_isert.c | 39 +++++++++++++++++++++++++++------
 drivers/infiniband/ulp/isert/ib_isert.h |  2 ++
 2 files changed, 34 insertions(+), 7 deletions(-)

Comments

Or Gerlitz Nov. 17, 2015, 8:09 a.m. UTC | #1 
On 11/16/2015 6:37 PM, Sagi Grimberg wrote:
> We'll use remote invalidate, according to negotiation result
> during connection establishment. If the initiator declared that
> it supports the remote invalidate exception then the target will
> use IB_WR_SEND_WITH_INV with the correct rkey for the response.

same comment as the one I posted for the initiator patch, check for
IB_DEVICE_MEM_MGT_EXTENSIONS

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sagi Grimberg Nov. 17, 2015, 9:50 a.m. UTC | #2 
On 17/11/2015 10:09, Or Gerlitz wrote:
> On 11/16/2015 6:37 PM, Sagi Grimberg wrote:
>> We'll use remote invalidate, according to negotiation result
>> during connection establishment. If the initiator declared that
>> it supports the remote invalidate exception then the target will
>> use IB_WR_SEND_WITH_INV with the correct rkey for the response.
>
> same comment as the one I posted for the initiator patch, check for
> IB_DEVICE_MEM_MGT_EXTENSIONS

SEND_WITH_INV doesn't have anything to do with
IB_DEVICE_MEM_MGT_EXTENSIONS does it? What is the relations between
the device capability to do FRWR and invalidating a remote rkey?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Or Gerlitz Nov. 17, 2015, 10:05 a.m. UTC | #3 
On 11/17/2015 11:50 AM, Sagi Grimberg wrote:
>> same comment as the one I posted for the initiator patch, check for
>> IB_DEVICE_MEM_MGT_EXTENSIONS
>
> SEND_WITH_INV doesn't have anything to do with 
> IB_DEVICE_MEM_MGT_EXTENSIONS does it? What is the relations between
> the device capability to do FRWR and invalidating a remote rkey? 

I am not sure.

I expect you as the author and leader for this series to dig into IBTA 
BMME annex and come with answers...

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig Nov. 17, 2015, 10:14 a.m. UTC | #4 
On Tue, Nov 17, 2015 at 11:50:15AM +0200, Sagi Grimberg wrote:
> 
> 
> On 17/11/2015 10:09, Or Gerlitz wrote:
> >On 11/16/2015 6:37 PM, Sagi Grimberg wrote:
> >>We'll use remote invalidate, according to negotiation result
> >>during connection establishment. If the initiator declared that
> >>it supports the remote invalidate exception then the target will
> >>use IB_WR_SEND_WITH_INV with the correct rkey for the response.
> >
> >same comment as the one I posted for the initiator patch, check for
> >IB_DEVICE_MEM_MGT_EXTENSIONS
> 
> SEND_WITH_INV doesn't have anything to do with
> IB_DEVICE_MEM_MGT_EXTENSIONS does it? What is the relations between
> the device capability to do FRWR and invalidating a remote rkey?

Both FRs and SEND_WITH_INV are part of the base memory management
extensions and also required for iWarp.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sagi Grimberg Nov. 17, 2015, 10:16 a.m. UTC | #5 
On 17/11/2015 12:14, Christoph Hellwig wrote:
> On Tue, Nov 17, 2015 at 11:50:15AM +0200, Sagi Grimberg wrote:
>>
>>
>> On 17/11/2015 10:09, Or Gerlitz wrote:
>>> On 11/16/2015 6:37 PM, Sagi Grimberg wrote:
>>>> We'll use remote invalidate, according to negotiation result
>>>> during connection establishment. If the initiator declared that
>>>> it supports the remote invalidate exception then the target will
>>>> use IB_WR_SEND_WITH_INV with the correct rkey for the response.
>>>
>>> same comment as the one I posted for the initiator patch, check for
>>> IB_DEVICE_MEM_MGT_EXTENSIONS
>>
>> SEND_WITH_INV doesn't have anything to do with
>> IB_DEVICE_MEM_MGT_EXTENSIONS does it? What is the relations between
>> the device capability to do FRWR and invalidating a remote rkey?
>
> Both FRs and SEND_WITH_INV are part of the base memory management
> extensions and also required for iWarp.

Thanks for clarifying. I'll add the check.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Or Gerlitz Nov. 19, 2015, 7:25 a.m. UTC | #6 
On 11/16/2015 6:37 PM, Sagi Grimberg wrote:
>   	struct isert_conn	*conn;
> @@ -209,6 +210,7 @@ struct isert_conn {
>   	struct work_struct	release_work;
>   	struct ib_recv_wr       beacon;
>   	bool                    logout_posted;
> +	bool                    snd_w_inv;
>   };

We've gone into this aspect few times in the past... my preference
is to use bit flags so the code would look like

conn->flags |= ISER_CONN_SEND_W_INV

or

if (conn->flags & ISER_CONN_LOGOUT_POSTED)

And you didn't want to go that way... but you did used
bit-fields in other areas of the driver, right?

Lets use some sort of BF-ing (bit fields or bit flags) all over the place
and not introduce new booleans everywhere we go.

Doing this over and over makes our fast path structures to grow and
maybe start crossing cache-lines (did you check that?)

Or.





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

Patch

diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index 667238f6253f..035fd12ec7ae 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -679,6 +679,25 @@  out_login_buf:
 	return ret;
 }
 
+static void
+isert_set_nego_params(struct isert_conn *isert_conn,
+		      struct rdma_conn_param *param)
+{
+	struct isert_device *device = isert_conn->device;
+
+	/* Set max inflight RDMA READ requests */
+	isert_conn->initiator_depth = min_t(u8, param->initiator_depth,
+				device->dev_attr.max_qp_init_rd_atom);
+	isert_dbg("Using initiator_depth: %u\n", isert_conn->initiator_depth);
+
+	if (param->private_data) {
+		u8 flags = *(u8 *)param->private_data;
+
+		isert_conn->snd_w_inv = !(flags & ISER_SEND_W_INV_NOT_SUP);
+		isert_info("Using remote invalidation\n");
+	}
+}
+
 static int
 isert_connect_request(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
 {
@@ -717,11 +736,7 @@  isert_connect_request(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
 	}
 	isert_conn->device = device;
 
-	/* Set max inflight RDMA READ requests */
-	isert_conn->initiator_depth = min_t(u8,
-				event->param.conn.initiator_depth,
-				device->dev_attr.max_qp_init_rd_atom);
-	isert_dbg("Using initiator_depth: %u\n", isert_conn->initiator_depth);
+	isert_set_nego_params(isert_conn, &event->param.conn);
 
 	ret = isert_conn_setup_qp(isert_conn, cma_id);
 	if (ret)
@@ -1100,7 +1115,14 @@  isert_init_send_wr(struct isert_conn *isert_conn, struct isert_cmd *isert_cmd,
 
 	isert_cmd->rdma_wr.iser_ib_op = ISER_IB_SEND;
 	send_wr->wr_id = (uintptr_t)&isert_cmd->tx_desc;
-	send_wr->opcode = IB_WR_SEND;
+
+	if (isert_conn->snd_w_inv && isert_cmd->inv_rkey) {
+		send_wr->opcode  = IB_WR_SEND_WITH_INV;
+		send_wr->ex.invalidate_rkey = isert_cmd->inv_rkey;
+	} else {
+		send_wr->opcode = IB_WR_SEND;
+	}
+
 	send_wr->sg_list = &tx_desc->tx_sg[0];
 	send_wr->num_sge = isert_cmd->tx_desc.num_sge;
 	send_wr->send_flags = IB_SEND_SIGNALED;
@@ -1489,6 +1511,7 @@  isert_rx_opcode(struct isert_conn *isert_conn, struct iser_rx_desc *rx_desc,
 		isert_cmd->read_va = read_va;
 		isert_cmd->write_stag = write_stag;
 		isert_cmd->write_va = write_va;
+		isert_cmd->inv_rkey = read_stag ? read_stag : write_stag;
 
 		ret = isert_handle_scsi_cmd(isert_conn, isert_cmd, cmd,
 					rx_desc, (unsigned char *)hdr);
@@ -3107,7 +3130,9 @@  isert_rdma_accept(struct isert_conn *isert_conn)
 	cp.rnr_retry_count = 7;
 
 	memset(&rsp_hdr, 0, sizeof(rsp_hdr));
-	rsp_hdr.flags = (ISERT_ZBVA_NOT_USED | ISERT_SEND_W_INV_NOT_USED);
+	rsp_hdr.flags = ISERT_ZBVA_NOT_USED;
+	if (!isert_conn->snd_w_inv)
+		rsp_hdr.flags = rsp_hdr.flags | ISERT_SEND_W_INV_NOT_USED;
 	cp.private_data = (void *)&rsp_hdr;
 	cp.private_data_len = sizeof(rsp_hdr);
 
diff --git a/drivers/infiniband/ulp/isert/ib_isert.h b/drivers/infiniband/ulp/isert/ib_isert.h
index 00edd5b68ca8..b23086d3d08b 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.h
+++ b/drivers/infiniband/ulp/isert/ib_isert.h
@@ -162,6 +162,7 @@  struct isert_cmd {
 	uint32_t		write_stag;
 	uint64_t		read_va;
 	uint64_t		write_va;
+	uint32_t		inv_rkey;
 	u64			pdu_buf_dma;
 	u32			pdu_buf_len;
 	struct isert_conn	*conn;
@@ -209,6 +210,7 @@  struct isert_conn {
 	struct work_struct	release_work;
 	struct ib_recv_wr       beacon;
 	bool                    logout_posted;
+	bool                    snd_w_inv;
 };
 
 #define ISERT_MAX_CQ 64