The MM subsystem is trying to reduce struct page to a single pointer.
The first step towards that is splitting struct page by its individual
users, as has already been done with folio and slab. This patchset does
that for netmem which is used for page pools.
There are some relatively significant reductions in kernel text size
from these changes. They don't appear to affect performance at all,
but it's nice to save a bit of memory.
v2:
- Rebase to next-20230105
- Add kernel-doc for struct netmem
- Add mlx5 compilation fixes from Jesper
- Folded in minor nit from Alex Duyck
Matthew Wilcox (Oracle) (24):
netmem: Create new type
netmem: Add utility functions
page_pool: Add netmem_set_dma_addr() and netmem_get_dma_addr()
page_pool: Convert page_pool_release_page() to
page_pool_release_netmem()
page_pool: Start using netmem in allocation path.
page_pool: Convert page_pool_return_page() to
page_pool_return_netmem()
page_pool: Convert __page_pool_put_page() to __page_pool_put_netmem()
page_pool: Convert pp_alloc_cache to contain netmem
page_pool: Convert page_pool_defrag_page() to
page_pool_defrag_netmem()
page_pool: Convert page_pool_put_defragged_page() to netmem
page_pool: Convert page_pool_empty_ring() to use netmem
page_pool: Convert page_pool_alloc_pages() to page_pool_alloc_netmem()
page_pool: Convert page_pool_dma_sync_for_device() to take a netmem
page_pool: Convert page_pool_recycle_in_cache() to netmem
page_pool: Remove page_pool_defrag_page()
page_pool: Use netmem in page_pool_drain_frag()
page_pool: Convert page_pool_return_skb_page() to use netmem
page_pool: Convert frag_page to frag_nmem
xdp: Convert to netmem
mm: Remove page pool members from struct page
page_pool: Pass a netmem to init_callback()
net: Add support for netmem in skb_frag
mvneta: Convert to netmem
mlx5: Convert to netmem
Documentation/networking/page_pool.rst | 5 +
drivers/net/ethernet/marvell/mvneta.c | 48 +--
drivers/net/ethernet/mellanox/mlx5/core/en.h | 10 +-
.../net/ethernet/mellanox/mlx5/core/en/txrx.h | 4 +-
.../net/ethernet/mellanox/mlx5/core/en/xdp.c | 24 +-
.../net/ethernet/mellanox/mlx5/core/en/xdp.h | 2 +-
.../net/ethernet/mellanox/mlx5/core/en_main.c | 12 +-
.../net/ethernet/mellanox/mlx5/core/en_rx.c | 130 +++++----
include/linux/mm_types.h | 22 --
include/linux/skbuff.h | 11 +
include/net/page_pool.h | 209 +++++++++++--
include/trace/events/page_pool.h | 28 +-
net/bpf/test_run.c | 4 +-
net/core/page_pool.c | 274 +++++++++---------
net/core/xdp.c | 7 +-
15 files changed, 471 insertions(+), 319 deletions(-)
--
2.35.1
Retrieve a netmem from the ptr_ring instead of a page.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
net/core/page_pool.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
Allow drivers to add netmem to skbs & retrieve them again. If the
VM_BUG_ON triggers, we can add a call to compound_head() either in
this function or in page_netmem().
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
include/linux/skbuff.h | 11 +++++++++++
1 file changed, 11 insertions(+)
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@infradead.org>
---
include/net/page_pool.h | 14 +++++++++++++-
net/core/page_pool.c | 13 ++++++-------
2 files changed, 19 insertions(+), 8 deletions(-)
@@ -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 */+staticinlinevoidpage_pool_put_full_netmem(structpage_pool*pool,+structnetmem*nmem,boolallow_direct)+{+page_pool_put_netmem(pool,nmem,-1,allow_direct);+}+staticinlinevoidpage_pool_put_full_page(structpage_pool*pool,structpage*page,boolallow_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 */
@@ -886,28 +886,27 @@ EXPORT_SYMBOL(page_pool_update_nid);boolpage_pool_return_skb_page(structpage*page){+structnetmem*nmem=page_netmem(compound_head(page));structpage_pool*pp;-page=compound_head(page);--/* page->pp_magic is OR'ed with PP_SIGNATURE after the allocation+/* nmem->pp_magic is OR'ed with PP_SIGNATURE after the allocation*inordertopreserveanyexistingbits,suchasbit0forthe*headpageofcompoundpageandbit1forpfmemallocpage,so*maskthosebitsforfreeingsidewhendoingbelowchecking,-*andpage_is_pfmemalloc()ischeckedin__page_pool_put_page()+*andnetmem_is_pfmemalloc()ischeckedin__page_pool_put_netmem()*toavoidrecyclingthepfmemallocpage.*/-if(unlikely((page->pp_magic&~0x3UL)!=PP_SIGNATURE))+if(unlikely((nmem->pp_magic&~0x3UL)!=PP_SIGNATURE))returnfalse;-pp=page->pp;+pp=nmem->pp;/* Driver set this to memory recycling info. Reset it on recycle.*Thiswill*not*workforNICusingasplit-pagememorymodel.*Thepagewillbereturnedtothepoolhereregardlessofthe*'flipped'fragmentbeinginuseornot.*/-page_pool_put_full_page(pp,page,false);+page_pool_put_full_netmem(pp,nmem,false);returntrue;}
We're not quite ready to change the API of page_pool_drain_frag(),
but we can remove the use of several wrappers by using the netmem
throughout.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
net/core/page_pool.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
@@ -672,17 +672,17 @@ static struct page *page_pool_drain_frag(struct page_pool *pool,longdrain_count=BIAS_MAX-pool->frag_users;/* Some user is still using the page frag */-if(likely(page_pool_defrag_page(page,drain_count)))+if(likely(page_pool_defrag_netmem(nmem,drain_count)))returnNULL;-if(page_ref_count(page)==1&&!page_is_pfmemalloc(page)){+if(netmem_ref_count(nmem)==1&&!netmem_is_pfmemalloc(nmem)){if(pool->p.flags&PP_FLAG_DMA_SYNC_DEV)page_pool_dma_sync_for_device(pool,nmem,-1);returnpage;}-page_pool_return_page(pool,page);+page_pool_return_netmem(pool,nmem);returnNULL;}
These are now split out into their own netmem struct.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
include/linux/mm_types.h | 22 ----------------------
include/net/page_pool.h | 4 ----
2 files changed, 26 deletions(-)
@@ -116,28 +116,6 @@ struct page {*/unsignedlongprivate;};-struct{/* page_pool used by netstack */-/**-*@pp_magic:magicvaluetoavoidrecyclingnon-*page_poolallocatedpages.-*/-unsignedlongpp_magic;-structpage_pool*pp;-unsignedlong_pp_mapping_pad;-unsignedlongdma_addr;-union{-/**-*dma_addr_upper:mightrequirea64-bit-*valueon32-bitarchitectures.-*/-unsignedlongdma_addr_upper;-/**-*Forfragpagesupport,notsupportedin-*32-bitarchitectureswith64-bitDMA.-*/-atomic_long_tpp_frag_count;-};-};struct{/* Tail pages of compound page */unsignedlongcompound_head;/* Bit zero is set */
Change the type here from page to netmem. It works out well to
convert page_pool_refill_alloc_cache() to return a netmem instead
of a page as part of this commit.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
include/net/page_pool.h | 2 +-
net/core/page_pool.c | 52 ++++++++++++++++++++---------------------
2 files changed, 27 insertions(+), 27 deletions(-)
@@ -229,10 +229,10 @@ void page_pool_return_page(struct page_pool *pool, struct page *page)}noinline-staticstructpage*page_pool_refill_alloc_cache(structpage_pool*pool)+staticstructnetmem*page_pool_refill_alloc_cache(structpage_pool*pool){structptr_ring*r=&pool->ring;-structpage*page;+structnetmem*nmem;intpref_nid;/* preferred NUMA node *//* Quicker fallback, avoid locks when ring is empty */
@@ -253,49 +253,49 @@ static struct page *page_pool_refill_alloc_cache(struct page_pool *pool)/* Refill alloc array, but only if NUMA match */do{-page=__ptr_ring_consume(r);-if(unlikely(!page))+nmem=__ptr_ring_consume(r);+if(unlikely(!nmem))break;-if(likely(page_to_nid(page)==pref_nid)){-pool->alloc.cache[pool->alloc.count++]=page;+if(likely(netmem_nid(nmem)==pref_nid)){+pool->alloc.cache[pool->alloc.count++]=nmem;}else{/* NUMA mismatch;*(1)release1pagetopage-allocatorand*(2)breakouttofallthroughtoalloc_pages_node.*Thislimitstressonpagebuddyalloactor.*/-page_pool_return_page(pool,page);+page_pool_return_netmem(pool,nmem);alloc_stat_inc(pool,waive);-page=NULL;+nmem=NULL;break;}}while(pool->alloc.count<PP_ALLOC_CACHE_REFILL);/* Return last page */if(likely(pool->alloc.count>0)){-page=pool->alloc.cache[--pool->alloc.count];+nmem=pool->alloc.cache[--pool->alloc.count];alloc_stat_inc(pool,refill);}-returnpage;+returnnmem;}/* fast path */staticstructpage*__page_pool_get_cached(structpage_pool*pool){-structpage*page;+structnetmem*nmem;/* Caller MUST guarantee safe non-concurrent access, e.g. softirq */if(likely(pool->alloc.count)){/* Fast-path */-page=pool->alloc.cache[--pool->alloc.count];+nmem=pool->alloc.cache[--pool->alloc.count];alloc_stat_inc(pool,fast);}else{-page=page_pool_refill_alloc_cache(pool);+nmem=page_pool_refill_alloc_cache(pool);}-returnpage;+returnnetmem_page(nmem);}staticvoidpage_pool_dma_sync_for_device(structpage_pool*pool,
@@ -391,13 +391,13 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool,/* Unnecessary as alloc cache is empty, but guarantees zero count */if(unlikely(pool->alloc.count>0))-returnpool->alloc.cache[--pool->alloc.count];+returnnetmem_page(pool->alloc.cache[--pool->alloc.count]);/* Mark empty alloc.cache slots "empty" for alloc_pages_bulk_array */memset(&pool->alloc.cache,0,sizeof(void*)*bulk);nr_pages=alloc_pages_bulk_array_node(gfp,pool->p.nid,bulk,-pool->alloc.cache);+(structpage**)pool->alloc.cache);if(unlikely(!nr_pages))returnNULL;
@@ -413,7 +413,7 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool,}page_pool_set_pp_info(pool,nmem);-pool->alloc.cache[pool->alloc.count++]=netmem_page(nmem);+pool->alloc.cache[pool->alloc.count++]=nmem;/* Track how many pages are held 'in-flight' */pool->pages_state_hold_cnt++;trace_page_pool_state_hold(pool,nmem,
@@ -878,15 +878,15 @@ EXPORT_SYMBOL(page_pool_destroy);/* Caller must provide appropriate safe context, e.g. NAPI. */voidpage_pool_update_nid(structpage_pool*pool,intnew_nid){-structpage*page;+structnetmem*nmem;trace_page_pool_update_nid(pool,new_nid);pool->p.nid=new_nid;/* Flush pool alloc cache, as refill will check NUMA node */while(pool->alloc.count){-page=pool->alloc.cache[--pool->alloc.count];-page_pool_return_page(pool,page);+nmem=pool->alloc.cache[--pool->alloc.count];+page_pool_return_netmem(pool,nmem);}}EXPORT_SYMBOL(page_pool_update_nid);
We dereference the 'pp' member of struct page, so we must use a netmem
here.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
net/core/xdp.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
@@ -375,17 +375,18 @@ EXPORT_SYMBOL_GPL(xdp_rxq_info_reg_mem_model);void__xdp_return(void*data,structxdp_mem_info*mem,boolnapi_direct,structxdp_buff*xdp){+structnetmem*nmem;structpage*page;switch(mem->type){caseMEM_TYPE_PAGE_POOL:-page=virt_to_head_page(data);+nmem=virt_to_netmem(data);if(napi_direct&&xdp_return_frame_no_direct())napi_direct=false;-/* No need to check ((page->pp_magic & ~0x3UL) == PP_SIGNATURE)+/* No need to check ((nmem->pp_magic & ~0x3UL) == PP_SIGNATURE)*asmem->typeknowsthisapage_poolpage*/-page_pool_put_full_page(page->pp,page,napi_direct);+page_pool_put_full_netmem(nmem->pp,nmem,napi_direct);break;caseMEM_TYPE_PAGE_SHARED:page_frag_free(data);
Remove page_pool_defrag_page() and page_pool_return_page() as they have
no more callers.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
include/net/page_pool.h | 17 ++++++---------
net/core/page_pool.c | 47 ++++++++++++++++++-----------------------
2 files changed, 26 insertions(+), 38 deletions(-)
@@ -343,7 +343,7 @@ static inline struct page *page_pool_dev_alloc_frag(struct page_pool *pool,{gfp_tgfp=(GFP_ATOMIC|__GFP_NOWARN);-returnpage_pool_alloc_frag(pool,offset,size,gfp);+returnnetmem_page(page_pool_alloc_frag(pool,offset,size,gfp));}/* get the stored dma direction. A driver might decide to treat this locally and
@@ -665,10 +659,9 @@ void page_pool_put_page_bulk(struct page_pool *pool, void **data,}EXPORT_SYMBOL(page_pool_put_page_bulk);-staticstructpage*page_pool_drain_frag(structpage_pool*pool,-structpage*page)+staticstructnetmem*page_pool_drain_frag(structpage_pool*pool,+structnetmem*nmem){-structnetmem*nmem=page_netmem(page);longdrain_count=BIAS_MAX-pool->frag_users;/* Some user is still using the page frag */
@@ -2440,10 +2440,10 @@ static int mvneta_rx_swbm(struct napi_struct *napi,structmvneta_rx_desc*rx_desc=mvneta_rxq_next_desc_get(rxq);u32rx_status,index;structsk_buff*skb;-structpage*page;+structnetmem*nmem;index=rx_desc-rxq->descs;-page=(structpage*)rxq->buf_virt_addr[index];+nmem=rxq->buf_virt_addr[index];rx_status=rx_desc->status;rx_proc++;
@@ -2461,17 +2461,17 @@ static int mvneta_rx_swbm(struct napi_struct *napi,desc_status=rx_status;mvneta_swbm_rx_frame(pp,rx_desc,rxq,&xdp_buf,-&size,page);+&size,nmem);}else{if(unlikely(!xdp_buf.data_hard_start)){rx_desc->buf_phys_addr=0;-page_pool_put_full_page(rxq->page_pool,page,+page_pool_put_full_netmem(rxq->page_pool,nmem,true);gotonext;}mvneta_swbm_add_rx_fragment(pp,rx_desc,rxq,&xdp_buf,-&size,page);+&size,nmem);}/* Middle or Last descriptor */if(!(rx_status&MVNETA_RXD_LAST_DESC))
@@ -676,6 +676,7 @@ EXPORT_SYMBOL(page_pool_put_page_bulk);staticstructpage*page_pool_drain_frag(structpage_pool*pool,structpage*page){+structnetmem*nmem=page_netmem(page);longdrain_count=BIAS_MAX-pool->frag_users;/* Some user is still using the page frag */
Add wrappers for page_pool_alloc_pages() and
page_pool_dev_alloc_netmem(). Also convert __page_pool_alloc_pages_slow()
to __page_pool_alloc_netmem_slow() and __page_pool_alloc_page_order()
to __page_pool_alloc_netmem(). __page_pool_get_cached() now returns
a netmem.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
include/net/page_pool.h | 13 ++++++++++++-
net/core/page_pool.c | 39 +++++++++++++++++++--------------------
2 files changed, 31 insertions(+), 21 deletions(-)
@@ -371,27 +371,27 @@ static struct page *__page_pool_alloc_page_order(struct page_pool *pool,/* Track how many pages are held 'in-flight' */pool->pages_state_hold_cnt++;trace_page_pool_state_hold(pool,nmem,pool->pages_state_hold_cnt);-returnnetmem_page(nmem);+returnnmem;}/* slow path */noinline-staticstructpage*__page_pool_alloc_pages_slow(structpage_pool*pool,+staticstructnetmem*__page_pool_alloc_netmem_slow(structpage_pool*pool,gfp_tgfp){constintbulk=PP_ALLOC_CACHE_REFILL;unsignedintpp_flags=pool->p.flags;unsignedintpp_order=pool->p.order;-structpage*page;+structnetmem*nmem;inti,nr_pages;/* Don't support bulk alloc for high-order pages */if(unlikely(pp_order))-return__page_pool_alloc_page_order(pool,gfp);+return__page_pool_alloc_netmem(pool,gfp);/* Unnecessary as alloc cache is empty, but guarantees zero count */if(unlikely(pool->alloc.count>0))-returnnetmem_page(pool->alloc.cache[--pool->alloc.count]);+returnpool->alloc.cache[--pool->alloc.count];/* Mark empty alloc.cache slots "empty" for alloc_pages_bulk_array */memset(&pool->alloc.cache,0,sizeof(void*)*bulk);
@@ -422,34 +422,33 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool,/* Return last page */if(likely(pool->alloc.count>0)){-page=netmem_page(pool->alloc.cache[--pool->alloc.count]);+nmem=pool->alloc.cache[--pool->alloc.count];alloc_stat_inc(pool,slow);}else{-page=NULL;+nmem=NULL;}/* When page just allocated it should have refcnt 1 (but may have*speculativereferences)*/-returnpage;+returnnmem;}/* For using page_pool replace: alloc_pages() API calls, but provide*synchronizationguaranteeforallocationside.*/-structpage*page_pool_alloc_pages(structpage_pool*pool,gfp_tgfp)+structnetmem*page_pool_alloc_netmem(structpage_pool*pool,gfp_tgfp){-structpage*page;+structnetmem*nmem;/* Fast-path: Get a page from cache */-page=__page_pool_get_cached(pool);-if(page)-returnpage;+nmem=__page_pool_get_cached(pool);+if(nmem)+returnnmem;/* Slow-path: cache empty, do real allocation */-page=__page_pool_alloc_pages_slow(pool,gfp);-returnpage;+return__page_pool_alloc_netmem_slow(pool,gfp);}-EXPORT_SYMBOL(page_pool_alloc_pages);+EXPORT_SYMBOL(page_pool_alloc_netmem);/* Calculate distance between two u32 values, valid if distance is below 2^(31)*https://en.wikipedia.org/wiki/Serial_number_arithmetic#General_Solution
netmem_page() is defined this way to preserve constness. page_netmem()
doesn't call compound_head() because netmem users always use the head
page; it does include a debugging assert to check that it's true.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
include/net/page_pool.h | 59 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 59 insertions(+)
Use the netmem APIs instead of the page_pool APIs. Possibly we should
add a netmem equivalent of skb_add_rx_frag(), but that can happen
later. Saves one call to compound_head() in the call to put_page()
in mlx5e_page_release_dynamic() which saves 58 bytes of text.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
drivers/net/ethernet/mellanox/mlx5/core/en.h | 10 +-
.../net/ethernet/mellanox/mlx5/core/en/txrx.h | 4 +-
.../net/ethernet/mellanox/mlx5/core/en/xdp.c | 24 ++--
.../net/ethernet/mellanox/mlx5/core/en/xdp.h | 2 +-
.../net/ethernet/mellanox/mlx5/core/en_main.c | 12 +-
.../net/ethernet/mellanox/mlx5/core/en_rx.c | 130 +++++++++---------
6 files changed, 94 insertions(+), 88 deletions(-)
@@ -157,7 +158,7 @@ mlx5e_xmit_xdp_buff(struct mlx5e_xdpsq *sq, struct mlx5e_rq *rq,}/* returns true if packet was consumed by xdp */-boolmlx5e_xdp_handle(structmlx5e_rq*rq,structpage*page,+boolmlx5e_xdp_handle(structmlx5e_rq*rq,structnetmem*nmem,structbpf_prog*prog,structxdp_buff*xdp){u32act;
@@ -168,19 +169,19 @@ bool mlx5e_xdp_handle(struct mlx5e_rq *rq, struct page *page,caseXDP_PASS:returnfalse;caseXDP_TX:-if(unlikely(!mlx5e_xmit_xdp_buff(rq->xdpsq,rq,page,xdp)))+if(unlikely(!mlx5e_xmit_xdp_buff(rq->xdpsq,rq,nmem,xdp)))gotoxdp_abort;__set_bit(MLX5E_RQ_FLAG_XDP_XMIT,rq->flags);/* non-atomic */returntrue;caseXDP_REDIRECT:-/* When XDP enabled then page-refcnt==1 here */+/* When XDP enabled then nmem->refcnt==1 here */err=xdp_do_redirect(rq->netdev,xdp,prog);if(unlikely(err))gotoxdp_abort;__set_bit(MLX5E_RQ_FLAG_XDP_XMIT,rq->flags);__set_bit(MLX5E_RQ_FLAG_XDP_REDIRECT,rq->flags);if(xdp->rxq->mem.type!=MEM_TYPE_XSK_BUFF_POOL)-mlx5e_page_dma_unmap(rq,page);+mlx5e_nmem_dma_unmap(rq,nmem);rq->stats->xdp_redirect++;returntrue;default:
@@ -1610,7 +1612,7 @@ mlx5e_skb_from_cqe_linear(struct mlx5e_rq *rq, struct mlx5e_wqe_frag_info *wi,net_prefetchw(va);/* xdp_frame data area */mlx5e_fill_xdp_buff(rq,va,rx_headroom,cqe_bcnt,&xdp);-if(mlx5e_xdp_handle(rq,au->page,prog,&xdp))+if(mlx5e_xdp_handle(rq,au->nmem,prog,&xdp))returnNULL;/* page/packet was consumed by XDP */rx_headroom=xdp.data-xdp.data_hard_start;
@@ -1623,7 +1625,7 @@ mlx5e_skb_from_cqe_linear(struct mlx5e_rq *rq, struct mlx5e_wqe_frag_info *wi,returnNULL;/* queue up for recycling/reuse */-page_ref_inc(au->page);+netmem_get(au->nmem);returnskb;}
@@ -1645,10 +1647,10 @@ mlx5e_skb_from_cqe_nonlinear(struct mlx5e_rq *rq, struct mlx5e_wqe_frag_info *wiu32truesize;void*va;-va=page_address(au->page)+wi->offset;+va=netmem_address(au->nmem)+wi->offset;frag_consumed_bytes=min_t(u32,frag_info->frag_size,cqe_bcnt);-addr=page_pool_get_dma_addr(au->page);+addr=netmem_get_dma_addr(au->nmem);dma_sync_single_range_for_cpu(rq->pdev,addr,wi->offset,rq->buff.frame0_sz,rq->buff.map_dir);net_prefetchw(va);/* xdp_frame data area */
@@ -1967,8 +1969,8 @@ mlx5e_skb_from_cqe_mpwrq_nonlinear(struct mlx5e_rq *rq, struct mlx5e_mpw_info *wmlx5e_fill_skb_data(skb,rq,au,byte_cnt,frag_offset);/* copy header */-addr=page_pool_get_dma_addr(head_au->page);-mlx5e_copy_skb_header(rq,skb,head_au->page,addr,+addr=netmem_get_dma_addr(head_au->nmem);+mlx5e_copy_skb_header(rq,skb,head_au->nmem,addr,head_offset,head_offset,headlen);/* skb linear part was allocated with headlen and aligned to long */skb->tail+=headlen;
@@ -2059,7 +2061,7 @@ mlx5e_skb_from_cqe_shampo(struct mlx5e_rq *rq, struct mlx5e_mpw_info *wi,returnNULL;/* queue up for recycling/reuse */-page_ref_inc(head->page);+netmem_get(head->nmem);}else{/* allocate SKB and copy header for large header */
@@ -2072,7 +2074,7 @@ mlx5e_skb_from_cqe_shampo(struct mlx5e_rq *rq, struct mlx5e_mpw_info *wi,}prefetchw(skb->data);-mlx5e_copy_skb_header(rq,skb,head->page,head->addr,+mlx5e_copy_skb_header(rq,skb,head->nmem,head->addr,head_offset+rx_headroom,rx_headroom,head_size);/* skb linear part was allocated with headlen and aligned to long */
Also convert page_pool_clear_pp_info() and trace_page_pool_state_release()
to take a netmem. Include a wrapper for page_pool_release_page() to
avoid converting all callers.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
include/net/page_pool.h | 14 ++++++++++----
include/trace/events/page_pool.h | 14 +++++++-------
net/core/page_pool.c | 18 +++++++++---------
3 files changed, 26 insertions(+), 20 deletions(-)
@@ -478,23 +478,23 @@ void page_pool_release_page(struct page_pool *pool, struct page *page)*/gotoskip_dma_unmap;-dma=page_pool_get_dma_addr(page);+dma=netmem_get_dma_addr(nmem);/* When page is unmapped, it cannot be returned to our pool */dma_unmap_page_attrs(pool->p.dev,dma,PAGE_SIZE<<pool->p.order,pool->p.dma_dir,DMA_ATTR_SKIP_CPU_SYNC);-page_pool_set_dma_addr(page,0);+netmem_set_dma_addr(nmem,0);skip_dma_unmap:-page_pool_clear_pp_info(page);+page_pool_clear_pp_info(nmem);/* This may be the last page returned, releasing the pool, so*itisnotsafetoreferencepoolafterwards.*/count=atomic_inc_return_relaxed(&pool->pages_state_release_cnt);-trace_page_pool_state_release(pool,page,count);+trace_page_pool_state_release(pool,nmem,count);}-EXPORT_SYMBOL(page_pool_release_page);+EXPORT_SYMBOL(page_pool_release_netmem);/* Return a page to the page allocator, cleaning up our state */staticvoidpage_pool_return_page(structpage_pool*pool,structpage*page)
As part of simplifying struct page, create a new netmem type which
mirrors the page_pool members in struct page.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
Documentation/networking/page_pool.rst | 5 +++
include/net/page_pool.h | 46 ++++++++++++++++++++++++++
2 files changed, 51 insertions(+)
@@ -50,6 +50,52 @@PP_FLAG_DMA_SYNC_DEV|\PP_FLAG_PAGE_FRAG)+/**+*structnetmem-Amemoryallocationfroma&structpage_pool.+*@flags:Thesameasthepageflags.Donotusedirectly.+*@pp_magic:Magicvaluetoavoidrecyclingnonpage_poolallocatedpages.+*@pp:Thepagepoolthisnetmemwasallocatedfrom.+*@dma_addr:Callnetmem_get_dma_addr()toreadthisvalue.+*@dma_addr_upper:Mightneedtobe64-biton32-bitarchitectures.+*@pp_frag_count:Forfragpagesupport,notsupportedin32-bit+*architectureswith64-bitDMA.+*@_mapcount:Donotaccessthismemberdirectly.+*@_refcount:Donotaccessthismemberdirectly.Readitusing+*netmem_ref_count()andmanipulateitwithnetmem_get()andnetmem_put().+*+*Thisstructoverlaysstructpagefornow.Donotmodifywithouta+*goodunderstandingoftheissues.+*/+structnetmem{+unsignedlongflags;+unsignedlongpp_magic;+structpage_pool*pp;+/* private: no need to document this padding */+unsignedlong_pp_mapping_pad;/* aliases with folio->mapping */+/* public: */+unsignedlongdma_addr;+union{+unsignedlongdma_addr_upper;+atomic_long_tpp_frag_count;+};+atomic_t_mapcount;+atomic_t_refcount;+};++#define NETMEM_MATCH(pg, nm) \+static_assert(offsetof(structpage,pg)==offsetof(structnetmem,nm))+NETMEM_MATCH(flags,flags);+NETMEM_MATCH(lru,pp_magic);+NETMEM_MATCH(pp,pp);+NETMEM_MATCH(mapping,_pp_mapping_pad);+NETMEM_MATCH(dma_addr,dma_addr);+NETMEM_MATCH(dma_addr_upper,dma_addr_upper);+NETMEM_MATCH(pp_frag_count,pp_frag_count);+NETMEM_MATCH(_mapcount,_mapcount);+NETMEM_MATCH(_refcount,_refcount);+#undef NETMEM_MATCH+static_assert(sizeof(structnetmem)<=sizeof(structpage));+/**Fastallocationsidecachearray/stack*
Convert __page_pool_alloc_page_order() and __page_pool_alloc_pages_slow()
to use netmem internally. This removes a couple of calls
to compound_head() that are hidden inside put_page().
Convert trace_page_pool_state_hold(), page_pool_dma_map() and
page_pool_set_pp_info() to take a netmem argument.
Saves 83 bytes of text in __page_pool_alloc_page_order() and 98 in
__page_pool_alloc_pages_slow() for a total of 181 bytes.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
include/trace/events/page_pool.h | 14 +++++------
net/core/page_pool.c | 42 +++++++++++++++++---------------
2 files changed, 29 insertions(+), 27 deletions(-)
@@ -345,26 +346,26 @@ static void page_pool_clear_pp_info(struct netmem *nmem)staticstructpage*__page_pool_alloc_page_order(structpage_pool*pool,gfp_tgfp){-structpage*page;+structnetmem*nmem;gfp|=__GFP_COMP;-page=alloc_pages_node(pool->p.nid,gfp,pool->p.order);-if(unlikely(!page))+nmem=page_netmem(alloc_pages_node(pool->p.nid,gfp,pool->p.order));+if(unlikely(!nmem))returnNULL;if((pool->p.flags&PP_FLAG_DMA_MAP)&&-unlikely(!page_pool_dma_map(pool,page))){-put_page(page);+unlikely(!page_pool_dma_map(pool,nmem))){+netmem_put(nmem);returnNULL;}alloc_stat_inc(pool,slow_high_order);-page_pool_set_pp_info(pool,page);+page_pool_set_pp_info(pool,nmem);/* Track how many pages are held 'in-flight' */pool->pages_state_hold_cnt++;-trace_page_pool_state_hold(pool,page,pool->pages_state_hold_cnt);-returnpage;+trace_page_pool_state_hold(pool,nmem,pool->pages_state_hold_cnt);+returnnetmem_page(nmem);}/* slow path */
@@ -398,18 +399,18 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool,*pageelementhavenotbeen(possibly)DMAmapped.*/for(i=0;i<nr_pages;i++){-page=pool->alloc.cache[i];+structnetmem*nmem=page_netmem(pool->alloc.cache[i]);if((pp_flags&PP_FLAG_DMA_MAP)&&-unlikely(!page_pool_dma_map(pool,page))){-put_page(page);+unlikely(!page_pool_dma_map(pool,nmem))){+netmem_put(nmem);continue;}-page_pool_set_pp_info(pool,page);-pool->alloc.cache[pool->alloc.count++]=page;+page_pool_set_pp_info(pool,nmem);+pool->alloc.cache[pool->alloc.count++]=netmem_page(nmem);/* Track how many pages are held 'in-flight' */pool->pages_state_hold_cnt++;-trace_page_pool_state_hold(pool,page,+trace_page_pool_state_hold(pool,nmem,pool->pages_state_hold_cnt);}
@@ -421,7 +422,8 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool,page=NULL;}-/* When page just alloc'ed is should/must have refcnt 1. */+/* When page just allocated it should have refcnt 1 (but may have+*speculativereferences)*/returnpage;}
@@ -420,15 +420,15 @@ static inline long page_pool_defrag_page(struct page *page, long nr)}staticinlineboolpage_pool_is_last_frag(structpage_pool*pool,-structpage*page)+structnetmem*nmem){/* If fragments aren't enabled or count is 0 we were the last user */return!(pool->p.flags&PP_FLAG_PAGE_FRAG)||-(page_pool_defrag_page(page,1)==0);+(page_pool_defrag_netmem(nmem,1)==0);}-staticinlinevoidpage_pool_put_page(structpage_pool*pool,-structpage*page,+staticinlinevoidpage_pool_put_netmem(structpage_pool*pool,+structnetmem*nmem,unsignedintdma_sync_size,boolallow_direct){
@@ -436,13 +436,22 @@ static inline void page_pool_put_page(struct page_pool *pool,*allowregisteringMEM_TYPE_PAGE_POOL,butshieldlinker.*/#ifdef CONFIG_PAGE_POOL-if(!page_pool_is_last_frag(pool,page))+if(!page_pool_is_last_frag(pool,nmem))return;-page_pool_put_defragged_page(pool,page,dma_sync_size,allow_direct);+page_pool_put_defragged_netmem(pool,nmem,dma_sync_size,allow_direct);#endif}+staticinlinevoidpage_pool_put_page(structpage_pool*pool,+structpage*page,+unsignedintdma_sync_size,+boolallow_direct)+{+page_pool_put_netmem(pool,page_netmem(page),dma_sync_size,+allow_direct);+}+/* Same as above but will try to sync the entire area pool->max_len */staticinlinevoidpage_pool_put_full_page(structpage_pool*pool,structpage*page,boolallow_direct)
@@ -516,14 +516,15 @@ static void page_pool_return_netmem(struct page_pool *pool, struct netmem *nmem)*/}-staticboolpage_pool_recycle_in_ring(structpage_pool*pool,structpage*page)+staticboolpage_pool_recycle_in_ring(structpage_pool*pool,+structnetmem*nmem){intret;/* BH protection not needed if current is serving softirq */if(in_serving_softirq())-ret=ptr_ring_produce(&pool->ring,page);+ret=ptr_ring_produce(&pool->ring,nmem);else-ret=ptr_ring_produce_bh(&pool->ring,page);+ret=ptr_ring_produce_bh(&pool->ring,nmem);if(!ret){recycle_stat_inc(pool,ring);
@@ -615,17 +616,17 @@ __page_pool_put_page(struct page_pool *pool, struct page *page,dma_sync_size,allow_direct));}-voidpage_pool_put_defragged_page(structpage_pool*pool,structpage*page,+voidpage_pool_put_defragged_netmem(structpage_pool*pool,structnetmem*nmem,unsignedintdma_sync_size,boolallow_direct){-page=__page_pool_put_page(pool,page,dma_sync_size,allow_direct);-if(page&&!page_pool_recycle_in_ring(pool,page)){+nmem=__page_pool_put_netmem(pool,nmem,dma_sync_size,allow_direct);+if(nmem&&!page_pool_recycle_in_ring(pool,nmem)){/* Cache full, fallback to free pages */recycle_stat_inc(pool,ring_full);-page_pool_return_page(pool,page);+page_pool_return_netmem(pool,nmem);}}-EXPORT_SYMBOL(page_pool_put_defragged_page);+EXPORT_SYMBOL(page_pool_put_defragged_netmem);/* Caller must not use data area after call, as this function overwrites it */voidpage_pool_put_page_bulk(structpage_pool*pool,void**data,
@@ -634,16 +635,16 @@ void page_pool_put_page_bulk(struct page_pool *pool, void **data,inti,bulk_len=0;for(i=0;i<count;i++){-structpage*page=virt_to_head_page(data[i]);+structnetmem*nmem=virt_to_netmem(data[i]);/* It is not the last user for the page frag case */-if(!page_pool_is_last_frag(pool,page))+if(!page_pool_is_last_frag(pool,nmem))continue;-page=__page_pool_put_page(pool,page,-1,false);+nmem=__page_pool_put_netmem(pool,nmem,-1,false);/* Approved for bulk recycling in ptr_ring cache */-if(page)-data[bulk_len++]=page;+if(nmem)+data[bulk_len++]=nmem;}if(unlikely(!bulk_len))
Removes the call to compound_head() hidden in put_page() which
saves 169 bytes of kernel text as __page_pool_put_page() is
inlined twice.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
net/core/page_pool.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
@@ -558,8 +558,8 @@ static bool page_pool_recycle_in_cache(struct page *page,*Ifthepagerefcnt!=1,thenthepagewillbereturnedtomemory*subsystem.*/-static__always_inlinestructpage*-__page_pool_put_page(structpage_pool*pool,structpage*page,+static__always_inlinestructnetmem*+__page_pool_put_netmem(structpage_pool*pool,structnetmem*nmem,unsignedintdma_sync_size,boolallow_direct){/* This allocator is optimized for the XDP mode that uses
@@ -571,19 +571,20 @@ __page_pool_put_page(struct page_pool *pool, struct page *page,*pageisNOTreusablewhenallocatedwhensystemisunder*somepressure.(page_is_pfmemalloc)*/-if(likely(page_ref_count(page)==1&&!page_is_pfmemalloc(page))){-/* Read barrier done in page_ref_count / READ_ONCE */+if(likely(netmem_ref_count(nmem)==1&&+!netmem_is_pfmemalloc(nmem))){+/* Read barrier done in netmem_ref_count / READ_ONCE */if(pool->p.flags&PP_FLAG_DMA_SYNC_DEV)-page_pool_dma_sync_for_device(pool,page,+page_pool_dma_sync_for_device(pool,netmem_page(nmem),dma_sync_size);if(allow_direct&&in_serving_softirq()&&-page_pool_recycle_in_cache(page,pool))+page_pool_recycle_in_cache(netmem_page(nmem),pool))returnNULL;/* Page found as candidate for recycling */-returnpage;+returnnmem;}/* Fallback/non-XDP mode: API user have elevated refcnt.*
@@ -599,13 +600,21 @@ __page_pool_put_page(struct page_pool *pool, struct page *page,*willbeinvokingput_page.*/recycle_stat_inc(pool,released_refcnt);-/* Do not replace this with page_pool_return_page() */-page_pool_release_page(pool,page);-put_page(page);+/* Do not replace this with page_pool_return_netmem() */+page_pool_release_netmem(pool,nmem);+netmem_put(nmem);returnNULL;}+static__always_inlinestructpage*+__page_pool_put_page(structpage_pool*pool,structpage*page,+unsignedintdma_sync_size,boolallow_direct)+{+returnnetmem_page(__page_pool_put_netmem(pool,page_netmem(page),+dma_sync_size,allow_direct));+}+voidpage_pool_put_defragged_page(structpage_pool*pool,structpage*page,unsignedintdma_sync_size,boolallow_direct){
Removes a call to compound_head(), saving 464 bytes of kernel text
as page_pool_return_page() is inlined seven times.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
net/core/page_pool.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
@@ -499,11 +505,11 @@ void page_pool_release_netmem(struct page_pool *pool, struct netmem *nmem)EXPORT_SYMBOL(page_pool_release_netmem);/* Return a page to the page allocator, cleaning up our state */-staticvoidpage_pool_return_page(structpage_pool*pool,structpage*page)+staticvoidpage_pool_return_netmem(structpage_pool*pool,structnetmem*nmem){-page_pool_release_page(pool,page);+page_pool_release_netmem(pool,nmem);-put_page(page);+netmem_put(nmem);/* An optimization would be to call __free_pages(page, pool->p.order)*knowingpageisnotpartofpage-cache(thusavoidinga*__page_cache_release()call).
On 1/5/2023 1:46 PM, Matthew Wilcox (Oracle) wrote:
The MM subsystem is trying to reduce struct page to a single pointer.
The first step towards that is splitting struct page by its individual
users, as has already been done with folio and slab. This patchset does
that for netmem which is used for page pools.
There are some relatively significant reductions in kernel text size
from these changes. They don't appear to affect performance at all,
but it's nice to save a bit of memory.
v2:
- Rebase to next-20230105
- Add kernel-doc for struct netmem
- Add mlx5 compilation fixes from Jesper
- Folded in minor nit from Alex Duyck
I had a brief look over the whole series and didn't see anything bad
that stood out to me.
The series is remarkably small, with many small and easy to review
patches, so thanks for taking the time to do those.
Reviewed-by: Jesse Brandeburg <redacted>
From: kernel test robot <hidden> Date: 2023-01-06 02:24:49
Hi Matthew,
I love your patch! Perhaps something to improve:
[auto build test WARNING on bpf-next/master]
[also build test WARNING on bpf/master net/master net-next/master linus/master v6.2-rc2 next-20230105]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Matthew-Wilcox-Oracle/netmem-Create-new-type/20230106-054852
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/r/20230105214631.3939268-3-willy%40infradead.org
patch subject: [PATCH v2 02/24] netmem: Add utility functions
config: arm-randconfig-r014-20230105
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 8d9828ef5aa9688500657d36cd2aefbe12bbd162)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
# https://github.com/intel-lab-lkp/linux/commit/167a5c35da10f97c077af525e9f537fab4438b94
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Matthew-Wilcox-Oracle/netmem-Create-new-type/20230106-054852
git checkout 167a5c35da10f97c077af525e9f537fab4438b94
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash kernel/ net/core/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot [off-list ref]
All warnings (new ones prefixed by >>):
kernel/fork.c:162:13: warning: no previous prototype for function 'arch_release_task_struct' [-Wmissing-prototypes]
void __weak arch_release_task_struct(struct task_struct *tsk)
^
kernel/fork.c:162:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void __weak arch_release_task_struct(struct task_struct *tsk)
^
static
kernel/fork.c:862:20: warning: no previous prototype for function 'arch_task_cache_init' [-Wmissing-prototypes]
void __init __weak arch_task_cache_init(void) { }
^
kernel/fork.c:862:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void __init __weak arch_task_cache_init(void) { }
^
static
kernel/fork.c:957:12: warning: no previous prototype for function 'arch_dup_task_struct' [-Wmissing-prototypes]
int __weak arch_dup_task_struct(struct task_struct *dst,
^
kernel/fork.c:957:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int __weak arch_dup_task_struct(struct task_struct *dst,
^
static
In file included from kernel/fork.c:1087:
In file included from include/linux/init_task.h:18:
In file included from include/net/net_namespace.h:43:
In file included from include/linux/skbuff.h:41:
quoted
include/net/page_pool.h:111:21: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_pfn(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:116:21: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_nid(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:126:22: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_virt(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:131:22: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_address(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:136:24: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_ref_count(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
8 warnings generated.
--
In file included from kernel/exit.c:55:
In file included from include/linux/init_task.h:18:
In file included from include/net/net_namespace.h:43:
In file included from include/linux/skbuff.h:41:
quoted
include/net/page_pool.h:111:21: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_pfn(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:116:21: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_nid(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:126:22: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_virt(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:131:22: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_address(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:136:24: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_ref_count(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
5 warnings generated.
--
In file included from kernel/kallsyms.c:25:
In file included from include/linux/filter.h:12:
In file included from include/linux/skbuff.h:41:
quoted
include/net/page_pool.h:111:21: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_pfn(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:116:21: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_nid(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:126:22: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_virt(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:131:22: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_address(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:136:24: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_ref_count(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
kernel/kallsyms.c:663:12: warning: no previous prototype for function 'arch_get_kallsym' [-Wmissing-prototypes]
int __weak arch_get_kallsym(unsigned int symnum, unsigned long *value,
^
kernel/kallsyms.c:663:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int __weak arch_get_kallsym(unsigned int symnum, unsigned long *value,
^
static
6 warnings generated.
--
In file included from net/core/flow_dissector.c:3:
In file included from include/linux/skbuff.h:41:
quoted
include/net/page_pool.h:111:21: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_pfn(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:116:21: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_nid(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:126:22: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_virt(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:131:22: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_address(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:136:24: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_ref_count(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
In file included from net/core/flow_dissector.c:21:
In file included from include/linux/if_pppox.h:17:
include/uapi/linux/if_pppox.h:71:4: warning: field sa_addr within 'struct sockaddr_pppox' is less aligned than 'union (unnamed union at include/uapi/linux/if_pppox.h:68:2)' and is usually due to 'struct sockaddr_pppox' being packed, which can lead to unaligned accesses [-Wunaligned-access]
} sa_addr;
^
6 warnings generated.
--
In file included from net/core/dev.c:89:
In file included from include/linux/if_ether.h:19:
In file included from include/linux/skbuff.h:41:
quoted
include/net/page_pool.h:111:21: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_pfn(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:116:21: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_nid(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:126:22: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_virt(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:131:22: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_address(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:136:24: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_ref_count(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
net/core/dev.c:5098:1: warning: unused function 'sch_handle_ingress' [-Wunused-function]
sch_handle_ingress(struct sk_buff *skb, struct packet_type **pt_prev, int *ret,
^
net/core/dev.c:5251:19: warning: unused function 'nf_ingress' [-Wunused-function]
static inline int nf_ingress(struct sk_buff *skb, struct packet_type **pt_prev,
^
7 warnings generated.
--
In file included from kernel/sched/core.c:44:
In file included from include/linux/init_task.h:18:
In file included from include/net/net_namespace.h:43:
In file included from include/linux/skbuff.h:41:
quoted
include/net/page_pool.h:111:21: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_pfn(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:116:21: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_nid(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:126:22: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_virt(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:131:22: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_address(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:136:24: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_ref_count(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
kernel/sched/core.c:6664:35: warning: no previous prototype for function 'schedule_user' [-Wmissing-prototypes]
asmlinkage __visible void __sched schedule_user(void)
^
kernel/sched/core.c:6664:22: note: declare 'static' if the function is not intended to be used outside of this translation unit
asmlinkage __visible void __sched schedule_user(void)
^
static
kernel/sched/core.c:3579:20: warning: unused function 'rq_has_pinned_tasks' [-Wunused-function]
static inline bool rq_has_pinned_tasks(struct rq *rq)
^
kernel/sched/core.c:5673:20: warning: unused function 'sched_tick_start' [-Wunused-function]
static inline void sched_tick_start(int cpu) { }
^
kernel/sched/core.c:5674:20: warning: unused function 'sched_tick_stop' [-Wunused-function]
static inline void sched_tick_stop(int cpu) { }
^
kernel/sched/core.c:6367:20: warning: unused function 'sched_core_cpu_starting' [-Wunused-function]
static inline void sched_core_cpu_starting(unsigned int cpu) {}
^
kernel/sched/core.c:6368:20: warning: unused function 'sched_core_cpu_deactivate' [-Wunused-function]
static inline void sched_core_cpu_deactivate(unsigned int cpu) {}
^
kernel/sched/core.c:6369:20: warning: unused function 'sched_core_cpu_dying' [-Wunused-function]
static inline void sched_core_cpu_dying(unsigned int cpu) {}
^
12 warnings generated.
--
In file included from kernel/bpf/core.c:21:
In file included from include/linux/filter.h:12:
In file included from include/linux/skbuff.h:41:
quoted
include/net/page_pool.h:111:21: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_pfn(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:116:21: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_nid(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:126:22: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_virt(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:131:22: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_address(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:136:24: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_ref_count(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
kernel/bpf/core.c:1630:12: warning: no previous prototype for function 'bpf_probe_read_kernel' [-Wmissing-prototypes]
u64 __weak bpf_probe_read_kernel(void *dst, u32 size, const void *unsafe_ptr)
^
kernel/bpf/core.c:1630:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
u64 __weak bpf_probe_read_kernel(void *dst, u32 size, const void *unsafe_ptr)
^
static
kernel/bpf/core.c:2069:6: warning: no previous prototype for function 'bpf_patch_call_args' [-Wmissing-prototypes]
void bpf_patch_call_args(struct bpf_insn *insn, u32 stack_depth)
^
kernel/bpf/core.c:2069:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void bpf_patch_call_args(struct bpf_insn *insn, u32 stack_depth)
^
static
7 warnings generated.
vim +111 include/net/page_pool.h
108
109 static inline unsigned long netmem_pfn(const struct netmem *nmem)
110 {
> 111 return page_to_pfn(netmem_page(nmem));
112 }
113
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
From: kernel test robot <hidden> Date: 2023-01-06 02:36:30
Hi Matthew,
I love your patch! Perhaps something to improve:
[auto build test WARNING on bpf-next/master]
[also build test WARNING on bpf/master net/master net-next/master linus/master v6.2-rc2 next-20230105]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Matthew-Wilcox-Oracle/netmem-Create-new-type/20230106-054852
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/r/20230105214631.3939268-6-willy%40infradead.org
patch subject: [PATCH v2 05/24] page_pool: Start using netmem in allocation path.
config: arm64-randconfig-r033-20230105
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 8d9828ef5aa9688500657d36cd2aefbe12bbd162)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/37522b9f56a9bac5d16428140c925698c26b07d1
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Matthew-Wilcox-Oracle/netmem-Create-new-type/20230106-054852
git checkout 37522b9f56a9bac5d16428140c925698c26b07d1
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash net/core/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot [off-list ref]
All warnings (new ones prefixed by >>):
In file included from net/core/page_pool.c:13:
include/net/page_pool.h:111:21: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_pfn(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:116:21: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_nid(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:126:22: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_to_virt(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:126:22: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:131:22: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_address(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
include/net/page_pool.h:136:24: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return page_ref_count(netmem_page(nmem));
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
quoted
net/core/page_pool.c:309:22: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
struct page *page = netmem_page(nmem);
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
net/core/page_pool.c:337:25: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
pool->p.init_callback(netmem_page(nmem), pool->p.init_arg);
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
net/core/page_pool.c:368:9: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
return netmem_page(nmem);
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
net/core/page_pool.c:410:44: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
pool->alloc.cache[pool->alloc.count++] = netmem_page(nmem);
^
include/net/page_pool.h:100:8: note: expanded from macro 'netmem_page'
const struct netmem: (const struct page *)nmem, \
^
10 warnings generated.
vim +309 net/core/page_pool.c
306
307 static bool page_pool_dma_map(struct page_pool *pool, struct netmem *nmem)
308 {
> 309 struct page *page = netmem_page(nmem);
310 dma_addr_t dma;
311
312 /* Setup DMA mapping: use 'struct page' area for storing DMA-addr
313 * since dma_addr_t can be either 32 or 64 bits and does not always fit
314 * into page private data (i.e 32bit cpu with 64bit DMA caps)
315 * This mapping is kept for lifetime of page, until leaving pool.
316 */
317 dma = dma_map_page_attrs(pool->p.dev, page, 0,
318 (PAGE_SIZE << pool->p.order),
319 pool->p.dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
320 if (dma_mapping_error(pool->p.dev, dma))
321 return false;
322
323 page_pool_set_dma_addr(page, dma);
324
325 if (pool->p.flags & PP_FLAG_DMA_SYNC_DEV)
326 page_pool_dma_sync_for_device(pool, page, pool->p.max_len);
327
328 return true;
329 }
330
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
As part of simplifying struct page, create a new netmem type which
mirrors the page_pool members in struct page.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
Documentation/networking/page_pool.rst | 5 +++
include/net/page_pool.h | 46 ++++++++++++++++++++++++++
2 files changed, 51 insertions(+)
On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
netmem_page() is defined this way to preserve constness. page_netmem()
doesn't call compound_head() because netmem users always use the head
page; it does include a debugging assert to check that it's true.
Signed-off-by: Matthew Wilcox (Oracle)<willy@infradead.org>
---
include/net/page_pool.h | 59 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 59 insertions(+)
On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
Also convert page_pool_clear_pp_info() and trace_page_pool_state_release()
to take a netmem. Include a wrapper for page_pool_release_page() to
avoid converting all callers.
Signed-off-by: Matthew Wilcox (Oracle)<willy@infradead.org>
---
include/net/page_pool.h | 14 ++++++++++----
include/trace/events/page_pool.h | 14 +++++++-------
net/core/page_pool.c | 18 +++++++++---------
3 files changed, 26 insertions(+), 20 deletions(-)
On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
Convert __page_pool_alloc_page_order() and __page_pool_alloc_pages_slow()
to use netmem internally. This removes a couple of calls
to compound_head() that are hidden inside put_page().
Convert trace_page_pool_state_hold(), page_pool_dma_map() and
page_pool_set_pp_info() to take a netmem argument.
Saves 83 bytes of text in __page_pool_alloc_page_order() and 98 in
__page_pool_alloc_pages_slow() for a total of 181 bytes.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
include/trace/events/page_pool.h | 14 +++++------
net/core/page_pool.c | 42 +++++++++++++++++---------------
2 files changed, 29 insertions(+), 27 deletions(-)
@@ -421,7 +422,8 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool, page = NULL; }- /* When page just alloc'ed is should/must have refcnt 1. */+ /* When page just allocated it should have refcnt 1 (but may have+ * speculative references) */ return page;
What does it mean page may have speculative references ?
And do I/we need to worry about that for page_pool?
--Jesper
On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
Removes the call to compound_head() hidden in put_page() which
saves 169 bytes of kernel text as __page_pool_put_page() is
inlined twice.
Signed-off-by: Matthew Wilcox (Oracle)<willy@infradead.org>
---
net/core/page_pool.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
Change the type here from page to netmem. It works out well to
convert page_pool_refill_alloc_cache() to return a netmem instead
of a page as part of this commit.
Signed-off-by: Matthew Wilcox (Oracle)<willy@infradead.org>
---
include/net/page_pool.h | 2 +-
net/core/page_pool.c | 52 ++++++++++++++++++++---------------------
2 files changed, 27 insertions(+), 27 deletions(-)
On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
quoted hunk
Retrieve a netmem from the ptr_ring instead of a page.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
net/core/page_pool.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
Add wrappers for page_pool_alloc_pages() and
page_pool_dev_alloc_netmem(). Also convert __page_pool_alloc_pages_slow()
to __page_pool_alloc_netmem_slow() and __page_pool_alloc_page_order()
to __page_pool_alloc_netmem(). __page_pool_get_cached() now returns
a netmem.
Signed-off-by: Matthew Wilcox (Oracle)<willy@infradead.org>
---
include/net/page_pool.h | 13 ++++++++++++-
net/core/page_pool.c | 39 +++++++++++++++++++--------------------
2 files changed, 31 insertions(+), 21 deletions(-)
On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
This wrapper is no longer used.
Signed-off-by: Matthew Wilcox (Oracle)<willy@infradead.org>
---
net/core/page_pool.c | 8 --------
1 file changed, 8 deletions(-)
On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
We're not quite ready to change the API of page_pool_drain_frag(),
but we can remove the use of several wrappers by using the netmem
throughout.
Signed-off-by: Matthew Wilcox (Oracle)<willy@infradead.org>
---
net/core/page_pool.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
From: Matthew Wilcox <willy@infradead.org> Date: 2023-01-06 15:36:04
On Fri, Jan 06, 2023 at 02:59:30PM +0100, Jesper Dangaard Brouer wrote:
On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
quoted
Convert __page_pool_alloc_page_order() and __page_pool_alloc_pages_slow()
to use netmem internally. This removes a couple of calls
to compound_head() that are hidden inside put_page().
Convert trace_page_pool_state_hold(), page_pool_dma_map() and
page_pool_set_pp_info() to take a netmem argument.
Saves 83 bytes of text in __page_pool_alloc_page_order() and 98 in
__page_pool_alloc_pages_slow() for a total of 181 bytes.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
include/trace/events/page_pool.h | 14 +++++------
net/core/page_pool.c | 42 +++++++++++++++++---------------
2 files changed, 29 insertions(+), 27 deletions(-)
@@ -421,7 +422,8 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool, page = NULL; }- /* When page just alloc'ed is should/must have refcnt 1. */+ /* When page just allocated it should have refcnt 1 (but may have+ * speculative references) */ return page;
What does it mean page may have speculative references ?
And do I/we need to worry about that for page_pool?
An excellent question. There are two code paths (known to me) which
take speculative references on a page, and there may well be more. One
is in GUP and the other is in the page cache. Both take the form of:
rcu_read_lock();
again:
look-up-page
try-get-page-ref
check-lookup
if lookup-failed drop-page-ref; goto again;
rcu_read_unlock();
If a page _has been_ in the page tables, then GUP can find it. If a
page _has been_ in the page cache, then filemap can find it. Because
there's no RCU grace period between freeing and reallocating a page, it
actually means that any page can see its refcount temporarily raised.
Usually the biggest problem is consumers assuming that they will be the
last code to call put_page() / folio_put(), and can do their cleanup
at that time (when the last caller of folio_put() may be GUP or filemap
which knows nothing of what you're using the page for).
I didn't notice any problems with temporarily elevated refcounts while
doing the netmem conversion, and it's something I'm fairly sensitive to,
so I think you got it all right and there is no need to be concerned.
Hope that's helpful!
On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
quoted hunk
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@infradead.org>
---
include/net/page_pool.h | 14 +++++++++++++-
net/core/page_pool.c | 13 ++++++-------
2 files changed, 19 insertions(+), 8 deletions(-)
@@ -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 */+staticinlinevoidpage_pool_put_full_netmem(structpage_pool*pool,+structnetmem*nmem,boolallow_direct)+{+page_pool_put_netmem(pool,nmem,-1,allow_direct);+}+staticinlinevoidpage_pool_put_full_page(structpage_pool*pool,structpage*page,boolallow_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 */
^^^^
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.
--Jesper
(cut rest of patch which looked fine)
On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
Remove page_pool_defrag_page() and page_pool_return_page() as they have
no more callers.
Signed-off-by: Matthew Wilcox (Oracle)<willy@infradead.org>
---
include/net/page_pool.h | 17 ++++++---------
net/core/page_pool.c | 47 ++++++++++++++++++-----------------------
2 files changed, 26 insertions(+), 38 deletions(-)
On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
We dereference the 'pp' member of struct page, so we must use a netmem
here.
Signed-off-by: Matthew Wilcox (Oracle)<willy@infradead.org>
---
net/core/xdp.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
These are now split out into their own netmem struct.
Signed-off-by: Matthew Wilcox (Oracle)<willy@infradead.org>
---
include/linux/mm_types.h | 22 ----------------------
include/net/page_pool.h | 4 ----
2 files changed, 26 deletions(-)
To Saeed and Tariq, please review.
This reminds me, that IMHO we/nvidia/mellanox should remove the local
mlx5e_page_cache functionality, as SKBs can now recycle page_pool pages.
This should simplify the driver and we get rid of the head-of-line
blocking issue with the local page cache (refcnt elevation tricks).
It might look good in microbencmarks, but my experience from prod
systems are that this local cache isn't utilized. And I believe we
should be able to get good/similar microbenchmark with page_pool, which
will continue to recycle and have no HoL issues for prod use-cases.
On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
quoted hunk
Use the netmem APIs instead of the page_pool APIs. Possibly we should
add a netmem equivalent of skb_add_rx_frag(), but that can happen
later. Saves one call to compound_head() in the call to put_page()
in mlx5e_page_release_dynamic() which saves 58 bytes of text.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
drivers/net/ethernet/mellanox/mlx5/core/en.h | 10 +-
.../net/ethernet/mellanox/mlx5/core/en/txrx.h | 4 +-
.../net/ethernet/mellanox/mlx5/core/en/xdp.c | 24 ++--
.../net/ethernet/mellanox/mlx5/core/en/xdp.h | 2 +-
.../net/ethernet/mellanox/mlx5/core/en_main.c | 12 +-
.../net/ethernet/mellanox/mlx5/core/en_rx.c | 130 +++++++++---------
6 files changed, 94 insertions(+), 88 deletions(-)
@@ -157,7 +158,7 @@ mlx5e_xmit_xdp_buff(struct mlx5e_xdpsq *sq, struct mlx5e_rq *rq,}/* returns true if packet was consumed by xdp */-boolmlx5e_xdp_handle(structmlx5e_rq*rq,structpage*page,+boolmlx5e_xdp_handle(structmlx5e_rq*rq,structnetmem*nmem,structbpf_prog*prog,structxdp_buff*xdp){u32act;
@@ -168,19 +169,19 @@ bool mlx5e_xdp_handle(struct mlx5e_rq *rq, struct page *page,caseXDP_PASS:returnfalse;caseXDP_TX:-if(unlikely(!mlx5e_xmit_xdp_buff(rq->xdpsq,rq,page,xdp)))+if(unlikely(!mlx5e_xmit_xdp_buff(rq->xdpsq,rq,nmem,xdp)))gotoxdp_abort;__set_bit(MLX5E_RQ_FLAG_XDP_XMIT,rq->flags);/* non-atomic */returntrue;caseXDP_REDIRECT:-/* When XDP enabled then page-refcnt==1 here */+/* When XDP enabled then nmem->refcnt==1 here */err=xdp_do_redirect(rq->netdev,xdp,prog);if(unlikely(err))gotoxdp_abort;__set_bit(MLX5E_RQ_FLAG_XDP_XMIT,rq->flags);__set_bit(MLX5E_RQ_FLAG_XDP_REDIRECT,rq->flags);if(xdp->rxq->mem.type!=MEM_TYPE_XSK_BUFF_POOL)-mlx5e_page_dma_unmap(rq,page);+mlx5e_nmem_dma_unmap(rq,nmem);rq->stats->xdp_redirect++;returntrue;default:
@@ -328,43 +328,45 @@ static inline int mlx5e_page_alloc_pool(struct mlx5e_rq *rq, union mlx5e_alloc_uif(mlx5e_rx_cache_get(rq,au))return0;-au->page=page_pool_dev_alloc_pages(rq->page_pool);-if(unlikely(!au->page))+au->nmem=page_pool_dev_alloc_netmem(rq->page_pool);+if(unlikely(!au->nmem))return-ENOMEM;/* Non-XSK always uses PAGE_SIZE. */-addr=dma_map_page(rq->pdev,au->page,0,PAGE_SIZE,rq->buff.map_dir);+addr=dma_map_page(rq->pdev,netmem_page(au->nmem),0,PAGE_SIZE,+rq->buff.map_dir);if(unlikely(dma_mapping_error(rq->pdev,addr))){-page_pool_recycle_direct(rq->page_pool,au->page);-au->page=NULL;+page_pool_recycle_netmem(rq->page_pool,au->nmem);+au->nmem=NULL;return-ENOMEM;}-page_pool_set_dma_addr(au->page,addr);+netmem_set_dma_addr(au->nmem,addr);return0;}-voidmlx5e_page_dma_unmap(structmlx5e_rq*rq,structpage*page)+voidmlx5e_nmem_dma_unmap(structmlx5e_rq*rq,structnetmem*nmem){-dma_addr_tdma_addr=page_pool_get_dma_addr(page);+dma_addr_tdma_addr=netmem_get_dma_addr(nmem);dma_unmap_page_attrs(rq->pdev,dma_addr,PAGE_SIZE,rq->buff.map_dir,DMA_ATTR_SKIP_CPU_SYNC);-page_pool_set_dma_addr(page,0);+netmem_set_dma_addr(nmem,0);}-voidmlx5e_page_release_dynamic(structmlx5e_rq*rq,structpage*page,boolrecycle)+voidmlx5e_page_release_dynamic(structmlx5e_rq*rq,structnetmem*nmem,+boolrecycle){if(likely(recycle)){-if(mlx5e_rx_cache_put(rq,page))+if(mlx5e_rx_cache_put(rq,nmem))return;-mlx5e_page_dma_unmap(rq,page);-page_pool_recycle_direct(rq->page_pool,page);+mlx5e_nmem_dma_unmap(rq,nmem);+page_pool_recycle_netmem(rq->page_pool,nmem);
I see page_pool_recycle_direct() is replaced with
page_pool_recycle_netmem().
It does make the allow_direct=true correct, but I don't like the name
page_pool_recycle_netmem()
because driver developers might mistake this for a safe thing to call.
Can we rename it to page_pool_recycle_direct_netmem() ?
From: Matthew Wilcox <willy@infradead.org> Date: 2023-01-06 16:53:06
On Fri, Jan 06, 2023 at 04:49:12PM +0100, Jesper Dangaard Brouer wrote:
On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
quoted
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@infradead.org>
---
include/net/page_pool.h | 14 +++++++++++++-
net/core/page_pool.c | 13 ++++++-------
2 files changed, 19 insertions(+), 8 deletions(-)
@@ -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 */+staticinlinevoidpage_pool_put_full_netmem(structpage_pool*pool,+structnetmem*nmem,boolallow_direct)+{+page_pool_put_netmem(pool,nmem,-1,allow_direct);+}+staticinlinevoidpage_pool_put_full_page(structpage_pool*pool,structpage*page,boolallow_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 */
^^^^
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?
On Fri, Jan 06, 2023 at 04:49:12PM +0100, Jesper Dangaard Brouer wrote:
quoted
On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
quoted
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@infradead.org>
---
include/net/page_pool.h | 14 +++++++++++++-
net/core/page_pool.c | 13 ++++++-------
2 files changed, 19 insertions(+), 8 deletions(-)
@@ -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 */+staticinlinevoidpage_pool_put_full_netmem(structpage_pool*pool,+structnetmem*nmem,boolallow_direct)+{+page_pool_put_netmem(pool,nmem,-1,allow_direct);+}+staticinlinevoidpage_pool_put_full_page(structpage_pool*pool,structpage*page,boolallow_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 */
^^^^
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.
--Jesper
From: Matthew Wilcox <willy@infradead.org> Date: 2023-01-06 20:35:20
On Fri, Jan 06, 2023 at 10:24:30AM +0800, kernel test robot wrote:
quoted
quoted
include/net/page_pool.h:111:21: warning: due to lvalue conversion of the controlling expression, association of type 'const struct netmem' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
OK, figured out what this error means.
#define netmem_page(nmem) (_Generic((*nmem), \
const struct netmem: (const struct page *)nmem, \
struct netmem: (struct page *)nmem))
Because I defined this with _Generic((*nmem),...) instead of
_Generic((nmem),...) (like page_folio() is defined), clang always
selects the second case and not the const case. Apparently lvalue
coversions remove the const (backed up by
https://en.cppreference.com/w/c/language/conversion) but I had no idea
that _Generic applied lvalue conversion to the controlling-expression
(it does! https://en.cppreference.com/w/c/language/generic)
So, yay for clang's extra warning. I'll fix this up (as below) and send
a v3 next week including the various R-b that I've received.
-#define netmem_page(nmem) (_Generic((*nmem), \
- const struct netmem: (const struct page *)nmem, \
- struct netmem: (struct page *)nmem))
+#define netmem_page(nmem) (_Generic((nmem), \
+ const struct netmem *: (const struct page *)nmem, \
+ struct netmem *: (struct page *)nmem))
On 06/01/2023 18:31, Jesper Dangaard Brouer wrote:
To Saeed and Tariq, please review.
Adding Dragos, Gal.
Hi Jesper,
Thanks for the ping. I'm on it.
This reminds me, that IMHO we/nvidia/mellanox should remove the local
mlx5e_page_cache functionality, as SKBs can now recycle page_pool pages.
This should simplify the driver and we get rid of the head-of-line
blocking issue with the local page cache (refcnt elevation tricks).
Totally agree.
Dragos is currently working on this task. This should clean up
significant amount of code, and improve performance. We target this for
the next submission window, to kernel v6.4.
It might look good in microbencmarks, but my experience from prod
systems are that this local cache isn't utilized. And I believe we
should be able to get good/similar microbenchmark with page_pool, which
will continue to recycle and have no HoL issues for prod use-cases.
On 05/01/2023 23:46, Matthew Wilcox (Oracle) wrote:
Use the netmem APIs instead of the page_pool APIs. Possibly we should
add a netmem equivalent of skb_add_rx_frag(), but that can happen
later. Saves one call to compound_head() in the call to put_page()
in mlx5e_page_release_dynamic() which saves 58 bytes of text.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
drivers/net/ethernet/mellanox/mlx5/core/en.h | 10 +-
.../net/ethernet/mellanox/mlx5/core/en/txrx.h | 4 +-
.../net/ethernet/mellanox/mlx5/core/en/xdp.c | 24 ++--
.../net/ethernet/mellanox/mlx5/core/en/xdp.h | 2 +-
.../net/ethernet/mellanox/mlx5/core/en_main.c | 12 +-
.../net/ethernet/mellanox/mlx5/core/en_rx.c | 130 +++++++++---------
6 files changed, 94 insertions(+), 88 deletions(-)
Thanks for your patch!
Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
Tariq
On Thu, 5 Jan 2023 at 23:46, Matthew Wilcox (Oracle)
[off-list ref] wrote:
As part of simplifying struct page, create a new netmem type which
mirrors the page_pool members in struct page.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
Documentation/networking/page_pool.rst | 5 +++
include/net/page_pool.h | 46 ++++++++++++++++++++++++++
2 files changed, 51 insertions(+)
From: Matthew Wilcox <willy@infradead.org> Date: 2023-01-09 18:36:57
On Fri, Jan 06, 2023 at 09:16:25PM +0100, Jesper Dangaard Brouer wrote:
On 06/01/2023 17.53, Matthew Wilcox wrote:
quoted
On Fri, Jan 06, 2023 at 04:49:12PM +0100, Jesper Dangaard Brouer wrote:
quoted
On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
quoted
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@infradead.org>
---
include/net/page_pool.h | 14 +++++++++++++-
net/core/page_pool.c | 13 ++++++-------
2 files changed, 19 insertions(+), 8 deletions(-)
@@ -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 */+staticinlinevoidpage_pool_put_full_netmem(structpage_pool*pool,+structnetmem*nmem,boolallow_direct)+{+page_pool_put_netmem(pool,nmem,-1,allow_direct);+}+staticinlinevoidpage_pool_put_full_page(structpage_pool*pool,structpage*page,boolallow_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 */
^^^^
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:
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) [off-list ref]
@@ -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 */-staticinlinevoidpage_pool_recycle_direct(structpage_pool*pool,+staticinlinevoid__page_pool_recycle_direct(structpage_pool*pool,+structnetmem*nmem)+{+page_pool_put_full_netmem(pool,nmem,true);+}++staticinlinevoid__page_pool_recycle_page_direct(structpage_pool*pool,structpage*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), \+structnetmem*:__page_pool_recycle_direct(pool,(structnetmem*)mem),\+structpage*:__page_pool_recycle_page_direct(pool,(structpage*)mem))+#define PAGE_POOL_DMA_USE_PP_FRAG_COUNT \(sizeof(dma_addr_t)>sizeof(unsignedlong))
Hi Matthew,
On Thu, Jan 05, 2023 at 09:46:11PM +0000, Matthew Wilcox (Oracle) wrote:
quoted hunk
Also convert page_pool_clear_pp_info() and trace_page_pool_state_release()
to take a netmem. Include a wrapper for page_pool_release_page() to
avoid converting all callers.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
include/net/page_pool.h | 14 ++++++++++----
include/trace/events/page_pool.h | 14 +++++++-------
net/core/page_pool.c | 18 +++++++++---------
3 files changed, 26 insertions(+), 20 deletions(-)
I think it's worth commenting here that page_pool_release_page() is
eventually going to be removed once we convert all drivers and shouldn't
be used anymore
@@ -478,23 +478,23 @@ void page_pool_release_page(struct page_pool *pool, struct page *page)*/gotoskip_dma_unmap;-dma=page_pool_get_dma_addr(page);+dma=netmem_get_dma_addr(nmem);/* When page is unmapped, it cannot be returned to our pool */dma_unmap_page_attrs(pool->p.dev,dma,PAGE_SIZE<<pool->p.order,pool->p.dma_dir,DMA_ATTR_SKIP_CPU_SYNC);-page_pool_set_dma_addr(page,0);+netmem_set_dma_addr(nmem,0);skip_dma_unmap:-page_pool_clear_pp_info(page);+page_pool_clear_pp_info(nmem);/* This may be the last page returned, releasing the pool, so*itisnotsafetoreferencepoolafterwards.*/count=atomic_inc_return_relaxed(&pool->pages_state_release_cnt);-trace_page_pool_state_release(pool,page,count);+trace_page_pool_state_release(pool,nmem,count);}-EXPORT_SYMBOL(page_pool_release_page);+EXPORT_SYMBOL(page_pool_release_netmem);/* Return a page to the page allocator, cleaning up our state */staticvoidpage_pool_return_page(structpage_pool*pool,structpage*page)--
On Thu, Jan 05, 2023 at 09:46:12PM +0000, Matthew Wilcox (Oracle) wrote:
quoted hunk
Convert __page_pool_alloc_page_order() and __page_pool_alloc_pages_slow()
to use netmem internally. This removes a couple of calls
to compound_head() that are hidden inside put_page().
Convert trace_page_pool_state_hold(), page_pool_dma_map() and
page_pool_set_pp_info() to take a netmem argument.
Saves 83 bytes of text in __page_pool_alloc_page_order() and 98 in
__page_pool_alloc_pages_slow() for a total of 181 bytes.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
include/trace/events/page_pool.h | 14 +++++------
net/core/page_pool.c | 42 +++++++++++++++++---------------
2 files changed, 29 insertions(+), 27 deletions(-)
@@ -345,26 +346,26 @@ static void page_pool_clear_pp_info(struct netmem *nmem)staticstructpage*__page_pool_alloc_page_order(structpage_pool*pool,gfp_tgfp){-structpage*page;+structnetmem*nmem;gfp|=__GFP_COMP;-page=alloc_pages_node(pool->p.nid,gfp,pool->p.order);-if(unlikely(!page))+nmem=page_netmem(alloc_pages_node(pool->p.nid,gfp,pool->p.order));+if(unlikely(!nmem))returnNULL;if((pool->p.flags&PP_FLAG_DMA_MAP)&&-unlikely(!page_pool_dma_map(pool,page))){-put_page(page);+unlikely(!page_pool_dma_map(pool,nmem))){+netmem_put(nmem);returnNULL;}alloc_stat_inc(pool,slow_high_order);-page_pool_set_pp_info(pool,page);+page_pool_set_pp_info(pool,nmem);/* Track how many pages are held 'in-flight' */pool->pages_state_hold_cnt++;-trace_page_pool_state_hold(pool,page,pool->pages_state_hold_cnt);-returnpage;+trace_page_pool_state_hold(pool,nmem,pool->pages_state_hold_cnt);+returnnetmem_page(nmem);}/* slow path */
@@ -398,18 +399,18 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool,*pageelementhavenotbeen(possibly)DMAmapped.*/for(i=0;i<nr_pages;i++){-page=pool->alloc.cache[i];+structnetmem*nmem=page_netmem(pool->alloc.cache[i]);if((pp_flags&PP_FLAG_DMA_MAP)&&-unlikely(!page_pool_dma_map(pool,page))){-put_page(page);+unlikely(!page_pool_dma_map(pool,nmem))){+netmem_put(nmem);continue;}-page_pool_set_pp_info(pool,page);-pool->alloc.cache[pool->alloc.count++]=page;+page_pool_set_pp_info(pool,nmem);+pool->alloc.cache[pool->alloc.count++]=netmem_page(nmem);/* Track how many pages are held 'in-flight' */pool->pages_state_hold_cnt++;-trace_page_pool_state_hold(pool,page,+trace_page_pool_state_hold(pool,nmem,pool->pages_state_hold_cnt);}
@@ -421,7 +422,8 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool,page=NULL;}-/* When page just alloc'ed is should/must have refcnt 1. */+/* When page just allocated it should have refcnt 1 (but may have+*speculativereferences)*/returnpage;}--
On Thu, Jan 05, 2023 at 09:46:13PM +0000, Matthew Wilcox (Oracle) wrote:
quoted hunk
Removes a call to compound_head(), saving 464 bytes of kernel text
as page_pool_return_page() is inlined seven times.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
net/core/page_pool.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
@@ -499,11 +505,11 @@ void page_pool_release_netmem(struct page_pool *pool, struct netmem *nmem)EXPORT_SYMBOL(page_pool_release_netmem);/* Return a page to the page allocator, cleaning up our state */-staticvoidpage_pool_return_page(structpage_pool*pool,structpage*page)+staticvoidpage_pool_return_netmem(structpage_pool*pool,structnetmem*nmem){-page_pool_release_page(pool,page);+page_pool_release_netmem(pool,nmem);-put_page(page);+netmem_put(nmem);/* An optimization would be to call __free_pages(page, pool->p.order)*knowingpageisnotpartofpage-cache(thusavoidinga*__page_cache_release()call).--
On Thu, Jan 05, 2023 at 09:46:14PM +0000, Matthew Wilcox (Oracle) wrote:
quoted hunk
Removes the call to compound_head() hidden in put_page() which
saves 169 bytes of kernel text as __page_pool_put_page() is
inlined twice.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
net/core/page_pool.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
@@ -558,8 +558,8 @@ static bool page_pool_recycle_in_cache(struct page *page,*Ifthepagerefcnt!=1,thenthepagewillbereturnedtomemory*subsystem.*/-static__always_inlinestructpage*-__page_pool_put_page(structpage_pool*pool,structpage*page,+static__always_inlinestructnetmem*+__page_pool_put_netmem(structpage_pool*pool,structnetmem*nmem,unsignedintdma_sync_size,boolallow_direct){/* This allocator is optimized for the XDP mode that uses
@@ -571,19 +571,20 @@ __page_pool_put_page(struct page_pool *pool, struct page *page,*pageisNOTreusablewhenallocatedwhensystemisunder*somepressure.(page_is_pfmemalloc)*/-if(likely(page_ref_count(page)==1&&!page_is_pfmemalloc(page))){-/* Read barrier done in page_ref_count / READ_ONCE */+if(likely(netmem_ref_count(nmem)==1&&+!netmem_is_pfmemalloc(nmem))){+/* Read barrier done in netmem_ref_count / READ_ONCE */if(pool->p.flags&PP_FLAG_DMA_SYNC_DEV)-page_pool_dma_sync_for_device(pool,page,+page_pool_dma_sync_for_device(pool,netmem_page(nmem),dma_sync_size);if(allow_direct&&in_serving_softirq()&&-page_pool_recycle_in_cache(page,pool))+page_pool_recycle_in_cache(netmem_page(nmem),pool))returnNULL;/* Page found as candidate for recycling */-returnpage;+returnnmem;}/* Fallback/non-XDP mode: API user have elevated refcnt.*
@@ -599,13 +600,21 @@ __page_pool_put_page(struct page_pool *pool, struct page *page,*willbeinvokingput_page.*/recycle_stat_inc(pool,released_refcnt);-/* Do not replace this with page_pool_return_page() */-page_pool_release_page(pool,page);-put_page(page);+/* Do not replace this with page_pool_return_netmem() */+page_pool_release_netmem(pool,nmem);+netmem_put(nmem);returnNULL;}+static__always_inlinestructpage*+__page_pool_put_page(structpage_pool*pool,structpage*page,+unsignedintdma_sync_size,boolallow_direct)+{+returnnetmem_page(__page_pool_put_netmem(pool,page_netmem(page),+dma_sync_size,allow_direct));+}+voidpage_pool_put_defragged_page(structpage_pool*pool,structpage*page,unsignedintdma_sync_size,boolallow_direct){--
On Thu, Jan 05, 2023 at 09:46:15PM +0000, Matthew Wilcox (Oracle) wrote:
quoted hunk
Change the type here from page to netmem. It works out well to
convert page_pool_refill_alloc_cache() to return a netmem instead
of a page as part of this commit.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
include/net/page_pool.h | 2 +-
net/core/page_pool.c | 52 ++++++++++++++++++++---------------------
2 files changed, 27 insertions(+), 27 deletions(-)
@@ -229,10 +229,10 @@ void page_pool_return_page(struct page_pool *pool, struct page *page)}noinline-staticstructpage*page_pool_refill_alloc_cache(structpage_pool*pool)+staticstructnetmem*page_pool_refill_alloc_cache(structpage_pool*pool){structptr_ring*r=&pool->ring;-structpage*page;+structnetmem*nmem;intpref_nid;/* preferred NUMA node *//* Quicker fallback, avoid locks when ring is empty */
@@ -253,49 +253,49 @@ static struct page *page_pool_refill_alloc_cache(struct page_pool *pool)/* Refill alloc array, but only if NUMA match */do{-page=__ptr_ring_consume(r);-if(unlikely(!page))+nmem=__ptr_ring_consume(r);+if(unlikely(!nmem))break;-if(likely(page_to_nid(page)==pref_nid)){-pool->alloc.cache[pool->alloc.count++]=page;+if(likely(netmem_nid(nmem)==pref_nid)){+pool->alloc.cache[pool->alloc.count++]=nmem;}else{/* NUMA mismatch;*(1)release1pagetopage-allocatorand*(2)breakouttofallthroughtoalloc_pages_node.*Thislimitstressonpagebuddyalloactor.*/-page_pool_return_page(pool,page);+page_pool_return_netmem(pool,nmem);alloc_stat_inc(pool,waive);-page=NULL;+nmem=NULL;break;}}while(pool->alloc.count<PP_ALLOC_CACHE_REFILL);/* Return last page */if(likely(pool->alloc.count>0)){-page=pool->alloc.cache[--pool->alloc.count];+nmem=pool->alloc.cache[--pool->alloc.count];alloc_stat_inc(pool,refill);}-returnpage;+returnnmem;}/* fast path */staticstructpage*__page_pool_get_cached(structpage_pool*pool){-structpage*page;+structnetmem*nmem;/* Caller MUST guarantee safe non-concurrent access, e.g. softirq */if(likely(pool->alloc.count)){/* Fast-path */-page=pool->alloc.cache[--pool->alloc.count];+nmem=pool->alloc.cache[--pool->alloc.count];alloc_stat_inc(pool,fast);}else{-page=page_pool_refill_alloc_cache(pool);+nmem=page_pool_refill_alloc_cache(pool);}-returnpage;+returnnetmem_page(nmem);}staticvoidpage_pool_dma_sync_for_device(structpage_pool*pool,
@@ -391,13 +391,13 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool,/* Unnecessary as alloc cache is empty, but guarantees zero count */if(unlikely(pool->alloc.count>0))-returnpool->alloc.cache[--pool->alloc.count];+returnnetmem_page(pool->alloc.cache[--pool->alloc.count]);/* Mark empty alloc.cache slots "empty" for alloc_pages_bulk_array */memset(&pool->alloc.cache,0,sizeof(void*)*bulk);nr_pages=alloc_pages_bulk_array_node(gfp,pool->p.nid,bulk,-pool->alloc.cache);+(structpage**)pool->alloc.cache);if(unlikely(!nr_pages))returnNULL;
@@ -413,7 +413,7 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool,}page_pool_set_pp_info(pool,nmem);-pool->alloc.cache[pool->alloc.count++]=netmem_page(nmem);+pool->alloc.cache[pool->alloc.count++]=nmem;/* Track how many pages are held 'in-flight' */pool->pages_state_hold_cnt++;trace_page_pool_state_hold(pool,nmem,
@@ -878,15 +878,15 @@ EXPORT_SYMBOL(page_pool_destroy);/* Caller must provide appropriate safe context, e.g. NAPI. */voidpage_pool_update_nid(structpage_pool*pool,intnew_nid){-structpage*page;+structnetmem*nmem;trace_page_pool_update_nid(pool,new_nid);pool->p.nid=new_nid;/* Flush pool alloc cache, as refill will check NUMA node */while(pool->alloc.count){-page=pool->alloc.cache[--pool->alloc.count];-page_pool_return_page(pool,page);+nmem=pool->alloc.cache[--pool->alloc.count];+page_pool_return_netmem(pool,nmem);}}EXPORT_SYMBOL(page_pool_update_nid);--
On Fri, Jan 06, 2023 at 09:16:25PM +0100, Jesper Dangaard Brouer wrote:
quoted
On 06/01/2023 17.53, Matthew Wilcox wrote:
quoted
On Fri, Jan 06, 2023 at 04:49:12PM +0100, Jesper Dangaard Brouer wrote:
quoted
On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
quoted
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@infradead.org>
---
include/net/page_pool.h | 14 +++++++++++++-
net/core/page_pool.c | 13 ++++++-------
2 files changed, 19 insertions(+), 8 deletions(-)
@@ -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 */+staticinlinevoidpage_pool_put_full_netmem(structpage_pool*pool,+structnetmem*nmem,boolallow_direct)+{+page_pool_put_netmem(pool,nmem,-1,allow_direct);+}+staticinlinevoidpage_pool_put_full_page(structpage_pool*pool,structpage*page,boolallow_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 */
^^^^
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).
quoted hunk
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) [off-list ref]
@@ -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 */-staticinlinevoidpage_pool_recycle_direct(structpage_pool*pool,+staticinlinevoid__page_pool_recycle_direct(structpage_pool*pool,+structnetmem*nmem)+{+page_pool_put_full_netmem(pool,nmem,true);+}++staticinlinevoid__page_pool_recycle_page_direct(structpage_pool*pool,structpage*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), \+structnetmem*:__page_pool_recycle_direct(pool,(structnetmem*)mem),\+structpage*:__page_pool_recycle_page_direct(pool,(structpage*)mem))+#define PAGE_POOL_DMA_USE_PP_FRAG_COUNT \(sizeof(dma_addr_t)>sizeof(unsignedlong))
On Thu, Jan 05, 2023 at 09:46:18PM +0000, Matthew Wilcox (Oracle) wrote:
quoted hunk
Retrieve a netmem from the ptr_ring instead of a page.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
net/core/page_pool.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
On Thu, Jan 05, 2023 at 09:46:19PM +0000, Matthew Wilcox (Oracle) wrote:
quoted hunk
Add wrappers for page_pool_alloc_pages() and
page_pool_dev_alloc_netmem(). Also convert __page_pool_alloc_pages_slow()
to __page_pool_alloc_netmem_slow() and __page_pool_alloc_page_order()
to __page_pool_alloc_netmem(). __page_pool_get_cached() now returns
a netmem.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
include/net/page_pool.h | 13 ++++++++++++-
net/core/page_pool.c | 39 +++++++++++++++++++--------------------
2 files changed, 31 insertions(+), 21 deletions(-)
@@ -371,27 +371,27 @@ static struct page *__page_pool_alloc_page_order(struct page_pool *pool,/* Track how many pages are held 'in-flight' */pool->pages_state_hold_cnt++;trace_page_pool_state_hold(pool,nmem,pool->pages_state_hold_cnt);-returnnetmem_page(nmem);+returnnmem;}/* slow path */noinline-staticstructpage*__page_pool_alloc_pages_slow(structpage_pool*pool,+staticstructnetmem*__page_pool_alloc_netmem_slow(structpage_pool*pool,gfp_tgfp){constintbulk=PP_ALLOC_CACHE_REFILL;unsignedintpp_flags=pool->p.flags;unsignedintpp_order=pool->p.order;-structpage*page;+structnetmem*nmem;inti,nr_pages;/* Don't support bulk alloc for high-order pages */if(unlikely(pp_order))-return__page_pool_alloc_page_order(pool,gfp);+return__page_pool_alloc_netmem(pool,gfp);/* Unnecessary as alloc cache is empty, but guarantees zero count */if(unlikely(pool->alloc.count>0))-returnnetmem_page(pool->alloc.cache[--pool->alloc.count]);+returnpool->alloc.cache[--pool->alloc.count];/* Mark empty alloc.cache slots "empty" for alloc_pages_bulk_array */memset(&pool->alloc.cache,0,sizeof(void*)*bulk);
@@ -422,34 +422,33 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool,/* Return last page */if(likely(pool->alloc.count>0)){-page=netmem_page(pool->alloc.cache[--pool->alloc.count]);+nmem=pool->alloc.cache[--pool->alloc.count];alloc_stat_inc(pool,slow);}else{-page=NULL;+nmem=NULL;}/* When page just allocated it should have refcnt 1 (but may have*speculativereferences)*/-returnpage;+returnnmem;}/* For using page_pool replace: alloc_pages() API calls, but provide*synchronizationguaranteeforallocationside.*/-structpage*page_pool_alloc_pages(structpage_pool*pool,gfp_tgfp)+structnetmem*page_pool_alloc_netmem(structpage_pool*pool,gfp_tgfp){-structpage*page;+structnetmem*nmem;/* Fast-path: Get a page from cache */-page=__page_pool_get_cached(pool);-if(page)-returnpage;+nmem=__page_pool_get_cached(pool);+if(nmem)+returnnmem;/* Slow-path: cache empty, do real allocation */-page=__page_pool_alloc_pages_slow(pool,gfp);-returnpage;+return__page_pool_alloc_netmem_slow(pool,gfp);}-EXPORT_SYMBOL(page_pool_alloc_pages);+EXPORT_SYMBOL(page_pool_alloc_netmem);/* Calculate distance between two u32 values, valid if distance is below 2^(31)*https://en.wikipedia.org/wiki/Serial_number_arithmetic#General_Solution--
@@ -676,6 +676,7 @@ EXPORT_SYMBOL(page_pool_put_page_bulk);staticstructpage*page_pool_drain_frag(structpage_pool*pool,structpage*page){+structnetmem*nmem=page_netmem(page);longdrain_count=BIAS_MAX-pool->frag_users;/* Some user is still using the page frag */
On Thu, Jan 05, 2023 at 09:46:23PM +0000, Matthew Wilcox (Oracle) wrote:
quoted hunk
We're not quite ready to change the API of page_pool_drain_frag(),
but we can remove the use of several wrappers by using the netmem
throughout.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
net/core/page_pool.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
@@ -672,17 +672,17 @@ static struct page *page_pool_drain_frag(struct page_pool *pool,longdrain_count=BIAS_MAX-pool->frag_users;/* Some user is still using the page frag */-if(likely(page_pool_defrag_page(page,drain_count)))+if(likely(page_pool_defrag_netmem(nmem,drain_count)))returnNULL;-if(page_ref_count(page)==1&&!page_is_pfmemalloc(page)){+if(netmem_ref_count(nmem)==1&&!netmem_is_pfmemalloc(nmem)){if(pool->p.flags&PP_FLAG_DMA_SYNC_DEV)page_pool_dma_sync_for_device(pool,nmem,-1);returnpage;}-page_pool_return_page(pool,page);+page_pool_return_netmem(pool,nmem);returnNULL;}--