* [PATCH net-next v11 5/6] page_pool: update document about fragment API
2023-10-13 6:48 [PATCH net-next v11 0/6] introduce page_pool_alloc() related API Yunsheng Lin
@ 2023-10-13 6:48 ` Yunsheng Lin
2023-10-13 6:48 ` [PATCH net-next v11 6/6] net: veth: use newly added page pool API for veth with xdp Yunsheng Lin
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Yunsheng Lin @ 2023-10-13 6:48 UTC (permalink / raw)
To: davem, kuba, pabeni
Cc: netdev, linux-kernel, Yunsheng Lin, Lorenzo Bianconi,
Alexander Duyck, Liang Chen, Alexander Lobakin, Dima Tisnek,
Jesper Dangaard Brouer, Ilias Apalodimas, Eric Dumazet,
Jonathan Corbet, Alexei Starovoitov, Daniel Borkmann,
John Fastabend, linux-doc, bpf
As more drivers begin to use the fragment API, update the
document about how to decide which API to use for the
driver author.
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
CC: Lorenzo Bianconi <lorenzo@kernel.org>
CC: Alexander Duyck <alexander.duyck@gmail.com>
CC: Liang Chen <liangchen.linux@gmail.com>
CC: Alexander Lobakin <aleksander.lobakin@intel.com>
CC: Dima Tisnek <dimaqq@gmail.com>
---
Documentation/networking/page_pool.rst | 4 +-
include/net/page_pool/helpers.h | 93 ++++++++++++++++++++++----
2 files changed, 82 insertions(+), 15 deletions(-)
diff --git a/Documentation/networking/page_pool.rst b/Documentation/networking/page_pool.rst
index 215ebc92752c..0c0705994f51 100644
--- a/Documentation/networking/page_pool.rst
+++ b/Documentation/networking/page_pool.rst
@@ -58,7 +58,9 @@ a page will cause no race conditions is enough.
.. kernel-doc:: include/net/page_pool/helpers.h
:identifiers: page_pool_put_page page_pool_put_full_page
- page_pool_recycle_direct page_pool_dev_alloc_pages
+ page_pool_recycle_direct page_pool_cache_free
+ page_pool_dev_alloc_pages page_pool_dev_alloc_frag
+ page_pool_dev_alloc page_pool_dev_cache_alloc
page_pool_get_dma_addr page_pool_get_dma_dir
.. kernel-doc:: net/core/page_pool.c
diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h
index 674f480d9f2e..7550beeacf3d 100644
--- a/include/net/page_pool/helpers.h
+++ b/include/net/page_pool/helpers.h
@@ -8,23 +8,46 @@
/**
* DOC: page_pool allocator
*
- * The page_pool allocator is optimized for the XDP mode that
- * uses one frame per-page, but it can fallback on the
- * regular page allocator APIs.
+ * The page_pool allocator is optimized for recycling page or page fragment used
+ * by skb packet and xdp frame.
*
- * Basic use involves replacing alloc_pages() calls with the
- * page_pool_alloc_pages() call. Drivers should use
- * page_pool_dev_alloc_pages() replacing dev_alloc_pages().
+ * Basic use involves replacing napi_alloc_frag() and alloc_pages() calls with
+ * page_pool_cache_alloc() and page_pool_alloc(), which allocate memory with or
+ * without page splitting depending on the requested memory size.
*
- * The API keeps track of in-flight pages, in order to let API users know
- * when it is safe to free a page_pool object. Thus, API users
- * must call page_pool_put_page() to free the page, or attach
- * the page to a page_pool-aware object like skbs marked with
- * skb_mark_for_recycle().
+ * If the driver knows that it always requires full pages or its allocations are
+ * always smaller than half a page, it can use one of the more specific API
+ * calls:
*
- * API users must call page_pool_put_page() once on a page, as it
- * will either recycle the page, or in case of refcnt > 1, it will
- * release the DMA mapping and in-flight state accounting.
+ * 1. page_pool_alloc_pages(): allocate memory without page splitting when
+ * driver knows that the memory it need is always bigger than half of the page
+ * allocated from page pool. There is no cache line dirtying for 'struct page'
+ * when a page is recycled back to the page pool.
+ *
+ * 2. page_pool_alloc_frag(): allocate memory with page splitting when driver
+ * knows that the memory it need is always smaller than or equal to half of the
+ * page allocated from page pool. Page splitting enables memory saving and thus
+ * avoids TLB/cache miss for data access, but there also is some cost to
+ * implement page splitting, mainly some cache line dirtying/bouncing for
+ * 'struct page' and atomic operation for page->pp_frag_count.
+ *
+ * The API keeps track of in-flight pages, in order to let API users know when
+ * it is safe to free a page_pool object, the API users must call
+ * page_pool_put_page() or page_pool_cache_free() to free the pp page or the pp
+ * buffer, or attach the pp page or the pp buffer to a page_pool-aware object
+ * like skbs marked with skb_mark_for_recycle().
+ *
+ * page_pool_put_page() may be called multi times on the same page if a page is
+ * split into multi fragments. For the last fragment, it will either recycle the
+ * page, or in case of page->_refcount > 1, it will release the DMA mapping and
+ * in-flight state accounting.
+ *
+ * dma_sync_single_range_for_device() is only called for the last fragment when
+ * page_pool is created with PP_FLAG_DMA_SYNC_DEV flag, so it depends on the
+ * last freed fragment to do the sync_for_device operation for all fragments in
+ * the same page when a page is split, the API user must setup pool->p.max_len
+ * and pool->p.offset correctly and ensure that page_pool_put_page() is called
+ * with dma_sync_size being -1 for fragment API.
*/
#ifndef _NET_PAGE_POOL_HELPERS_H
#define _NET_PAGE_POOL_HELPERS_H
@@ -73,6 +96,17 @@ static inline struct page *page_pool_dev_alloc_pages(struct page_pool *pool)
return page_pool_alloc_pages(pool, gfp);
}
+/**
+ * page_pool_dev_alloc_frag() - allocate a page fragment.
+ * @pool: pool from which to allocate
+ * @offset: offset to the allocated page
+ * @size: requested size
+ *
+ * Get a page fragment from the page allocator or page_pool caches.
+ *
+ * Return:
+ * Return allocated page fragment, otherwise return NULL.
+ */
static inline struct page *page_pool_dev_alloc_frag(struct page_pool *pool,
unsigned int *offset,
unsigned int size)
@@ -111,6 +145,19 @@ static inline struct page *page_pool_alloc(struct page_pool *pool,
return page;
}
+/**
+ * page_pool_dev_alloc() - allocate a page or a page fragment.
+ * @pool: pool from which to allocate
+ * @offset: offset to the allocated page
+ * @size: in as the requested size, out as the allocated size
+ *
+ * Get a page or a page fragment from the page allocator or page_pool caches
+ * depending on the requested size in order to allocate memory with least memory
+ * utilization and performance penalty.
+ *
+ * Return:
+ * Return allocated page or page fragment, otherwise return NULL.
+ */
static inline struct page *page_pool_dev_alloc(struct page_pool *pool,
unsigned int *offset,
unsigned int *size)
@@ -133,6 +180,16 @@ static inline void *page_pool_cache_alloc(struct page_pool *pool,
return page_address(page) + offset;
}
+/**
+ * page_pool_dev_cache_alloc() - allocate a cache.
+ * @pool: pool from which to allocate
+ * @size: in as the requested size, out as the allocated size
+ *
+ * Get a cache from the page allocator or page_pool caches.
+ *
+ * Return:
+ * Return the addr for the allocated cache, otherwise return NULL.
+ */
static inline void *page_pool_dev_cache_alloc(struct page_pool *pool,
unsigned int *size)
{
@@ -280,6 +337,14 @@ static inline void page_pool_recycle_direct(struct page_pool *pool,
#define PAGE_POOL_32BIT_ARCH_WITH_64BIT_DMA \
(sizeof(dma_addr_t) > sizeof(unsigned long))
+/**
+ * page_pool_cache_free() - free a cache into the page_pool
+ * @pool: pool from which cache was allocated
+ * @data: addr of cache to be free
+ * @allow_direct: freed by the consumer, allow lockless caching
+ *
+ * Free a cache allocated from page_pool_dev_cache_alloc().
+ */
static inline void page_pool_cache_free(struct page_pool *pool, void *data,
bool allow_direct)
{
--
2.33.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next v11 6/6] net: veth: use newly added page pool API for veth with xdp
2023-10-13 6:48 [PATCH net-next v11 0/6] introduce page_pool_alloc() related API Yunsheng Lin
2023-10-13 6:48 ` [PATCH net-next v11 5/6] page_pool: update document about fragment API Yunsheng Lin
@ 2023-10-13 6:48 ` Yunsheng Lin
2023-10-17 1:27 ` [PATCH net-next v11 0/6] introduce page_pool_alloc() related API Jakub Kicinski
2023-10-17 4:10 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 12+ messages in thread
From: Yunsheng Lin @ 2023-10-13 6:48 UTC (permalink / raw)
To: davem, kuba, pabeni
Cc: netdev, linux-kernel, Yunsheng Lin, Lorenzo Bianconi,
Alexander Duyck, Liang Chen, Alexander Lobakin, Eric Dumazet,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, bpf
Use page_pool[_cache]_alloc() API to allocate memory with
least memory utilization and performance penalty.
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
CC: Lorenzo Bianconi <lorenzo@kernel.org>
CC: Alexander Duyck <alexander.duyck@gmail.com>
CC: Liang Chen <liangchen.linux@gmail.com>
CC: Alexander Lobakin <aleksander.lobakin@intel.com>
---
drivers/net/veth.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 0deefd1573cf..470791b0b533 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -737,10 +737,11 @@ static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq,
if (skb_shared(skb) || skb_head_is_locked(skb) ||
skb_shinfo(skb)->nr_frags ||
skb_headroom(skb) < XDP_PACKET_HEADROOM) {
- u32 size, len, max_head_size, off;
+ u32 size, len, max_head_size, off, truesize, page_offset;
struct sk_buff *nskb;
struct page *page;
int i, head_off;
+ void *data;
/* We need a private copy of the skb and data buffers since
* the ebpf program can modify it. We segment the original skb
@@ -753,14 +754,17 @@ static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq,
if (skb->len > PAGE_SIZE * MAX_SKB_FRAGS + max_head_size)
goto drop;
+ size = min_t(u32, skb->len, max_head_size);
+ truesize = SKB_HEAD_ALIGN(size) + VETH_XDP_HEADROOM;
+
/* Allocate skb head */
- page = page_pool_dev_alloc_pages(rq->page_pool);
- if (!page)
+ data = page_pool_dev_cache_alloc(rq->page_pool, &truesize);
+ if (!data)
goto drop;
- nskb = napi_build_skb(page_address(page), PAGE_SIZE);
+ nskb = napi_build_skb(data, truesize);
if (!nskb) {
- page_pool_put_full_page(rq->page_pool, page, true);
+ page_pool_cache_free(rq->page_pool, data, true);
goto drop;
}
@@ -768,7 +772,6 @@ static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq,
skb_copy_header(nskb, skb);
skb_mark_for_recycle(nskb);
- size = min_t(u32, skb->len, max_head_size);
if (skb_copy_bits(skb, 0, nskb->data, size)) {
consume_skb(nskb);
goto drop;
@@ -783,14 +786,18 @@ static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq,
len = skb->len - off;
for (i = 0; i < MAX_SKB_FRAGS && off < skb->len; i++) {
- page = page_pool_dev_alloc_pages(rq->page_pool);
+ size = min_t(u32, len, PAGE_SIZE);
+ truesize = size;
+
+ page = page_pool_dev_alloc(rq->page_pool, &page_offset,
+ &truesize);
if (!page) {
consume_skb(nskb);
goto drop;
}
- size = min_t(u32, len, PAGE_SIZE);
- skb_add_rx_frag(nskb, i, page, 0, size, PAGE_SIZE);
+ skb_add_rx_frag(nskb, i, page, page_offset, size,
+ truesize);
if (skb_copy_bits(skb, off, page_address(page),
size)) {
consume_skb(nskb);
--
2.33.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH net-next v11 0/6] introduce page_pool_alloc() related API
2023-10-13 6:48 [PATCH net-next v11 0/6] introduce page_pool_alloc() related API Yunsheng Lin
2023-10-13 6:48 ` [PATCH net-next v11 5/6] page_pool: update document about fragment API Yunsheng Lin
2023-10-13 6:48 ` [PATCH net-next v11 6/6] net: veth: use newly added page pool API for veth with xdp Yunsheng Lin
@ 2023-10-17 1:27 ` Jakub Kicinski
2023-10-17 7:56 ` Yunsheng Lin
2023-10-17 4:10 ` patchwork-bot+netdevbpf
3 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2023-10-17 1:27 UTC (permalink / raw)
To: Yunsheng Lin
Cc: davem, pabeni, netdev, linux-kernel, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Matthias Brugger, AngeloGioacchino Del Regno, bpf,
linux-arm-kernel, linux-mediatek
On Fri, 13 Oct 2023 14:48:20 +0800 Yunsheng Lin wrote:
> v5 RFC: Add a new page_pool_cache_alloc() API, and other minor
> change as discussed in v4. As there seems to be three
> comsumers that might be made use of the new API, so
> repost it as RFC and CC the relevant authors to see
> if the new API fits their need.
I have looked thru the v4 discussion (admittedly it was pretty huge).
I can't find where the "cache" API was suggested.
And I can't figure out now what the "cache" in the name is referring to.
Looks like these are just convenience wrappers which return VA instead
of struct page..
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v11 0/6] introduce page_pool_alloc() related API
2023-10-17 1:27 ` [PATCH net-next v11 0/6] introduce page_pool_alloc() related API Jakub Kicinski
@ 2023-10-17 7:56 ` Yunsheng Lin
2023-10-17 15:13 ` Jakub Kicinski
0 siblings, 1 reply; 12+ messages in thread
From: Yunsheng Lin @ 2023-10-17 7:56 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, pabeni, netdev, linux-kernel, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Matthias Brugger, AngeloGioacchino Del Regno, bpf,
linux-arm-kernel, linux-mediatek
On 2023/10/17 9:27, Jakub Kicinski wrote:
> On Fri, 13 Oct 2023 14:48:20 +0800 Yunsheng Lin wrote:
>> v5 RFC: Add a new page_pool_cache_alloc() API, and other minor
>> change as discussed in v4. As there seems to be three
>> comsumers that might be made use of the new API, so
>> repost it as RFC and CC the relevant authors to see
>> if the new API fits their need.
>
> I have looked thru the v4 discussion (admittedly it was pretty huge).
> I can't find where the "cache" API was suggested.
Actually, the discussion happened in V3 as both of discussions in V3
and V4 seems to be happening concurrently:
https://lore.kernel.org/all/f8ce176f-f975-af11-641c-b56c53a8066a@redhat.com/
> And I can't figure out now what the "cache" in the name is referring to.
> Looks like these are just convenience wrappers which return VA instead
> of struct page..
Yes, it is corresponding to some API like napi_alloc_frag() returning va
instead of 'struct page' mentioned in patch 5.
Anyway, naming is hard, any suggestion for a better naming is always
welcomed:)
> .
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v11 0/6] introduce page_pool_alloc() related API
2023-10-17 7:56 ` Yunsheng Lin
@ 2023-10-17 15:13 ` Jakub Kicinski
2023-10-17 15:14 ` Jakub Kicinski
2023-10-18 11:47 ` Yunsheng Lin
0 siblings, 2 replies; 12+ messages in thread
From: Jakub Kicinski @ 2023-10-17 15:13 UTC (permalink / raw)
To: Yunsheng Lin
Cc: davem, pabeni, netdev, linux-kernel, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Matthias Brugger, AngeloGioacchino Del Regno, bpf,
linux-arm-kernel, linux-mediatek, Alexander Duyck
On Tue, 17 Oct 2023 15:56:48 +0800 Yunsheng Lin wrote:
> > And I can't figure out now what the "cache" in the name is referring to.
> > Looks like these are just convenience wrappers which return VA instead
> > of struct page..
>
> Yes, it is corresponding to some API like napi_alloc_frag() returning va
> instead of 'struct page' mentioned in patch 5.
>
> Anyway, naming is hard, any suggestion for a better naming is always
> welcomed:)
I'd just throw a _va (for virtual address) at the end. And not really
mention it in the documentation. Plus the kdoc of the function should
say that this is just a thin wrapper around other page pool APIs, and
it's safe to mix it with other page pool APIs?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v11 0/6] introduce page_pool_alloc() related API
2023-10-17 15:13 ` Jakub Kicinski
@ 2023-10-17 15:14 ` Jakub Kicinski
2023-10-18 11:47 ` Yunsheng Lin
1 sibling, 0 replies; 12+ messages in thread
From: Jakub Kicinski @ 2023-10-17 15:14 UTC (permalink / raw)
To: Yunsheng Lin
Cc: davem, pabeni, netdev, linux-kernel, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Matthias Brugger, AngeloGioacchino Del Regno, bpf,
linux-arm-kernel, linux-mediatek, Alexander Duyck
On Tue, 17 Oct 2023 08:13:03 -0700 Jakub Kicinski wrote:
> I'd just throw a _va (for virtual address) at the end
To be clear I mean:
page_pool_alloc_va()
page_pool_dev_alloc_va()
page_pool_free_va()
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v11 0/6] introduce page_pool_alloc() related API
2023-10-17 15:13 ` Jakub Kicinski
2023-10-17 15:14 ` Jakub Kicinski
@ 2023-10-18 11:47 ` Yunsheng Lin
2023-10-18 15:35 ` Jakub Kicinski
1 sibling, 1 reply; 12+ messages in thread
From: Yunsheng Lin @ 2023-10-18 11:47 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, pabeni, netdev, linux-kernel, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Matthias Brugger, AngeloGioacchino Del Regno, bpf,
linux-arm-kernel, linux-mediatek, Alexander Duyck
On 2023/10/17 23:13, Jakub Kicinski wrote:
> On Tue, 17 Oct 2023 15:56:48 +0800 Yunsheng Lin wrote:
>>> And I can't figure out now what the "cache" in the name is referring to.
>>> Looks like these are just convenience wrappers which return VA instead
>>> of struct page..
>>
>> Yes, it is corresponding to some API like napi_alloc_frag() returning va
>> instead of 'struct page' mentioned in patch 5.
>>
>> Anyway, naming is hard, any suggestion for a better naming is always
>> welcomed:)
>
> I'd just throw a _va (for virtual address) at the end. And not really
_va seems fine:)
> mention it in the documentation. Plus the kdoc of the function should
> say that this is just a thin wrapper around other page pool APIs, and
> it's safe to mix it with other page pool APIs?
I am not sure I understand what do 'safe' and 'mix' mean here.
For 'safe' part, I suppose you mean if there is a va accociated with
a 'struct page' without calling some API like kmap()? For that, I suppose
it is safe when the driver is calling page_pool API without the
__GFP_HIGHMEM flag. Maybe we should mention that in the kdoc and give a
warning if page_pool_*alloc_va() is called with the __GFP_HIGHMEM flag?
For the 'mix', I suppose you mean the below:
1. Allocate a page with the page_pool_*alloc_va() API and free a page with
page_pool_free() API.
2. Allocate a page with the page_pool_*alloc() API and free a page with
page_pool_free_va() API.
For 1, it seems it is ok as some virt_to_head_page() and page_address() call
between va and 'struct page' does not seem to change anything if we have
enforce page_pool_*alloc_va() to be called without the __GFP_HIGHMEM flag.
For 2, If the va is returned from page_address() which the allocation API is
called without __GFP_HIGHMEM flag. If not, the va is from kmap*()? which means
we may be calling page_pool_free_va() before kunmap*()? Is that possible?
> .
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v11 0/6] introduce page_pool_alloc() related API
2023-10-18 11:47 ` Yunsheng Lin
@ 2023-10-18 15:35 ` Jakub Kicinski
2023-10-19 13:22 ` Yunsheng Lin
0 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2023-10-18 15:35 UTC (permalink / raw)
To: Yunsheng Lin
Cc: davem, pabeni, netdev, linux-kernel, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Matthias Brugger, AngeloGioacchino Del Regno, bpf,
linux-arm-kernel, linux-mediatek, Alexander Duyck
On Wed, 18 Oct 2023 19:47:16 +0800 Yunsheng Lin wrote:
> > mention it in the documentation. Plus the kdoc of the function should
> > say that this is just a thin wrapper around other page pool APIs, and
> > it's safe to mix it with other page pool APIs?
>
> I am not sure I understand what do 'safe' and 'mix' mean here.
>
> For 'safe' part, I suppose you mean if there is a va accociated with
> a 'struct page' without calling some API like kmap()? For that, I suppose
> it is safe when the driver is calling page_pool API without the
> __GFP_HIGHMEM flag. Maybe we should mention that in the kdoc and give a
> warning if page_pool_*alloc_va() is called with the __GFP_HIGHMEM flag?
Sounds good. Warning wrapped in #if CONFIG_DEBUG_NET perhaps?
> For the 'mix', I suppose you mean the below:
> 1. Allocate a page with the page_pool_*alloc_va() API and free a page with
> page_pool_free() API.
> 2. Allocate a page with the page_pool_*alloc() API and free a page with
> page_pool_free_va() API.
>
> For 1, it seems it is ok as some virt_to_head_page() and page_address() call
> between va and 'struct page' does not seem to change anything if we have
> enforce page_pool_*alloc_va() to be called without the __GFP_HIGHMEM flag.
>
> For 2, If the va is returned from page_address() which the allocation API is
> called without __GFP_HIGHMEM flag. If not, the va is from kmap*()? which means
> we may be calling page_pool_free_va() before kunmap*()? Is that possible?
Right, if someone passes kmap()'ed address they are trying quite hard
to break their own driver. Technically possible but I wouldn't worry.
I just mean that in the common case of non-HIGHMEM page, calling
page_pool_free_va() with the address returned by page_address()
is perfectly legal.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v11 0/6] introduce page_pool_alloc() related API
2023-10-18 15:35 ` Jakub Kicinski
@ 2023-10-19 13:22 ` Yunsheng Lin
2023-10-19 13:56 ` Jakub Kicinski
0 siblings, 1 reply; 12+ messages in thread
From: Yunsheng Lin @ 2023-10-19 13:22 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, pabeni, netdev, linux-kernel, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Matthias Brugger, AngeloGioacchino Del Regno, bpf,
linux-arm-kernel, linux-mediatek, Alexander Duyck
On 2023/10/18 23:35, Jakub Kicinski wrote:
> On Wed, 18 Oct 2023 19:47:16 +0800 Yunsheng Lin wrote:
>>> mention it in the documentation. Plus the kdoc of the function should
>>> say that this is just a thin wrapper around other page pool APIs, and
>>> it's safe to mix it with other page pool APIs?
>>
>> I am not sure I understand what do 'safe' and 'mix' mean here.
>>
>> For 'safe' part, I suppose you mean if there is a va accociated with
>> a 'struct page' without calling some API like kmap()? For that, I suppose
>> it is safe when the driver is calling page_pool API without the
>> __GFP_HIGHMEM flag. Maybe we should mention that in the kdoc and give a
>> warning if page_pool_*alloc_va() is called with the __GFP_HIGHMEM flag?
>
> Sounds good. Warning wrapped in #if CONFIG_DEBUG_NET perhaps?
How about something like __get_free_pages() does with gfp flags?
https://elixir.free-electrons.com/linux/v6.4-rc6/source/mm/page_alloc.c#L4818
how about something like below on top of this patchset:
diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h
index 7550beeacf3d..61cee55606c0 100644
--- a/include/net/page_pool/helpers.h
+++ b/include/net/page_pool/helpers.h
@@ -167,13 +167,13 @@ static inline struct page *page_pool_dev_alloc(struct page_pool *pool,
return page_pool_alloc(pool, offset, size, gfp);
}
-static inline void *page_pool_cache_alloc(struct page_pool *pool,
- unsigned int *size, gfp_t gfp)
+static inline void *page_pool_alloc_va(struct page_pool *pool,
+ unsigned int *size, gfp_t gfp)
{
unsigned int offset;
struct page *page;
- page = page_pool_alloc(pool, &offset, size, gfp);
+ page = page_pool_alloc(pool, &offset, size, gfp & ~__GFP_HIGHMEM);
if (unlikely(!page))
return NULL;
@@ -181,21 +181,22 @@ static inline void *page_pool_cache_alloc(struct page_pool *pool,
}
/**
- * page_pool_dev_cache_alloc() - allocate a cache.
+ * page_pool_dev_alloc_va() - allocate a page or a page fragment.
* @pool: pool from which to allocate
* @size: in as the requested size, out as the allocated size
*
- * Get a cache from the page allocator or page_pool caches.
+ * This is just a thin wrapper around the page_pool_alloc() API, and
+ * it returns va of the allocated page or page fragment.
*
* Return:
- * Return the addr for the allocated cache, otherwise return NULL.
+ * Return the va for the allocated page or page fragment, otherwise return NULL.
*/
-static inline void *page_pool_dev_cache_alloc(struct page_pool *pool,
- unsigned int *size)
+static inline void *page_pool_dev_alloc_va(struct page_pool *pool,
+ unsigned int *size)
{
gfp_t gfp = (GFP_ATOMIC | __GFP_NOWARN);
- return page_pool_cache_alloc(pool, size, gfp);
+ return page_pool_alloc_va(pool, size, gfp);
}
/**
@@ -338,17 +339,17 @@ static inline void page_pool_recycle_direct(struct page_pool *pool,
(sizeof(dma_addr_t) > sizeof(unsigned long))
/**
- * page_pool_cache_free() - free a cache into the page_pool
- * @pool: pool from which cache was allocated
- * @data: addr of cache to be free
+ * page_pool_free_va() - free a va into the page_pool
+ * @pool: pool from which va was allocated
+ * @va: va to be free
* @allow_direct: freed by the consumer, allow lockless caching
*
* Free a cache allocated from page_pool_dev_cache_alloc().
*/
-static inline void page_pool_cache_free(struct page_pool *pool, void *data,
- bool allow_direct)
+static inline void page_pool_free_va(struct page_pool *pool, void *va,
+ bool allow_direct)
{
- page_pool_put_page(pool, virt_to_head_page(data), -1, allow_direct);
+ page_pool_put_page(pool, virt_to_head_page(va), -1, allow_direct);
}
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH net-next v11 0/6] introduce page_pool_alloc() related API
2023-10-19 13:22 ` Yunsheng Lin
@ 2023-10-19 13:56 ` Jakub Kicinski
0 siblings, 0 replies; 12+ messages in thread
From: Jakub Kicinski @ 2023-10-19 13:56 UTC (permalink / raw)
To: Yunsheng Lin
Cc: davem, pabeni, netdev, linux-kernel, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Matthias Brugger, AngeloGioacchino Del Regno, bpf,
linux-arm-kernel, linux-mediatek, Alexander Duyck
On Thu, 19 Oct 2023 21:22:07 +0800 Yunsheng Lin wrote:
> > Sounds good. Warning wrapped in #if CONFIG_DEBUG_NET perhaps?
>
> How about something like __get_free_pages() does with gfp flags?
> https://elixir.free-electrons.com/linux/v6.4-rc6/source/mm/page_alloc.c#L4818
Fine by me!
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v11 0/6] introduce page_pool_alloc() related API
2023-10-13 6:48 [PATCH net-next v11 0/6] introduce page_pool_alloc() related API Yunsheng Lin
` (2 preceding siblings ...)
2023-10-17 1:27 ` [PATCH net-next v11 0/6] introduce page_pool_alloc() related API Jakub Kicinski
@ 2023-10-17 4:10 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 12+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-10-17 4:10 UTC (permalink / raw)
To: Yunsheng Lin
Cc: davem, kuba, pabeni, netdev, linux-kernel, ast, daniel, hawk,
john.fastabend, matthias.bgg, angelogioacchino.delregno, bpf,
linux-arm-kernel, linux-mediatek
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Fri, 13 Oct 2023 14:48:20 +0800 you wrote:
> In [1] & [2] & [3], there are usecases for veth and virtio_net
> to use frag support in page pool to reduce memory usage, and it
> may request different frag size depending on the head/tail
> room space for xdp_frame/shinfo and mtu/packet size. When the
> requested frag size is large enough that a single page can not
> be split into more than one frag, using frag support only have
> performance penalty because of the extra frag count handling
> for frag support.
>
> [...]
Here is the summary with links:
- [net-next,v11,1/6] page_pool: fragment API support for 32-bit arch with 64-bit DMA
https://git.kernel.org/netdev/net-next/c/90de47f020db
- [net-next,v11,2/6] page_pool: unify frag_count handling in page_pool_is_last_frag()
(no matching commit)
- [net-next,v11,3/6] page_pool: remove PP_FLAG_PAGE_FRAG
(no matching commit)
- [net-next,v11,4/6] page_pool: introduce page_pool[_cache]_alloc() API
(no matching commit)
- [net-next,v11,5/6] page_pool: update document about fragment API
(no matching commit)
- [net-next,v11,6/6] net: veth: use newly added page pool API for veth with xdp
(no matching commit)
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] 12+ messages in thread