On Sun, Oct 06, 2024 at 11:44:42AM +0300, Ido Schimmel wrote:
> On Tue, Oct 01, 2024 at 03:39:53PM -0700, Jakub Kicinski wrote:
> > On Tue, 1 Oct 2024 16:38:39 +0300 Ido Schimmel wrote:
> > > > 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.
> > >
> > > Jakub / Jiri, what is your preference here? This patch or cherry picking
> > > a lot of code from 6.3?
> >
> > No preference here. The fix as posted looks correct. The backport of
> > the upstream commit should be correct too (I don't see any
> > incompatibilities) but as you said the code has moved and got exposed
> > via a header, so the diff will look quite different.
> >
> > I think Greg would still prefer to use the bastardized upstream commit
> > in such cases.
>
> Greg, if I augment the commit message with the necessary information,
> would you be willing to take this patch instead of a much larger patch?
I almost always want to take whatever is in Linus's tree to ensure that
it will be easier to maintain over time due to other changes needing to
happen in the same area over the next 5+ years. ALSO, almost always,
without fail, whenever we take code that is NOT in Linus's tree, it's
wrong, and it needs to be fixed up again as it's going outside of our
normal development and review and testing processes.
So unless there is some MAJOR reason why we can't just take what is in
Linus's tree, I strongly prefer that. It is trivial for us to take 10's
and even 100's of patches of backports over a one-off change that is
going to end up costing us more work and effort over time.
thanks,
greg k-h
