[PATCH net-next v2 0/5] devmem TCP fixes

[PATCH net-next v2 0/5] devmem TCP fixes

[Mina Almasry]

Couple unrelated devmem TCP fixes bundled in a series for some
convenience.

- fix naming and provide page_pool_alloc_netmem for fragged
netmem.

- fix issues with dma-buf dma addresses being potentially
passed to dma_sync_for_* helpers.

- add netmem_prefetch helper.

---

v2:
- Fork off the syzbot fixes to net tree, resubmit the rest here.


Mina Almasry (4):
  net: page_pool: rename page_pool_alloc_netmem to *_netmems
  net: page_pool: create page_pool_alloc_netmem
  page_pool: disable sync for cpu for dmabuf memory provider
  netmem: add netmem_prefetch

Samiullah Khawaja (1):
  page_pool: Set `dma_sync` to false for devmem memory provider

 include/net/netmem.h            |  7 +++++
 include/net/page_pool/helpers.h | 50 +++++++++++++++++++++++----------
 include/net/page_pool/types.h   |  2 +-
 net/core/devmem.c               |  9 +++---
 net/core/page_pool.c            | 11 +++++---
 5 files changed, 54 insertions(+), 25 deletions(-)

-- 
2.47.0.277.g8800431eea-goog

[PATCH net-next v2 1/5] net: page_pool: rename page_pool_alloc_netmem to *_netmems

[Mina Almasry]

page_pool_alloc_netmem (without an s) was the mirror of
page_pool_alloc_pages (with an s), which was confusing.

Rename to page_pool_alloc_netmems so it's the mirror of
page_pool_alloc_pages.

Signed-off-by: Mina Almasry <almasrymina@google.com>
---
 include/net/page_pool/types.h | 2 +-
 net/core/page_pool.c          | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h
index c022c410abe3..8f564fe9eb9a 100644
--- a/include/net/page_pool/types.h
+++ b/include/net/page_pool/types.h
@@ -242,7 +242,7 @@ struct page_pool {
 };
 
 struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp);
-netmem_ref page_pool_alloc_netmem(struct page_pool *pool, gfp_t gfp);
+netmem_ref page_pool_alloc_netmems(struct page_pool *pool, gfp_t gfp);
 struct page *page_pool_alloc_frag(struct page_pool *pool, unsigned int *offset,
 				  unsigned int size, gfp_t gfp);
 netmem_ref page_pool_alloc_frag_netmem(struct page_pool *pool,
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index a813d30d2135..77de79c1933b 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -574,7 +574,7 @@ static noinline netmem_ref __page_pool_alloc_pages_slow(struct page_pool *pool,
 /* For using page_pool replace: alloc_pages() API calls, but provide
  * synchronization guarantee for allocation side.
  */
-netmem_ref page_pool_alloc_netmem(struct page_pool *pool, gfp_t gfp)
+netmem_ref page_pool_alloc_netmems(struct page_pool *pool, gfp_t gfp)
 {
 	netmem_ref netmem;
 
@@ -590,11 +590,11 @@ netmem_ref page_pool_alloc_netmem(struct page_pool *pool, gfp_t gfp)
 		netmem = __page_pool_alloc_pages_slow(pool, gfp);
 	return netmem;
 }
-EXPORT_SYMBOL(page_pool_alloc_netmem);
+EXPORT_SYMBOL(page_pool_alloc_netmems);
 
 struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp)
 {
-	return netmem_to_page(page_pool_alloc_netmem(pool, gfp));
+	return netmem_to_page(page_pool_alloc_netmems(pool, gfp));
 }
 EXPORT_SYMBOL(page_pool_alloc_pages);
 ALLOW_ERROR_INJECTION(page_pool_alloc_pages, NULL);
@@ -956,7 +956,7 @@ netmem_ref page_pool_alloc_frag_netmem(struct page_pool *pool,
 	}
 
 	if (!netmem) {
-		netmem = page_pool_alloc_netmem(pool, gfp);
+		netmem = page_pool_alloc_netmems(pool, gfp);
 		if (unlikely(!netmem)) {
 			pool->frag_page = 0;
 			return 0;
-- 
2.47.0.277.g8800431eea-goog

[PATCH net-next v2 2/5] net: page_pool: create page_pool_alloc_netmem

[Mina Almasry]

Create page_pool_alloc_netmem to be the mirror of page_pool_alloc.

This enables drivers that want currently use page_pool_alloc to
transition to netmem by converting the call sites to
page_pool_alloc_netmem.

Signed-off-by: Mina Almasry <almasrymina@google.com>
---
 include/net/page_pool/helpers.h | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h
index 793e6fd78bc5..8e548ff3044c 100644
--- a/include/net/page_pool/helpers.h
+++ b/include/net/page_pool/helpers.h
@@ -116,22 +116,22 @@ static inline struct page *page_pool_dev_alloc_frag(struct page_pool *pool,
 	return page_pool_alloc_frag(pool, offset, size, gfp);
 }
 
-static inline struct page *page_pool_alloc(struct page_pool *pool,
-					   unsigned int *offset,
-					   unsigned int *size, gfp_t gfp)
+static inline netmem_ref page_pool_alloc_netmem(struct page_pool *pool,
+						unsigned int *offset,
+						unsigned int *size, gfp_t gfp)
 {
 	unsigned int max_size = PAGE_SIZE << pool->p.order;
-	struct page *page;
+	netmem_ref netmem;
 
 	if ((*size << 1) > max_size) {
 		*size = max_size;
 		*offset = 0;
-		return page_pool_alloc_pages(pool, gfp);
+		return page_pool_alloc_netmems(pool, gfp);
 	}
 
-	page = page_pool_alloc_frag(pool, offset, *size, gfp);
-	if (unlikely(!page))
-		return NULL;
+	netmem = page_pool_alloc_frag_netmem(pool, offset, *size, gfp);
+	if (unlikely(!netmem))
+		return 0;
 
 	/* There is very likely not enough space for another fragment, so append
 	 * the remaining size to the current fragment to avoid truesize
@@ -142,7 +142,14 @@ static inline struct page *page_pool_alloc(struct page_pool *pool,
 		pool->frag_offset = max_size;
 	}
 
-	return page;
+	return netmem;
+}
+
+static inline struct page *page_pool_alloc(struct page_pool *pool,
+					   unsigned int *offset,
+					   unsigned int *size, gfp_t gfp)
+{
+	return netmem_to_page(page_pool_alloc_netmem(pool, offset, size, gfp));
 }
 
 /**
-- 
2.47.0.277.g8800431eea-goog

[PATCH net-next v2 3/5] page_pool: Set `dma_sync` to false for devmem memory provider

[Mina Almasry]

From: Samiullah Khawaja <skhawaja@google.com>

Move the `dma_map` and `dma_sync` checks to `page_pool_init` to make
them generic. Set dma_sync to false for devmem memory provider because
the dma_sync APIs should not be used for dma_buf backed devmem memory
provider.

Cc: Jason Gunthorpe <jgg@ziepe.ca>
Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
Signed-off-by: Mina Almasry <almasrymina@google.com>

---
 net/core/devmem.c    | 9 ++++-----
 net/core/page_pool.c | 3 +++
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/net/core/devmem.c b/net/core/devmem.c
index 11b91c12ee11..826d0b00159f 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -331,11 +331,10 @@ int mp_dmabuf_devmem_init(struct page_pool *pool)
 	if (!binding)
 		return -EINVAL;
 
-	if (!pool->dma_map)
-		return -EOPNOTSUPP;
-
-	if (pool->dma_sync)
-		return -EOPNOTSUPP;
+	/* dma-buf dma addresses should not be used with
+	 * dma_sync_for_cpu/device. Force disable dma_sync.
+	 */
+	pool->dma_sync = false;
 
 	if (pool->p.order != 0)
 		return -E2BIG;
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index 77de79c1933b..528dd4d18eab 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -287,6 +287,9 @@ static int page_pool_init(struct page_pool *pool,
 	}
 
 	if (pool->mp_priv) {
+		if (!pool->dma_map || !pool->dma_sync)
+			return -EOPNOTSUPP;
+
 		err = mp_dmabuf_devmem_init(pool);
 		if (err) {
 			pr_warn("%s() mem-provider init failed %d\n", __func__,
-- 
2.47.0.277.g8800431eea-goog

[PATCH net-next v2 4/5] page_pool: disable sync for cpu for dmabuf memory provider

[Mina Almasry]

dmabuf dma-addresses should not be dma_sync'd for CPU/device. Typically
its the driver responsibility to dma_sync for CPU, but the driver should
not dma_sync for CPU if the netmem is actually coming from a dmabuf
memory provider.

The page_pool already exposes a helper for dma_sync_for_cpu:
page_pool_dma_sync_for_cpu. Upgrade this existing helper to handle
netmem, and have it skip dma_sync if the memory is from a dmabuf memory
provider. Drivers should migrate to using this helper when adding
support for netmem.

Cc: Jason Gunthorpe <jgg@ziepe.ca>
Signed-off-by: Mina Almasry <almasrymina@google.com>

---
 include/net/page_pool/helpers.h | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h
index 8e548ff3044c..ad4fed4a791c 100644
--- a/include/net/page_pool/helpers.h
+++ b/include/net/page_pool/helpers.h
@@ -429,9 +429,10 @@ static inline dma_addr_t page_pool_get_dma_addr(const struct page *page)
 }
 
 /**
- * page_pool_dma_sync_for_cpu - sync Rx page for CPU after it's written by HW
+ * page_pool_dma_sync_netmem_for_cpu - sync Rx page for CPU after it's written
+ *				       by HW
  * @pool: &page_pool the @page belongs to
- * @page: page to sync
+ * @netmem: netmem to sync
  * @offset: offset from page start to "hard" start if using PP frags
  * @dma_sync_size: size of the data written to the page
  *
@@ -440,16 +441,28 @@ static inline dma_addr_t page_pool_get_dma_addr(const struct page *page)
  * Note that this version performs DMA sync unconditionally, even if the
  * associated PP doesn't perform sync-for-device.
  */
-static inline void page_pool_dma_sync_for_cpu(const struct page_pool *pool,
-					      const struct page *page,
-					      u32 offset, u32 dma_sync_size)
+static inline void
+page_pool_dma_sync_netmem_for_cpu(const struct page_pool *pool,
+				  const netmem_ref netmem, u32 offset,
+				  u32 dma_sync_size)
 {
+	if (pool->mp_priv)
+		return;
+
 	dma_sync_single_range_for_cpu(pool->p.dev,
-				      page_pool_get_dma_addr(page),
+				      page_pool_get_dma_addr_netmem(netmem),
 				      offset + pool->p.offset, dma_sync_size,
 				      page_pool_get_dma_dir(pool));
 }
 
+static inline void page_pool_dma_sync_for_cpu(const struct page_pool *pool,
+					      struct page *page, u32 offset,
+					      u32 dma_sync_size)
+{
+	page_pool_dma_sync_netmem_for_cpu(pool, page_to_netmem(page), offset,
+					  dma_sync_size);
+}
+
 static inline bool page_pool_put(struct page_pool *pool)
 {
 	return refcount_dec_and_test(&pool->user_cnt);
-- 
2.47.0.277.g8800431eea-goog

[PATCH net-next v2 5/5] netmem: add netmem_prefetch

[Mina Almasry]

prefect(page) is a common thing to be called from drivers. Add
netmem_prefetch that can be called on generic netmem. Skips the prefetch
for net_iovs.

Signed-off-by: Mina Almasry <almasrymina@google.com>
---
 include/net/netmem.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/net/netmem.h b/include/net/netmem.h
index 8a6e20be4b9d..923c47147aa8 100644
--- a/include/net/netmem.h
+++ b/include/net/netmem.h
@@ -171,4 +171,11 @@ static inline unsigned long netmem_get_dma_addr(netmem_ref netmem)
 	return __netmem_clear_lsb(netmem)->dma_addr;
 }
 
+static inline void netmem_prefetch(netmem_ref netmem)
+{
+	if (netmem_is_net_iov(netmem))
+		return;
+
+	prefetch(netmem_to_page(netmem));
+}
 #endif /* _NET_NETMEM_H */
-- 
2.47.0.277.g8800431eea-goog

Re: [PATCH net-next v2 5/5] netmem: add netmem_prefetch

[kernel test robot]

Hi Mina,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Mina-Almasry/net-page_pool-rename-page_pool_alloc_netmem-to-_netmems/20241108-052530
base:   net-next/main
patch link:    https://lore.kernel.org/r/20241107212309.3097362-6-almasrymina%40google.com
patch subject: [PATCH net-next v2 5/5] netmem: add netmem_prefetch
config: parisc-allmodconfig (https://download.01.org/0day-ci/archive/20241108/202411081014.jmz4uf6C-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241108/202411081014.jmz4uf6C-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411081014.jmz4uf6C-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/skbuff.h:40,
                    from net/ipv4/tcp_vegas.c:37:
   include/net/netmem.h: In function 'netmem_prefetch':
   include/net/netmem.h:179:9: error: implicit declaration of function 'prefetch' [-Wimplicit-function-declaration]
     179 |         prefetch(netmem_to_page(netmem));
         |         ^~~~~~~~
   net/ipv4/tcp_vegas.c: At top level:
>> net/ipv4/tcp_vegas.c:46:12: warning: 'gamma' defined but not used [-Wunused-variable]
      46 | static int gamma = 1;
         |            ^~~~~


vim +/gamma +46 net/ipv4/tcp_vegas.c

7752237e9f07b3 Stephen Hemminger 2007-04-23  43  
8d3a564da34e58 Doug Leith        2008-12-09  44  static int alpha = 2;
8d3a564da34e58 Doug Leith        2008-12-09  45  static int beta  = 4;
8d3a564da34e58 Doug Leith        2008-12-09 @46  static int gamma = 1;
b87d8561d8667d Stephen Hemminger 2005-06-23  47  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next v2 5/5] netmem: add netmem_prefetch

[kernel test robot]

Hi Mina,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Mina-Almasry/net-page_pool-rename-page_pool_alloc_netmem-to-_netmems/20241108-052530
base:   net-next/main
patch link:    https://lore.kernel.org/r/20241107212309.3097362-6-almasrymina%40google.com
patch subject: [PATCH net-next v2 5/5] netmem: add netmem_prefetch
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20241108/202411081105.Z1kEMaBE-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 592c0fe55f6d9a811028b5f3507be91458ab2713)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241108/202411081105.Z1kEMaBE-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411081105.Z1kEMaBE-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from arch/s390/kernel/asm-offsets.c:11:
   In file included from include/linux/kvm_host.h:16:
   In file included from include/linux/mm.h:2213:
   include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     505 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     512 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     524 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     525 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   In file included from arch/s390/kernel/asm-offsets.c:11:
   In file included from include/linux/kvm_host.h:19:
   In file included from include/linux/msi.h:24:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/s390/include/asm/io.h:95:
   include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     548 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     561 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
         |                                                           ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
     102 | #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
         |                                                      ^
   In file included from arch/s390/kernel/asm-offsets.c:11:
   In file included from include/linux/kvm_host.h:19:
   In file included from include/linux/msi.h:24:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/s390/include/asm/io.h:95:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     574 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
         |                                                           ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
     115 | #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
         |                                                      ^
   In file included from arch/s390/kernel/asm-offsets.c:11:
   In file included from include/linux/kvm_host.h:19:
   In file included from include/linux/msi.h:24:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/s390/include/asm/io.h:95:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     585 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     595 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     605 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:693:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     693 |         readsb(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:701:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     701 |         readsw(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:709:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     709 |         readsl(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:718:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     718 |         writesb(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:727:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     727 |         writesw(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:736:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     736 |         writesl(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   In file included from arch/s390/kernel/asm-offsets.c:11:
   In file included from include/linux/kvm_host.h:41:
   In file included from include/linux/kvm_para.h:5:
   In file included from include/uapi/linux/kvm_para.h:37:
   In file included from arch/s390/include/asm/kvm_para.h:25:
   In file included from arch/s390/include/asm/diag.h:12:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:40:
>> include/net/netmem.h:179:2: error: call to undeclared function 'prefetch'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     179 |         prefetch(netmem_to_page(netmem));
         |         ^
   16 warnings and 1 error generated.
   make[3]: *** [scripts/Makefile.build:102: arch/s390/kernel/asm-offsets.s] Error 1
   make[3]: Target 'prepare' not remade because of errors.
   make[2]: *** [Makefile:1203: prepare0] Error 2
   make[2]: Target 'prepare' not remade because of errors.
   make[1]: *** [Makefile:224: __sub-make] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:224: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.


vim +/prefetch +179 include/net/netmem.h

   173	
   174	static inline void netmem_prefetch(netmem_ref netmem)
   175	{
   176		if (netmem_is_net_iov(netmem))
   177			return;
   178	
 > 179		prefetch(netmem_to_page(netmem));

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next v2 5/5] netmem: add netmem_prefetch

[kernel test robot]

Hi Mina,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Mina-Almasry/net-page_pool-rename-page_pool_alloc_netmem-to-_netmems/20241108-052530
base:   net-next/main
patch link:    https://lore.kernel.org/r/20241107212309.3097362-6-almasrymina%40google.com
patch subject: [PATCH net-next v2 5/5] netmem: add netmem_prefetch
config: s390-allyesconfig (https://download.01.org/0day-ci/archive/20241108/202411081130.CxioaC2A-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241108/202411081130.CxioaC2A-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411081130.CxioaC2A-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/linux/skbuff.h:40,
                    from include/linux/if_ether.h:19,
                    from arch/s390/include/asm/diag.h:12,
                    from arch/s390/include/asm/kvm_para.h:25,
                    from include/uapi/linux/kvm_para.h:37,
                    from include/linux/kvm_para.h:5,
                    from include/linux/kvm_host.h:41,
                    from arch/s390/kernel/asm-offsets.c:11:
   include/net/netmem.h: In function 'netmem_prefetch':
>> include/net/netmem.h:179:9: error: implicit declaration of function 'prefetch' [-Wimplicit-function-declaration]
     179 |         prefetch(netmem_to_page(netmem));
         |         ^~~~~~~~
   make[3]: *** [scripts/Makefile.build:102: arch/s390/kernel/asm-offsets.s] Error 1
   make[3]: Target 'prepare' not remade because of errors.
   make[2]: *** [Makefile:1203: prepare0] Error 2
   make[2]: Target 'prepare' not remade because of errors.
   make[1]: *** [Makefile:224: __sub-make] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:224: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.


vim +/prefetch +179 include/net/netmem.h

   173	
   174	static inline void netmem_prefetch(netmem_ref netmem)
   175	{
   176		if (netmem_is_net_iov(netmem))
   177			return;
   178	
 > 179		prefetch(netmem_to_page(netmem));

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next v2 4/5] page_pool: disable sync for cpu for dmabuf memory provider

[Jason Gunthorpe]

On Thu, Nov 07, 2024 at 09:23:08PM +0000, Mina Almasry wrote:
> dmabuf dma-addresses should not be dma_sync'd for CPU/device. Typically
> its the driver responsibility to dma_sync for CPU, but the driver should
> not dma_sync for CPU if the netmem is actually coming from a dmabuf
> memory provider.

This is not completely true, it is not *all* dmabuf, just the parts of
the dmabuf that are actually MMIO.

If you do this you may want to block accepting dmabufs that have CPU
pages inside them.

Jason

Re: [PATCH net-next v2 1/5] net: page_pool: rename page_pool_alloc_netmem to *_netmems

[Stanislav Fomichev]

On 11/07, Mina Almasry wrote:
> page_pool_alloc_netmem (without an s) was the mirror of
> page_pool_alloc_pages (with an s), which was confusing.
> 
> Rename to page_pool_alloc_netmems so it's the mirror of
> page_pool_alloc_pages.
> 
> Signed-off-by: Mina Almasry <almasrymina@google.com>

Acked-by: Stanislav Fomichev <sdf@fomichev.me>

Re: [PATCH net-next v2 2/5] net: page_pool: create page_pool_alloc_netmem

[Stanislav Fomichev]

On 11/07, Mina Almasry wrote:
> Create page_pool_alloc_netmem to be the mirror of page_pool_alloc.
> 
> This enables drivers that want currently use page_pool_alloc to
> transition to netmem by converting the call sites to
> page_pool_alloc_netmem.
> 
> Signed-off-by: Mina Almasry <almasrymina@google.com>

Acked-by: Stanislav Fomichev <sdf@fomichev.me>

Re: [PATCH net-next v2 3/5] page_pool: Set `dma_sync` to false for devmem memory provider

[Stanislav Fomichev]

On 11/07, Mina Almasry wrote:
> From: Samiullah Khawaja <skhawaja@google.com>
> 
> Move the `dma_map` and `dma_sync` checks to `page_pool_init` to make
> them generic. Set dma_sync to false for devmem memory provider because
> the dma_sync APIs should not be used for dma_buf backed devmem memory
> provider.
> 
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
> Signed-off-by: Mina Almasry <almasrymina@google.com>

Acked-by: Stanislav Fomichev <sdf@fomichev.me>

Re: [PATCH net-next v2 5/5] netmem: add netmem_prefetch

[Stanislav Fomichev]

On 11/07, Mina Almasry wrote:
> prefect(page) is a common thing to be called from drivers. Add
> netmem_prefetch that can be called on generic netmem. Skips the prefetch
> for net_iovs.
> 
> Signed-off-by: Mina Almasry <almasrymina@google.com>

Needs some extra includes to make the build bot happy, after that:
Acked-by: Stanislav Fomichev <sdf@fomichev.me>

---
pw-bot: cr

Re: [PATCH net-next v2 4/5] page_pool: disable sync for cpu for dmabuf memory provider

[Stanislav Fomichev]

On 11/08, Jason Gunthorpe wrote:
> On Thu, Nov 07, 2024 at 09:23:08PM +0000, Mina Almasry wrote:
> > dmabuf dma-addresses should not be dma_sync'd for CPU/device. Typically
> > its the driver responsibility to dma_sync for CPU, but the driver should
> > not dma_sync for CPU if the netmem is actually coming from a dmabuf
> > memory provider.
> 
> This is not completely true, it is not *all* dmabuf, just the parts of
> the dmabuf that are actually MMIO.
> 
> If you do this you may want to block accepting dmabufs that have CPU
> pages inside them.

We still want udmabufs to work, so probably need some new helper to test
whether a particular netmem is backed by the cpu memory?

Re: [PATCH net-next v2 3/5] page_pool: Set `dma_sync` to false for devmem memory provider

[Pavel Begunkov]

On 11/7/24 21:23, Mina Almasry wrote:
> From: Samiullah Khawaja <skhawaja@google.com>
> 
> Move the `dma_map` and `dma_sync` checks to `page_pool_init` to make
> them generic. Set dma_sync to false for devmem memory provider because
> the dma_sync APIs should not be used for dma_buf backed devmem memory
> provider.
> 
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
> Signed-off-by: Mina Almasry <almasrymina@google.com>
> 
> ---
>   net/core/devmem.c    | 9 ++++-----
>   net/core/page_pool.c | 3 +++
>   2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/net/core/devmem.c b/net/core/devmem.c
> index 11b91c12ee11..826d0b00159f 100644
> --- a/net/core/devmem.c
> +++ b/net/core/devmem.c
> @@ -331,11 +331,10 @@ int mp_dmabuf_devmem_init(struct page_pool *pool)
>   	if (!binding)
>   		return -EINVAL;
>   
> -	if (!pool->dma_map)
> -		return -EOPNOTSUPP;
> -
> -	if (pool->dma_sync)
> -		return -EOPNOTSUPP;
> +	/* dma-buf dma addresses should not be used with
> +	 * dma_sync_for_cpu/device. Force disable dma_sync.
> +	 */
> +	pool->dma_sync = false;
>   
>   	if (pool->p.order != 0)
>   		return -E2BIG;
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index 77de79c1933b..528dd4d18eab 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -287,6 +287,9 @@ static int page_pool_init(struct page_pool *pool,
>   	}
>   
>   	if (pool->mp_priv) {
> +		if (!pool->dma_map || !pool->dma_sync)

Is there a reason why ->dma_sync is enforced for all providers?
It sounds like a devmem fix, on the other hand I don't really
care as we don't have drivers yet implementing queue api / etc.
but not passing PP_FLAG_DMA_SYNC_DEV.

Also, putting fixes first in the series or sending them separately
reduces backporting headache.

-- 
Pavel Begunkov

Re: [PATCH net-next v2 4/5] page_pool: disable sync for cpu for dmabuf memory provider

[Pavel Begunkov]

On 11/8/24 15:58, Stanislav Fomichev wrote:
> On 11/08, Jason Gunthorpe wrote:
>> On Thu, Nov 07, 2024 at 09:23:08PM +0000, Mina Almasry wrote:
>>> dmabuf dma-addresses should not be dma_sync'd for CPU/device. Typically
>>> its the driver responsibility to dma_sync for CPU, but the driver should
>>> not dma_sync for CPU if the netmem is actually coming from a dmabuf
>>> memory provider.
>>
>> This is not completely true, it is not *all* dmabuf, just the parts of
>> the dmabuf that are actually MMIO.
>>
>> If you do this you may want to block accepting dmabufs that have CPU
>> pages inside them.
> 
> We still want udmabufs to work, so probably need some new helper to test
> whether a particular netmem is backed by the cpu memory?

Agree. I guess it's fair to assume that page pool is backed either
by one or another, so could be a page pool flag that devmem.c can set
on init.

-- 
Pavel Begunkov

Re: [PATCH net-next v2 4/5] page_pool: disable sync for cpu for dmabuf memory provider

[Mina Almasry]

On Fri, Nov 8, 2024 at 6:18 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Thu, Nov 07, 2024 at 09:23:08PM +0000, Mina Almasry wrote:
> > dmabuf dma-addresses should not be dma_sync'd for CPU/device. Typically
> > its the driver responsibility to dma_sync for CPU, but the driver should
> > not dma_sync for CPU if the netmem is actually coming from a dmabuf
> > memory provider.
>
> This is not completely true, it is not *all* dmabuf, just the parts of
> the dmabuf that are actually MMIO.
>

Thanks Jason, I can clarify the commit message when/if I send another iteration.

> If you do this you may want to block accepting dmabufs that have CPU
> pages inside them.
>

How do I check if the dmabuf has CPU pages inside of it? The only way
I can think to do that is to sg_page a scatterlist entry, then
!is_zone_device_page() the page. Or something like that, but I thought
calling sg_page() on the dmabuf scatterlist was banned now.

But as others mentioned, we've taken a dependency on using udmabuf for
testing, so we'd like that to still work, we probably need to find
another way than just blocking them entirely.

I'm thinking, we already use the dmabuf sync APIs when we want to read
the udmabuf from userspace. In ncdevmem.c, we do:

sync.flags = DMA_BUF_SYNC_READ | DMA_BUF_SYNC_START;
ioctl(buf, DMA_BUF_IOCTL_SYNC, &sync);

Before we read the udmabuf data. Doesn't that do the same job as
dma_sync_single_range_for_cpu? I'm thinking dmabufs with user pages
would work as long as the user calls the dmabuf sync APIs, so we don't
need to block them entirely from devmem tcp.



-- 
Thanks,
Mina

Re: [PATCH net-next v2 3/5] page_pool: Set `dma_sync` to false for devmem memory provider

[Jakub Kicinski]

On Thu,  7 Nov 2024 21:23:07 +0000 Mina Almasry wrote:
> From: Samiullah Khawaja <skhawaja@google.com>
> 
> Move the `dma_map` and `dma_sync` checks to `page_pool_init` to make
> them generic. Set dma_sync to false for devmem memory provider because
> the dma_sync APIs should not be used for dma_buf backed devmem memory
> provider.

Let's start from specifying the expectations from the drivers,
and documenting them under Documentation/

We should require that drivers set PP_FLAG_DMA_SYNC_DEV on the page
pool and always use (a new form of) page_pool_dma_sync_for_cpu()
to sync for CPU.

Behind the scenes we will configure the PP to act accordingly.
For dmabuf backed PP I suspect we just say "syncing is
the responsibility of userspace", and configure the PP
to do no syncing at all.

For other providers we can keep the syncing enabled. But drivers
shouldn't be allowed to use any netmem if they don't set
PP_FLAG_DMA_SYNC_DEV, just to keep things uniform.

Re: [PATCH net-next v2 5/5] netmem: add netmem_prefetch

[Jakub Kicinski]

On Thu,  7 Nov 2024 21:23:09 +0000 Mina Almasry wrote:
> prefect(page) is a common thing to be called from drivers. Add
> netmem_prefetch that can be called on generic netmem. Skips the prefetch
> for net_iovs.

Why would you ever need to prefetch a payload buffer?
Looks wrong..

Re: [PATCH net-next v2 4/5] page_pool: disable sync for cpu for dmabuf memory provider

[Jason Gunthorpe]

On Fri, Nov 08, 2024 at 11:01:21AM -0800, Mina Almasry wrote:

> > If you do this you may want to block accepting dmabufs that have CPU
> > pages inside them.
> >
> 
> How do I check if the dmabuf has CPU pages inside of it? The only way
> I can think to do that is to sg_page a scatterlist entry, then
> !is_zone_device_page() the page. Or something like that, but I thought
> calling sg_page() on the dmabuf scatterlist was banned now.

I don't know. Many dmabuf scenarios abuse scatter list and the CPU
list is invalid, so you can't reference sg_page().

I think you'd need to discuss with the dmabuf maintainers.

Jason

Re: [PATCH net-next v2 4/5] page_pool: disable sync for cpu for dmabuf memory provider

[Samiullah Khawaja]

On Thu, Nov 14, 2024 at 5:59 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Fri, Nov 08, 2024 at 11:01:21AM -0800, Mina Almasry wrote:
>
> > > If you do this you may want to block accepting dmabufs that have CPU
> > > pages inside them.

I believe we should be following the dmabuf API for this purpose, if
we really want to sync for CPU in. page_pool, and should not assume
the behaviour of the backing memory.

The dmabuf exporters are supposed to implement the begin_cpu_access
(optional) and the sync for cpu is done based on the exporter
implementation.

For example udmabuf implementation calls `dma_sync_sg_for_cpu` on the
underlying pages.

I think following dmabuf APIs can be used to ensure the backing memory
is synced:

Before cpu access to sync for cpu
`dma_buf_begin_cpu_access`

After cpu access to be synced for device
`dma_buf_end_cpu_access`

Sami
> > >
> >
> > How do I check if the dmabuf has CPU pages inside of it? The only way
> > I can think to do that is to sg_page a scatterlist entry, then
> > !is_zone_device_page() the page. Or something like that, but I thought
> > calling sg_page() on the dmabuf scatterlist was banned now.
>
> I don't know. Many dmabuf scenarios abuse scatter list and the CPU
> list is invalid, so you can't reference sg_page().
>
> I think you'd need to discuss with the dmabuf maintainers.
>
> Jason

Re: [PATCH net-next v2 4/5] page_pool: disable sync for cpu for dmabuf memory provider

[Jakub Kicinski]

On Fri, 22 Nov 2024 14:10:28 -0800 Samiullah Khawaja wrote:
> > > > If you do this you may want to block accepting dmabufs that have CPU
> > > > pages inside them.  
> 
> I believe we should be following the dmabuf API for this purpose, if
> we really want to sync for CPU in. page_pool, and should not assume
> the behaviour of the backing memory.
> 
> The dmabuf exporters are supposed to implement the begin_cpu_access
> (optional) and the sync for cpu is done based on the exporter
> implementation.

DMABUF is a bit of a distraction here.

What's key is that we need to fill in the gap in the driver facing
page_pool API, because DMABUF will not be the only provider we can
plug in.