* [PATCH net-next v11 0/6] introduce page_pool_alloc() related API
@ 2023-10-13 6:48 Yunsheng Lin
2023-10-13 6:48 ` [PATCH net-next v11 3/6] page_pool: remove PP_FLAG_PAGE_FRAG Yunsheng Lin
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Yunsheng Lin @ 2023-10-13 6:48 UTC (permalink / raw)
To: davem, kuba, pabeni
Cc: netdev, linux-kernel, Yunsheng Lin, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Matthias Brugger, AngeloGioacchino Del Regno, bpf,
linux-arm-kernel, linux-mediatek
In [1] & [2] & [3], there are usecases for veth and virtio_net
to use frag support in page pool to reduce memory usage, and it
may request different frag size depending on the head/tail
room space for xdp_frame/shinfo and mtu/packet size. When the
requested frag size is large enough that a single page can not
be split into more than one frag, using frag support only have
performance penalty because of the extra frag count handling
for frag support.
So this patchset provides a page pool API for the driver to
allocate memory with least memory utilization and performance
penalty when it doesn't know the size of memory it need
beforehand.
1. https://patchwork.kernel.org/project/netdevbpf/patch/d3ae6bd3537fbce379382ac6a42f67e22f27ece2.1683896626.git.lorenzo@kernel.org/
2. https://patchwork.kernel.org/project/netdevbpf/patch/20230526054621.18371-3-liangchen.linux@gmail.com/
3. https://github.com/alobakin/linux/tree/iavf-pp-frag
V11: Repost based on the latest net-next branch and collect
Tested-by Tag from Alexander.
V10: Use fragment instead of frag in English docs.
Remove PP_FLAG_PAGE_FRAG usage in idpf driver.
V9: Update some performance info in patch 2.
V8: Store the dma addr on a shifted u32 instead of using
dma_addr_t explicitly for 32-bit arch with 64-bit DMA.
Update document according to discussion in v7.
V7: Fix a compile error, a few typo and use kernel-doc syntax.
V6: Add a PP_FLAG_PAGE_SPLIT_IN_DRIVER flag to fail the page_pool
creation for 32-bit arch with 64-bit DMA when driver tries to
do the page splitting itself, adjust the requested size to
include head/tail room in veth, and rebased on the latest
next-net.
v5 RFC: Add a new page_pool_cache_alloc() API, and other minor
change as discussed in v4. As there seems to be three
comsumers that might be made use of the new API, so
repost it as RFC and CC the relevant authors to see
if the new API fits their need.
V4. Fix a typo and add a patch to update document about frag
API, PAGE_POOL_DMA_USE_PP_FRAG_COUNT is not renamed yet
as we may need a different thread to discuss that.
V3: Incorporate changes from the disscusion with Alexander,
mostly the inline wraper, PAGE_POOL_DMA_USE_PP_FRAG_COUNT
change split to separate patch and comment change.
V2: Add patch to remove PP_FLAG_PAGE_FRAG flags and mention
virtio_net usecase in the cover letter.
V1: Drop RFC tag and page_pool_frag patch.
Yunsheng Lin (6):
page_pool: fragment API support for 32-bit arch with 64-bit DMA
page_pool: unify frag_count handling in page_pool_is_last_frag()
page_pool: remove PP_FLAG_PAGE_FRAG
page_pool: introduce page_pool[_cache]_alloc() API
page_pool: update document about fragment API
net: veth: use newly added page pool API for veth with xdp
Documentation/networking/page_pool.rst | 4 +-
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 -
.../net/ethernet/hisilicon/hns3/hns3_enet.c | 3 +-
drivers/net/ethernet/intel/idpf/idpf_txrx.c | 3 -
.../marvell/octeontx2/nic/otx2_common.c | 2 +-
.../net/ethernet/mellanox/mlx5/core/en_main.c | 2 +-
drivers/net/veth.c | 25 +-
drivers/net/wireless/mediatek/mt76/mac80211.c | 2 +-
include/linux/mm_types.h | 13 +-
include/net/page_pool/helpers.h | 231 +++++++++++++++---
include/net/page_pool/types.h | 6 +-
net/core/page_pool.c | 31 ++-
net/core/skbuff.c | 2 +-
13 files changed, 243 insertions(+), 83 deletions(-)
--
2.33.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH net-next v11 3/6] page_pool: remove PP_FLAG_PAGE_FRAG 2023-10-13 6:48 [PATCH net-next v11 0/6] introduce page_pool_alloc() related API Yunsheng Lin @ 2023-10-13 6:48 ` Yunsheng Lin 2023-10-17 1:27 ` [PATCH net-next v11 0/6] introduce page_pool_alloc() related API Jakub Kicinski 2023-10-17 4:10 ` patchwork-bot+netdevbpf 2 siblings, 0 replies; 11+ messages in thread From: Yunsheng Lin @ 2023-10-13 6:48 UTC (permalink / raw) To: davem, kuba, pabeni Cc: netdev, linux-kernel, Yunsheng Lin, Lorenzo Bianconi, Alexander Duyck, Liang Chen, Alexander Lobakin, Michael Chan, Eric Dumazet, Yisen Zhuang, Salil Mehta, Jesse Brandeburg, Tony Nguyen, Sunil Goutham, Geetha sowjanya, Subbaraya Sundeep, hariprasad, Saeed Mahameed, Leon Romanovsky, Felix Fietkau, Ryder Lee, Shayne Chen, Sean Wang, Kalle Valo, Matthias Brugger, AngeloGioacchino Del Regno, Jesper Dangaard Brouer, Ilias Apalodimas, intel-wired-lan, linux-rdma, linux-wireless, linux-arm-kernel, linux-mediatek PP_FLAG_PAGE_FRAG is not really needed after pp_frag_count handling is unified and page_pool_alloc_frag() is supported in 32-bit arch with 64-bit DMA, so remove it. Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com> CC: Lorenzo Bianconi <lorenzo@kernel.org> CC: Alexander Duyck <alexander.duyck@gmail.com> CC: Liang Chen <liangchen.linux@gmail.com> CC: Alexander Lobakin <aleksander.lobakin@intel.com> --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 -- drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 3 +-- drivers/net/ethernet/intel/idpf/idpf_txrx.c | 3 --- drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c | 2 +- drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 2 +- drivers/net/wireless/mediatek/mt76/mac80211.c | 2 +- include/net/page_pool/types.h | 6 ++---- net/core/page_pool.c | 3 +-- net/core/skbuff.c | 2 +- 9 files changed, 8 insertions(+), 17 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index b0ca3b319e4f..96d11f41dd38 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -3250,8 +3250,6 @@ static int bnxt_alloc_rx_page_pool(struct bnxt *bp, pp.dma_dir = bp->rx_dir; pp.max_len = PAGE_SIZE; pp.flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV; - if (PAGE_SIZE > BNXT_RX_PAGE_SIZE) - pp.flags |= PP_FLAG_PAGE_FRAG; rxr->page_pool = page_pool_create(&pp); if (IS_ERR(rxr->page_pool)) { diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c index cf50368441b7..06117502001f 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c @@ -4940,8 +4940,7 @@ static void hns3_put_ring_config(struct hns3_nic_priv *priv) static void hns3_alloc_page_pool(struct hns3_enet_ring *ring) { struct page_pool_params pp_params = { - .flags = PP_FLAG_DMA_MAP | PP_FLAG_PAGE_FRAG | - PP_FLAG_DMA_SYNC_DEV, + .flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV, .order = hns3_page_order(ring), .pool_size = ring->desc_num * hns3_buf_size(ring) / (PAGE_SIZE << hns3_page_order(ring)), diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c index 6fa79898c42c..55a099986b55 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c @@ -595,9 +595,6 @@ static struct page_pool *idpf_rx_create_page_pool(struct idpf_queue *rxbufq) .offset = 0, }; - if (rxbufq->rx_buf_size == IDPF_RX_BUF_2048) - pp.flags |= PP_FLAG_PAGE_FRAG; - return page_pool_create(&pp); } diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c index 818ce76185b2..1a42bfded872 100644 --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c @@ -1404,7 +1404,7 @@ int otx2_pool_init(struct otx2_nic *pfvf, u16 pool_id, } pp_params.order = get_order(buf_size); - pp_params.flags = PP_FLAG_PAGE_FRAG | PP_FLAG_DMA_MAP; + pp_params.flags = PP_FLAG_DMA_MAP; pp_params.pool_size = min(OTX2_PAGE_POOL_SZ, numptrs); pp_params.nid = NUMA_NO_NODE; pp_params.dev = pfvf->dev; diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c index acb40770cf0c..a5441dea3463 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c @@ -834,7 +834,7 @@ static int mlx5e_alloc_rq(struct mlx5e_params *params, struct page_pool_params pp_params = { 0 }; pp_params.order = 0; - pp_params.flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV | PP_FLAG_PAGE_FRAG; + pp_params.flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV; pp_params.pool_size = pool_size; pp_params.nid = node; pp_params.dev = rq->pdev; diff --git a/drivers/net/wireless/mediatek/mt76/mac80211.c b/drivers/net/wireless/mediatek/mt76/mac80211.c index d158320bc15d..fe7cc67b7ee2 100644 --- a/drivers/net/wireless/mediatek/mt76/mac80211.c +++ b/drivers/net/wireless/mediatek/mt76/mac80211.c @@ -566,7 +566,7 @@ int mt76_create_page_pool(struct mt76_dev *dev, struct mt76_queue *q) { struct page_pool_params pp_params = { .order = 0, - .flags = PP_FLAG_PAGE_FRAG, + .flags = 0, .nid = NUMA_NO_NODE, .dev = dev->dma_dev, }; diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h index 887e7946a597..6fc5134095ed 100644 --- a/include/net/page_pool/types.h +++ b/include/net/page_pool/types.h @@ -17,10 +17,8 @@ * Please note DMA-sync-for-CPU is still * device driver responsibility */ -#define PP_FLAG_PAGE_FRAG BIT(2) /* for page frag feature */ #define PP_FLAG_ALL (PP_FLAG_DMA_MAP |\ - PP_FLAG_DMA_SYNC_DEV |\ - PP_FLAG_PAGE_FRAG) + PP_FLAG_DMA_SYNC_DEV) /* * Fast allocation side cache array/stack @@ -45,7 +43,7 @@ struct pp_alloc_cache { /** * struct page_pool_params - page pool parameters - * @flags: PP_FLAG_DMA_MAP, PP_FLAG_DMA_SYNC_DEV, PP_FLAG_PAGE_FRAG + * @flags: PP_FLAG_DMA_MAP, PP_FLAG_DMA_SYNC_DEV * @order: 2^order pages on allocation * @pool_size: size of the ptr_ring * @nid: NUMA node id to allocate from pages from diff --git a/net/core/page_pool.c b/net/core/page_pool.c index 953535cab081..2a3671c97ca7 100644 --- a/net/core/page_pool.c +++ b/net/core/page_pool.c @@ -756,8 +756,7 @@ struct page *page_pool_alloc_frag(struct page_pool *pool, unsigned int max_size = PAGE_SIZE << pool->p.order; struct page *page = pool->frag_page; - if (WARN_ON(!(pool->p.flags & PP_FLAG_PAGE_FRAG) || - size > max_size)) + if (WARN_ON(size > max_size)) return NULL; size = ALIGN(size, dma_get_cache_alignment()); diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 0401f40973a5..ced4549b06c5 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -5764,7 +5764,7 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from, /* In general, avoid mixing page_pool and non-page_pool allocated * pages within the same SKB. Additionally avoid dealing with clones * with page_pool pages, in case the SKB is using page_pool fragment - * references (PP_FLAG_PAGE_FRAG). Since we only take full page + * references (page_pool_alloc_frag()). Since we only take full page * references for cloned SKBs at the moment that would result in * inconsistent reference counts. * In theory we could take full references if @from is cloned and -- 2.33.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v11 0/6] introduce page_pool_alloc() related API 2023-10-13 6:48 [PATCH net-next v11 0/6] introduce page_pool_alloc() related API Yunsheng Lin 2023-10-13 6:48 ` [PATCH net-next v11 3/6] page_pool: remove PP_FLAG_PAGE_FRAG Yunsheng Lin @ 2023-10-17 1:27 ` Jakub Kicinski 2023-10-17 7:56 ` Yunsheng Lin 2023-10-17 4:10 ` patchwork-bot+netdevbpf 2 siblings, 1 reply; 11+ messages in thread From: Jakub Kicinski @ 2023-10-17 1:27 UTC (permalink / raw) To: Yunsheng Lin Cc: davem, pabeni, netdev, linux-kernel, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Matthias Brugger, AngeloGioacchino Del Regno, bpf, linux-arm-kernel, linux-mediatek On Fri, 13 Oct 2023 14:48:20 +0800 Yunsheng Lin wrote: > v5 RFC: Add a new page_pool_cache_alloc() API, and other minor > change as discussed in v4. As there seems to be three > comsumers that might be made use of the new API, so > repost it as RFC and CC the relevant authors to see > if the new API fits their need. I have looked thru the v4 discussion (admittedly it was pretty huge). I can't find where the "cache" API was suggested. And I can't figure out now what the "cache" in the name is referring to. Looks like these are just convenience wrappers which return VA instead of struct page.. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v11 0/6] introduce page_pool_alloc() related API 2023-10-17 1:27 ` [PATCH net-next v11 0/6] introduce page_pool_alloc() related API Jakub Kicinski @ 2023-10-17 7:56 ` Yunsheng Lin 2023-10-17 15:13 ` Jakub Kicinski 0 siblings, 1 reply; 11+ messages in thread From: Yunsheng Lin @ 2023-10-17 7:56 UTC (permalink / raw) To: Jakub Kicinski Cc: davem, pabeni, netdev, linux-kernel, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Matthias Brugger, AngeloGioacchino Del Regno, bpf, linux-arm-kernel, linux-mediatek On 2023/10/17 9:27, Jakub Kicinski wrote: > On Fri, 13 Oct 2023 14:48:20 +0800 Yunsheng Lin wrote: >> v5 RFC: Add a new page_pool_cache_alloc() API, and other minor >> change as discussed in v4. As there seems to be three >> comsumers that might be made use of the new API, so >> repost it as RFC and CC the relevant authors to see >> if the new API fits their need. > > I have looked thru the v4 discussion (admittedly it was pretty huge). > I can't find where the "cache" API was suggested. Actually, the discussion happened in V3 as both of discussions in V3 and V4 seems to be happening concurrently: https://lore.kernel.org/all/f8ce176f-f975-af11-641c-b56c53a8066a@redhat.com/ > And I can't figure out now what the "cache" in the name is referring to. > Looks like these are just convenience wrappers which return VA instead > of struct page.. Yes, it is corresponding to some API like napi_alloc_frag() returning va instead of 'struct page' mentioned in patch 5. Anyway, naming is hard, any suggestion for a better naming is always welcomed:) > . > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v11 0/6] introduce page_pool_alloc() related API 2023-10-17 7:56 ` Yunsheng Lin @ 2023-10-17 15:13 ` Jakub Kicinski 2023-10-17 15:14 ` Jakub Kicinski 2023-10-18 11:47 ` Yunsheng Lin 0 siblings, 2 replies; 11+ messages in thread From: Jakub Kicinski @ 2023-10-17 15:13 UTC (permalink / raw) To: Yunsheng Lin Cc: davem, pabeni, netdev, linux-kernel, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Matthias Brugger, AngeloGioacchino Del Regno, bpf, linux-arm-kernel, linux-mediatek, Alexander Duyck On Tue, 17 Oct 2023 15:56:48 +0800 Yunsheng Lin wrote: > > And I can't figure out now what the "cache" in the name is referring to. > > Looks like these are just convenience wrappers which return VA instead > > of struct page.. > > Yes, it is corresponding to some API like napi_alloc_frag() returning va > instead of 'struct page' mentioned in patch 5. > > Anyway, naming is hard, any suggestion for a better naming is always > welcomed:) I'd just throw a _va (for virtual address) at the end. And not really mention it in the documentation. Plus the kdoc of the function should say that this is just a thin wrapper around other page pool APIs, and it's safe to mix it with other page pool APIs? _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v11 0/6] introduce page_pool_alloc() related API 2023-10-17 15:13 ` Jakub Kicinski @ 2023-10-17 15:14 ` Jakub Kicinski 2023-10-18 11:47 ` Yunsheng Lin 1 sibling, 0 replies; 11+ messages in thread From: Jakub Kicinski @ 2023-10-17 15:14 UTC (permalink / raw) To: Yunsheng Lin Cc: davem, pabeni, netdev, linux-kernel, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Matthias Brugger, AngeloGioacchino Del Regno, bpf, linux-arm-kernel, linux-mediatek, Alexander Duyck On Tue, 17 Oct 2023 08:13:03 -0700 Jakub Kicinski wrote: > I'd just throw a _va (for virtual address) at the end To be clear I mean: page_pool_alloc_va() page_pool_dev_alloc_va() page_pool_free_va() _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v11 0/6] introduce page_pool_alloc() related API 2023-10-17 15:13 ` Jakub Kicinski 2023-10-17 15:14 ` Jakub Kicinski @ 2023-10-18 11:47 ` Yunsheng Lin 2023-10-18 15:35 ` Jakub Kicinski 1 sibling, 1 reply; 11+ messages in thread From: Yunsheng Lin @ 2023-10-18 11:47 UTC (permalink / raw) To: Jakub Kicinski Cc: davem, pabeni, netdev, linux-kernel, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Matthias Brugger, AngeloGioacchino Del Regno, bpf, linux-arm-kernel, linux-mediatek, Alexander Duyck On 2023/10/17 23:13, Jakub Kicinski wrote: > On Tue, 17 Oct 2023 15:56:48 +0800 Yunsheng Lin wrote: >>> And I can't figure out now what the "cache" in the name is referring to. >>> Looks like these are just convenience wrappers which return VA instead >>> of struct page.. >> >> Yes, it is corresponding to some API like napi_alloc_frag() returning va >> instead of 'struct page' mentioned in patch 5. >> >> Anyway, naming is hard, any suggestion for a better naming is always >> welcomed:) > > I'd just throw a _va (for virtual address) at the end. And not really _va seems fine:) > mention it in the documentation. Plus the kdoc of the function should > say that this is just a thin wrapper around other page pool APIs, and > it's safe to mix it with other page pool APIs? I am not sure I understand what do 'safe' and 'mix' mean here. For 'safe' part, I suppose you mean if there is a va accociated with a 'struct page' without calling some API like kmap()? For that, I suppose it is safe when the driver is calling page_pool API without the __GFP_HIGHMEM flag. Maybe we should mention that in the kdoc and give a warning if page_pool_*alloc_va() is called with the __GFP_HIGHMEM flag? For the 'mix', I suppose you mean the below: 1. Allocate a page with the page_pool_*alloc_va() API and free a page with page_pool_free() API. 2. Allocate a page with the page_pool_*alloc() API and free a page with page_pool_free_va() API. For 1, it seems it is ok as some virt_to_head_page() and page_address() call between va and 'struct page' does not seem to change anything if we have enforce page_pool_*alloc_va() to be called without the __GFP_HIGHMEM flag. For 2, If the va is returned from page_address() which the allocation API is called without __GFP_HIGHMEM flag. If not, the va is from kmap*()? which means we may be calling page_pool_free_va() before kunmap*()? Is that possible? > . > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v11 0/6] introduce page_pool_alloc() related API 2023-10-18 11:47 ` Yunsheng Lin @ 2023-10-18 15:35 ` Jakub Kicinski 2023-10-19 13:22 ` Yunsheng Lin 0 siblings, 1 reply; 11+ messages in thread From: Jakub Kicinski @ 2023-10-18 15:35 UTC (permalink / raw) To: Yunsheng Lin Cc: davem, pabeni, netdev, linux-kernel, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Matthias Brugger, AngeloGioacchino Del Regno, bpf, linux-arm-kernel, linux-mediatek, Alexander Duyck On Wed, 18 Oct 2023 19:47:16 +0800 Yunsheng Lin wrote: > > mention it in the documentation. Plus the kdoc of the function should > > say that this is just a thin wrapper around other page pool APIs, and > > it's safe to mix it with other page pool APIs? > > I am not sure I understand what do 'safe' and 'mix' mean here. > > For 'safe' part, I suppose you mean if there is a va accociated with > a 'struct page' without calling some API like kmap()? For that, I suppose > it is safe when the driver is calling page_pool API without the > __GFP_HIGHMEM flag. Maybe we should mention that in the kdoc and give a > warning if page_pool_*alloc_va() is called with the __GFP_HIGHMEM flag? Sounds good. Warning wrapped in #if CONFIG_DEBUG_NET perhaps? > For the 'mix', I suppose you mean the below: > 1. Allocate a page with the page_pool_*alloc_va() API and free a page with > page_pool_free() API. > 2. Allocate a page with the page_pool_*alloc() API and free a page with > page_pool_free_va() API. > > For 1, it seems it is ok as some virt_to_head_page() and page_address() call > between va and 'struct page' does not seem to change anything if we have > enforce page_pool_*alloc_va() to be called without the __GFP_HIGHMEM flag. > > For 2, If the va is returned from page_address() which the allocation API is > called without __GFP_HIGHMEM flag. If not, the va is from kmap*()? which means > we may be calling page_pool_free_va() before kunmap*()? Is that possible? Right, if someone passes kmap()'ed address they are trying quite hard to break their own driver. Technically possible but I wouldn't worry. I just mean that in the common case of non-HIGHMEM page, calling page_pool_free_va() with the address returned by page_address() is perfectly legal. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v11 0/6] introduce page_pool_alloc() related API 2023-10-18 15:35 ` Jakub Kicinski @ 2023-10-19 13:22 ` Yunsheng Lin 2023-10-19 13:56 ` Jakub Kicinski 0 siblings, 1 reply; 11+ messages in thread From: Yunsheng Lin @ 2023-10-19 13:22 UTC (permalink / raw) To: Jakub Kicinski Cc: davem, pabeni, netdev, linux-kernel, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Matthias Brugger, AngeloGioacchino Del Regno, bpf, linux-arm-kernel, linux-mediatek, Alexander Duyck On 2023/10/18 23:35, Jakub Kicinski wrote: > On Wed, 18 Oct 2023 19:47:16 +0800 Yunsheng Lin wrote: >>> mention it in the documentation. Plus the kdoc of the function should >>> say that this is just a thin wrapper around other page pool APIs, and >>> it's safe to mix it with other page pool APIs? >> >> I am not sure I understand what do 'safe' and 'mix' mean here. >> >> For 'safe' part, I suppose you mean if there is a va accociated with >> a 'struct page' without calling some API like kmap()? For that, I suppose >> it is safe when the driver is calling page_pool API without the >> __GFP_HIGHMEM flag. Maybe we should mention that in the kdoc and give a >> warning if page_pool_*alloc_va() is called with the __GFP_HIGHMEM flag? > > Sounds good. Warning wrapped in #if CONFIG_DEBUG_NET perhaps? How about something like __get_free_pages() does with gfp flags? https://elixir.free-electrons.com/linux/v6.4-rc6/source/mm/page_alloc.c#L4818 how about something like below on top of this patchset: diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h index 7550beeacf3d..61cee55606c0 100644 --- a/include/net/page_pool/helpers.h +++ b/include/net/page_pool/helpers.h @@ -167,13 +167,13 @@ static inline struct page *page_pool_dev_alloc(struct page_pool *pool, return page_pool_alloc(pool, offset, size, gfp); } -static inline void *page_pool_cache_alloc(struct page_pool *pool, - unsigned int *size, gfp_t gfp) +static inline void *page_pool_alloc_va(struct page_pool *pool, + unsigned int *size, gfp_t gfp) { unsigned int offset; struct page *page; - page = page_pool_alloc(pool, &offset, size, gfp); + page = page_pool_alloc(pool, &offset, size, gfp & ~__GFP_HIGHMEM); if (unlikely(!page)) return NULL; @@ -181,21 +181,22 @@ static inline void *page_pool_cache_alloc(struct page_pool *pool, } /** - * page_pool_dev_cache_alloc() - allocate a cache. + * page_pool_dev_alloc_va() - allocate a page or a page fragment. * @pool: pool from which to allocate * @size: in as the requested size, out as the allocated size * - * Get a cache from the page allocator or page_pool caches. + * This is just a thin wrapper around the page_pool_alloc() API, and + * it returns va of the allocated page or page fragment. * * Return: - * Return the addr for the allocated cache, otherwise return NULL. + * Return the va for the allocated page or page fragment, otherwise return NULL. */ -static inline void *page_pool_dev_cache_alloc(struct page_pool *pool, - unsigned int *size) +static inline void *page_pool_dev_alloc_va(struct page_pool *pool, + unsigned int *size) { gfp_t gfp = (GFP_ATOMIC | __GFP_NOWARN); - return page_pool_cache_alloc(pool, size, gfp); + return page_pool_alloc_va(pool, size, gfp); } /** @@ -338,17 +339,17 @@ static inline void page_pool_recycle_direct(struct page_pool *pool, (sizeof(dma_addr_t) > sizeof(unsigned long)) /** - * page_pool_cache_free() - free a cache into the page_pool - * @pool: pool from which cache was allocated - * @data: addr of cache to be free + * page_pool_free_va() - free a va into the page_pool + * @pool: pool from which va was allocated + * @va: va to be free * @allow_direct: freed by the consumer, allow lockless caching * * Free a cache allocated from page_pool_dev_cache_alloc(). */ -static inline void page_pool_cache_free(struct page_pool *pool, void *data, - bool allow_direct) +static inline void page_pool_free_va(struct page_pool *pool, void *va, + bool allow_direct) { - page_pool_put_page(pool, virt_to_head_page(data), -1, allow_direct); + page_pool_put_page(pool, virt_to_head_page(va), -1, allow_direct); } _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v11 0/6] introduce page_pool_alloc() related API 2023-10-19 13:22 ` Yunsheng Lin @ 2023-10-19 13:56 ` Jakub Kicinski 0 siblings, 0 replies; 11+ messages in thread From: Jakub Kicinski @ 2023-10-19 13:56 UTC (permalink / raw) To: Yunsheng Lin Cc: davem, pabeni, netdev, linux-kernel, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Matthias Brugger, AngeloGioacchino Del Regno, bpf, linux-arm-kernel, linux-mediatek, Alexander Duyck On Thu, 19 Oct 2023 21:22:07 +0800 Yunsheng Lin wrote: > > Sounds good. Warning wrapped in #if CONFIG_DEBUG_NET perhaps? > > How about something like __get_free_pages() does with gfp flags? > https://elixir.free-electrons.com/linux/v6.4-rc6/source/mm/page_alloc.c#L4818 Fine by me! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v11 0/6] introduce page_pool_alloc() related API 2023-10-13 6:48 [PATCH net-next v11 0/6] introduce page_pool_alloc() related API Yunsheng Lin 2023-10-13 6:48 ` [PATCH net-next v11 3/6] page_pool: remove PP_FLAG_PAGE_FRAG Yunsheng Lin 2023-10-17 1:27 ` [PATCH net-next v11 0/6] introduce page_pool_alloc() related API Jakub Kicinski @ 2023-10-17 4:10 ` patchwork-bot+netdevbpf 2 siblings, 0 replies; 11+ messages in thread From: patchwork-bot+netdevbpf @ 2023-10-17 4:10 UTC (permalink / raw) To: Yunsheng Lin Cc: davem, kuba, pabeni, netdev, linux-kernel, ast, daniel, hawk, john.fastabend, matthias.bgg, angelogioacchino.delregno, bpf, linux-arm-kernel, linux-mediatek Hello: This series was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Fri, 13 Oct 2023 14:48:20 +0800 you wrote: > In [1] & [2] & [3], there are usecases for veth and virtio_net > to use frag support in page pool to reduce memory usage, and it > may request different frag size depending on the head/tail > room space for xdp_frame/shinfo and mtu/packet size. When the > requested frag size is large enough that a single page can not > be split into more than one frag, using frag support only have > performance penalty because of the extra frag count handling > for frag support. > > [...] Here is the summary with links: - [net-next,v11,1/6] page_pool: fragment API support for 32-bit arch with 64-bit DMA https://git.kernel.org/netdev/net-next/c/90de47f020db - [net-next,v11,2/6] page_pool: unify frag_count handling in page_pool_is_last_frag() (no matching commit) - [net-next,v11,3/6] page_pool: remove PP_FLAG_PAGE_FRAG (no matching commit) - [net-next,v11,4/6] page_pool: introduce page_pool[_cache]_alloc() API (no matching commit) - [net-next,v11,5/6] page_pool: update document about fragment API (no matching commit) - [net-next,v11,6/6] net: veth: use newly added page pool API for veth with xdp (no matching commit) You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-10-19 13:56 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-13 6:48 [PATCH net-next v11 0/6] introduce page_pool_alloc() related API Yunsheng Lin 2023-10-13 6:48 ` [PATCH net-next v11 3/6] page_pool: remove PP_FLAG_PAGE_FRAG Yunsheng Lin 2023-10-17 1:27 ` [PATCH net-next v11 0/6] introduce page_pool_alloc() related API Jakub Kicinski 2023-10-17 7:56 ` Yunsheng Lin 2023-10-17 15:13 ` Jakub Kicinski 2023-10-17 15:14 ` Jakub Kicinski 2023-10-18 11:47 ` Yunsheng Lin 2023-10-18 15:35 ` Jakub Kicinski 2023-10-19 13:22 ` Yunsheng Lin 2023-10-19 13:56 ` Jakub Kicinski 2023-10-17 4:10 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).