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?