linux-kernel - Re: [PATCH net-next] page_pool: add debug for release to cache from wrong CPU

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: <n4umixor7ph2gnmc5osxaenksd6eybeuntljeywudi5h7ctamy@qfqlucvdcihq>
Date: Mon, 22 Sep 2025 17:18:14 +0000
From: Dragos Tatulea <dtatulea@...dia.com>
To: Jesper Dangaard Brouer <hawk@...nel.org>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, 
	Ilias Apalodimas <ilias.apalodimas@...aro.org>, Sebastian Andrzej Siewior <bigeasy@...utronix.de>, 
	Clark Williams <clrkwllms@...nel.org>, Steven Rostedt <rostedt@...dmis.org>, 
	Mina Almasry <almasrymina@...gle.com>
Cc: netdev@...r.kernel.org, Tariq Toukan <tariqt@...dia.com>, 
	linux-kernel@...r.kernel.org, linux-rt-devel@...ts.linux.dev
Subject: Re: [PATCH net-next] page_pool: add debug for release to cache from
 wrong CPU

On Sat, Sep 20, 2025 at 07:36:49PM +0200, Jesper Dangaard Brouer wrote:
> 
> 
> On 18/09/2025 10.48, Dragos Tatulea wrote:
> > Direct page releases to cache must be done on the same CPU as where NAPI
> > is running. Not doing so results in races that are quite difficult to
> > debug.
> > 
> > This change adds a debug configuration which issues a warning when
> > such buggy behaviour is encountered.
> > 
> > Signed-off-by: Dragos Tatulea<dtatulea@...dia.com>
> > Reviewed-by: Tariq Toukan<tariqt@...dia.com>
> > ---
> >   net/Kconfig.debug    | 10 +++++++
> >   net/core/page_pool.c | 66 ++++++++++++++++++++++++++------------------
> >   2 files changed, 49 insertions(+), 27 deletions(-)
> > 
> [...]
> 
> > @@ -768,6 +795,18 @@ static bool page_pool_recycle_in_cache(netmem_ref netmem,
> >   		return false;
> >   	}
> > +#ifdef CONFIG_DEBUG_PAGE_POOL_CACHE_RELEASE
> > +	if (unlikely(!page_pool_napi_local(pool))) {
> > +		u32 pp_cpuid = READ_ONCE(pool->cpuid);
> > +		u32 cpuid = smp_processor_id();
> > +
> > +		WARN_RATELIMIT(1, "page_pool %d: direct page release from wrong CPU %d, expected CPU %d",
> > +			       pool->user.id, cpuid, pp_cpuid);
> > +
> > +		return false;
> > +	}
> > +#endif
> 
> The page_pool_recycle_in_cache() is an extreme fast-path for page_pool.
> I know this is a debugging patch, but I would like to know the overhead
> this adds (when enabled, compared to not enabled).
> 
> We (Mina) recently added a benchmark module for page_pool
> under tools/testing/selftests/net/bench/page_pool/ that you can use.
>
Is there a way to trigger the fast-path? It doesn't seem to run
in_softirq() ...

I will also have to do some additional configuration so that
page_pool_napi_local() doesn't return false. I assume that we want to
test the perf of the non-erroneous case. Right?

> Adding a WARN in fast-path code need extra careful review (maybe is it
> okay here), this is because it adds an asm instruction (on Intel CPUs
> ud2) what influence instruction cache prefetching.  Looks like this only
> gets inlined two places (page_pool_put_unrefed_netmem and
> page_pool_put_page_bulk), and it might be okay... I think it is.
> See how I worked around this in commit 34cc0b338a61 ("xdp: Xdp_frame add
> member frame_sz and handle in convert_to_xdp_frame").
>
Thanks for the explanation and the pointer. Will do this in the next
version.

Thanks,
Dragos