nvme/rdma: Don't reg MRs for cmds and rsps

Since now cmds and rsps buffers are allocated
from huge pages, there are already registered
MR for this memory. In that way  we can avoid
registering 2 additional MRs per qpair, just
perform memory translation to get lkey.

Signed-off-by: Aleksey Marchuk <alexeymar@nvidia.com>
Change-Id: I2cb39a15e5d224698c293ac18af00a909840eaa8
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/14428
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Shuhei Matsumoto <smatsumoto@nvidia.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>

lib/nvme/nvme_rdma.c

@@ -148,12 +148,6 @@ struct nvme_rdma_poll_group {
 	uint32_t					num_pollers;
 };
 
-/* Memory regions */
-union nvme_rdma_mr {
-	struct ibv_mr	*mr;
-	uint64_t	key;
-};
-
 enum nvme_rdma_qpair_state {
 	NVME_RDMA_QPAIR_STATE_INVALID = 0,
 	NVME_RDMA_QPAIR_STATE_STALE_CONN,
@@ -195,19 +189,12 @@ struct nvme_rdma_qpair {
 	struct spdk_nvme_rdma_rsp		*rsps;
 
 	struct ibv_recv_wr			*rsp_recv_wrs;
-
-	/* Memory region describing all rsps for this qpair */
-	union nvme_rdma_mr			rsp_mr;
-
 	/*
 	 * Array of num_entries NVMe commands registered as RDMA message buffers.
 	 * Indexed by rdma_req->id.
 	 */
 	struct spdk_nvmf_cmd			*cmds;
 
-	/* Memory region describing all cmds for this qpair */
-	union nvme_rdma_mr			cmd_mr;
-
 	struct spdk_rdma_mem_map		*mr_map;
 
 	TAILQ_HEAD(, spdk_nvme_rdma_req)	free_reqs;
@@ -894,58 +881,6 @@ nvme_rdma_post_recv(struct nvme_rdma_qpair *rqpair, uint16_t rsp_idx)
 	return nvme_rdma_qpair_queue_recv_wr(rqpair, wr);
 }
 
-static int
-nvme_rdma_reg_mr(struct ibv_pd *pd, union nvme_rdma_mr *mr, void *mem, size_t length)
-{
-	if (!g_nvme_hooks.get_rkey) {
-		mr->mr = ibv_reg_mr(pd, mem, length, IBV_ACCESS_LOCAL_WRITE);
-		if (mr->mr == NULL) {
-			SPDK_ERRLOG("Unable to register mr: %s (%d)\n",
-				    spdk_strerror(errno), errno);
-			return -1;
-		}
-	} else {
-		mr->key = g_nvme_hooks.get_rkey(pd, mem, length);
-	}
-
-	return 0;
-}
-
-static void
-nvme_rdma_dereg_mr(union nvme_rdma_mr *mr)
-{
-	if (!g_nvme_hooks.get_rkey) {
-		if (mr->mr && ibv_dereg_mr(mr->mr)) {
-			SPDK_ERRLOG("Unable to de-register mr\n");
-		}
-	} else {
-		if (mr->key) {
-			g_nvme_hooks.put_rkey(mr->key);
-		}
-	}
-	memset(mr, 0, sizeof(*mr));
-}
-
-static uint32_t
-nvme_rdma_mr_get_lkey(union nvme_rdma_mr *mr)
-{
-	uint32_t lkey;
-
-	if (!g_nvme_hooks.get_rkey) {
-		lkey = mr->mr->lkey;
-	} else {
-		lkey = *((uint64_t *) mr->key);
-	}
-
-	return lkey;
-}
-
-static void
-nvme_rdma_unregister_rsps(struct nvme_rdma_qpair *rqpair)
-{
-	nvme_rdma_dereg_mr(&rqpair->rsp_mr);
-}
-
 static void
 nvme_rdma_free_rsps(struct nvme_rdma_qpair *rqpair)
 {
@@ -990,18 +925,9 @@ fail:
 static int
 nvme_rdma_register_rsps(struct nvme_rdma_qpair *rqpair)
 {
+	struct spdk_rdma_memory_translation translation;
 	uint16_t i;
 	int rc;
-	uint32_t lkey;
-
-	rc = nvme_rdma_reg_mr(rqpair->rdma_qp->qp->pd, &rqpair->rsp_mr,
-			      rqpair->rsps, rqpair->num_entries * sizeof(*rqpair->rsps));
-
-	if (rc < 0) {
-		goto fail;
-	}
-
-	lkey = nvme_rdma_mr_get_lkey(&rqpair->rsp_mr);
 
 	for (i = 0; i < rqpair->num_entries; i++) {
 		struct ibv_sge *rsp_sgl = &rqpair->rsp_sgls[i];
@@ -1012,7 +938,12 @@ nvme_rdma_register_rsps(struct nvme_rdma_qpair *rqpair)
 		rsp->idx = i;
 		rsp_sgl->addr = (uint64_t)&rqpair->rsps[i];
 		rsp_sgl->length = sizeof(struct spdk_nvme_cpl);
-		rsp_sgl->lkey = lkey;
+		rc = spdk_rdma_get_translation(rqpair->mr_map, &rqpair->rsps[i], sizeof(*rqpair->rsps),
+					       &translation);
+		if (rc) {
+			return rc;
+		}
+		rsp_sgl->lkey = spdk_rdma_memory_translation_get_lkey(&translation);
 
 		rqpair->rsp_recv_wrs[i].wr_id = (uint64_t)&rsp->rdma_wr;
 		rqpair->rsp_recv_wrs[i].next = NULL;
@@ -1021,26 +952,16 @@ nvme_rdma_register_rsps(struct nvme_rdma_qpair *rqpair)
 
 		rc = nvme_rdma_post_recv(rqpair, i);
 		if (rc) {
-			goto fail;
+			return rc;
 		}
 	}
 
 	rc = nvme_rdma_qpair_submit_recvs(rqpair);
 	if (rc) {
-		goto fail;
+		return rc;
 	}
 
 	return 0;
-
-fail:
-	nvme_rdma_unregister_rsps(rqpair);
-	return rc;
-}
-
-static void
-nvme_rdma_unregister_reqs(struct nvme_rdma_qpair *rqpair)
-{
-	nvme_rdma_dereg_mr(&rqpair->cmd_mr);
 }
 
 static void
@@ -1111,28 +1032,20 @@ fail:
 static int
 nvme_rdma_register_reqs(struct nvme_rdma_qpair *rqpair)
 {
-	int i;
+	struct spdk_rdma_memory_translation translation;
+	uint16_t i;
 	int rc;
-	uint32_t lkey;
-
-	rc = nvme_rdma_reg_mr(rqpair->rdma_qp->qp->pd, &rqpair->cmd_mr,
-			      rqpair->cmds, rqpair->num_entries * sizeof(*rqpair->cmds));
-
-	if (rc < 0) {
-		goto fail;
-	}
-
-	lkey = nvme_rdma_mr_get_lkey(&rqpair->cmd_mr);
 
 	for (i = 0; i < rqpair->num_entries; i++) {
-		rqpair->rdma_reqs[i].send_sgl[0].lkey = lkey;
+		rc = spdk_rdma_get_translation(rqpair->mr_map, &rqpair->cmds[i], sizeof(*rqpair->cmds),
+					       &translation);
+		if (rc) {
+			return rc;
+		}
+		rqpair->rdma_reqs[i].send_sgl[0].lkey = spdk_rdma_memory_translation_get_lkey(&translation);
 	}
 
 	return 0;
-
-fail:
-	nvme_rdma_unregister_reqs(rqpair);
-	return -ENOMEM;
 }
 
 static int nvme_rdma_connect(struct nvme_rdma_qpair *rqpair);
@@ -1230,6 +1143,13 @@ nvme_rdma_connect_established(struct nvme_rdma_qpair *rqpair, int ret)
 		return ret;
 	}
 
+	rqpair->mr_map = spdk_rdma_create_mem_map(rqpair->rdma_qp->qp->pd, &g_nvme_hooks,
+			 SPDK_RDMA_MEMORY_MAP_ROLE_INITIATOR);
+	if (!rqpair->mr_map) {
+		SPDK_ERRLOG("Unable to register RDMA memory translation map\n");
+		return -1;
+	}
+
 	ret = nvme_rdma_register_reqs(rqpair);
 	SPDK_DEBUGLOG(nvme, "rc =%d\n", ret);
 	if (ret) {
@@ -1246,13 +1166,6 @@ nvme_rdma_connect_established(struct nvme_rdma_qpair *rqpair, int ret)
 	}
 	SPDK_DEBUGLOG(nvme, "RDMA responses registered\n");
 
-	rqpair->mr_map = spdk_rdma_create_mem_map(rqpair->rdma_qp->qp->pd, &g_nvme_hooks,
-			 SPDK_RDMA_MEMORY_MAP_ROLE_INITIATOR);
-	if (!rqpair->mr_map) {
-		SPDK_ERRLOG("Unable to register RDMA memory translation map\n");
-		return -1;
-	}
-
 	rqpair->state = NVME_RDMA_QPAIR_STATE_FABRIC_CONNECT_SEND;
 
 	return 0;
@@ -1946,8 +1859,6 @@ nvme_rdma_qpair_destroy(struct nvme_rdma_qpair *rqpair)
 	struct nvme_rdma_cm_event_entry *entry, *tmp;
 
 	spdk_rdma_free_mem_map(&rqpair->mr_map);
-	nvme_rdma_unregister_reqs(rqpair);
-	nvme_rdma_unregister_rsps(rqpair);
 
 	if (rqpair->evt) {
 		rdma_ack_cm_event(rqpair->evt);

test/unit/lib/nvme/nvme_rdma.c/nvme_rdma_ut.c

@@ -737,33 +737,6 @@ test_nvme_rdma_qpair_process_cm_event(void)
 	CU_ASSERT(rc == 0);
 }
 
-static void
-test_nvme_rdma_mr_get_lkey(void)
-{
-	union nvme_rdma_mr mr = {};
-	struct ibv_mr	ibv_mr = {};
-	uint64_t mr_key;
-	uint32_t lkey;
-
-	memset(&g_nvme_hooks, 0, sizeof(g_nvme_hooks));
-	ibv_mr.lkey = 1;
-	mr_key = 2;
-
-	/* Case 1:  get key form key address */
-	mr.key = (uint64_t)&mr_key;
-	g_nvme_hooks.get_rkey = (void *)0xAEADBEEF;
-
-	lkey = nvme_rdma_mr_get_lkey(&mr);
-	CU_ASSERT(lkey == mr_key);
-
-	/* Case 2: Get key from ibv_mr  */
-	g_nvme_hooks.get_rkey = NULL;
-	mr.mr = &ibv_mr;
-
-	lkey = nvme_rdma_mr_get_lkey(&mr);
-	CU_ASSERT(lkey == ibv_mr.lkey);
-}
-
 static void
 test_nvme_rdma_ctrlr_construct(void)
 {
@@ -1013,34 +986,20 @@ test_nvme_rdma_register_and_unregister_reqs(void)
 	rdma_qp.qp = &qp;
 	g_nvme_hooks.get_rkey = NULL;
 	rqpair.rdma_reqs = rdma_reqs;
-	/* case 1: nvme_rdma_register_req: nvme_rdma_reg_mr fail, expect: fail */
-	rqpair.num_entries = 0;
 
-	rc = nvme_rdma_register_reqs(&rqpair);
+	/* case 1: nvme_rdma_register_req: single entry, expect: PASS */
-	CU_ASSERT(rc == -ENOMEM);
-	CU_ASSERT(rqpair.cmd_mr.mr == NULL);
-
-	/* case 2: nvme_rdma_register_req: single entry, expect: PASS */
 	rqpair.num_entries = 1;
-
 	rc = nvme_rdma_register_reqs(&rqpair);
 	CU_ASSERT(rc == 0);
-	CU_ASSERT(rqpair.cmd_mr.mr == &g_rdma_mr);
+	CU_ASSERT(rqpair.rdma_reqs[0].send_sgl[0].lkey == g_rdma_mr.lkey);
-	CU_ASSERT(rqpair.rdma_reqs[0].send_sgl[0].lkey == rqpair.cmd_mr.mr->lkey);
 
-	/* case 3: nvme_rdma_register_req: multiple entry, expect: PASS */
+	/* case 2: nvme_rdma_register_req: multiple entry, expect: PASS */
 	rqpair.num_entries = 50;
-
 	rc = nvme_rdma_register_reqs(&rqpair);
 	CU_ASSERT(rc == 0);
-	CU_ASSERT(rqpair.cmd_mr.mr == &g_rdma_mr);
 	for (int i = 0; i < rqpair.num_entries; i++) {
-		CU_ASSERT(rqpair.rdma_reqs[i].send_sgl[0].lkey == rqpair.cmd_mr.mr->lkey);
+		CU_ASSERT(rqpair.rdma_reqs[0].send_sgl[0].lkey == g_rdma_mr.lkey);
 	}
-
-	/* case4: nvme_rdma_unregister_reqs, expect: PASS */
-	nvme_rdma_unregister_reqs(&rqpair);
-	CU_ASSERT(rqpair.cmd_mr.mr == NULL);
 }
 
 static void
@@ -1554,7 +1513,6 @@ main(int argc, char **argv)
 	CU_ADD_TEST(suite, test_nvme_rdma_ctrlr_create_qpair);
 	CU_ADD_TEST(suite, test_nvme_rdma_poller_create);
 	CU_ADD_TEST(suite, test_nvme_rdma_qpair_process_cm_event);
-	CU_ADD_TEST(suite, test_nvme_rdma_mr_get_lkey);
 	CU_ADD_TEST(suite, test_nvme_rdma_ctrlr_construct);
 	CU_ADD_TEST(suite, test_nvme_rdma_req_put_and_get);
 	CU_ADD_TEST(suite, test_nvme_rdma_req_init);