netdev - Re: [net-next v3 00/10] page_pool: Add page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 2 Feb 2022 15:31:40 +0100
From:   Jesper Dangaard Brouer <jbrouer@...hat.com>
To:     Joe Damato <jdamato@...tly.com>, netdev@...r.kernel.org,
        kuba@...nel.org, ilias.apalodimas@...aro.org, davem@...emloft.net,
        hawk@...nel.org, Tariq Toukan <ttoukan.linux@...il.com>,
        Saeed Mahameed <saeed@...nel.org>
Cc:     brouer@...hat.com
Subject: Re: [net-next v3 00/10] page_pool: Add page_pool stat counters


Adding Cc. Tariq and Saeed, as they wanted page_pool stats in the past.

On 02/02/2022 02.12, Joe Damato wrote:
> Greetings:
> 
> Sending a v3 as I noted some issues with the procfs code in patch 10 I
> submit in v2 (thanks, kernel test robot) and fixing the placement of the
> refill stat increment in patch 8.

Could you explain why a single global stats (/proc/net/page_pool_stat) 
for all page_pool instances for all RX-queues makes sense?

I think this argument/explanation belongs in the cover letter.

What are you using this for?

And do Tariq and Saeeds agree with this single global stats approach?


> I only modified the placement of the refill stat, but decided to re-run the
> benchmarks used in the v2 [1], and the results are:

I appreciate that you are running the benchmarks.

> Test system:
> 	- 2x Intel(R) Xeon(R) Gold 6140 CPU @ 2.30GHz
> 	- 2 NUMA zones, with 18 cores per zone and 2 threads per core
> 
> bench_page_pool_simple results:
> test name			stats enabled		stats disabled
> 				cycles	nanosec		cycles	nanosec
> 
> for_loop			0	0.335		0	0.334

I think you can drop the 'for_loop' results, we can see that the 
overhead is insignificant.

> atomic_inc 			13	6.028		13	6.035
> lock				32	14.017		31	13.552
> 
> no-softirq-page_pool01		45	19.832		46	20.193
> no-softirq-page_pool02		44	19.478		46	20.083
> no-softirq-page_pool03		110	48.365		109	47.699
> 
> tasklet_page_pool01_fast_path	14	6.204		13	6.021
> tasklet_page_pool02_ptr_ring	41	18.115		42	18.699
> tasklet_page_pool03_slow	110	48.085		108	47.395
> 
> bench_page_pool_cross_cpu results:
> test name			stats enabled		stats disabled
> 				cycles	nanosec		cycles	nanosec
> 
> page_pool_cross_cpu CPU(0)	2216	966.179		2101	915.692
> page_pool_cross_cpu CPU(1)	2211	963.914		2159	941.087
> page_pool_cross_cpu CPU(2)	1108	483.097		1079	470.573
> 
> page_pool_cross_cpu average	1845	-		1779	-
> 
> v2 -> v3:
> 	- patch 8/10 ("Add stat tracking cache refill") fixed placement of
> 	  counter increment.
> 	- patch 10/10 ("net-procfs: Show page pool stats in proc") updated:
> 		- fix unused label warning from kernel test robot,
> 		- fixed page_pool_seq_show to only display the refill stat
> 		  once,
> 		- added a remove_proc_entry for page_pool_stat to
> 		  dev_proc_net_exit.
> 
> v1 -> v2:
> 	- A new kernel config option has been added, which defaults to N,
> 	   preventing this code from being compiled in by default
> 	- The stats structure has been converted to a per-cpu structure
> 	- The stats are now exported via proc (/proc/net/page_pool_stat)
> 
> Thanks.
> 
> [1]:
> https://lore.kernel.org/all/1643499540-8351-1-git-send-email-jdamato@fastly.com/T/#md82c6d5233e35bb518bc40c8fd7dff7a7a17e199
> 
> Joe Damato (10):
>    page_pool: kconfig: Add flag for page pool stats
>    page_pool: Add per-cpu page_pool_stats struct
>    page_pool: Add a macro for incrementing stats
>    page_pool: Add stat tracking fast path allocations
>    page_pool: Add slow path order 0 allocation stat
>    page_pool: Add slow path high order allocation stat
>    page_pool: Add stat tracking empty ring
>    page_pool: Add stat tracking cache refill
>    page_pool: Add a stat tracking waived pages
>    net-procfs: Show page pool stats in proc
> 
>   include/net/page_pool.h | 20 +++++++++++++++
>   net/Kconfig             | 12 +++++++++
>   net/core/net-procfs.c   | 67 +++++++++++++++++++++++++++++++++++++++++++++++++
>   net/core/page_pool.c    | 28 ++++++++++++++++++---
>   4 files changed, 124 insertions(+), 3 deletions(-)
>