diff mbox

[v2,00/26] IB: Optimize DMA mapping

Message ID 1484690386.2729.8.camel@sandisk.com (mailing list archive)
State Superseded
Headers show

Commit Message

Bart Van Assche Jan. 17, 2017, 10 p.m. UTC
On Tue, 2017-01-17 at 13:48 -0800, Bart Van Assche wrote:
> On Sat, 2017-01-14 at 02:05 +0000, Estrin, Alex wrote:
> > [ ... ]
> > please see hfi1/verbs.c @ hfi1_register_ib_device()  
> > [ ... ]
> 
> Hello Alex,
> 
> I think I figured out what I did wrong: both the hfi1 and the qib drivers need two
> sets of DMA mapping operations. ULPs have to use &dma_virt_ops and the SDMA code
> has to use the PCIe DMA mapping operations. My patch series made the SDMA code use
> dma_virt_ops and that's wrong. The attached patch should fix this. Unfortunately I
> do not have access to a hfi1 or qib test setup. Can you help me by testing the
> attached patch on top of the already posted patches?

Hello Alex,

The wrong version of my patch was attached to previous e-mail. Sorry for this. Please
use the patch attached to this e-mail.

Thanks,

Bart.

Comments

Jason Gunthorpe Jan. 17, 2017, 10:27 p.m. UTC | #1
On Tue, Jan 17, 2017 at 10:00:00PM +0000, Bart Van Assche wrote:
> +	/*
> +	 * qib and hfi1 use two sets of DMA operations:
> +	 * - The DMA operations of the PCIe device for SDMA.
> +	 * - dma_virt_ops for users of the qib and hfi1 drivers.
> +	 * The only purpose of @dma_device is to provide a struct device that
> +	 * provides dma_virt_ops.
> +	 */
> +	struct device dma_device;

Creating a struct device, not calling device_initialize() and then
passing it around it other places is pretty ugly and potentially
dangerous.

Is there some other way?

Can you setup ibdev to be the 'dma device' for the user side?

Jason
--
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
Bart Van Assche Jan. 17, 2017, 10:33 p.m. UTC | #2
On Tue, 2017-01-17 at 15:27 -0700, Jason Gunthorpe wrote:
> On Tue, Jan 17, 2017 at 10:00:00PM +0000, Bart Van Assche wrote:
> > +	/*
> > +	 * qib and hfi1 use two sets of DMA operations:
> > +	 * - The DMA operations of the PCIe device for SDMA.
> > +	 * - dma_virt_ops for users of the qib and hfi1 drivers.
> > +	 * The only purpose of @dma_device is to provide a struct device that
> > +	 * provides dma_virt_ops.
> > +	 */
> > +	struct device dma_device;
> 
> Creating a struct device, not calling device_initialize() and then
> passing it around it other places is pretty ugly and potentially
> dangerous.
> 
> Is there some other way?
> 
> Can you setup ibdev to be the 'dma device' for the user side?

Hello Jason,

The only member of this struct device that is used is dma_ops so this approach
should be fine as an interim solution. My plan is, if this patch is sufficient
to unbreak the hfi1 driver, to remove the struct device cited in your e-mail
again and also the dma_device pointer from struct ib_device. Since the
dev.dma_ops pointer in struct ib_device is not yet used dev.dma_ops can be used
instead of the dma_device pointer.

Bart.--
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
Jason Gunthorpe Jan. 17, 2017, 10:50 p.m. UTC | #3
On Tue, Jan 17, 2017 at 10:33:16PM +0000, Bart Van Assche wrote:

> again and also the dma_device pointer from struct ib_device. Since the
> dev.dma_ops pointer in struct ib_device is not yet used dev.dma_ops can be used
> instead of the dma_device pointer.

okay great

Jason
--
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

From a462991fadfd1c350ecb41635f1157e227e133ef Mon Sep 17 00:00:00 2001
From: Bart Van Assche <bart.vanassche@sandisk.com>
Date: Tue, 17 Jan 2017 13:22:35 -0800
Subject: [PATCH] hfi1, qib, rdmavt [v2]: Use proper DMA mapping operations

Make users of the hfi1 and qib drivers use dma_virt_ops. Use the PCIe
mapping operations for SDMA. Make sure that the rdmavt code does not
change the PCIe DMA mapping operations pointer into &dma_virt_ops.
---
 drivers/infiniband/hw/hfi1/mad.c      | 2 +-
 drivers/infiniband/hw/hfi1/verbs.c    | 1 -
 drivers/infiniband/hw/qib/qib_verbs.c | 1 -
 drivers/infiniband/sw/rdmavt/vt.c     | 2 ++
 include/rdma/rdma_vt.h                | 8 ++++++++
 5 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/mad.c b/drivers/infiniband/hw/hfi1/mad.c
index 6e595afca24c..041d503c399b 100644
--- a/drivers/infiniband/hw/hfi1/mad.c
+++ b/drivers/infiniband/hw/hfi1/mad.c
@@ -4406,7 +4406,7 @@  int hfi1_process_mad(struct ib_device *ibdev, int mad_flags, u8 port,
 	switch (in_mad->base_version) {
 	case OPA_MGMT_BASE_VERSION:
 		if (unlikely(in_mad_size != sizeof(struct opa_mad))) {
-			dev_err(ibdev->dma_device, "invalid in_mad_size\n");
+			dev_err(&ibdev->dev, "invalid in_mad_size\n");
 			return IB_MAD_RESULT_FAILURE;
 		}
 		return hfi1_process_opa_mad(ibdev, mad_flags, port,
diff --git a/drivers/infiniband/hw/hfi1/verbs.c b/drivers/infiniband/hw/hfi1/verbs.c
index 95ed4d6da510..fa0ff3e11597 100644
--- a/drivers/infiniband/hw/hfi1/verbs.c
+++ b/drivers/infiniband/hw/hfi1/verbs.c
@@ -1784,7 +1784,6 @@  int hfi1_register_ib_device(struct hfi1_devdata *dd)
 	strlcpy(ibdev->name + lcpysz, "_%d", IB_DEVICE_NAME_MAX - lcpysz);
 	ibdev->owner = THIS_MODULE;
 	ibdev->phys_port_cnt = dd->num_pports;
-	ibdev->dma_device = &dd->pcidev->dev;
 	ibdev->modify_device = modify_device;
 	ibdev->alloc_hw_stats = alloc_hw_stats;
 	ibdev->get_hw_stats = get_hw_stats;
diff --git a/drivers/infiniband/hw/qib/qib_verbs.c b/drivers/infiniband/hw/qib/qib_verbs.c
index 4b54c0ddd08a..daa28a7b7574 100644
--- a/drivers/infiniband/hw/qib/qib_verbs.c
+++ b/drivers/infiniband/hw/qib/qib_verbs.c
@@ -1632,7 +1632,6 @@  int qib_register_ib_device(struct qib_devdata *dd)
 	ibdev->owner = THIS_MODULE;
 	ibdev->node_guid = ppd->guid;
 	ibdev->phys_port_cnt = dd->num_pports;
-	ibdev->dma_device = &dd->pcidev->dev;
 	ibdev->modify_device = qib_modify_device;
 	ibdev->process_mad = qib_process_mad;
 
diff --git a/drivers/infiniband/sw/rdmavt/vt.c b/drivers/infiniband/sw/rdmavt/vt.c
index 6a81b179f631..99b3cb39ed67 100644
--- a/drivers/infiniband/sw/rdmavt/vt.c
+++ b/drivers/infiniband/sw/rdmavt/vt.c
@@ -103,6 +103,8 @@  struct rvt_dev_info *rvt_alloc_device(size_t size, int nports)
 	if (!rdi->ports)
 		ib_dealloc_device(&rdi->ibdev);
 
+	rdi->ibdev.dma_device = &rdi->dma_device;
+
 	return rdi;
 }
 EXPORT_SYMBOL(rvt_alloc_device);
diff --git a/include/rdma/rdma_vt.h b/include/rdma/rdma_vt.h
index 861e23eaebda..b7259c9b056e 100644
--- a/include/rdma/rdma_vt.h
+++ b/include/rdma/rdma_vt.h
@@ -339,6 +339,14 @@  struct rvt_driver_provided {
 
 struct rvt_dev_info {
 	struct ib_device ibdev; /* Keep this first. Nothing above here */
+	/*
+	 * qib and hfi1 use two sets of DMA operations:
+	 * - The DMA operations of the PCIe device for SDMA.
+	 * - dma_virt_ops for users of the qib and hfi1 drivers.
+	 * The only purpose of @dma_device is to provide a struct device that
+	 * provides dma_virt_ops.
+	 */
+	struct device dma_device;
 
 	/*
 	 * Prior to calling for registration the driver will be responsible for
-- 
2.11.0