On 09/01/2023 19.36, Matthew Wilcox wrote:
On Fri, Jan 06, 2023 at 09:16:25PM +0100, Jesper Dangaard Brouer wrote:
On 06/01/2023 17.53, Matthew Wilcox wrote:
On Fri, Jan 06, 2023 at 04:49:12PM +0100, Jesper Dangaard Brouer wrote:
On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
This function accesses the pagepool members of struct page directly,
so it needs to become netmem. Add page_pool_put_full_netmem() and
page_pool_recycle_netmem().
Signed-off-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
---
include/net/page_pool.h | 14 +++++++++++++-
net/core/page_pool.c | 13 ++++++-------
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/include/net/page_pool.h b/include/net/page_pool.h
index fbb653c9f1da..126c04315929 100644
--- a/include/net/page_pool.h
+++ b/include/net/page_pool.h
@@ -464,10 +464,16 @@ static inline void page_pool_put_page(struct page_pool *pool,
}
/* Same as above but will try to sync the entire area pool->max_len */
+static inline void page_pool_put_full_netmem(struct page_pool *pool,
+ struct netmem *nmem, bool allow_direct)
+{
+ page_pool_put_netmem(pool, nmem, -1, allow_direct);
+}
+
static inline void page_pool_put_full_page(struct page_pool *pool,
struct page *page, bool allow_direct)
{
- page_pool_put_page(pool, page, -1, allow_direct);
+ page_pool_put_full_netmem(pool, page_netmem(page), allow_direct);
}
/* Same as above but the caller must guarantee safe context. e.g NAPI */
@@ -477,6 +483,12 @@ static inline void page_pool_recycle_direct(struct page_pool *pool,
page_pool_put_full_page(pool, page, true);
}
+static inline void page_pool_recycle_netmem(struct page_pool *pool,
+ struct netmem *nmem)
+{
+ page_pool_put_full_netmem(pool, nmem, true);
^^^^
It is not clear in what context page_pool_recycle_netmem() will be used,
but I think the 'true' (allow_direct=true) might be wrong here.
It is only in limited special cases (RX-NAPI context) we can allow
direct return to the RX-alloc-cache.
Mmm. It's a c'n'p of the previous function:
static inline void page_pool_recycle_direct(struct page_pool *pool,
struct page *page)
{
page_pool_put_full_page(pool, page, true);
}
so perhaps it's just badly named?
Yes, I think so.
Can we name it:
page_pool_recycle_netmem_direct
And perhaps add a comment with a warning like:
/* Caller must guarantee safe context. e.g NAPI */
Like the page_pool_recycle_direct() function has a comment.
I don't really like the new name you're proposing here. Really,
page_pool_recycle_direct() is the perfect name, it just has the wrong
type.
I considered the attached megapatch, but I don't think that's a great
idea.
So here's what I'm planning instead:
I do like below patch.
I must admit I had to lookup _Generic() when I started reviewing this
patchset. I think it makes a lot of sense to use here as it allow us to
easier convert drivers over.
We have 22 call spots in drivers:
$ git grep page_pool_recycle_direct drivers/net/ethernet/ | wc -l
22
But approx 9 drivers doing this (as each driver calls it in multiple
places).
page_pool: Allow page_pool_recycle_direct() to take a netmem or a page
With no better name for a variant of page_pool_recycle_direct() which
takes a netmem instead of a page, use _Generic() to allow it to take
either a page or a netmem argument. It's a bit ugly, but maybe not
the worst alternative?
Signed-off-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
diff --git a/include/net/page_pool.h b/include/net/page_pool.h
index abe3822a1125..1eed8ed2dcc1 100644
--- a/include/net/page_pool.h
+++ b/include/net/page_pool.h
@@ -477,12 +477,22 @@ static inline void page_pool_put_full_page(struct page_pool *pool,
}
/* Same as above but the caller must guarantee safe context. e.g NAPI */
-static inline void page_pool_recycle_direct(struct page_pool *pool,
+static inline void __page_pool_recycle_direct(struct page_pool *pool,
+ struct netmem *nmem)
+{
+ page_pool_put_full_netmem(pool, nmem, true);
+}
+
+static inline void __page_pool_recycle_page_direct(struct page_pool *pool,
struct page *page)
{
- page_pool_put_full_page(pool, page, true);
+ page_pool_put_full_netmem(pool, page_netmem(page), true);
}
+#define page_pool_recycle_direct(pool, mem) _Generic((mem), \
+ struct netmem *: __page_pool_recycle_direct(pool, (struct netmem *)mem), \
+ struct page *: __page_pool_recycle_page_direct(pool, (struct page *)mem))
+
#define PAGE_POOL_DMA_USE_PP_FRAG_COUNT \
(sizeof(dma_addr_t) > sizeof(unsigned long))
