On Sat, Sep 20, 2025 at 07:36:49PM +0200, Jesper Dangaard Brouer wrote:

On 18/09/2025 10.48, Dragos Tatulea wrote:

quoted

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@nvidia.com>
Reviewed-by: Tariq Toukan<tariqt@nvidia.com>
---
  net/Kconfig.debug    | 10 +++++++
  net/core/page_pool.c | 66 ++++++++++++++++++++++++++------------------
  2 files changed, 49 insertions(+), 27 deletions(-)

[...]

quoted

@@ -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