The main reason for this change was to allow mixing pages from different
&page_pools within one &xdp_buff/&xdp_frame. Why not?
Adjust xdp_return_frame_bulk() and page_pool_put_page_bulk(), so that
they won't be tied to a particular pool. Let the latter create a
separate bulk of pages which's PP is different and process it after
the main loop.
This greatly optimizes xdp_return_frame_bulk(): no more hashtable
lookups. Also make xdp_flush_frame_bulk() inline, as it's just one if +
function call + one u32 read, not worth extending the call ladder.
Co-developed-by: Toke Høiland-Jørgensen <toke@xxxxxxxxxx> # iterative
Signed-off-by: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>
Signed-off-by: Alexander Lobakin <aleksander.lobakin@xxxxxxxxx>
---
include/net/page_pool/types.h | 6 ++--
include/net/xdp.h | 16 +++++++---
net/core/page_pool.c | 60 +++++++++++++++++++++++++++--------
net/core/xdp.c | 29 +----------------
4 files changed, 62 insertions(+), 49 deletions(-)
diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h
index 6c1be99a5959..269ee1788388 100644
--- a/include/net/page_pool/types.h
+++ b/include/net/page_pool/types.h
@@ -259,8 +259,7 @@ void page_pool_disable_direct_recycling(struct page_pool *pool);
void page_pool_destroy(struct page_pool *pool);
void page_pool_use_xdp_mem(struct page_pool *pool, void (*disconnect)(void *),
const struct xdp_mem_info *mem);
-void page_pool_put_page_bulk(struct page_pool *pool, struct page **data,
- u32 count);
+void page_pool_put_page_bulk(struct page **data, u32 count);
#else
static inline void page_pool_destroy(struct page_pool *pool)
{
@@ -272,8 +271,7 @@ static inline void page_pool_use_xdp_mem(struct page_pool *pool,
{
}
-static inline void page_pool_put_page_bulk(struct page_pool *pool,
- struct page **data, u32 count)
+static inline void page_pool_put_page_bulk(struct page **data, u32 count)
{
}
#endif
diff --git a/include/net/xdp.h b/include/net/xdp.h
index 4416cd4b5086..cce819ee17d3 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -11,6 +11,8 @@
#include <linux/netdevice.h>
#include <linux/skbuff.h> /* skb_shared_info */
+#include <net/page_pool/types.h>
+
/**
* DOC: XDP RX-queue information
*
@@ -193,14 +195,12 @@ xdp_frame_is_frag_pfmemalloc(const struct xdp_frame *frame)
#define XDP_BULK_QUEUE_SIZE 16
struct xdp_frame_bulk {
int count;
- void *xa;
struct page *q[XDP_BULK_QUEUE_SIZE];
};
static __always_inline void xdp_frame_bulk_init(struct xdp_frame_bulk *bq)
{
- /* bq->count will be zero'ed when bq->xa gets updated */
- bq->xa = NULL;
+ bq->count = 0;
}
static inline struct skb_shared_info *
@@ -317,10 +317,18 @@ void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
void xdp_return_frame(struct xdp_frame *xdpf);
void xdp_return_frame_rx_napi(struct xdp_frame *xdpf);
void xdp_return_buff(struct xdp_buff *xdp);
-void xdp_flush_frame_bulk(struct xdp_frame_bulk *bq);
void xdp_return_frame_bulk(struct xdp_frame *xdpf,
struct xdp_frame_bulk *bq);
+static inline void xdp_flush_frame_bulk(struct xdp_frame_bulk *bq)
+{
+ if (unlikely(!bq->count))
+ return;
+
+ page_pool_put_page_bulk(bq->q, bq->count);
+ bq->count = 0;
+}
+
static __always_inline unsigned int
xdp_get_frame_len(const struct xdp_frame *xdpf)
{
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index ad219206ee8d..2d67c2cc3d77 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -841,7 +841,6 @@ EXPORT_SYMBOL(page_pool_put_unrefed_page);
/**
* page_pool_put_page_bulk() - release references on multiple pages
- * @pool: pool from which pages were allocated
* @data: array holding page pointers
* @count: number of pages in @data
*
@@ -854,35 +853,61 @@ EXPORT_SYMBOL(page_pool_put_unrefed_page);
* Please note the caller must not use data area after running
* page_pool_put_page_bulk(), as this function overwrites it.
*/
-void page_pool_put_page_bulk(struct page_pool *pool, struct page **data,
- u32 count)
+void page_pool_put_page_bulk(struct page **data, u32 count)
{
- int i, bulk_len = 0;
+ netmem_ref bulk[XDP_BULK_QUEUE_SIZE];
+ int i, bulk_len, foreign;
+ struct page_pool *pool;
+ bool again = false;
bool allow_direct;
bool in_softirq;
- allow_direct = page_pool_napi_local(pool);
+again:
+ pool = NULL;
+ bulk_len = 0;
+ foreign = 0;
for (i = 0; i < count; i++) {
- netmem_ref netmem = page_to_netmem(compound_head(data[i]));
+ struct page *page;
+ netmem_ref netmem;
+
+ if (!again) {
+ page = compound_head(data[i]);
+ netmem = page_to_netmem(page);
- /* It is not the last user for the page frag case */
- if (!page_pool_is_last_ref(netmem))
+ /* It is not the last user for the page frag case */
+ if (!page_pool_is_last_ref(netmem))
+ continue;
+ } else {
+ page = data[i];
+ netmem = page_to_netmem(page);
+ }
+
+ if (unlikely(!pool)) {
+ pool = page->pp;
+ allow_direct = page_pool_napi_local(pool);
+ } else if (page->pp != pool) {
+ /*
+ * If the page belongs to a different page_pool, save
+ * it for a second round after the main loop.
+ */
+ data[foreign++] = page;
continue;
+ }
netmem = __page_pool_put_page(pool, netmem, -1, allow_direct);
/* Approved for bulk recycling in ptr_ring cache */
if (netmem)
- data[bulk_len++] = (__force void *)netmem;
+ bulk[bulk_len++] = netmem;
}
if (!bulk_len)
- return;
+ goto out;
/* Bulk producer into ptr_ring page_pool cache */
in_softirq = page_pool_producer_lock(pool);
for (i = 0; i < bulk_len; i++) {
- if (__ptr_ring_produce(&pool->ring, data[i])) {
+ if (__ptr_ring_produce(&pool->ring, (__force void *)bulk[i])) {
/* ring full */
recycle_stat_inc(pool, ring_full);
break;
@@ -893,13 +918,22 @@ void page_pool_put_page_bulk(struct page_pool *pool, struct page **data,
/* Hopefully all pages was return into ptr_ring */
if (likely(i == bulk_len))
- return;
+ goto out;
/* ptr_ring cache full, free remaining pages outside producer lock
* since put_page() with refcnt == 1 can be an expensive operation
*/
for (; i < bulk_len; i++)
- page_pool_return_page(pool, (__force netmem_ref)data[i]);
+ page_pool_return_page(pool, bulk[i]);
+
+out:
+ if (!foreign)
+ return;
+
+ count = foreign;
+ again = true;
+
+ goto again;
}
EXPORT_SYMBOL(page_pool_put_page_bulk);
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 779e646f347b..0fde1bb54192 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -508,46 +508,19 @@ EXPORT_SYMBOL_GPL(xdp_return_frame_rx_napi);
* xdp_frame_bulk is usually stored/allocated on the function
* call-stack to avoid locking penalties.
*/
-void xdp_flush_frame_bulk(struct xdp_frame_bulk *bq)
-{
- struct xdp_mem_allocator *xa = bq->xa;
-
- if (unlikely(!xa || !bq->count))
- return;
-
- page_pool_put_page_bulk(xa->page_pool, bq->q, bq->count);
- /* bq->xa is not cleared to save lookup, if mem.id same in next bulk */
- bq->count = 0;
-}
-EXPORT_SYMBOL_GPL(xdp_flush_frame_bulk);
/* Must be called with rcu_read_lock held */
void xdp_return_frame_bulk(struct xdp_frame *xdpf,
struct xdp_frame_bulk *bq)
{
- struct xdp_mem_info *mem = &xdpf->mem;
- struct xdp_mem_allocator *xa;
-
- if (mem->type != MEM_TYPE_PAGE_POOL) {
+ if (xdpf->mem.type != MEM_TYPE_PAGE_POOL) {
xdp_return_frame(xdpf);
return;
}
- xa = bq->xa;
- if (unlikely(!xa)) {
- xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
- bq->count = 0;
- bq->xa = xa;
- }
-
if (bq->count == XDP_BULK_QUEUE_SIZE)
xdp_flush_frame_bulk(bq);
- if (unlikely(mem->id != xa->mem.id)) {
- xdp_flush_frame_bulk(bq);
- bq->xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
- }
-
if (unlikely(xdp_frame_has_frags(xdpf))) {
struct skb_shared_info *sinfo;
int i;
--
2.47.0
