Skip to contentSkip to conversation

nvmet-rdma: Fix missing dma sync to nvme data structures

PATCH Superseded Parav Pandit 3 participants 6 messages Jan 19, 2017

Subsystems (3)
InfiniBand NVMe NVMe target

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Parav Pandit

author
#0 lore raw

PATCH v3 1/1 nvmet-rdma: Fix missing dma sync to nvme data structures

This patch performs dma sync operations on nvme_command and nvme_completion.

nvme_command is synced
(a) on receiving of the recv queue completion for cpu access.
(b) before posting recv wqe back to rdma adapter for device access.

nvme_completion is synced
(a) on receiving of the recv queue completion of associated nvme_command for cpu access.
(b) before posting send wqe to rdma adapter for device access.

This patch is generated for git://git.infradead.org/nvme-fabrics.git
Branch: nvmf-4.10

Signed-off-by Parav Pandit <parav at mellanox.com>
Reviewed-by Max Gurtovoy <maxg at mellanox.com>
---
 drivers/nvme/target/rdma.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
drivers/nvme/target/rdma.c +16 -0
index 6c1c368..0599217 100644
@@ -438,6 +438,10 @@ static int nvmet_rdma_post_recv(struct nvmet_rdma_device *ndev,
 {
 	struct ib_recv_wr *bad_wr;
 
+	ib_dma_sync_single_for_device(ndev->device,
+		cmd->sge[0].addr, cmd->sge[0].length,
+		DMA_FROM_DEVICE);
+
 	if (ndev->srq)
 		return ib_post_srq_recv(ndev->srq, &cmd->wr, &bad_wr);
 	return ib_post_recv(cmd->queue->cm_id->qp, &cmd->wr, &bad_wr);

@@ -538,6 +542,11 @@ static void nvmet_rdma_queue_response(struct nvmet_req *req)
 		first_wr = &rsp->send_wr;
 
 	nvmet_rdma_post_recv(rsp->queue->dev, rsp->cmd);
+
+	ib_dma_sync_single_for_device(rsp->queue->dev->device,
+			rsp->send_sge.addr, rsp->send_sge.length,
+			DMA_TO_DEVICE);
+
 	if (ib_post_send(cm_id->qp, first_wr, &bad_wr)) {
 		pr_err("sending cmd response failed\n");
 		nvmet_rdma_release_rsp(rsp);

@@ -698,6 +707,13 @@ static void nvmet_rdma_handle_command(struct nvmet_rdma_queue *queue,
 	cmd->n_rdma = 0;
 	cmd->req.port = queue->port;
 
+	ib_dma_sync_single_for_cpu(queue->dev->device,
+		cmd->cmd->sge[0].addr, cmd->cmd->sge[0].length,
+		DMA_FROM_DEVICE);
+	ib_dma_sync_single_for_cpu(queue->dev->device,
+			cmd->send_sge.addr, cmd->send_sge.length,
+			DMA_TO_DEVICE);
+
 	if (!nvmet_req_init(&cmd->req, &queue->nvme_cq,
 			&queue->nvme_sq, &nvmet_rdma_ops))
 		return;
Signature · 1 line
1.8.3.1
maintainer
#1 lore raw

PATCH v3 1/1 nvmet-rdma: Fix missing dma sync to nvme data structures

Looks much better, I'm fine with it.

Christoph?

maintainer
#2 lore raw

PATCH v3 1/1 nvmet-rdma: Fix missing dma sync to nvme data structures

In reply to Parav Pandit
Quoted 49 lines · Parav Pandit

This patch performs dma sync operations on nvme_command and nvme_completion.

nvme_command is synced
(a) on receiving of the recv queue completion for cpu access.
(b) before posting recv wqe back to rdma adapter for device access.

nvme_completion is synced
(a) on receiving of the recv queue completion of associated nvme_command for cpu access.
(b) before posting send wqe to rdma adapter for device access.

This patch is generated for git://git.infradead.org/nvme-fabrics.git
Branch: nvmf-4.10

Signed-off-by Parav Pandit <parav at mellanox.com>
Reviewed-by Max Gurtovoy <maxg at mellanox.com>
---
 drivers/nvme/target/rdma.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
drivers/nvme/target/rdma.c +9 -0
index 6c1c368..0599217 100644
@@ -438,6 +438,10 @@ static int nvmet_rdma_post_recv(struct nvmet_rdma_device *ndev,
 {
 	struct ib_recv_wr *bad_wr;
 
+	ib_dma_sync_single_for_device(ndev->device,
+		cmd->sge[0].addr, cmd->sge[0].length,
+		DMA_FROM_DEVICE);
+
 	if (ndev->srq)
 		return ib_post_srq_recv(ndev->srq, &cmd->wr, &bad_wr);
 	return ib_post_recv(cmd->queue->cm_id->qp, &cmd->wr, &bad_wr);

@@ -538,6 +542,11 @@ static void nvmet_rdma_queue_response(struct nvmet_req *req)
 		first_wr = &rsp->send_wr;
 
 	nvmet_rdma_post_recv(rsp->queue->dev, rsp->cmd);
+
+	ib_dma_sync_single_for_device(rsp->queue->dev->device,
+			rsp->send_sge.addr, rsp->send_sge.length,
+			DMA_TO_DEVICE);
+
 	if (ib_post_send(cm_id->qp, first_wr, &bad_wr)) {
 		pr_err("sending cmd response failed\n");
 		nvmet_rdma_release_rsp(rsp);

drivers/nvme/target/rdma.c static void nvmet_rdma_handle_command(struct nvmet_rdma_queue *queue, · line 715

Quoted 9 lines
drivers/nvme/target/rdma.c +6 -0
@@ -698,6 +707,13 @@ static void nvmet_rdma_handle_command(struct nvmet_rdma_queue *queue,
 	cmd->n_rdma = 0;
 	cmd->req.port = queue->port;
 
+	ib_dma_sync_single_for_cpu(queue->dev->device,
+		cmd->cmd->sge[0].addr, cmd->cmd->sge[0].length,
+		DMA_FROM_DEVICE);
+	ib_dma_sync_single_for_cpu(queue->dev->device,
+			cmd->send_sge.addr, cmd->send_sge.length,
+			DMA_TO_DEVICE);

Why the different indentation here? Both one or two tab indents look fine to me in this context, but don't mix them.

Except for that this looks fine:

Reviewed-by Christoph Hellwig <hch at lst.de>
author
#3 lore raw

PATCH v3 1/1 nvmet-rdma: Fix missing dma sync to nvme data structures

Hi Christoph,

Quoted 10 lines

-----Original Message-----
From: Christoph Hellwig [mailto:hch at lst.de]
Sent: Thursday, January 19, 2017 9:23 AM
To: Parav Pandit <parav at mellanox.com>
Cc: hch at lst.de; sagi at grimberg.me; linux-nvme at lists.infradead.org; linux- rdma at vger.kernel.org; dledford at redhat.com
Subject: Re: [PATCHv3] nvmet-rdma: Fix missing dma sync to nvme data structures

Quoted 1 line
Quoted 10 lines

drivers/nvme/target/rdma.c static void nvmet_rdma_handle_command(struct nvmet_rdma_queue *queue, · line 715

Quoted 6 lines
drivers/nvme/target/rdma.c +6 -0
+	ib_dma_sync_single_for_cpu(queue->dev->device,
+		cmd->cmd->sge[0].addr, cmd->cmd->sge[0].length,
+		DMA_FROM_DEVICE);
+	ib_dma_sync_single_for_cpu(queue->dev->device,
+			cmd->send_sge.addr, cmd->send_sge.length,
+			DMA_TO_DEVICE);

Why the different indentation here? Both one or two tab indents look fine to me in this context, but don't mix them.

I agree. I should have taken care of that.
Shall I resend patch with same indentation at all places or this can be taken care during commit?

Quoted 3 lines

Except for that this looks fine:

Reviewed-by Christoph Hellwig <hch at lst.de>
maintainer
#4 lore raw

PATCH v3 1/1 nvmet-rdma: Fix missing dma sync to nvme data structures

Quoted 2 lines · Parav Pandit

I agree. I should have taken care of that.
Shall I resend patch with same indentation at all places or this can be taken care during commit?

I think Sagi or I can just take care of it on commit.

author
#5 lore raw

PATCH v3 1/1 nvmet-rdma: Fix missing dma sync to nvme data structures

After sending I realized that Doug might also need to do that to merge this to Linux-rdma so that Bart's dma_ops series works correctly. So its work for more people.
Let me just resend it. Its minor.

Parav

Quoted 16 lines

-----Original Message-----
From: Christoph Hellwig [mailto:hch at lst.de]
Sent: Thursday, January 19, 2017 9:46 AM
To: Parav Pandit <parav at mellanox.com>
Cc: Christoph Hellwig <hch at lst.de>; sagi at grimberg.me; linux- nvme at lists.infradead.org; linux-rdma at vger.kernel.org; dledford at redhat.com
Subject: Re: [PATCHv3] nvmet-rdma: Fix missing dma sync to nvme data structures

Quoted 2 lines · Parav Pandit

I agree. I should have taken care of that.
Shall I resend patch with same indentation at all places or this can be taken

care during commit?

I think Sagi or I can just take care of it on commit.