From: Jie Wang <[email protected]>
Currently NIC packet receiving performance based on page pool deteriorates
occasionally. To analysis the causes of this problem page allocation stats
are collected. Here are the stats when NIC rx performance deteriorates:
bandwidth(Gbits/s) 16.8 6.91
rx_pp_alloc_fast 13794308 21141869
rx_pp_alloc_slow 108625 166481
rx_pp_alloc_slow_h 0 0
rx_pp_alloc_empty 8192 8192
rx_pp_alloc_refill 0 0
rx_pp_alloc_waive 100433 158289
rx_pp_recycle_cached 0 0
rx_pp_recycle_cache_full 0 0
rx_pp_recycle_ring 362400 420281
rx_pp_recycle_ring_full 6064893 9709724
rx_pp_recycle_released_ref 0 0
The rx_pp_alloc_waive count indicates that a large number of pages' numa
node are inconsistent with the NIC device numa node. Therefore these pages
can't be reused by the page pool. As a result, many new pages would be
allocated by __page_pool_alloc_pages_slow which is time consuming. This
causes the NIC rx performance fluctuations.
The main reason of huge numa mismatch pages in page pool is that page pool
uses alloc_pages_bulk_array to allocate original pages. This function is
not suitable for page allocation in NUMA scenario. So this patch uses
alloc_pages_bulk_array_node which has a NUMA id input parameter to ensure
the NUMA consistent between NIC device and allocated pages.
Repeated NIC rx performance tests are performed 40 times. NIC rx bandwidth
is higher and more stable compared to the datas above. Here are three test
stats, the rx_pp_alloc_waive count is zero and rx_pp_alloc_slow which
indicates pages allocated from slow patch is relatively low.
bandwidth(Gbits/s) 93 93.9 93.8
rx_pp_alloc_fast 60066264 61266386 60938254
rx_pp_alloc_slow 16512 16517 16539
rx_pp_alloc_slow_ho 0 0 0
rx_pp_alloc_empty 16512 16517 16539
rx_pp_alloc_refill 473841 481910 481585
rx_pp_alloc_waive 0 0 0
rx_pp_recycle_cached 0 0 0
rx_pp_recycle_cache_full 0 0 0
rx_pp_recycle_ring 29754145 30358243 30194023
rx_pp_recycle_ring_full 0 0 0
rx_pp_recycle_released_ref 0 0 0
Signed-off-by: Jie Wang <[email protected]>
---
net/core/page_pool.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index f18e6e771993..15997fcd78f3 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -377,6 +377,7 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool,
unsigned int pp_order = pool->p.order;
struct page *page;
int i, nr_pages;
+ int pref_nid; /* preferred NUMA node */
/* Don't support bulk alloc for high-order pages */
if (unlikely(pp_order))
@@ -386,10 +387,18 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool,
if (unlikely(pool->alloc.count > 0))
return pool->alloc.cache[--pool->alloc.count];
+#ifdef CONFIG_NUMA
+ pref_nid = (pool->p.nid == NUMA_NO_NODE) ? numa_mem_id() : pool->p.nid;
+#else
+ /* Ignore pool->p.nid setting if !CONFIG_NUMA, helps compiler */
+ pref_nid = numa_mem_id(); /* will be zero like page_to_nid() */
+#endif
+
/* 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(gfp, bulk, pool->alloc.cache);
+ nr_pages = alloc_pages_bulk_array_node(gfp, pref_nid, bulk,
+ pool->alloc.cache);
if (unlikely(!nr_pages))
return NULL;
--
2.33.0
On 24/06/2022 11.36, Guangbin Huang wrote:
> From: Jie Wang <[email protected]>
>
> Currently NIC packet receiving performance based on page pool deteriorates
> occasionally. To analysis the causes of this problem page allocation stats
> are collected. Here are the stats when NIC rx performance deteriorates:
>
> bandwidth(Gbits/s) 16.8 6.91
> rx_pp_alloc_fast 13794308 21141869
> rx_pp_alloc_slow 108625 166481
> rx_pp_alloc_slow_h 0 0
> rx_pp_alloc_empty 8192 8192
> rx_pp_alloc_refill 0 0
> rx_pp_alloc_waive 100433 158289
> rx_pp_recycle_cached 0 0
> rx_pp_recycle_cache_full 0 0
> rx_pp_recycle_ring 362400 420281
> rx_pp_recycle_ring_full 6064893 9709724
> rx_pp_recycle_released_ref 0 0
>
> The rx_pp_alloc_waive count indicates that a large number of pages' numa
> node are inconsistent with the NIC device numa node. Therefore these pages
> can't be reused by the page pool. As a result, many new pages would be
> allocated by __page_pool_alloc_pages_slow which is time consuming. This
> causes the NIC rx performance fluctuations.
>
> The main reason of huge numa mismatch pages in page pool is that page pool
> uses alloc_pages_bulk_array to allocate original pages. This function is
> not suitable for page allocation in NUMA scenario. So this patch uses
> alloc_pages_bulk_array_node which has a NUMA id input parameter to ensure
> the NUMA consistent between NIC device and allocated pages.
>
> Repeated NIC rx performance tests are performed 40 times. NIC rx bandwidth
> is higher and more stable compared to the datas above. Here are three test
> stats, the rx_pp_alloc_waive count is zero and rx_pp_alloc_slow which
> indicates pages allocated from slow patch is relatively low.
>
> bandwidth(Gbits/s) 93 93.9 93.8
> rx_pp_alloc_fast 60066264 61266386 60938254
> rx_pp_alloc_slow 16512 16517 16539
> rx_pp_alloc_slow_ho 0 0 0
> rx_pp_alloc_empty 16512 16517 16539
> rx_pp_alloc_refill 473841 481910 481585
> rx_pp_alloc_waive 0 0 0
> rx_pp_recycle_cached 0 0 0
> rx_pp_recycle_cache_full 0 0 0
> rx_pp_recycle_ring 29754145 30358243 30194023
> rx_pp_recycle_ring_full 0 0 0
> rx_pp_recycle_released_ref 0 0 0
>
> Signed-off-by: Jie Wang <[email protected]>
> ---
> net/core/page_pool.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
Thanks for improving this, but we need some small adjustments below.
And then you need to send a V2 of the patch.
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index f18e6e771993..15997fcd78f3 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -377,6 +377,7 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool,
> unsigned int pp_order = pool->p.order;
> struct page *page;
> int i, nr_pages;
> + int pref_nid; /* preferred NUMA node */
>
> /* Don't support bulk alloc for high-order pages */
> if (unlikely(pp_order))
> @@ -386,10 +387,18 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool,
> if (unlikely(pool->alloc.count > 0))
> return pool->alloc.cache[--pool->alloc.count];
>
> +#ifdef CONFIG_NUMA
> + pref_nid = (pool->p.nid == NUMA_NO_NODE) ? numa_mem_id() : pool->p.nid;
> +#else
> + /* Ignore pool->p.nid setting if !CONFIG_NUMA, helps compiler */
Remove "helps compiler" from comments, it only make sense in the code
this was copy-pasted from.
> + pref_nid = numa_mem_id(); /* will be zero like page_to_nid() */
The comment about "page_to_nid()" is only relevant in the code
this was copy-pasted from.
Change to:
pref_nid = NUMA_NO_NODE;
As alloc_pages_bulk_array_node() will be inlined, the effect (generated
asm code) will be the same, but it will be better for code maintenance.
> +#endif
> +
> /* 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(gfp, bulk, pool->alloc.cache);
> + nr_pages = alloc_pages_bulk_array_node(gfp, pref_nid, bulk,
> + pool->alloc.cache);
> if (unlikely(!nr_pages))
> return NULL;
>
On 2022/6/27 17:50, Jesper Dangaard Brouer wrote:
>
>
> On 24/06/2022 11.36, Guangbin Huang wrote:
>> From: Jie Wang <[email protected]>
>>
>> Currently NIC packet receiving performance based on page pool
>> deteriorates
>> occasionally. To analysis the causes of this problem page allocation
>> stats
>> are collected. Here are the stats when NIC rx performance deteriorates:
>>
>> bandwidth(Gbits/s) 16.8 6.91
>> rx_pp_alloc_fast 13794308 21141869
>> rx_pp_alloc_slow 108625 166481
>> rx_pp_alloc_slow_h 0 0
>> rx_pp_alloc_empty 8192 8192
>> rx_pp_alloc_refill 0 0
>> rx_pp_alloc_waive 100433 158289
>> rx_pp_recycle_cached 0 0
>> rx_pp_recycle_cache_full 0 0
>> rx_pp_recycle_ring 362400 420281
>> rx_pp_recycle_ring_full 6064893 9709724
>> rx_pp_recycle_released_ref 0 0
>>
>> The rx_pp_alloc_waive count indicates that a large number of pages' numa
>> node are inconsistent with the NIC device numa node. Therefore these
>> pages
>> can't be reused by the page pool. As a result, many new pages would be
>> allocated by __page_pool_alloc_pages_slow which is time consuming. This
>> causes the NIC rx performance fluctuations.
>>
>> The main reason of huge numa mismatch pages in page pool is that page
>> pool
>> uses alloc_pages_bulk_array to allocate original pages. This function is
>> not suitable for page allocation in NUMA scenario. So this patch uses
>> alloc_pages_bulk_array_node which has a NUMA id input parameter to ensure
>> the NUMA consistent between NIC device and allocated pages.
>>
>> Repeated NIC rx performance tests are performed 40 times. NIC rx
>> bandwidth
>> is higher and more stable compared to the datas above. Here are three
>> test
>> stats, the rx_pp_alloc_waive count is zero and rx_pp_alloc_slow which
>> indicates pages allocated from slow patch is relatively low.
>>
>> bandwidth(Gbits/s) 93 93.9 93.8
>> rx_pp_alloc_fast 60066264 61266386 60938254
>> rx_pp_alloc_slow 16512 16517 16539
>> rx_pp_alloc_slow_ho 0 0 0
>> rx_pp_alloc_empty 16512 16517 16539
>> rx_pp_alloc_refill 473841 481910 481585
>> rx_pp_alloc_waive 0 0 0
>> rx_pp_recycle_cached 0 0 0
>> rx_pp_recycle_cache_full 0 0 0
>> rx_pp_recycle_ring 29754145 30358243 30194023
>> rx_pp_recycle_ring_full 0 0 0
>> rx_pp_recycle_released_ref 0 0 0
>>
>> Signed-off-by: Jie Wang <[email protected]>
>> ---
>> net/core/page_pool.c | 11 ++++++++++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> Thanks for improving this, but we need some small adjustments below.
> And then you need to send a V2 of the patch.
>
>> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
>> index f18e6e771993..15997fcd78f3 100644
>> --- a/net/core/page_pool.c
>> +++ b/net/core/page_pool.c
>> @@ -377,6 +377,7 @@ static struct page
>> *__page_pool_alloc_pages_slow(struct page_pool *pool,
>> unsigned int pp_order = pool->p.order;
>> struct page *page;
>> int i, nr_pages;
>> + int pref_nid; /* preferred NUMA node */
>> /* Don't support bulk alloc for high-order pages */
>> if (unlikely(pp_order))
>> @@ -386,10 +387,18 @@ static struct page
>> *__page_pool_alloc_pages_slow(struct page_pool *pool,
>> if (unlikely(pool->alloc.count > 0))
>> return pool->alloc.cache[--pool->alloc.count];
>> +#ifdef CONFIG_NUMA
>> + pref_nid = (pool->p.nid == NUMA_NO_NODE) ? numa_mem_id() :
>> pool->p.nid;
>> +#else
>> + /* Ignore pool->p.nid setting if !CONFIG_NUMA, helps compiler */
>
> Remove "helps compiler" from comments, it only make sense in the code
> this was copy-pasted from.
>
>
>> + pref_nid = numa_mem_id(); /* will be zero like page_to_nid() */
>
> The comment about "page_to_nid()" is only relevant in the code
> this was copy-pasted from.
>
> Change to:
> pref_nid = NUMA_NO_NODE;
>
> As alloc_pages_bulk_array_node() will be inlined, the effect (generated
> asm code) will be the same, but it will be better for code maintenance.
>
OKļ¼thanks for your review, I will fix it in next version.
>> +#endif
>> +
>> /* 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(gfp, bulk, pool->alloc.cache);
>> + nr_pages = alloc_pages_bulk_array_node(gfp, pref_nid, bulk,
>> + pool->alloc.cache);
>> if (unlikely(!nr_pages))
>> return NULL;
>>
>
>
> .
>