BPF Archive on lore.kernel.org
 help / color / Atom feed
* add an API to check if a streamming mapping needs sync calls
@ 2020-06-29 13:03 Christoph Hellwig
  2020-06-29 13:03 ` [PATCH 1/4] dma-mapping: add a new dma_need_sync API Christoph Hellwig
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Christoph Hellwig @ 2020-06-29 13:03 UTC (permalink / raw)
  To: Björn Töpel
  Cc: Magnus Karlsson, Jonathan Lemon, iommu, netdev, bpf, linux-kernel

Hi all,

this series lifts the somewhat hacky checks in the XSK code if a DMA
streaming mapping needs dma_sync_single_for_{device,cpu} calls to the
DMA API.


Diffstat:
 Documentation/core-api/dma-api.rst |    8 +++++
 include/linux/dma-direct.h         |    1 
 include/linux/dma-mapping.h        |    5 +++
 include/net/xsk_buff_pool.h        |    6 ++--
 kernel/dma/direct.c                |    6 ++++
 kernel/dma/mapping.c               |   10 ++++++
 net/xdp/xsk_buff_pool.c            |   54 ++-----------------------------------
 7 files changed, 37 insertions(+), 53 deletions(-)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/4] dma-mapping: add a new dma_need_sync API
  2020-06-29 13:03 add an API to check if a streamming mapping needs sync calls Christoph Hellwig
@ 2020-06-29 13:03 ` Christoph Hellwig
  2020-07-06 19:42   ` Jonathan Lemon
  2020-06-29 13:03 ` [PATCH 2/4] xsk: replace the cheap_dma flag with a dma_need_sync flag Christoph Hellwig
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Christoph Hellwig @ 2020-06-29 13:03 UTC (permalink / raw)
  To: Björn Töpel
  Cc: Magnus Karlsson, Jonathan Lemon, iommu, netdev, bpf, linux-kernel

Add a new API to check if calls to dma_sync_single_for_{device,cpu} are
required for a given DMA streaming mapping.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 Documentation/core-api/dma-api.rst |  8 ++++++++
 include/linux/dma-direct.h         |  1 +
 include/linux/dma-mapping.h        |  5 +++++
 kernel/dma/direct.c                |  6 ++++++
 kernel/dma/mapping.c               | 10 ++++++++++
 5 files changed, 30 insertions(+)

diff --git a/Documentation/core-api/dma-api.rst b/Documentation/core-api/dma-api.rst
index 2d8d2fed731720..f41620439ef349 100644
--- a/Documentation/core-api/dma-api.rst
+++ b/Documentation/core-api/dma-api.rst
@@ -204,6 +204,14 @@ Returns the maximum size of a mapping for the device. The size parameter
 of the mapping functions like dma_map_single(), dma_map_page() and
 others should not be larger than the returned value.
 
+::
+
+	bool
+	dma_need_sync(struct device *dev, dma_addr_t dma_addr);
+
+Returns %true if dma_sync_single_for_{device,cpu} calls are required to
+transfer memory ownership.  Returns %false if those calls can be skipped.
+
 ::
 
 	unsigned long
diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index cdfa400f89b3d3..5184735a0fe8eb 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -85,4 +85,5 @@ int dma_direct_mmap(struct device *dev, struct vm_area_struct *vma,
 		void *cpu_addr, dma_addr_t dma_addr, size_t size,
 		unsigned long attrs);
 int dma_direct_supported(struct device *dev, u64 mask);
+bool dma_direct_need_sync(struct device *dev, dma_addr_t dma_addr);
 #endif /* _LINUX_DMA_DIRECT_H */
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 78f677cf45ab69..a33ed3954ed465 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -461,6 +461,7 @@ int dma_set_mask(struct device *dev, u64 mask);
 int dma_set_coherent_mask(struct device *dev, u64 mask);
 u64 dma_get_required_mask(struct device *dev);
 size_t dma_max_mapping_size(struct device *dev);
+bool dma_need_sync(struct device *dev, dma_addr_t dma_addr);
 unsigned long dma_get_merge_boundary(struct device *dev);
 #else /* CONFIG_HAS_DMA */
 static inline dma_addr_t dma_map_page_attrs(struct device *dev,
@@ -571,6 +572,10 @@ static inline size_t dma_max_mapping_size(struct device *dev)
 {
 	return 0;
 }
+static inline bool dma_need_sync(struct device *dev, dma_addr_t dma_addr)
+{
+	return false;
+}
 static inline unsigned long dma_get_merge_boundary(struct device *dev)
 {
 	return 0;
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 93f578a8e613ba..95866b64758100 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -539,3 +539,9 @@ size_t dma_direct_max_mapping_size(struct device *dev)
 		return swiotlb_max_mapping_size(dev);
 	return SIZE_MAX;
 }
+
+bool dma_direct_need_sync(struct device *dev, dma_addr_t dma_addr)
+{
+	return !dev_is_dma_coherent(dev) ||
+		is_swiotlb_buffer(dma_to_phys(dev, dma_addr));
+}
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index 98e3d873792ea4..a8c18c9a796fdc 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -397,6 +397,16 @@ size_t dma_max_mapping_size(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(dma_max_mapping_size);
 
+bool dma_need_sync(struct device *dev, dma_addr_t dma_addr)
+{
+	const struct dma_map_ops *ops = get_dma_ops(dev);
+
+	if (dma_is_direct(ops))
+		return dma_direct_need_sync(dev, dma_addr);
+	return ops->sync_single_for_cpu || ops->sync_single_for_device;
+}
+EXPORT_SYMBOL_GPL(dma_need_sync);
+
 unsigned long dma_get_merge_boundary(struct device *dev)
 {
 	const struct dma_map_ops *ops = get_dma_ops(dev);
-- 
2.26.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/4] xsk: replace the cheap_dma flag with a dma_need_sync flag
  2020-06-29 13:03 add an API to check if a streamming mapping needs sync calls Christoph Hellwig
  2020-06-29 13:03 ` [PATCH 1/4] dma-mapping: add a new dma_need_sync API Christoph Hellwig
@ 2020-06-29 13:03 ` Christoph Hellwig
  2020-06-29 13:03 ` [PATCH 3/4] xsk: remove a double pool->dev assignment in xp_dma_map Christoph Hellwig
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2020-06-29 13:03 UTC (permalink / raw)
  To: Björn Töpel
  Cc: Magnus Karlsson, Jonathan Lemon, iommu, netdev, bpf, linux-kernel

Invert the polarity and better name the flag so that the use case is
properly documented.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/net/xsk_buff_pool.h | 6 +++---
 net/xdp/xsk_buff_pool.c     | 5 ++---
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/include/net/xsk_buff_pool.h b/include/net/xsk_buff_pool.h
index a4ff226505c99c..6842990e2712bd 100644
--- a/include/net/xsk_buff_pool.h
+++ b/include/net/xsk_buff_pool.h
@@ -40,7 +40,7 @@ struct xsk_buff_pool {
 	u32 headroom;
 	u32 chunk_size;
 	u32 frame_len;
-	bool cheap_dma;
+	bool dma_need_sync;
 	bool unaligned;
 	void *addrs;
 	struct device *dev;
@@ -80,7 +80,7 @@ static inline dma_addr_t xp_get_frame_dma(struct xdp_buff_xsk *xskb)
 void xp_dma_sync_for_cpu_slow(struct xdp_buff_xsk *xskb);
 static inline void xp_dma_sync_for_cpu(struct xdp_buff_xsk *xskb)
 {
-	if (xskb->pool->cheap_dma)
+	if (!xskb->pool->dma_need_sync)
 		return;
 
 	xp_dma_sync_for_cpu_slow(xskb);
@@ -91,7 +91,7 @@ void xp_dma_sync_for_device_slow(struct xsk_buff_pool *pool, dma_addr_t dma,
 static inline void xp_dma_sync_for_device(struct xsk_buff_pool *pool,
 					  dma_addr_t dma, size_t size)
 {
-	if (pool->cheap_dma)
+	if (!pool->dma_need_sync)
 		return;
 
 	xp_dma_sync_for_device_slow(pool, dma, size);
diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 540ed75e44821c..9fe84c797a7060 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -55,7 +55,6 @@ struct xsk_buff_pool *xp_create(struct page **pages, u32 nr_pages, u32 chunks,
 	pool->free_heads_cnt = chunks;
 	pool->headroom = headroom;
 	pool->chunk_size = chunk_size;
-	pool->cheap_dma = true;
 	pool->unaligned = unaligned;
 	pool->frame_len = chunk_size - headroom - XDP_PACKET_HEADROOM;
 	INIT_LIST_HEAD(&pool->free_list);
@@ -195,7 +194,7 @@ int xp_dma_map(struct xsk_buff_pool *pool, struct device *dev,
 		xp_check_dma_contiguity(pool);
 
 	pool->dev = dev;
-	pool->cheap_dma = xp_check_cheap_dma(pool);
+	pool->dma_need_sync = !xp_check_cheap_dma(pool);
 	return 0;
 }
 EXPORT_SYMBOL(xp_dma_map);
@@ -280,7 +279,7 @@ struct xdp_buff *xp_alloc(struct xsk_buff_pool *pool)
 	xskb->xdp.data = xskb->xdp.data_hard_start + XDP_PACKET_HEADROOM;
 	xskb->xdp.data_meta = xskb->xdp.data;
 
-	if (!pool->cheap_dma) {
+	if (pool->dma_need_sync) {
 		dma_sync_single_range_for_device(pool->dev, xskb->dma, 0,
 						 pool->frame_len,
 						 DMA_BIDIRECTIONAL);
-- 
2.26.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 3/4] xsk: remove a double pool->dev assignment in xp_dma_map
  2020-06-29 13:03 add an API to check if a streamming mapping needs sync calls Christoph Hellwig
  2020-06-29 13:03 ` [PATCH 1/4] dma-mapping: add a new dma_need_sync API Christoph Hellwig
  2020-06-29 13:03 ` [PATCH 2/4] xsk: replace the cheap_dma flag with a dma_need_sync flag Christoph Hellwig
@ 2020-06-29 13:03 ` Christoph Hellwig
  2020-06-29 13:03 ` [PATCH 4/4] xsk: use dma_need_sync instead of reimplenting it Christoph Hellwig
  2020-06-29 13:39 ` add an API to check if a streamming mapping needs sync calls Björn Töpel
  4 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2020-06-29 13:03 UTC (permalink / raw)
  To: Björn Töpel
  Cc: Magnus Karlsson, Jonathan Lemon, iommu, netdev, bpf, linux-kernel

->dev is already assigned at the top of the function, remove the duplicate
one at the end.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 net/xdp/xsk_buff_pool.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 9fe84c797a7060..6733e2c59e4835 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -193,7 +193,6 @@ int xp_dma_map(struct xsk_buff_pool *pool, struct device *dev,
 	if (pool->unaligned)
 		xp_check_dma_contiguity(pool);
 
-	pool->dev = dev;
 	pool->dma_need_sync = !xp_check_cheap_dma(pool);
 	return 0;
 }
-- 
2.26.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 4/4] xsk: use dma_need_sync instead of reimplenting it
  2020-06-29 13:03 add an API to check if a streamming mapping needs sync calls Christoph Hellwig
                   ` (2 preceding siblings ...)
  2020-06-29 13:03 ` [PATCH 3/4] xsk: remove a double pool->dev assignment in xp_dma_map Christoph Hellwig
@ 2020-06-29 13:03 ` Christoph Hellwig
  2020-06-29 13:39 ` add an API to check if a streamming mapping needs sync calls Björn Töpel
  4 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2020-06-29 13:03 UTC (permalink / raw)
  To: Björn Töpel
  Cc: Magnus Karlsson, Jonathan Lemon, iommu, netdev, bpf, linux-kernel

Use the dma_need_sync helper instead of (not always entirely correctly)
poking into the dma-mapping internals.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 net/xdp/xsk_buff_pool.c | 50 +++--------------------------------------
 1 file changed, 3 insertions(+), 47 deletions(-)

diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 6733e2c59e4835..08b80669f64955 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -2,9 +2,6 @@
 
 #include <net/xsk_buff_pool.h>
 #include <net/xdp_sock.h>
-#include <linux/dma-direct.h>
-#include <linux/dma-noncoherent.h>
-#include <linux/swiotlb.h>
 
 #include "xsk_queue.h"
 
@@ -124,48 +121,6 @@ static void xp_check_dma_contiguity(struct xsk_buff_pool *pool)
 	}
 }
 
-static bool __maybe_unused xp_check_swiotlb_dma(struct xsk_buff_pool *pool)
-{
-#if defined(CONFIG_SWIOTLB)
-	phys_addr_t paddr;
-	u32 i;
-
-	for (i = 0; i < pool->dma_pages_cnt; i++) {
-		paddr = dma_to_phys(pool->dev, pool->dma_pages[i]);
-		if (is_swiotlb_buffer(paddr))
-			return false;
-	}
-#endif
-	return true;
-}
-
-static bool xp_check_cheap_dma(struct xsk_buff_pool *pool)
-{
-#if defined(CONFIG_HAS_DMA)
-	const struct dma_map_ops *ops = get_dma_ops(pool->dev);
-
-	if (ops) {
-		return !ops->sync_single_for_cpu &&
-			!ops->sync_single_for_device;
-	}
-
-	if (!dma_is_direct(ops))
-		return false;
-
-	if (!xp_check_swiotlb_dma(pool))
-		return false;
-
-	if (!dev_is_dma_coherent(pool->dev)) {
-#if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU) ||		\
-	defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL) ||	\
-	defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE)
-		return false;
-#endif
-	}
-#endif
-	return true;
-}
-
 int xp_dma_map(struct xsk_buff_pool *pool, struct device *dev,
 	       unsigned long attrs, struct page **pages, u32 nr_pages)
 {
@@ -179,6 +134,7 @@ int xp_dma_map(struct xsk_buff_pool *pool, struct device *dev,
 
 	pool->dev = dev;
 	pool->dma_pages_cnt = nr_pages;
+	pool->dma_need_sync = false;
 
 	for (i = 0; i < pool->dma_pages_cnt; i++) {
 		dma = dma_map_page_attrs(dev, pages[i], 0, PAGE_SIZE,
@@ -187,13 +143,13 @@ int xp_dma_map(struct xsk_buff_pool *pool, struct device *dev,
 			xp_dma_unmap(pool, attrs);
 			return -ENOMEM;
 		}
+		if (dma_need_sync(dev, dma))
+			pool->dma_need_sync = true;
 		pool->dma_pages[i] = dma;
 	}
 
 	if (pool->unaligned)
 		xp_check_dma_contiguity(pool);
-
-	pool->dma_need_sync = !xp_check_cheap_dma(pool);
 	return 0;
 }
 EXPORT_SYMBOL(xp_dma_map);
-- 
2.26.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: add an API to check if a streamming mapping needs sync calls
  2020-06-29 13:03 add an API to check if a streamming mapping needs sync calls Christoph Hellwig
                   ` (3 preceding siblings ...)
  2020-06-29 13:03 ` [PATCH 4/4] xsk: use dma_need_sync instead of reimplenting it Christoph Hellwig
@ 2020-06-29 13:39 ` Björn Töpel
  2020-06-29 14:22   ` Christoph Hellwig
  2020-07-08  7:44   ` Christoph Hellwig
  4 siblings, 2 replies; 13+ messages in thread
From: Björn Töpel @ 2020-06-29 13:39 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Magnus Karlsson, Jonathan Lemon, iommu, netdev, bpf, linux-kernel

On 2020-06-29 15:03, Christoph Hellwig wrote:
> Hi all,
> 
> this series lifts the somewhat hacky checks in the XSK code if a DMA
> streaming mapping needs dma_sync_single_for_{device,cpu} calls to the
> DMA API.
>

Thanks a lot for working on, and fixing this, Christoph!

I took the series for a spin, and there are (obviously) no performance
regressions.

Would the patches go through the net/bpf trees or somewhere else?

For the series:
Tested-by: Björn Töpel <bjorn.topel@intel.com>
Acked-by: Björn Töpel <bjorn.topel@intel.com>


Björn

> 
> Diffstat:
>   Documentation/core-api/dma-api.rst |    8 +++++
>   include/linux/dma-direct.h         |    1
>   include/linux/dma-mapping.h        |    5 +++
>   include/net/xsk_buff_pool.h        |    6 ++--
>   kernel/dma/direct.c                |    6 ++++
>   kernel/dma/mapping.c               |   10 ++++++
>   net/xdp/xsk_buff_pool.c            |   54 ++-----------------------------------
>   7 files changed, 37 insertions(+), 53 deletions(-)
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: add an API to check if a streamming mapping needs sync calls
  2020-06-29 13:39 ` add an API to check if a streamming mapping needs sync calls Björn Töpel
@ 2020-06-29 14:22   ` Christoph Hellwig
  2020-07-08  7:44   ` Christoph Hellwig
  1 sibling, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2020-06-29 14:22 UTC (permalink / raw)
  To: Björn Töpel
  Cc: Christoph Hellwig, Magnus Karlsson, Jonathan Lemon, iommu,
	netdev, bpf, linux-kernel

On Mon, Jun 29, 2020 at 03:39:01PM +0200, Björn Töpel wrote:
> On 2020-06-29 15:03, Christoph Hellwig wrote:
>> Hi all,
>>
>> this series lifts the somewhat hacky checks in the XSK code if a DMA
>> streaming mapping needs dma_sync_single_for_{device,cpu} calls to the
>> DMA API.
>>
>
> Thanks a lot for working on, and fixing this, Christoph!
>
> I took the series for a spin, and there are (obviously) no performance
> regressions.
>
> Would the patches go through the net/bpf trees or somewhere else?

either the net tree or (my) dma-mapping tree would be fine with me.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] dma-mapping: add a new dma_need_sync API
  2020-06-29 13:03 ` [PATCH 1/4] dma-mapping: add a new dma_need_sync API Christoph Hellwig
@ 2020-07-06 19:42   ` Jonathan Lemon
  2020-07-07  6:47     ` Christoph Hellwig
  0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Lemon @ 2020-07-06 19:42 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Björn Töpel, Magnus Karlsson, iommu, netdev, bpf, linux-kernel

On Mon, Jun 29, 2020 at 03:03:56PM +0200, Christoph Hellwig wrote:
> Add a new API to check if calls to dma_sync_single_for_{device,cpu} are
> required for a given DMA streaming mapping.
> 
> +::
> +
> +	bool
> +	dma_need_sync(struct device *dev, dma_addr_t dma_addr);
> +
> +Returns %true if dma_sync_single_for_{device,cpu} calls are required to
> +transfer memory ownership.  Returns %false if those calls can be skipped.

Hi Christoph -

Thie call above is for a specific dma_addr.  For correctness, would I
need to check every addr, or can I assume that for a specific memory
type (pages returned from malloc), that the answer would be identical?
-- 
Jonathan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] dma-mapping: add a new dma_need_sync API
  2020-07-06 19:42   ` Jonathan Lemon
@ 2020-07-07  6:47     ` Christoph Hellwig
  2020-07-07 15:11       ` Jonathan Lemon
  0 siblings, 1 reply; 13+ messages in thread
From: Christoph Hellwig @ 2020-07-07  6:47 UTC (permalink / raw)
  To: Jonathan Lemon
  Cc: Christoph Hellwig, Björn Töpel, Magnus Karlsson, iommu,
	netdev, bpf, linux-kernel

On Mon, Jul 06, 2020 at 12:42:27PM -0700, Jonathan Lemon wrote:
> On Mon, Jun 29, 2020 at 03:03:56PM +0200, Christoph Hellwig wrote:
> > Add a new API to check if calls to dma_sync_single_for_{device,cpu} are
> > required for a given DMA streaming mapping.
> > 
> > +::
> > +
> > +	bool
> > +	dma_need_sync(struct device *dev, dma_addr_t dma_addr);
> > +
> > +Returns %true if dma_sync_single_for_{device,cpu} calls are required to
> > +transfer memory ownership.  Returns %false if those calls can be skipped.
> 
> Hi Christoph -
> 
> Thie call above is for a specific dma_addr.  For correctness, would I
> need to check every addr, or can I assume that for a specific memory
> type (pages returned from malloc), that the answer would be identical?

You need to check every mapping.  E.g. this API pairs with a
dma_map_single/page call.  For S/G mappings you'd need to call it for
each entry, although if you have a use case for that we really should
add a dma_sg_need_sync helper instea of open coding the scatterlist walk.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] dma-mapping: add a new dma_need_sync API
  2020-07-07  6:47     ` Christoph Hellwig
@ 2020-07-07 15:11       ` Jonathan Lemon
  2020-07-07 15:14         ` Christoph Hellwig
  0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Lemon @ 2020-07-07 15:11 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Björn Töpel, Magnus Karlsson, iommu, netdev, bpf, linux-kernel

On Tue, Jul 07, 2020 at 08:47:30AM +0200, Christoph Hellwig wrote:
> On Mon, Jul 06, 2020 at 12:42:27PM -0700, Jonathan Lemon wrote:
> > On Mon, Jun 29, 2020 at 03:03:56PM +0200, Christoph Hellwig wrote:
> > > Add a new API to check if calls to dma_sync_single_for_{device,cpu} are
> > > required for a given DMA streaming mapping.
> > > 
> > > +::
> > > +
> > > +	bool
> > > +	dma_need_sync(struct device *dev, dma_addr_t dma_addr);
> > > +
> > > +Returns %true if dma_sync_single_for_{device,cpu} calls are required to
> > > +transfer memory ownership.  Returns %false if those calls can be skipped.
> > 
> > Hi Christoph -
> > 
> > Thie call above is for a specific dma_addr.  For correctness, would I
> > need to check every addr, or can I assume that for a specific memory
> > type (pages returned from malloc), that the answer would be identical?
> 
> You need to check every mapping.  E.g. this API pairs with a
> dma_map_single/page call.  For S/G mappings you'd need to call it for
> each entry, although if you have a use case for that we really should
> add a dma_sg_need_sync helper instea of open coding the scatterlist walk.

My use case is setting up a pinned memory area, and caching the dma
mappings.  I'd like to bypass storing the DMA addresses if they aren't
needed.  For example:

setup()
{
    if (dma_need_sync(dev, addr, len)) {
        kvmalloc_array(...)
        cache_dma_mappings(...)
    }


dev_get_dma(page)
{
    if (!cache)
        return page_to_phys(page)

    return dma_cache_lookup(...)



The reason for doing it this way is that the page in question may be
backed by either system memory, or device memory such as a GPU.  For the
latter, the GPU provides a table of DMA addresses where data may be
accessed, so I'm unable to use the dma_map_page() API.
-- 
Jonathan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] dma-mapping: add a new dma_need_sync API
  2020-07-07 15:11       ` Jonathan Lemon
@ 2020-07-07 15:14         ` Christoph Hellwig
  0 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2020-07-07 15:14 UTC (permalink / raw)
  To: Jonathan Lemon
  Cc: Christoph Hellwig, Björn Töpel, Magnus Karlsson, iommu,
	netdev, bpf, linux-kernel

On Tue, Jul 07, 2020 at 08:11:09AM -0700, Jonathan Lemon wrote:
> > You need to check every mapping.  E.g. this API pairs with a
> > dma_map_single/page call.  For S/G mappings you'd need to call it for
> > each entry, although if you have a use case for that we really should
> > add a dma_sg_need_sync helper instea of open coding the scatterlist walk.
> 
> My use case is setting up a pinned memory area, and caching the dma
> mappings.  I'd like to bypass storing the DMA addresses if they aren't
> needed.  For example:
> 
> setup()
> {
>     if (dma_need_sync(dev, addr, len)) {
>         kvmalloc_array(...)
>         cache_dma_mappings(...)
>     }
> 
> 
> dev_get_dma(page)
> {
>     if (!cache)
>         return page_to_phys(page)
> 
>     return dma_cache_lookup(...)
> 
> 
> 
> The reason for doing it this way is that the page in question may be
> backed by either system memory, or device memory such as a GPU.  For the
> latter, the GPU provides a table of DMA addresses where data may be
> accessed, so I'm unable to use the dma_map_page() API.

dma_need_sync doesn't tell you if the unmap needs the dma_addr_t.
I've been think about replacing CONFIG_NEED_DMA_MAP_STATE with a runtime
for a while, which would give you exattly what you need.  For now it
isn't very useful as there are very few configs left that do not have
CONFIG_NEED_DMA_MAP_STATE set.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: add an API to check if a streamming mapping needs sync calls
  2020-06-29 13:39 ` add an API to check if a streamming mapping needs sync calls Björn Töpel
  2020-06-29 14:22   ` Christoph Hellwig
@ 2020-07-08  7:44   ` Christoph Hellwig
  2020-07-08  9:01     ` Daniel Borkmann
  1 sibling, 1 reply; 13+ messages in thread
From: Christoph Hellwig @ 2020-07-08  7:44 UTC (permalink / raw)
  To: Björn Töpel
  Cc: Christoph Hellwig, Magnus Karlsson, Jonathan Lemon, iommu,
	netdev, bpf, linux-kernel

On Mon, Jun 29, 2020 at 03:39:01PM +0200, Björn Töpel wrote:
> On 2020-06-29 15:03, Christoph Hellwig wrote:
>> Hi all,
>>
>> this series lifts the somewhat hacky checks in the XSK code if a DMA
>> streaming mapping needs dma_sync_single_for_{device,cpu} calls to the
>> DMA API.
>>
>
> Thanks a lot for working on, and fixing this, Christoph!
>
> I took the series for a spin, and there are (obviously) no performance
> regressions.
>
> Would the patches go through the net/bpf trees or somewhere else?

Where did this end up?  I still don't see it in Linus' tree and this
is getting urgent now.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: add an API to check if a streamming mapping needs sync calls
  2020-07-08  7:44   ` Christoph Hellwig
@ 2020-07-08  9:01     ` Daniel Borkmann
  0 siblings, 0 replies; 13+ messages in thread
From: Daniel Borkmann @ 2020-07-08  9:01 UTC (permalink / raw)
  To: Christoph Hellwig, Björn Töpel
  Cc: Magnus Karlsson, Jonathan Lemon, iommu, netdev, bpf, linux-kernel

On 7/8/20 9:44 AM, Christoph Hellwig wrote:
> On Mon, Jun 29, 2020 at 03:39:01PM +0200, Björn Töpel wrote:
>> On 2020-06-29 15:03, Christoph Hellwig wrote:
>>> Hi all,
>>>
>>> this series lifts the somewhat hacky checks in the XSK code if a DMA
>>> streaming mapping needs dma_sync_single_for_{device,cpu} calls to the
>>> DMA API.
>>>
>>
>> Thanks a lot for working on, and fixing this, Christoph!
>>
>> I took the series for a spin, and there are (obviously) no performance
>> regressions.
>>
>> Would the patches go through the net/bpf trees or somewhere else?
> 
> Where did this end up?  I still don't see it in Linus' tree and this
> is getting urgent now.

It was merged into bpf tree and we sent the PR to DaveM which was merged into
net tree around a week ago [0]; I assume the PR for net might go to Linus soon
this week.

   [0] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=e708e2bd55c921f5bb554fa5837d132a878951cf

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, back to index

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-29 13:03 add an API to check if a streamming mapping needs sync calls Christoph Hellwig
2020-06-29 13:03 ` [PATCH 1/4] dma-mapping: add a new dma_need_sync API Christoph Hellwig
2020-07-06 19:42   ` Jonathan Lemon
2020-07-07  6:47     ` Christoph Hellwig
2020-07-07 15:11       ` Jonathan Lemon
2020-07-07 15:14         ` Christoph Hellwig
2020-06-29 13:03 ` [PATCH 2/4] xsk: replace the cheap_dma flag with a dma_need_sync flag Christoph Hellwig
2020-06-29 13:03 ` [PATCH 3/4] xsk: remove a double pool->dev assignment in xp_dma_map Christoph Hellwig
2020-06-29 13:03 ` [PATCH 4/4] xsk: use dma_need_sync instead of reimplenting it Christoph Hellwig
2020-06-29 13:39 ` add an API to check if a streamming mapping needs sync calls Björn Töpel
2020-06-29 14:22   ` Christoph Hellwig
2020-07-08  7:44   ` Christoph Hellwig
2020-07-08  9:01     ` Daniel Borkmann

BPF Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/bpf/0 bpf/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 bpf bpf/ https://lore.kernel.org/bpf \
		bpf@vger.kernel.org
	public-inbox-index bpf

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.bpf


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git