Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Dan Carpenter <dan.carpenter@linaro.org>]

On Thu, Jul 18, 2024 at 09:34:04AM +0200, Takashi Iwai wrote:
> On Thu, 18 Jul 2024 00:05:06 +0200,
> Dan Carpenter wrote:
> > 
> > On Tue, Jul 16, 2024 at 02:20:42PM +0200, Takashi Iwai wrote:
> > > > > > The one question I have is for patches which pre-date git.  I was told
> > > > > > to leave the Fixes tag off in that case, but I think that's out of date.
> > > > > > It's more useful to say "Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")".
> > > > > 
> > > > > If a thing was there since before git and only now is being discovered
> > > > > it either needs to be explicitly marked for stable with cc: or we can
> > > > > just keep the fix in newer kernels. IMO this particular "Fixes" tag does
> > > > > not bring any value.
> > > > 
> > > > On the contrary, if I see just a "cc: stable" and no "Fixes:" tag, I do
> > > > a "best effort" to backport to all current stable/lts trees.  If it
> > > > fails on 5.15 and older, great, I don't warn anyone about that as there
> > > > was no Fixes: tag for me to know how far back it should go.
> > > 
> > > That's what I expected for the cases I put only Cc-to-stable; they
> > > have been mostly some trivial quirks only for new devices, and the
> > > stuff that can be taken safely when applicable -- otherwise just
> > > skip.
> > > 
> > 
> > That's the same thing that happens if you add the Fixes: 1da177e4c3f4
> > ("Linux-2.6.12-rc2") tag.  Greg and Sasha aren't manually backporting
> > patches unless they seem security related.  When a patch doesn't
> > backport cleanly, the stable maintainers rely on other people to backport
> > patches they care about.
> 
> Well, in my case, it's not always Fixes:Linux-2.6.12-rc2; the relevant
> code has been often added later.  But pointing somewhere new as Fixes
> also won't work, since there are other dependent fixes.  And tracking
> all those is really hard, and not worthy.

One thing that I think is that Fixes tags aren't really fair.  For
example, imagine someone sends an "Add bounds checking" patch but the
bounds checking has an integer overflow.  The "Fix integer overflow"
patch will point to the bounds checking patch even though the bug was
there in the original code as well.

Blaming the second patch works from a review perspective because
we can see why the integer overflow was added.  It also works from a
backporting perspective because the patches depend on each other.

Unfortunately, from just looking at the tags someone might think that
the "Add bounds checking" patch added a regression.  For example, LWN
sometimes looks at how many regressions are added to stable kernels.
(The LWN numbers are always going to be approximations it's fine.  I
love LWN).  But I've seen people complain about the unfairness of
assigning fixes tags this way and I just think that from a practical
perspective it's the only thing which makes sense.

> It's a thing that can be
> backported safely whenever it's cleanly applied and built, but
> otherwise it can be simply skipped.

That's how it's going to work either way regardless of if there is a
Fixes tag.

> It's no "regression", per se, but
> a new feature that didn't exist in the past, after all.
> 

If it's not a regression then don't add a Fixes tag.

regards,
dan carpenter