linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RESEND v3 net-next 0/5] net: consolidate page_is_pfmemalloc() usage
@ 2021-02-02 13:30 Alexander Lobakin
  2021-02-02 13:30 ` [PATCH RESEND v3 net-next 1/5] mm: constify page_is_pfmemalloc() argument Alexander Lobakin
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Alexander Lobakin @ 2021-02-02 13:30 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: John Hubbard, David Rientjes, Yisen Zhuang, Salil Mehta,
	Jesse Brandeburg, Tony Nguyen, Saeed Mahameed, Leon Romanovsky,
	Andrew Morton, Jesper Dangaard Brouer, Ilias Apalodimas,
	Jonathan Lemon, Willem de Bruijn, Randy Dunlap,
	Pablo Neira Ayuso, Dexuan Cui, Jakub Sitnicki, Marco Elver,
	Paolo Abeni, Alexander Lobakin, netdev, linux-kernel,
	intel-wired-lan, linux-rdma, linux-mm

page_is_pfmemalloc() is used mostly by networking drivers to test
if a page can be considered for reusing/recycling.
It doesn't write anything to the struct page itself, so its sole
argument can be constified, as well as the first argument of
skb_propagate_pfmemalloc().
In Page Pool core code, it can be simply inlined instead.
Most of the callers from NIC drivers were just doppelgangers of
the same condition tests. Derive them into a new common function
do deduplicate the code.

Resend of v3 [2]:
 - it missed Patchwork and Netdev archives, probably due to server-side
   issues.

Since v2 [1]:
 - use more intuitive name for the new inline function since there's
   nothing "reserved" in remote pages (Jakub Kicinski, John Hubbard);
 - fold likely() inside the helper itself to make driver code a bit
   fancier (Jakub Kicinski);
 - split function introduction and using into two separate commits;
 - collect some more tags (Jesse Brandeburg, David Rientjes).

Since v1 [0]:
 - new: reduce code duplication by introducing a new common function
   to test if a page can be reused/recycled (David Rientjes);
 - collect autographs for Page Pool bits (Jesper Dangaard Brouer,
   Ilias Apalodimas).

[0] https://lore.kernel.org/netdev/20210125164612.243838-1-alobakin@pm.me
[1] https://lore.kernel.org/netdev/20210127201031.98544-1-alobakin@pm.me
[2] https://lore.kernel.org/lkml/20210131120844.7529-1-alobakin@pm.me

Alexander Lobakin (5):
  mm: constify page_is_pfmemalloc() argument
  skbuff: constify skb_propagate_pfmemalloc() "page" argument
  net: introduce common dev_page_is_reusable()
  net: use the new dev_page_is_reusable() instead of private versions
  net: page_pool: simplify page recycling condition tests

 .../net/ethernet/hisilicon/hns3/hns3_enet.c   | 17 ++++++----------
 drivers/net/ethernet/intel/fm10k/fm10k_main.c | 13 ++++--------
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 15 +-------------
 drivers/net/ethernet/intel/iavf/iavf_txrx.c   | 15 +-------------
 drivers/net/ethernet/intel/ice/ice_txrx.c     | 13 ++----------
 drivers/net/ethernet/intel/igb/igb_main.c     |  9 ++-------
 drivers/net/ethernet/intel/igc/igc_main.c     |  9 ++-------
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  9 ++-------
 .../net/ethernet/intel/ixgbevf/ixgbevf_main.c |  9 ++-------
 .../net/ethernet/mellanox/mlx5/core/en_rx.c   |  7 +------
 include/linux/mm.h                            |  2 +-
 include/linux/skbuff.h                        | 20 +++++++++++++++++--
 net/core/page_pool.c                          | 14 ++++---------
 13 files changed, 46 insertions(+), 106 deletions(-)

-- 
2.30.0



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

* [PATCH RESEND v3 net-next 1/5] mm: constify page_is_pfmemalloc() argument
  2021-02-02 13:30 [PATCH RESEND v3 net-next 0/5] net: consolidate page_is_pfmemalloc() usage Alexander Lobakin
@ 2021-02-02 13:30 ` Alexander Lobakin
  2021-02-02 13:31 ` [PATCH RESEND v3 net-next 2/5] skbuff: constify skb_propagate_pfmemalloc() "page" argument Alexander Lobakin
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Alexander Lobakin @ 2021-02-02 13:30 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: John Hubbard, David Rientjes, Yisen Zhuang, Salil Mehta,
	Jesse Brandeburg, Tony Nguyen, Saeed Mahameed, Leon Romanovsky,
	Andrew Morton, Jesper Dangaard Brouer, Ilias Apalodimas,
	Jonathan Lemon, Willem de Bruijn, Randy Dunlap,
	Pablo Neira Ayuso, Dexuan Cui, Jakub Sitnicki, Marco Elver,
	Paolo Abeni, Alexander Lobakin, netdev, linux-kernel,
	intel-wired-lan, linux-rdma, linux-mm

The function only tests for page->index, so its argument should be
const.

Signed-off-by: Alexander Lobakin <alobakin@pm.me>
Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Acked-by: David Rientjes <rientjes@google.com>
---
 include/linux/mm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index ecdf8a8cd6ae..078633d43af9 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1584,7 +1584,7 @@ struct address_space *page_mapping_file(struct page *page);
  * ALLOC_NO_WATERMARKS and the low watermark was not
  * met implying that the system is under some pressure.
  */
-static inline bool page_is_pfmemalloc(struct page *page)
+static inline bool page_is_pfmemalloc(const struct page *page)
 {
 	/*
 	 * Page index cannot be this large so this must be
-- 
2.30.0



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

* [PATCH RESEND v3 net-next 2/5] skbuff: constify skb_propagate_pfmemalloc() "page" argument
  2021-02-02 13:30 [PATCH RESEND v3 net-next 0/5] net: consolidate page_is_pfmemalloc() usage Alexander Lobakin
  2021-02-02 13:30 ` [PATCH RESEND v3 net-next 1/5] mm: constify page_is_pfmemalloc() argument Alexander Lobakin
@ 2021-02-02 13:31 ` Alexander Lobakin
  2021-02-02 13:31 ` [PATCH RESEND v3 net-next 3/5] net: introduce common dev_page_is_reusable() Alexander Lobakin
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Alexander Lobakin @ 2021-02-02 13:31 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: John Hubbard, David Rientjes, Yisen Zhuang, Salil Mehta,
	Jesse Brandeburg, Tony Nguyen, Saeed Mahameed, Leon Romanovsky,
	Andrew Morton, Jesper Dangaard Brouer, Ilias Apalodimas,
	Jonathan Lemon, Willem de Bruijn, Randy Dunlap,
	Pablo Neira Ayuso, Dexuan Cui, Jakub Sitnicki, Marco Elver,
	Paolo Abeni, Alexander Lobakin, netdev, linux-kernel,
	intel-wired-lan, linux-rdma, linux-mm

The function doesn't write anything to the page struct itself,
so this argument can be const.

Misc: align second argument to the brace while at it.

Signed-off-by: Alexander Lobakin <alobakin@pm.me>
Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Acked-by: David Rientjes <rientjes@google.com>
---
 include/linux/skbuff.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 9313b5aaf45b..b027526da4f9 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2943,8 +2943,8 @@ static inline struct page *dev_alloc_page(void)
  *	@page: The page that was allocated from skb_alloc_page
  *	@skb: The skb that may need pfmemalloc set
  */
-static inline void skb_propagate_pfmemalloc(struct page *page,
-					     struct sk_buff *skb)
+static inline void skb_propagate_pfmemalloc(const struct page *page,
+					    struct sk_buff *skb)
 {
 	if (page_is_pfmemalloc(page))
 		skb->pfmemalloc = true;
-- 
2.30.0



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

* [PATCH RESEND v3 net-next 3/5] net: introduce common dev_page_is_reusable()
  2021-02-02 13:30 [PATCH RESEND v3 net-next 0/5] net: consolidate page_is_pfmemalloc() usage Alexander Lobakin
  2021-02-02 13:30 ` [PATCH RESEND v3 net-next 1/5] mm: constify page_is_pfmemalloc() argument Alexander Lobakin
  2021-02-02 13:31 ` [PATCH RESEND v3 net-next 2/5] skbuff: constify skb_propagate_pfmemalloc() "page" argument Alexander Lobakin
@ 2021-02-02 13:31 ` Alexander Lobakin
  2021-02-02 13:31 ` [PATCH RESEND v3 net-next 4/5] net: use the new dev_page_is_reusable() instead of private versions Alexander Lobakin
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Alexander Lobakin @ 2021-02-02 13:31 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: John Hubbard, David Rientjes, Yisen Zhuang, Salil Mehta,
	Jesse Brandeburg, Tony Nguyen, Saeed Mahameed, Leon Romanovsky,
	Andrew Morton, Jesper Dangaard Brouer, Ilias Apalodimas,
	Jonathan Lemon, Willem de Bruijn, Randy Dunlap,
	Pablo Neira Ayuso, Dexuan Cui, Jakub Sitnicki, Marco Elver,
	Paolo Abeni, Alexander Lobakin, netdev, linux-kernel,
	intel-wired-lan, linux-rdma, linux-mm

A bunch of drivers test the page before reusing/recycling for two
common conditions:
 - if a page was allocated under memory pressure (pfmemalloc page);
 - if a page was allocated at a distant memory node (to exclude
   slowdowns).

Introduce a new common inline for doing this, with likely() already
folded inside to make driver code a bit simpler.

Suggested-by: David Rientjes <rientjes@google.com>
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Cc: John Hubbard <jhubbard@nvidia.com>
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Acked-by: David Rientjes <rientjes@google.com>
---
 include/linux/skbuff.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index b027526da4f9..0e42c53b8ca9 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2938,6 +2938,22 @@ static inline struct page *dev_alloc_page(void)
 	return dev_alloc_pages(0);
 }
 
+/**
+ * dev_page_is_reusable - check whether a page can be reused for network Rx
+ * @page: the page to test
+ *
+ * A page shouldn't be considered for reusing/recycling if it was allocated
+ * under memory pressure or at a distant memory node.
+ *
+ * Returns false if this page should be returned to page allocator, true
+ * otherwise.
+ */
+static inline bool dev_page_is_reusable(const struct page *page)
+{
+	return likely(page_to_nid(page) == numa_mem_id() &&
+		      !page_is_pfmemalloc(page));
+}
+
 /**
  *	skb_propagate_pfmemalloc - Propagate pfmemalloc if skb is allocated after RX page
  *	@page: The page that was allocated from skb_alloc_page
-- 
2.30.0



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

* [PATCH RESEND v3 net-next 4/5] net: use the new dev_page_is_reusable() instead of private versions
  2021-02-02 13:30 [PATCH RESEND v3 net-next 0/5] net: consolidate page_is_pfmemalloc() usage Alexander Lobakin
                   ` (2 preceding siblings ...)
  2021-02-02 13:31 ` [PATCH RESEND v3 net-next 3/5] net: introduce common dev_page_is_reusable() Alexander Lobakin
@ 2021-02-02 13:31 ` Alexander Lobakin
  2021-02-04 19:52   ` Jesse Brandeburg
  2021-02-02 13:31 ` [PATCH RESEND v3 net-next 5/5] net: page_pool: simplify page recycling condition tests Alexander Lobakin
  2021-02-05  2:40 ` [PATCH RESEND v3 net-next 0/5] net: consolidate page_is_pfmemalloc() usage patchwork-bot+netdevbpf
  5 siblings, 1 reply; 8+ messages in thread
From: Alexander Lobakin @ 2021-02-02 13:31 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: John Hubbard, David Rientjes, Yisen Zhuang, Salil Mehta,
	Jesse Brandeburg, Tony Nguyen, Saeed Mahameed, Leon Romanovsky,
	Andrew Morton, Jesper Dangaard Brouer, Ilias Apalodimas,
	Jonathan Lemon, Willem de Bruijn, Randy Dunlap,
	Pablo Neira Ayuso, Dexuan Cui, Jakub Sitnicki, Marco Elver,
	Paolo Abeni, Alexander Lobakin, netdev, linux-kernel,
	intel-wired-lan, linux-rdma, linux-mm

Now we can remove a bunch of identical functions from the drivers and
make them use common dev_page_is_reusable(). All {,un}likely() checks
are omitted since it's already present in this helper.
Also update some comments near the call sites.

Suggested-by: David Rientjes <rientjes@google.com>
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Cc: John Hubbard <jhubbard@nvidia.com>
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
 drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 17 ++++++-----------
 drivers/net/ethernet/intel/fm10k/fm10k_main.c   | 13 ++++---------
 drivers/net/ethernet/intel/i40e/i40e_txrx.c     | 15 +--------------
 drivers/net/ethernet/intel/iavf/iavf_txrx.c     | 15 +--------------
 drivers/net/ethernet/intel/ice/ice_txrx.c       | 13 ++-----------
 drivers/net/ethernet/intel/igb/igb_main.c       |  9 ++-------
 drivers/net/ethernet/intel/igc/igc_main.c       |  9 ++-------
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c   |  9 ++-------
 .../net/ethernet/intel/ixgbevf/ixgbevf_main.c   |  9 ++-------
 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c |  7 +------
 10 files changed, 23 insertions(+), 93 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 512080640cbc..f39f5b1c4cec 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -2800,12 +2800,6 @@ static void hns3_nic_alloc_rx_buffers(struct hns3_enet_ring *ring,
 	writel(i, ring->tqp->io_base + HNS3_RING_RX_RING_HEAD_REG);
 }
 
-static bool hns3_page_is_reusable(struct page *page)
-{
-	return page_to_nid(page) == numa_mem_id() &&
-		!page_is_pfmemalloc(page);
-}
-
 static bool hns3_can_reuse_page(struct hns3_desc_cb *cb)
 {
 	return (page_count(cb->priv) - cb->pagecnt_bias) == 1;
@@ -2823,10 +2817,11 @@ static void hns3_nic_reuse_page(struct sk_buff *skb, int i,
 	skb_add_rx_frag(skb, i, desc_cb->priv, desc_cb->page_offset + pull_len,
 			size - pull_len, truesize);
 
-	/* Avoid re-using remote pages, or the stack is still using the page
-	 * when page_offset rollback to zero, flag default unreuse
+	/* Avoid re-using remote and pfmemalloc pages, or the stack is still
+	 * using the page when page_offset rollback to zero, flag default
+	 * unreuse
 	 */
-	if (unlikely(!hns3_page_is_reusable(desc_cb->priv)) ||
+	if (!dev_page_is_reusable(desc_cb->priv) ||
 	    (!desc_cb->page_offset && !hns3_can_reuse_page(desc_cb))) {
 		__page_frag_cache_drain(desc_cb->priv, desc_cb->pagecnt_bias);
 		return;
@@ -3083,8 +3078,8 @@ static int hns3_alloc_skb(struct hns3_enet_ring *ring, unsigned int length,
 	if (length <= HNS3_RX_HEAD_SIZE) {
 		memcpy(__skb_put(skb, length), va, ALIGN(length, sizeof(long)));
 
-		/* We can reuse buffer as-is, just make sure it is local */
-		if (likely(hns3_page_is_reusable(desc_cb->priv)))
+		/* We can reuse buffer as-is, just make sure it is reusable */
+		if (dev_page_is_reusable(desc_cb->priv))
 			desc_cb->reuse_flag = 1;
 		else /* This page cannot be reused so discard it */
 			__page_frag_cache_drain(desc_cb->priv,
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index 99b8252eb969..247f44f4cb30 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -194,17 +194,12 @@ static void fm10k_reuse_rx_page(struct fm10k_ring *rx_ring,
 					 DMA_FROM_DEVICE);
 }
 
-static inline bool fm10k_page_is_reserved(struct page *page)
-{
-	return (page_to_nid(page) != numa_mem_id()) || page_is_pfmemalloc(page);
-}
-
 static bool fm10k_can_reuse_rx_page(struct fm10k_rx_buffer *rx_buffer,
 				    struct page *page,
 				    unsigned int __maybe_unused truesize)
 {
-	/* avoid re-using remote pages */
-	if (unlikely(fm10k_page_is_reserved(page)))
+	/* avoid re-using remote and pfmemalloc pages */
+	if (!dev_page_is_reusable(page))
 		return false;
 
 #if (PAGE_SIZE < 8192)
@@ -265,8 +260,8 @@ static bool fm10k_add_rx_frag(struct fm10k_rx_buffer *rx_buffer,
 	if (likely(size <= FM10K_RX_HDR_LEN)) {
 		memcpy(__skb_put(skb, size), va, ALIGN(size, sizeof(long)));
 
-		/* page is not reserved, we can reuse buffer as-is */
-		if (likely(!fm10k_page_is_reserved(page)))
+		/* page is reusable, we can reuse buffer as-is */
+		if (dev_page_is_reusable(page))
 			return true;
 
 		/* this page cannot be reused so discard it */
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 2574e78f7597..8d2ea4293d69 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -1843,19 +1843,6 @@ static bool i40e_cleanup_headers(struct i40e_ring *rx_ring, struct sk_buff *skb,
 	return false;
 }
 
-/**
- * i40e_page_is_reusable - check if any reuse is possible
- * @page: page struct to check
- *
- * A page is not reusable if it was allocated under low memory
- * conditions, or it's not in the same NUMA node as this CPU.
- */
-static inline bool i40e_page_is_reusable(struct page *page)
-{
-	return (page_to_nid(page) == numa_mem_id()) &&
-		!page_is_pfmemalloc(page);
-}
-
 /**
  * i40e_can_reuse_rx_page - Determine if this page can be reused by
  * the adapter for another receive
@@ -1891,7 +1878,7 @@ static bool i40e_can_reuse_rx_page(struct i40e_rx_buffer *rx_buffer,
 	struct page *page = rx_buffer->page;
 
 	/* Is any reuse possible? */
-	if (unlikely(!i40e_page_is_reusable(page)))
+	if (!dev_page_is_reusable(page))
 		return false;
 
 #if (PAGE_SIZE < 8192)
diff --git a/drivers/net/ethernet/intel/iavf/iavf_txrx.c b/drivers/net/ethernet/intel/iavf/iavf_txrx.c
index 256fa07d54d5..ffaf2742a2e0 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_txrx.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_txrx.c
@@ -1141,19 +1141,6 @@ static void iavf_reuse_rx_page(struct iavf_ring *rx_ring,
 	new_buff->pagecnt_bias	= old_buff->pagecnt_bias;
 }
 
-/**
- * iavf_page_is_reusable - check if any reuse is possible
- * @page: page struct to check
- *
- * A page is not reusable if it was allocated under low memory
- * conditions, or it's not in the same NUMA node as this CPU.
- */
-static inline bool iavf_page_is_reusable(struct page *page)
-{
-	return (page_to_nid(page) == numa_mem_id()) &&
-		!page_is_pfmemalloc(page);
-}
-
 /**
  * iavf_can_reuse_rx_page - Determine if this page can be reused by
  * the adapter for another receive
@@ -1187,7 +1174,7 @@ static bool iavf_can_reuse_rx_page(struct iavf_rx_buffer *rx_buffer)
 	struct page *page = rx_buffer->page;
 
 	/* Is any reuse possible? */
-	if (unlikely(!iavf_page_is_reusable(page)))
+	if (!dev_page_is_reusable(page))
 		return false;
 
 #if (PAGE_SIZE < 8192)
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c
index 2c2de56e2824..8ca63c6a6ba4 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
@@ -728,15 +728,6 @@ bool ice_alloc_rx_bufs(struct ice_ring *rx_ring, u16 cleaned_count)
 	return !!cleaned_count;
 }
 
-/**
- * ice_page_is_reserved - check if reuse is possible
- * @page: page struct to check
- */
-static bool ice_page_is_reserved(struct page *page)
-{
-	return (page_to_nid(page) != numa_mem_id()) || page_is_pfmemalloc(page);
-}
-
 /**
  * ice_rx_buf_adjust_pg_offset - Prepare Rx buffer for reuse
  * @rx_buf: Rx buffer to adjust
@@ -775,8 +766,8 @@ ice_can_reuse_rx_page(struct ice_rx_buf *rx_buf, int rx_buf_pgcnt)
 	unsigned int pagecnt_bias = rx_buf->pagecnt_bias;
 	struct page *page = rx_buf->page;
 
-	/* avoid re-using remote pages */
-	if (unlikely(ice_page_is_reserved(page)))
+	/* avoid re-using remote and pfmemalloc pages */
+	if (!dev_page_is_reusable(page))
 		return false;
 
 #if (PAGE_SIZE < 8192)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 84d4284b8b32..7d8e02b4d092 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -8215,18 +8215,13 @@ static void igb_reuse_rx_page(struct igb_ring *rx_ring,
 	new_buff->pagecnt_bias	= old_buff->pagecnt_bias;
 }
 
-static inline bool igb_page_is_reserved(struct page *page)
-{
-	return (page_to_nid(page) != numa_mem_id()) || page_is_pfmemalloc(page);
-}
-
 static bool igb_can_reuse_rx_page(struct igb_rx_buffer *rx_buffer)
 {
 	unsigned int pagecnt_bias = rx_buffer->pagecnt_bias;
 	struct page *page = rx_buffer->page;
 
-	/* avoid re-using remote pages */
-	if (unlikely(igb_page_is_reserved(page)))
+	/* avoid re-using remote and pfmemalloc pages */
+	if (!dev_page_is_reusable(page))
 		return false;
 
 #if (PAGE_SIZE < 8192)
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 43aec42e6d9d..ae0de7f08568 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -1648,18 +1648,13 @@ static void igc_reuse_rx_page(struct igc_ring *rx_ring,
 	new_buff->pagecnt_bias	= old_buff->pagecnt_bias;
 }
 
-static inline bool igc_page_is_reserved(struct page *page)
-{
-	return (page_to_nid(page) != numa_mem_id()) || page_is_pfmemalloc(page);
-}
-
 static bool igc_can_reuse_rx_page(struct igc_rx_buffer *rx_buffer)
 {
 	unsigned int pagecnt_bias = rx_buffer->pagecnt_bias;
 	struct page *page = rx_buffer->page;
 
-	/* avoid re-using remote pages */
-	if (unlikely(igc_page_is_reserved(page)))
+	/* avoid re-using remote and pfmemalloc pages */
+	if (!dev_page_is_reusable(page))
 		return false;
 
 #if (PAGE_SIZE < 8192)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index e08c01525fd2..237e09342f28 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1940,19 +1940,14 @@ static void ixgbe_reuse_rx_page(struct ixgbe_ring *rx_ring,
 	new_buff->pagecnt_bias	= old_buff->pagecnt_bias;
 }
 
-static inline bool ixgbe_page_is_reserved(struct page *page)
-{
-	return (page_to_nid(page) != numa_mem_id()) || page_is_pfmemalloc(page);
-}
-
 static bool ixgbe_can_reuse_rx_page(struct ixgbe_rx_buffer *rx_buffer,
 				    int rx_buffer_pgcnt)
 {
 	unsigned int pagecnt_bias = rx_buffer->pagecnt_bias;
 	struct page *page = rx_buffer->page;
 
-	/* avoid re-using remote pages */
-	if (unlikely(ixgbe_page_is_reserved(page)))
+	/* avoid re-using remote and pfmemalloc pages */
+	if (!dev_page_is_reusable(page))
 		return false;
 
 #if (PAGE_SIZE < 8192)
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index a14e55e7fce8..449d7d5b280d 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -781,18 +781,13 @@ static void ixgbevf_reuse_rx_page(struct ixgbevf_ring *rx_ring,
 	new_buff->pagecnt_bias = old_buff->pagecnt_bias;
 }
 
-static inline bool ixgbevf_page_is_reserved(struct page *page)
-{
-	return (page_to_nid(page) != numa_mem_id()) || page_is_pfmemalloc(page);
-}
-
 static bool ixgbevf_can_reuse_rx_page(struct ixgbevf_rx_buffer *rx_buffer)
 {
 	unsigned int pagecnt_bias = rx_buffer->pagecnt_bias;
 	struct page *page = rx_buffer->page;
 
-	/* avoid re-using remote pages */
-	if (unlikely(ixgbevf_page_is_reserved(page)))
+	/* avoid re-using remote and pfmemalloc pages */
+	if (!dev_page_is_reusable(page))
 		return false;
 
 #if (PAGE_SIZE < 8192)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index 98b56f495b32..e1b4cf506a15 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -213,11 +213,6 @@ static inline u32 mlx5e_decompress_cqes_start(struct mlx5e_rq *rq,
 	return mlx5e_decompress_cqes_cont(rq, wq, 1, budget_rem) - 1;
 }
 
-static inline bool mlx5e_page_is_reserved(struct page *page)
-{
-	return page_is_pfmemalloc(page) || page_to_nid(page) != numa_mem_id();
-}
-
 static inline bool mlx5e_rx_cache_put(struct mlx5e_rq *rq,
 				      struct mlx5e_dma_info *dma_info)
 {
@@ -230,7 +225,7 @@ static inline bool mlx5e_rx_cache_put(struct mlx5e_rq *rq,
 		return false;
 	}
 
-	if (unlikely(mlx5e_page_is_reserved(dma_info->page))) {
+	if (!dev_page_is_reusable(dma_info->page)) {
 		stats->cache_waive++;
 		return false;
 	}
-- 
2.30.0



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

* [PATCH RESEND v3 net-next 5/5] net: page_pool: simplify page recycling condition tests
  2021-02-02 13:30 [PATCH RESEND v3 net-next 0/5] net: consolidate page_is_pfmemalloc() usage Alexander Lobakin
                   ` (3 preceding siblings ...)
  2021-02-02 13:31 ` [PATCH RESEND v3 net-next 4/5] net: use the new dev_page_is_reusable() instead of private versions Alexander Lobakin
@ 2021-02-02 13:31 ` Alexander Lobakin
  2021-02-05  2:40 ` [PATCH RESEND v3 net-next 0/5] net: consolidate page_is_pfmemalloc() usage patchwork-bot+netdevbpf
  5 siblings, 0 replies; 8+ messages in thread
From: Alexander Lobakin @ 2021-02-02 13:31 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: John Hubbard, David Rientjes, Yisen Zhuang, Salil Mehta,
	Jesse Brandeburg, Tony Nguyen, Saeed Mahameed, Leon Romanovsky,
	Andrew Morton, Jesper Dangaard Brouer, Ilias Apalodimas,
	Jonathan Lemon, Willem de Bruijn, Randy Dunlap,
	Pablo Neira Ayuso, Dexuan Cui, Jakub Sitnicki, Marco Elver,
	Paolo Abeni, Alexander Lobakin, netdev, linux-kernel,
	intel-wired-lan, linux-rdma, linux-mm

pool_page_reusable() is a leftover from pre-NUMA-aware times. For now,
this function is just a redundant wrapper over page_is_pfmemalloc(),
so inline it into its sole call site.

Signed-off-by: Alexander Lobakin <alobakin@pm.me>
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Acked-by: David Rientjes <rientjes@google.com>
---
 net/core/page_pool.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index f3c690b8c8e3..ad8b0707af04 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -350,14 +350,6 @@ static bool page_pool_recycle_in_cache(struct page *page,
 	return true;
 }
 
-/* page is NOT reusable when:
- * 1) allocated when system is under some pressure. (page_is_pfmemalloc)
- */
-static bool pool_page_reusable(struct page_pool *pool, struct page *page)
-{
-	return !page_is_pfmemalloc(page);
-}
-
 /* If the page refcnt == 1, this will try to recycle the page.
  * if PP_FLAG_DMA_SYNC_DEV is set, we'll try to sync the DMA area for
  * the configured size min(dma_sync_size, pool->max_len).
@@ -373,9 +365,11 @@ __page_pool_put_page(struct page_pool *pool, struct page *page,
 	 * regular page allocator APIs.
 	 *
 	 * refcnt == 1 means page_pool owns page, and can recycle it.
+	 *
+	 * page is NOT reusable when allocated when system is under
+	 * some pressure. (page_is_pfmemalloc)
 	 */
-	if (likely(page_ref_count(page) == 1 &&
-		   pool_page_reusable(pool, page))) {
+	if (likely(page_ref_count(page) == 1 && !page_is_pfmemalloc(page))) {
 		/* Read barrier done in page_ref_count / READ_ONCE */
 
 		if (pool->p.flags & PP_FLAG_DMA_SYNC_DEV)
-- 
2.30.0



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

* Re: [PATCH RESEND v3 net-next 4/5] net: use the new dev_page_is_reusable() instead of private versions
  2021-02-02 13:31 ` [PATCH RESEND v3 net-next 4/5] net: use the new dev_page_is_reusable() instead of private versions Alexander Lobakin
@ 2021-02-04 19:52   ` Jesse Brandeburg
  0 siblings, 0 replies; 8+ messages in thread
From: Jesse Brandeburg @ 2021-02-04 19:52 UTC (permalink / raw)
  To: Alexander Lobakin
  Cc: David S. Miller, Jakub Kicinski, John Hubbard, David Rientjes,
	Yisen Zhuang, Salil Mehta, Tony Nguyen, Saeed Mahameed,
	Leon Romanovsky, Andrew Morton, Jesper Dangaard Brouer,
	Ilias Apalodimas, Jonathan Lemon, Willem de Bruijn, Randy Dunlap,
	Pablo Neira Ayuso, Dexuan Cui, Jakub Sitnicki, Marco Elver,
	Paolo Abeni, netdev, linux-kernel, intel-wired-lan, linux-rdma,
	linux-mm

Alexander Lobakin wrote:

> Now we can remove a bunch of identical functions from the drivers and
> make them use common dev_page_is_reusable(). All {,un}likely() checks
> are omitted since it's already present in this helper.
> Also update some comments near the call sites.
> 
> Suggested-by: David Rientjes <rientjes@google.com>
> Suggested-by: Jakub Kicinski <kuba@kernel.org>
> Cc: John Hubbard <jhubbard@nvidia.com>
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>

I don't know why it was missed in the series update, but:
Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

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

* Re: [PATCH RESEND v3 net-next 0/5] net: consolidate page_is_pfmemalloc() usage
  2021-02-02 13:30 [PATCH RESEND v3 net-next 0/5] net: consolidate page_is_pfmemalloc() usage Alexander Lobakin
                   ` (4 preceding siblings ...)
  2021-02-02 13:31 ` [PATCH RESEND v3 net-next 5/5] net: page_pool: simplify page recycling condition tests Alexander Lobakin
@ 2021-02-05  2:40 ` patchwork-bot+netdevbpf
  5 siblings, 0 replies; 8+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-02-05  2:40 UTC (permalink / raw)
  To: Alexander Lobakin
  Cc: davem, kuba, jhubbard, rientjes, yisen.zhuang, salil.mehta,
	jesse.brandeburg, anthony.l.nguyen, saeedm, leon, akpm, brouer,
	ilias.apalodimas, jonathan.lemon, willemb, rdunlap, pablo, decui,
	jakub, elver, pabeni, netdev, linux-kernel, intel-wired-lan,
	linux-rdma, linux-mm

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Tue, 02 Feb 2021 13:30:43 +0000 you wrote:
> page_is_pfmemalloc() is used mostly by networking drivers to test
> if a page can be considered for reusing/recycling.
> It doesn't write anything to the struct page itself, so its sole
> argument can be constified, as well as the first argument of
> skb_propagate_pfmemalloc().
> In Page Pool core code, it can be simply inlined instead.
> Most of the callers from NIC drivers were just doppelgangers of
> the same condition tests. Derive them into a new common function
> do deduplicate the code.
> 
> [...]

Here is the summary with links:
  - [RESEND,v3,net-next,1/5] mm: constify page_is_pfmemalloc() argument
    https://git.kernel.org/netdev/net-next/c/1d7bab6a9445
  - [RESEND,v3,net-next,2/5] skbuff: constify skb_propagate_pfmemalloc() "page" argument
    https://git.kernel.org/netdev/net-next/c/48f971c9c80a
  - [RESEND,v3,net-next,3/5] net: introduce common dev_page_is_reusable()
    https://git.kernel.org/netdev/net-next/c/bc38f30f8dbc
  - [RESEND,v3,net-next,4/5] net: use the new dev_page_is_reusable() instead of private versions
    https://git.kernel.org/netdev/net-next/c/a79afa78e625
  - [RESEND,v3,net-next,5/5] net: page_pool: simplify page recycling condition tests
    https://git.kernel.org/netdev/net-next/c/05656132a874

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2021-02-05  2:41 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-02 13:30 [PATCH RESEND v3 net-next 0/5] net: consolidate page_is_pfmemalloc() usage Alexander Lobakin
2021-02-02 13:30 ` [PATCH RESEND v3 net-next 1/5] mm: constify page_is_pfmemalloc() argument Alexander Lobakin
2021-02-02 13:31 ` [PATCH RESEND v3 net-next 2/5] skbuff: constify skb_propagate_pfmemalloc() "page" argument Alexander Lobakin
2021-02-02 13:31 ` [PATCH RESEND v3 net-next 3/5] net: introduce common dev_page_is_reusable() Alexander Lobakin
2021-02-02 13:31 ` [PATCH RESEND v3 net-next 4/5] net: use the new dev_page_is_reusable() instead of private versions Alexander Lobakin
2021-02-04 19:52   ` Jesse Brandeburg
2021-02-02 13:31 ` [PATCH RESEND v3 net-next 5/5] net: page_pool: simplify page recycling condition tests Alexander Lobakin
2021-02-05  2:40 ` [PATCH RESEND v3 net-next 0/5] net: consolidate page_is_pfmemalloc() usage patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).