netdev - Re: [PATCH net-next v6 8/8] page_pool: use list instead of array for alloc cache

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]

Message-ID: <20250107120344.GI33144@kernel.org>
Date: Tue, 7 Jan 2025 12:03:44 +0000
From: Simon Horman <horms@...nel.org>
To: Yunsheng Lin <linyunsheng@...wei.com>
Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
	liuyonglong@...wei.com, fanghaiqing@...wei.com,
	zhangkun09@...wei.com, Robin Murphy <robin.murphy@....com>,
	Alexander Duyck <alexander.duyck@...il.com>,
	IOMMU <iommu@...ts.linux.dev>,
	Jesper Dangaard Brouer <hawk@...nel.org>,
	Ilias Apalodimas <ilias.apalodimas@...aro.org>,
	Eric Dumazet <edumazet@...gle.com>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v6 8/8] page_pool: use list instead of array for
 alloc cache

On Mon, Jan 06, 2025 at 09:01:16PM +0800, Yunsheng Lin wrote:
> As the alloc cache is always protected by NAPI context
> protection, use encoded_next as a pointer to a next item
> to avoid the using the array.
> 
> Testing shows there is about 3ns improvement for the
> performance of 'time_bench_page_pool01_fast_path' test
> case.
> 
> CC: Robin Murphy <robin.murphy@....com>
> CC: Alexander Duyck <alexander.duyck@...il.com>
> CC: IOMMU <iommu@...ts.linux.dev>
> Signed-off-by: Yunsheng Lin <linyunsheng@...wei.com>

...

> diff --git a/net/core/page_pool.c b/net/core/page_pool.c

...

> @@ -677,10 +698,12 @@ static void __page_pool_return_page(struct page_pool *pool, netmem_ref netmem,
>  
>  static noinline netmem_ref page_pool_refill_alloc_cache(struct page_pool *pool)
>  {
> -	struct page_pool_item *refill;
> +	struct page_pool_item *refill, *alloc, *curr;
>  	netmem_ref netmem;
>  	int pref_nid; /* preferred NUMA node */
>  
> +	DEBUG_NET_WARN_ON_ONCE(pool->alloc.count || pool->alloc.list);
> +
>  	/* Quicker fallback, avoid locks when ring is empty */
>  	refill = pool->alloc.refill;
>  	if (unlikely(!refill && !READ_ONCE(pool->ring.list))) {
> @@ -698,6 +721,7 @@ static noinline netmem_ref page_pool_refill_alloc_cache(struct page_pool *pool)
>  	pref_nid = numa_mem_id(); /* will be zero like page_to_nid() */
>  #endif
>  
> +	alloc = NULL;
>  	/* Refill alloc array, but only if NUMA match */
>  	do {
>  		if (unlikely(!refill)) {
> @@ -706,10 +730,13 @@ static noinline netmem_ref page_pool_refill_alloc_cache(struct page_pool *pool)
>  				break;
>  		}
>  
> +		curr = refill;
>  		netmem = refill->pp_netmem;
>  		refill = page_pool_item_get_next(refill);
>  		if (likely(netmem_is_pref_nid(netmem, pref_nid))) {
> -			pool->alloc.cache[pool->alloc.count++] = netmem;
> +			page_pool_item_set_next(curr, alloc);
> +			pool->alloc.count++;
> +			alloc = curr;
>  		} else {
>  			/* NUMA mismatch;
>  			 * (1) release 1 page to page-allocator and
> @@ -729,7 +756,8 @@ static noinline netmem_ref page_pool_refill_alloc_cache(struct page_pool *pool)
>  	/* Return last page */
>  	if (likely(pool->alloc.count > 0)) {
>  		atomic_sub(pool->alloc.count, &pool->ring.count);
> -		netmem = pool->alloc.cache[--pool->alloc.count];
> +		pool->alloc.list = page_pool_item_get_next(alloc);
> +		pool->alloc.count--;
>  		alloc_stat_inc(pool, refill);
>  	}
>  

Hi Yunsheng Lin,

The following line of the code looks like this:

	return netmem;

And, with this patch applied, Smatch warns that netmem may be used
uninitialised here. I assume this is because it is no longer conditionally
initialised above.

...