From: Mina Almasry <almasrymina@google.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Dragos Tatulea <dtatulea@nvidia.com>,
"davem@davemloft.net" <davem@davemloft.net>,
"ilias.apalodimas@linaro.org" <ilias.apalodimas@linaro.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"jacob.e.keller@intel.com" <jacob.e.keller@intel.com>,
"pabeni@redhat.com" <pabeni@redhat.com>,
Jianbo Liu <jianbol@nvidia.com>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"edumazet@google.com" <edumazet@google.com>
Subject: Re: [RFC PATCH] net: Fix one page_pool page leak from skb_frag_unref
Date: Wed, 1 May 2024 09:23:20 -0700 [thread overview]
Message-ID: <CAHS8izO2PT3ZrDdHhNOZ7+AL_SKZfDko9ZUWuTd8O84bjOVDHg@mail.gmail.com> (raw)
In-Reply-To: <20240501072434.5720fd42@kernel.org>
On Wed, May 1, 2024 at 7:24 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 1 May 2024 00:48:43 -0700 Mina Almasry wrote:
> > > 1) Revert commit 2cc3aeb5eccc
> > > ("skbuff: Fix a potential race while recycling page_pool packets"). I tested
> > > this btw and it works (for this specific scenario).
> > >
> > > 2) Revert Mina's commit a580ea994fd3 ("net: mirror skb frag ref/unref helpers")
> > > for now.
> >
> > I vote for #1, and IIUC Jakub's feedback, he seems to prefer this as
> > well.
>
> I vote #2, actually :( Or #3 make page pool ref safe to acquire
> concurrently, but that plus fixing all the places where we do crazy
> things may be tricky.
>
> Even taking the ref is not as simple as using atomic_long_inc_not_zero()
> sadly, partly because we try to keep the refcount at one, in an apparent
> attempt to avoid dirtying the cache line twice.
>
> So maybe partial revert to stop be bleeding and retry after more testing
> is the way to go?
>
OK, I will upload a revert sometime today.
> I had a quick look at the code and there is also a bunch of functions
> which "shift" frags from one skb to another, without checking whether
> the pp_recycle state matches.
You posted a diff, I will pick it up in a separate patch.
--
Thanks,
Mina
next prev parent reply other threads:[~2024-05-01 16:23 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-24 16:56 [RFC PATCH] net: Fix one page_pool page leak from skb_frag_unref Dragos Tatulea
2024-04-24 17:25 ` Dragos Tatulea
2024-04-24 22:08 ` Mina Almasry
2024-04-25 8:17 ` Dragos Tatulea
2024-04-25 19:20 ` Mina Almasry
2024-04-25 19:47 ` Dragos Tatulea
2024-04-25 20:42 ` Mina Almasry
2024-04-26 14:59 ` Dragos Tatulea
2024-04-26 19:03 ` Mina Almasry
2024-04-26 23:08 ` Jakub Kicinski
2024-04-27 4:24 ` Mina Almasry
2024-04-29 15:00 ` Jakub Kicinski
2024-05-02 20:08 ` Mina Almasry
2024-05-03 0:46 ` Jakub Kicinski
2024-04-26 23:05 ` Jakub Kicinski
2024-04-29 7:39 ` Dragos Tatulea
2024-05-01 6:20 ` Dragos Tatulea
2024-05-01 7:48 ` Mina Almasry
2024-05-01 7:58 ` Dragos Tatulea
2024-05-01 14:24 ` Jakub Kicinski
2024-05-01 14:28 ` Jakub Kicinski
2024-05-01 16:23 ` Mina Almasry [this message]
2024-04-26 23:07 ` Jakub Kicinski
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=CAHS8izO2PT3ZrDdHhNOZ7+AL_SKZfDko9ZUWuTd8O84bjOVDHg@mail.gmail.com \
--to=almasrymina@google.com \
--cc=davem@davemloft.net \
--cc=dtatulea@nvidia.com \
--cc=edumazet@google.com \
--cc=ilias.apalodimas@linaro.org \
--cc=jacob.e.keller@intel.com \
--cc=jianbol@nvidia.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.