From: Aleksandr Nogikh <nogikh@google.com>
To: idosch@nvidia.com
Cc: davem@davemloft.net, edumazet@google.com,
gregkh@linuxfoundation.org, jiri@nvidia.com, kuba@kernel.org,
pabeni@redhat.com, sashal@kernel.org, stable@vger.kernel.org,
vkarri@nvidia.com, dvyukov@google.com
Subject: Re: Re: [PATCH stable 6.1] devlink: Fix RCU stall when unregistering a devlink instance
Date: Tue, 1 Oct 2024 18:47:59 +0200 [thread overview]
Message-ID: <20241001164759.469719-1-nogikh@google.com> (raw)
In-Reply-To: <Zvv7X7HgcQuFIVF1@shredder.lan>
Hi Ido,
On Tue, 1 Oct 2024 16:38:39 +0300, Ido Schimmel wrote:
>
> On Tue, Oct 01, 2024 at 02:11:27PM +0200, Greg KH wrote:
> > On Tue, Oct 01, 2024 at 02:20:35PM +0300, Ido Schimmel wrote:
> > > I read the stable rules and I am not providing an "upstream commit ID"
> > > since the code in upstream has been reworked, making this fix
> > > irrelevant. The only affected stable kernel is 6.1.y.
> >
> > You need to document the heck out of why this is only relevant for this
> > one specific kernel branch IN the changelog text, so that we understand
> > what is going on, AND you need to get acks from the relevant maintainers
> > of this area of the kernel to accept something that is not in Linus's
> > tree.
> >
> > But first of, why? Why not just take the upstrema commits instead?
>
> There were a lot of changes as part of the 6.3 cycle to completely
> rework the semantics of the devlink instance reference count. As part of
> these changes, commit d77278196441 ("devlink: bump the instance index
> directly when iterating") inadvertently fixed the bug mentioned in this
> patch. This commit cannot be applied to 6.1.y as-is because a prior
> commit (also in 6.3) moved the code to a different file (leftover.c ->
> core.c). There might be more dependencies that I'm currently unaware of.
>
> The alternative, proposed in this patch, is to provide a minimal and
> contained fix for the bug introduced in upstream commit c2368b19807a
> ("net: devlink: introduce "unregistering" mark and use it during
> devlinks iteration") as part of the 6.0 cycle.
>
> The above explains why the patch is only relevant to 6.1.y.
Thanks for bringing up this topic!
For what it's worth, syzbot would also greatly benefit from your fix:
https://github.com/google/syzkaller/issues/5328
I've built a kernel locally with your changes, run syzkaller against it,
and I can confirm that the kernel no longer crashes due to devlink.
--
Aleksandr
>
> Jakub / Jiri, what is your preference here? This patch or cherry picking
> a lot of code from 6.3?
next prev parent reply other threads:[~2024-10-01 16:48 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-01 11:20 [PATCH stable 6.1] devlink: Fix RCU stall when unregistering a devlink instance Ido Schimmel
2024-10-01 12:11 ` Greg KH
2024-10-01 13:38 ` Ido Schimmel
2024-10-01 16:47 ` Aleksandr Nogikh [this message]
2024-10-06 8:51 ` Ido Schimmel
2024-10-01 22:39 ` Jakub Kicinski
2024-10-06 8:44 ` Ido Schimmel
2024-10-06 10:11 ` Greg KH
2024-10-10 14:08 ` Sasha Levin
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=20241001164759.469719-1-nogikh@google.com \
--to=nogikh@google.com \
--cc=davem@davemloft.net \
--cc=dvyukov@google.com \
--cc=edumazet@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=idosch@nvidia.com \
--cc=jiri@nvidia.com \
--cc=kuba@kernel.org \
--cc=pabeni@redhat.com \
--cc=sashal@kernel.org \
--cc=stable@vger.kernel.org \
--cc=vkarri@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