Thread (11 messages) flat view 11 messages, 5 authors, 2025-02-22

Re: [PATCH net-next v9 3/4] page_pool: support unlimited number of inflight pages

From: Jesper Dangaard Brouer <hawk@kernel.org>
Date: 2025-02-21 10:13:01
Also in: linux-iommu, lkml

See below, I request changes to add memory accounting for these item_blocks.

On 12/02/2025 10.25, Yunsheng Lin wrote:
quoted hunk ↗ jump to hunk
Currently a fixed size of pre-allocated memory is used to
keep track of the inflight pages, in order to use the DMA
API correctly.

As mentioned [1], the number of inflight pages can be up to
73203 depending on the use cases. Allocate memory dynamically
to keep track of the inflight pages when pre-allocated memory
runs out.

The overhead of using dynamic memory allocation is about 10ns~
20ns, which causes 5%~10% performance degradation for the test
case of time_bench_page_pool03_slow() in [2].

1. https://lore.kernel.org/all/b8b7818a-e44b-45f5-91c2-d5eceaa5dd5b@kernel.org/ (local)
2. https://github.com/netoptimizer/prototype-kernel
CC: Robin Murphy <robin.murphy@arm.com>
CC: Alexander Duyck <redacted>
CC: IOMMU <redacted>
Fixes: f71fec47c2df ("page_pool: make sure struct device is stable")
Signed-off-by: Yunsheng Lin <redacted>
---
  include/net/page_pool/types.h | 10 +++++
  net/core/page_pool.c          | 80 ++++++++++++++++++++++++++++++++++-
  2 files changed, 89 insertions(+), 1 deletion(-)
diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h
index c131e2725e9a..c8c47ca67f4b 100644
--- a/include/net/page_pool/types.h
+++ b/include/net/page_pool/types.h
@@ -103,6 +103,7 @@ struct page_pool_params {
   * @waive:	pages obtained from the ptr ring that cannot be added to
   *		the cache due to a NUMA mismatch
   * @item_fast_empty: pre-allocated item cache is empty
+ * @item_slow_failed: failed to allocate memory for item_block
   */
  struct page_pool_alloc_stats {
  	u64 fast;
@@ -112,6 +113,7 @@ struct page_pool_alloc_stats {
  	u64 refill;
  	u64 waive;
  	u64 item_fast_empty;
+	u64 item_slow_failed;
  };
  
  /**
@@ -159,9 +161,16 @@ struct page_pool_item {
  struct page_pool_item_block {
  	struct page_pool *pp;
  	struct list_head list;
+	unsigned int flags;
+	refcount_t ref;
  	struct page_pool_item items[];
  };
  
+struct page_pool_slow_item {
+	struct page_pool_item_block *block;
+	unsigned int next_to_use;
+};
+
  /* Ensure the offset of 'pp' field for both 'page_pool_item_block' and
   * 'netmem_item_block' are the same.
   */
@@ -191,6 +200,7 @@ struct page_pool {
  	int cpuid;
  	u32 pages_state_hold_cnt;
  	struct llist_head hold_items;
+	struct page_pool_slow_item slow_items;
  
  	bool has_init_callback:1;	/* slow::init_callback is set */
  	bool dma_map:1;			/* Perform DMA mapping */
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index 508dd51b1a9a..3109bf015225 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -64,6 +64,7 @@ static const char pp_stats[][ETH_GSTRING_LEN] = {
  	"rx_pp_alloc_refill",
  	"rx_pp_alloc_waive",
  	"rx_pp_alloc_item_fast_empty",
+	"rx_pp_alloc_item_slow_failed",
  	"rx_pp_recycle_cached",
  	"rx_pp_recycle_cache_full",
  	"rx_pp_recycle_ring",
@@ -98,6 +99,7 @@ bool page_pool_get_stats(const struct page_pool *pool,
  	stats->alloc_stats.refill += pool->alloc_stats.refill;
  	stats->alloc_stats.waive += pool->alloc_stats.waive;
  	stats->alloc_stats.item_fast_empty += pool->alloc_stats.item_fast_empty;
+	stats->alloc_stats.item_slow_failed += pool->alloc_stats.item_slow_failed;
  
  	for_each_possible_cpu(cpu) {
  		const struct page_pool_recycle_stats *pcpu =
@@ -144,6 +146,7 @@ u64 *page_pool_ethtool_stats_get(u64 *data, const void *stats)
  	*data++ = pool_stats->alloc_stats.refill;
  	*data++ = pool_stats->alloc_stats.waive;
  	*data++ = pool_stats->alloc_stats.item_fast_empty;
+	*data++ = pool_stats->alloc_stats.item_slow_failed;
  	*data++ = pool_stats->recycle_stats.cached;
  	*data++ = pool_stats->recycle_stats.cache_full;
  	*data++ = pool_stats->recycle_stats.ring;
@@ -430,6 +433,7 @@ static void page_pool_item_uninit(struct page_pool *pool)
  					 struct page_pool_item_block,
  					 list);
  		list_del(&block->list);
+		WARN_ON(refcount_read(&block->ref));
  		put_page(virt_to_page(block));
  	}
  }
@@ -513,10 +517,43 @@ static struct page_pool_item *page_pool_fast_item_alloc(struct page_pool *pool)
  	return llist_entry(first, struct page_pool_item, lentry);
  }
  
+#define PAGE_POOL_SLOW_ITEM_BLOCK_BIT			BIT(0)
+static struct page_pool_item *page_pool_slow_item_alloc(struct page_pool *pool)
+{
+	if (unlikely(!pool->slow_items.block ||
+		     pool->slow_items.next_to_use >= ITEMS_PER_PAGE)) {
+		struct page_pool_item_block *block;
+		struct page *page;
+
+		page = alloc_pages_node(pool->p.nid, GFP_ATOMIC | __GFP_NOWARN |
+					__GFP_ZERO, 0);
+		if (!page) {
+			alloc_stat_inc(pool, item_slow_failed);
+			return NULL;
+		}
We also need stats on how many pages we allocate for these item_blocks
(and later free). This new scheme of keeping track of all pages
allocated via page_pool, is obviously going to consume more memory.

I want to be able to find out how much memory a page_pool is consuming.
(E.g. Kuba added a nice interface for querying inflight packets, even
though this is kept as two different counters).

What I worry about, is that fragmentation happens inside these
item_blocks. (I hope you understand what I mean by fragmentation, else
let me know).

Could you explain how code handles or avoids fragmentation?

quoted hunk ↗ jump to hunk
+
+		block = page_address(page);
+		block->pp = pool;
+		block->flags |= PAGE_POOL_SLOW_ITEM_BLOCK_BIT;
+		refcount_set(&block->ref, ITEMS_PER_PAGE);
+		pool->slow_items.block = block;
+		pool->slow_items.next_to_use = 0;
+
+		spin_lock_bh(&pool->item_lock);
+		list_add(&block->list, &pool->item_blocks);
+		spin_unlock_bh(&pool->item_lock);
+	}
+
+	return &pool->slow_items.block->items[pool->slow_items.next_to_use++];
+}
+
  static bool page_pool_set_item_info(struct page_pool *pool, netmem_ref netmem)
  {
  	struct page_pool_item *item = page_pool_fast_item_alloc(pool);
  
+	if (unlikely(!item))
+		item = page_pool_slow_item_alloc(pool);
+
  	if (likely(item)) {
  		item->pp_netmem = netmem;
  		page_pool_item_set_used(item);
@@ -526,6 +563,37 @@ static bool page_pool_set_item_info(struct page_pool *pool, netmem_ref netmem)
  	return !!item;
  }
  
+static void __page_pool_slow_item_free(struct page_pool *pool,
+				       struct page_pool_item_block *block)
+{
+	spin_lock_bh(&pool->item_lock);
+	list_del(&block->list);
+	spin_unlock_bh(&pool->item_lock);
+
+	put_page(virt_to_page(block));
Also counter for releasing block page here.

--Jesper
quoted hunk ↗ jump to hunk
+}
+
+static void page_pool_slow_item_drain(struct page_pool *pool)
+{
+	struct page_pool_item_block *block = pool->slow_items.block;
+
+	if (!block || pool->slow_items.next_to_use >= ITEMS_PER_PAGE)
+		return;
+
+	if (refcount_sub_and_test(ITEMS_PER_PAGE - pool->slow_items.next_to_use,
+				  &block->ref))
+		__page_pool_slow_item_free(pool, block);
+}
+
+static void page_pool_slow_item_free(struct page_pool *pool,
+				     struct page_pool_item_block *block)
+{
+	if (likely(!refcount_dec_and_test(&block->ref)))
+		return;
+
+	__page_pool_slow_item_free(pool, block);
+}
+
  static void page_pool_fast_item_free(struct page_pool *pool,
  				     struct page_pool_item *item)
  {
@@ -535,13 +603,22 @@ static void page_pool_fast_item_free(struct page_pool *pool,
  static void page_pool_clear_item_info(struct page_pool *pool, netmem_ref netmem)
  {
  	struct page_pool_item *item = netmem_get_pp_item(netmem);
+	struct page_pool_item_block *block;
  
  	DEBUG_NET_WARN_ON_ONCE(item->pp_netmem != netmem);
  	DEBUG_NET_WARN_ON_ONCE(page_pool_item_is_mapped(item));
  	DEBUG_NET_WARN_ON_ONCE(!page_pool_item_is_used(item));
  	page_pool_item_clear_used(item);
  	netmem_set_pp_item(netmem, NULL);
-	page_pool_fast_item_free(pool, item);
+
+	block = page_pool_item_to_block(item);
+	if (likely(!(block->flags & PAGE_POOL_SLOW_ITEM_BLOCK_BIT))) {
+		DEBUG_NET_WARN_ON_ONCE(refcount_read(&block->ref));
+		page_pool_fast_item_free(pool, item);
+		return;
+	}
+
+	page_pool_slow_item_free(pool, block);
  }
  
  /**
@@ -1383,6 +1460,7 @@ void page_pool_destroy(struct page_pool *pool)
  
  	page_pool_disable_direct_recycling(pool);
  	page_pool_free_frag(pool);
+	page_pool_slow_item_drain(pool);
  
  	if (!page_pool_release(pool))
  		return;
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help