bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Jesper Dangaard Brouer <hawk@kernel.org>
Cc: Dragos Tatulea <dtatulea@nvidia.com>,
	Chris Arges <carges@cloudflare.com>,
	Jesse Brandeburg <jbrandeburg@cloudflare.com>,
	netdev@vger.kernel.org, bpf@vger.kernel.org,
	kernel-team <kernel-team@cloudflare.com>,
	tariqt@nvidia.com, saeedm@nvidia.com,
	Leon Romanovsky <leon@kernel.org>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	John Fastabend <john.fastabend@gmail.com>,
	Simon Horman <horms@kernel.org>,
	Andrew Rzeznik <arzeznik@cloudflare.com>,
	Yan Zhai <yan@cloudflare.com>,
	Kumar Kartikeya Dwivedi <memxor@gmail.com>,
	Mina Almasry <almasrymina@google.com>
Subject: Re: [BUG] mlx5_core memory management issue
Date: Fri, 15 Aug 2025 09:36:44 -0700	[thread overview]
Message-ID: <20250815093644.5447c581@kernel.org> (raw)
In-Reply-To: <bd1a2f1f-20bb-4cc0-82ed-150c5e36b1da@kernel.org>

On Fri, 15 Aug 2025 18:02:10 +0200 Jesper Dangaard Brouer wrote:
> >> Yes, something like that, but I would like Kuba/Jakub's input, as IIRC
> >> he introduced the page_pool->cpuid and page_pool->napi.
> >>
> >> There are some corner-cases we need to consider if they are valid.  If
> >> cpumap get redirected to the *same* CPU as "previous" NAPI instance,
> >> which then makes page_pool->cpuid match, is it then still valid to do
> >> "direct" return(?).  
> > 
> > I think/hope so, but it depends on xdp_return only being called from
> > softirq context.. Since softirqs can't nest if producer and consumer
> > of the page pool pages are on the same CPU they can't race.  
> 
> That is true, softirqs can't nest.
> 
> Jesse pointed me at the tun device driver, where we in-principle are 
> missing a xdp_set_return_frame_no_direct() section. Except I believe, 
> that the memory type cannot be page_pool in this driver. (Code hint, 
> tun_xdp_act() calls xdp_do_redirect).
> 
> The tun driver made me realize, that we do have users that doesn't run 
> under a softirq, but they do remember to disable BH. (IIRC BH-disable 
> can nest).  Are we also race safe in this case(?).

Yes, it should be. But chances of direct recycling happening in this
case are rather low since NAPI needs to be pending to be considered
owned. If we're coming from process context BHs are likely not pending.

> Is the code change as simple as below or did I miss something?
> 
> void __xdp_return
>    [...]
>    case MEM_TYPE_PAGE_POOL:
>     [...]
>      if (napi_direct && READ_ONCE(pool->cpuid) != smp_processor_id())
> 	napi_direct = false;

cpuid is a different beast, NAPI-based direct recycling logic is in
page_pool_napi_local() (and we should not let it leak out to XDP,
just unref the page and PP will "override" the "napi_safe" argument).

> It is true, that when we exit NAPI, then pool->cpuid becomes -1.
> Or what that only during shutdown?

      reply	other threads:[~2025-08-15 16:36 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-03 15:49 [BUG] mlx5_core memory management issue Chris Arges
2025-07-04 12:37 ` Dragos Tatulea
2025-07-04 20:14   ` Dragos Tatulea
2025-07-07 22:07     ` Chris Arges
2025-07-23 18:48   ` Chris Arges
2025-07-24 17:01     ` Dragos Tatulea
2025-08-07 16:45       ` Chris Arges
2025-08-11  8:37         ` Dragos Tatulea
2025-08-12 15:44           ` Dragos Tatulea
2025-08-12 18:55             ` Jesse Brandeburg
2025-08-12 20:19               ` Dragos Tatulea
2025-08-12 21:25                 ` Chris Arges
2025-08-13 18:53                   ` Chris Arges
2025-08-13 19:26                     ` Dragos Tatulea
2025-08-13 20:24                       ` Dragos Tatulea
2025-08-14 11:26                         ` Jesper Dangaard Brouer
2025-08-14 14:42                           ` Dragos Tatulea
2025-08-14 15:58                             ` Jesper Dangaard Brouer
2025-08-14 16:45                               ` Dragos Tatulea
2025-08-15 14:59                               ` Jakub Kicinski
2025-08-15 16:02                                 ` Jesper Dangaard Brouer
2025-08-15 16:36                                   ` Jakub Kicinski [this message]

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=20250815093644.5447c581@kernel.org \
    --to=kuba@kernel.org \
    --cc=almasrymina@google.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=arzeznik@cloudflare.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=carges@cloudflare.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=dtatulea@nvidia.com \
    --cc=edumazet@google.com \
    --cc=hawk@kernel.org \
    --cc=horms@kernel.org \
    --cc=jbrandeburg@cloudflare.com \
    --cc=john.fastabend@gmail.com \
    --cc=kernel-team@cloudflare.com \
    --cc=leon@kernel.org \
    --cc=memxor@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=saeedm@nvidia.com \
    --cc=tariqt@nvidia.com \
    --cc=yan@cloudflare.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).