netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jesper Dangaard Brouer <hawk@kernel.org>
To: Dragos Tatulea <dtatulea@nvidia.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>,
	Ilias Apalodimas <ilias.apalodimas@linaro.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Clark Williams <clrkwllms@kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Mina Almasry <almasrymina@google.com>
Cc: netdev@vger.kernel.org, Tariq Toukan <tariqt@nvidia.com>,
	linux-kernel@vger.kernel.org, linux-rt-devel@lists.linux.dev
Subject: Re: [PATCH net-next] page_pool: add debug for release to cache from wrong CPU
Date: Sat, 20 Sep 2025 19:36:49 +0200	[thread overview]
Message-ID: <182ceffb-b038-4c4f-9c3b-383351a043d5@kernel.org> (raw)
In-Reply-To: <20250918084823.372000-1-dtatulea@nvidia.com>



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@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(-)
> 
[...]

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

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").

--Jesper

  parent reply	other threads:[~2025-09-20 17:36 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-18  8:48 [PATCH net-next] page_pool: add debug for release to cache from wrong CPU Dragos Tatulea
2025-09-18  9:13 ` Sebastian Andrzej Siewior
2025-09-18  9:51   ` Dragos Tatulea
2025-09-19 23:57 ` Jakub Kicinski
2025-09-20  9:25   ` Dragos Tatulea
2025-09-22 23:18     ` Jakub Kicinski
2025-09-23 15:23       ` Dragos Tatulea
2025-09-23 15:34         ` Jakub Kicinski
2025-09-23 16:00           ` Dragos Tatulea
2025-09-23 23:26             ` Jakub Kicinski
2025-09-20 17:36 ` Jesper Dangaard Brouer [this message]
2025-09-22 17:18   ` Dragos Tatulea

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=182ceffb-b038-4c4f-9c3b-383351a043d5@kernel.org \
    --to=hawk@kernel.org \
    --cc=almasrymina@google.com \
    --cc=bigeasy@linutronix.de \
    --cc=clrkwllms@kernel.org \
    --cc=davem@davemloft.net \
    --cc=dtatulea@nvidia.com \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=ilias.apalodimas@linaro.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-devel@lists.linux.dev \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=tariqt@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).