Proposal: Enhancing Commit Tagging for Stable Kernel Branches

Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Sasha Levin]

Hi folks,

The Linux kernel community relies heavily on commit tags to identify and
manage patches destined for stable kernel branches. Currently, we use a
"Stable tag" (cc: stable@kernel.org) to indicate that a patch should be
included in stable kernel branches, and a "Fixes tag" (Fixes:
012345678901 ("commit subject")) to point to an older commit that the
new commit fixes or improves. However, this scheme has led to some
unintended consequences.

One of the main issues is that most Fixes-tagged commits (>80%) end up
in a stable tree, leading some authors to omit the Stable tag
altogether. This means we may not be trying hard enough to include
critical commits in stable kernel branches. On the other hand, some
authors are unhappy when commits without a Stable tag end up in a stable
kernel branch. To address these shortcomings, I propose introducing an
"Improves tag" (Improves: 012345678901 ("commit subject")) and altering
the meaning of the Fixes tag.

The Improves tag would indicate that a commit improves something but
does not necessarily fix an issue. This new tag would imply that the
commit should not be included in a stable kernel branch, unless it's
needed as a dependency for a later commit. In contrast, the Fixes tag
would henceforth carry the same meaning as if the Stable tag were
present, ensuring that critical fixes are properly identified and
prioritized.

By introducing the Improves tag and updating the semantics of the Fixes
tag, we can provide authors with more nuanced options for describing
their commits. This change would also allow us to slowly deprecate the
Stable tag, making it easier on authors by having one less tag to add.
The recently introduced "not for stable tag" (Cc:
<stable+noautosel@kernel.org>) could still be used in cases where an
author explicitly wants to exclude their commit from a stable kernel
branch.

I believe this proposal would improve the overall quality and
consistency of our commit tagging scheme, making it easier for authors
to provide accurate metadata about their commits. This, in turn, would
facilitate more efficient management of patches destined for stable
kernel branches. I invite feedback and discussion from the Linux kernel
community on this proposal.

-- 
Thanks,
Sasha

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[James Bottomley]

On Sun, 2024-07-14 at 08:31 -0400, Sasha Levin wrote:
> Hi folks,
> 
> The Linux kernel community relies heavily on commit tags to identify
> and manage patches destined for stable kernel branches. Currently, we
> use a "Stable tag" (cc: stable@kernel.org) to indicate that a patch
> should be included in stable kernel branches, and a "Fixes tag"
> (Fixes: 012345678901 ("commit subject")) to point to an older commit
> that the new commit fixes or improves. However, this scheme has led
> to some unintended consequences.
> 
> One of the main issues is that most Fixes-tagged commits (>80%) end
> up in a stable tree, leading some authors to omit the Stable tag
> altogether. This means we may not be trying hard enough to include
> critical commits in stable kernel branches. On the other hand, some
> authors are unhappy when commits without a Stable tag end up in a
> stable kernel branch. To address these shortcomings, I propose
> introducing an "Improves tag" (Improves: 012345678901 ("commit
> subject")) and altering the meaning of the Fixes tag.

I've got to say this looks like a bad idea: your complaint is we're not
being clear enough about the cc:stable and fixes semantics, so the
proposal is to introduce a tag with even less clear semantics in the
hope that it will somehow improve the situation. Why not simply be much
more crisp about the meaning of Fixes and no cc:stable? If everyone
realised that Fixes without cc:stable meant the patch wouldn't be
included in a stable tree that surely gives all the semantics you need
without having to get everyone to try do differentiate between a fix
and an improvement?

One of the big reasons patches get Fixes without cc:stable is simply
that it's an -rc fix for a merge window regression (so no released
kernel has it in and therefore no stable kernel needs it), so you'd
also need to explain that case in the improve docs (because it's a
genuine fix, just not a stable candidate).

So the clear rules look like they should be

   1. every patch fixing something should have a fixes tag pointing to
      the fixed commit
   2. Only patches with cc:stable should go automatically in to stable
      trees and as far back as the fixes tag allows
   3. if a patch without cc:stable is later discovered to be a required
      fix, people can ask for it to be backported.

James

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Andrew Lunn]

> One of the big reasons patches get Fixes without cc:stable is simply
> that it's an -rc fix for a merge window regression (so no released
> kernel has it in and therefore no stable kernel needs it), so you'd
> also need to explain that case in the improve docs (because it's a
> genuine fix, just not a stable candidate).

There is also the case where a patch which needs fixing has been
queued into a -next tree by the Maintainer. So it is not even in an
-rc yet, it is waiting or the next merge window to open.

So what we seem to be talking about is a Fixes: tag pointing to a
patch in a released kernel, which does not have a stable: tag.

Why not simply get 0-day to enforce such patches must have a
not-stable: tag? Such emails should quickly train developers to do the
right thing.

Or is this not a binary condition. Is there a legitimate third option?

	Andrew

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[James Bottomley]

On Sun, 2024-07-14 at 17:35 +0200, Andrew Lunn wrote:
> > One of the big reasons patches get Fixes without cc:stable is
> > simply that it's an -rc fix for a merge window regression (so no
> > released kernel has it in and therefore no stable kernel needs it),
> > so you'd also need to explain that case in the improve docs
> > (because it's a genuine fix, just not a stable candidate).
> 
> There is also the case where a patch which needs fixing has been
> queued into a -next tree by the Maintainer. So it is not even in an
> -rc yet, it is waiting or the next merge window to open.
> 
> So what we seem to be talking about is a Fixes: tag pointing to a
> patch in a released kernel, which does not have a stable: tag.
> 
> Why not simply get 0-day to enforce such patches must have a
> not-stable: tag? Such emails should quickly train developers to do
> the right thing.

Because the not-stable tag seems yet another really hard to use thing.
When Maintainers look at patches, about the first question is where did
this come from (Fixes answers that and we're having good success
tagging most things with it because it's obvious how to apply it---and
not derailing this is another good reason not to split the tag).  The
next question is should this be backported.  This is a bit more
subjective because it's a combination of would anyone actually see any
effects from this or is it a serious enough problem in general that it
might lead to some exploit or bug.  However, this is the judgment call
that can often prove wrong with hindsight, so we have the send
separately to stable channel if this turns out to be the case.  The
not-stable tag seems to be a pejorative statement that only an idiot
would backport this so don't.  I can't see myself tagging any Fixes
tagged commit with that because it's a judgment call as likely to be
wrong as the initial cc:stable decision, so it seems more aimed at
enhancement patches that wouldn't have a Fixes tag anyway and therefore
shouldn't be backported.


I think the actual problem the not-stable tag is meant to solve is some
commits that aren't fixes tagged actually end up in stable when they
shouldn't be there; their presence causes bugs and whoever the
maintainer is gets annoyed that it got backported.

James

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Linus Torvalds]

On Sun, 14 Jul 2024 at 05:31, Sasha Levin <sashal@kernel.org> wrote:
>
> One of the main issues is that most Fixes-tagged commits (>80%) end up
> in a stable tree, leading some authors to omit the Stable tag
> altogether.

So as others have said, the fix to two confusing tags is not to
introduce *more* tags. That only adds to the confusion.

I honestly personally will likely not ever use any more tags than the
ones we already have. Every single "let's add a new tag" proposal I've
ever seen has been pure garbage.

People see one problem and want to fix it by introducing a tag, and
think tags magically would fix it, when adding more tags will only
cause more confusion and make the existing tags less obvious.

               Linus

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Sasha Levin]

On Sun, Jul 14, 2024 at 09:35:26AM -0400, James Bottomley wrote:
>On Sun, 2024-07-14 at 08:31 -0400, Sasha Levin wrote:
>> Hi folks,
>>
>> The Linux kernel community relies heavily on commit tags to identify
>> and manage patches destined for stable kernel branches. Currently, we
>> use a "Stable tag" (cc: stable@kernel.org) to indicate that a patch
>> should be included in stable kernel branches, and a "Fixes tag"
>> (Fixes: 012345678901 ("commit subject")) to point to an older commit
>> that the new commit fixes or improves. However, this scheme has led
>> to some unintended consequences.
>>
>> One of the main issues is that most Fixes-tagged commits (>80%) end
>> up in a stable tree, leading some authors to omit the Stable tag
>> altogether. This means we may not be trying hard enough to include
>> critical commits in stable kernel branches. On the other hand, some
>> authors are unhappy when commits without a Stable tag end up in a
>> stable kernel branch. To address these shortcomings, I propose
>> introducing an "Improves tag" (Improves: 012345678901 ("commit
>> subject")) and altering the meaning of the Fixes tag.
>
>I've got to say this looks like a bad idea: your complaint is we're not
>being clear enough about the cc:stable and fixes semantics, so the
>proposal is to introduce a tag with even less clear semantics in the
>hope that it will somehow improve the situation. Why not simply be much
>more crisp about the meaning of Fixes and no cc:stable? If everyone

What does it mean exactly? What should we be doing now that wasn't
happening before?

All our documentation explicitly says that a stable tag is a *must*,
we've been nagging folks to add it when they haven't, and we give them
the spiel whenever we're asked why a certain fixes-only commit didn't
make it into the stable trees.

>realised that Fixes without cc:stable meant the patch wouldn't be
>included in a stable tree that surely gives all the semantics you need
>without having to get everyone to try do differentiate between a fix
>and an improvement?

By your logic folks already have to do it now, right? The decision
around whether to add a stable tag or not means that there needs to be a
decision around whether something is a fix or an improvement each time a
Fixes tag is added.

>One of the big reasons patches get Fixes without cc:stable is simply
>that it's an -rc fix for a merge window regression (so no released
>kernel has it in and therefore no stable kernel needs it), so you'd
>also need to explain that case in the improve docs (because it's a
>genuine fix, just not a stable candidate).

This is a fairly common misconception around what happens in the -rc
cycles.

The statistic is that about 70% of commits that have a stable tag but no
fixes tag actually fix something in the previous releases rather than
something that was introduced in the current merge window.

When you couple that with commits that are stable tagged, nearly 85% of
commits in each -rc release actually address something that happened
prior to the merge window.

>
>So the clear rules look like they should be
>
>   1. every patch fixing something should have a fixes tag pointing to
>      the fixed commit
>   2. Only patches with cc:stable should go automatically in to stable
>      trees and as far back as the fixes tag allows
>   3. if a patch without cc:stable is later discovered to be a required
>      fix, people can ask for it to be backported.

These were the rules for a while, and the issue was that there were so
many commits without a stable tag that needed to be backported that the
model of "later discovered" simply overwhelmed the process.

We can't go back to that again.

-- 
Thanks,
Sasha

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Sasha Levin]

On Sun, Jul 14, 2024 at 10:07:48AM -0700, Linus Torvalds wrote:
>On Sun, 14 Jul 2024 at 05:31, Sasha Levin <sashal@kernel.org> wrote:
>>
>> One of the main issues is that most Fixes-tagged commits (>80%) end up
>> in a stable tree, leading some authors to omit the Stable tag
>> altogether.
>
>So as others have said, the fix to two confusing tags is not to
>introduce *more* tags. That only adds to the confusion.
>
>I honestly personally will likely not ever use any more tags than the
>ones we already have. Every single "let's add a new tag" proposal I've
>ever seen has been pure garbage.
>
>People see one problem and want to fix it by introducing a tag, and
>think tags magically would fix it, when adding more tags will only
>cause more confusion and make the existing tags less obvious.

I'm not trying to add an additional tag, but rather replace a "broken"
tag by adding something that is better aligned with how people are doing
things these days.

For that matter, we can look at Linus Torvalds, a prominent Linux
developer. In the past few years, Linus has authored:

  - 2023: 3 stable tagged commits.
  - 2022: 2 stable tagged commits.
  - 2021: 4 stable tagged commits.
  - 2020: 0 stable tagged commits.

If we all agree that Linus authored more stable-worthy commits than
that, then to me it's an indication that the stable tagging system isn't
working well here.

My thinking was that we can stop nagging folks about adding a stable tag
and leverage their current (positive) behavior around adding fixes tags,
but yes - training folks to use a new tag is hard.

What's the path forward here? Stable tags don't work, "Improves:" tag
might indeed be garbage, so what would make the folks who read this
mailing list more consistent about tagging patches they author for
backport?

-- 
Thanks,
Sasha

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[James Bottomley]

On Sun, 2024-07-14 at 14:38 -0400, Sasha Levin wrote:
> On Sun, Jul 14, 2024 at 09:35:26AM -0400, James Bottomley wrote:
> > On Sun, 2024-07-14 at 08:31 -0400, Sasha Levin wrote:
> > > Hi folks,
> > > 
> > > The Linux kernel community relies heavily on commit tags to
> > > identify and manage patches destined for stable kernel branches.
> > > Currently, we use a "Stable tag" (cc: stable@kernel.org) to
> > > indicate that a patch should be included in stable kernel
> > > branches, and a "Fixes tag" (Fixes: 012345678901 ("commit
> > > subject")) to point to an older commit that the new commit fixes
> > > or improves. However, this scheme has led to some unintended
> > > consequences.
> > > 
> > > One of the main issues is that most Fixes-tagged commits (>80%)
> > > end up in a stable tree, leading some authors to omit the Stable
> > > tag altogether. This means we may not be trying hard enough to
> > > include critical commits in stable kernel branches. On the other
> > > hand, some authors are unhappy when commits without a Stable tag
> > > end up in a stable kernel branch. To address these shortcomings,
> > > I propose introducing an "Improves tag" (Improves: 012345678901
> > > ("commit subject")) and altering the meaning of the Fixes tag.
> > 
> > I've got to say this looks like a bad idea: your complaint is we're
> > not being clear enough about the cc:stable and fixes semantics, so
> > the proposal is to introduce a tag with even less clear semantics
> > in the hope that it will somehow improve the situation. Why not
> > simply be much more crisp about the meaning of Fixes and no
> > cc:stable? If everyone
> 
> What does it mean exactly? What should we be doing now that wasn't
> happening before?

I outlined it below and you replied.

> All our documentation explicitly says that a stable tag is a *must*,
> we've been nagging folks to add it when they haven't, and we give
> them the spiel whenever we're asked why a certain fixes-only commit
> didn't make it into the stable trees.

Then you've muddied that by backporting a lot of stuff without
cc:stable leading to somewhat mixed messages.  However, most of the
trees I'm involved in are very fastidious about cc:stable which should
prove getting people to assist here isn't impossible.

> > realised that Fixes without cc:stable meant the patch wouldn't be
> > included in a stable tree that surely gives all the semantics you
> > need without having to get everyone to try do differentiate between
> > a fix and an improvement?
> 
> By your logic folks already have to do it now, right? The decision
> around whether to add a stable tag or not means that there needs to
> be a decision around whether something is a fix or an improvement
> each time a Fixes tag is added.

I believe most maintainers who haven't given up on the cc:stable
process do that, yes.

> > One of the big reasons patches get Fixes without cc:stable is
> > simply that it's an -rc fix for a merge window regression (so no
> > released kernel has it in and therefore no stable kernel needs it),
> > so you'd also need to explain that case in the improve docs
> > (because it's a genuine fix, just not a stable candidate).
> 
> This is a fairly common misconception around what happens in the -rc
> cycles.
> 
> The statistic is that about 70% of commits that have a stable tag but
> no fixes tag actually fix something in the previous releases rather
> than something that was introduced in the current merge window.

Given the number for fairly trivial spelling and other non user visible
fixes we get in the trees I look at that doesn't sound too surprising.
So the big question is how many of those *should* actually have had a
stable tag?

> When you couple that with commits that are stable tagged, nearly 85%
> of commits in each -rc release actually address something that
> happened prior to the merge window.

Here you seem to be assuming that every commit with a Fixes tag for a
prior release should be in stable ... I don't think that's true at all.

> > So the clear rules look like they should be
> > 
> >   1. every patch fixing something should have a fixes tag pointing
> > to the fixed commit
> >   2. Only patches with cc:stable should go automatically in to
> > stable trees and as far back as the fixes tag allows
> >   3. if a patch without cc:stable is later discovered to be a
> > required fix, people can ask for it to be backported.
> 
> These were the rules for a while, and the issue was that there were
> so many commits without a stable tag that needed to be backported
> that the model of "later discovered" simply overwhelmed the process.

How can it?  The cc:stable is automation driven, surely we could
automate the discovered after process as well.  Our control systems via
email has rather dramatically improved over the years, just look at
regzbot.

> We can't go back to that again.

Why?  I mean I'm assuming the actual number of patches would be the
same under a perfect cc:stable system as it would be under he cc:stable
but ask for backport if I got it wrong system, so it's really just a
question of using similar automation that drives the former for the
latter.  If you can't figure out how to automate it, I'm sure we can
lend a hand if that's all it takes.

Regards,

James

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Linus Torvalds]

On Sun, 14 Jul 2024 at 11:47, Sasha Levin <sashal@kernel.org> wrote:
>
> I'm not trying to add an additional tag,

What? You *literally* suggested exactly that - adding an "Improves:" tag.

I'm not going to use such an odd and pointless tag.

I would hope that *all* commits improve on something. And if it's an
actual fix to a previous commit, it should say so.

If it's just a random improvement, it shouldn't refer to a previous
commit at all.

What you seem to want is some made-up distinction between "fix that
wants backporting" and "fix that is not important for backporting".

We have long been told that commits that have a "Fixes" tag don't need
a "Cc: stable" because the stabl;e people already pick up on the
fixes, so now you're complaining about the lack of stable tagging.

I think that the *real* issue is "not everything is black and white".

But no amount of tagging will change reality. Reality is not black and
white. Tough.

              Linus

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Sasha Levin]

On Sun, Jul 14, 2024 at 03:20:48PM -0400, James Bottomley wrote:
>On Sun, 2024-07-14 at 14:38 -0400, Sasha Levin wrote:
>> On Sun, Jul 14, 2024 at 09:35:26AM -0400, James Bottomley wrote:
>> > On Sun, 2024-07-14 at 08:31 -0400, Sasha Levin wrote:
>> > > Hi folks,
>> > >
>> > > The Linux kernel community relies heavily on commit tags to
>> > > identify and manage patches destined for stable kernel branches.
>> > > Currently, we use a "Stable tag" (cc: stable@kernel.org) to
>> > > indicate that a patch should be included in stable kernel
>> > > branches, and a "Fixes tag" (Fixes: 012345678901 ("commit
>> > > subject")) to point to an older commit that the new commit fixes
>> > > or improves. However, this scheme has led to some unintended
>> > > consequences.
>> > >
>> > > One of the main issues is that most Fixes-tagged commits (>80%)
>> > > end up in a stable tree, leading some authors to omit the Stable
>> > > tag altogether. This means we may not be trying hard enough to
>> > > include critical commits in stable kernel branches. On the other
>> > > hand, some authors are unhappy when commits without a Stable tag
>> > > end up in a stable kernel branch. To address these shortcomings,
>> > > I propose introducing an "Improves tag" (Improves: 012345678901
>> > > ("commit subject")) and altering the meaning of the Fixes tag.
>> >
>> > I've got to say this looks like a bad idea: your complaint is we're
>> > not being clear enough about the cc:stable and fixes semantics, so
>> > the proposal is to introduce a tag with even less clear semantics
>> > in the hope that it will somehow improve the situation. Why not
>> > simply be much more crisp about the meaning of Fixes and no
>> > cc:stable? If everyone
>>
>> What does it mean exactly? What should we be doing now that wasn't
>> happening before?
>
>I outlined it below and you replied.
>
>> All our documentation explicitly says that a stable tag is a *must*,
>> we've been nagging folks to add it when they haven't, and we give
>> them the spiel whenever we're asked why a certain fixes-only commit
>> didn't make it into the stable trees.
>
>Then you've muddied that by backporting a lot of stuff without
>cc:stable leading to somewhat mixed messages.  However, most of the
>trees I'm involved in are very fastidious about cc:stable which should
>prove getting people to assist here isn't impossible.

And we appreciate the help, and some of it is indeed what you've pointed
out here: folks are used to "Fixes:" meaning automatic backport to
-stable.

[snip]

>> The statistic is that about 70% of commits that have a stable tag but
>> no fixes tag actually fix something in the previous releases rather
>> than something that was introduced in the current merge window.
>
>Given the number for fairly trivial spelling and other non user visible
>fixes we get in the trees I look at that doesn't sound too surprising.
>So the big question is how many of those *should* actually have had a
>stable tag?

I'm going to argue that if you look at commits with only a Fixes tag in
any of the recent -rc cycles, you'll find that over half of them should
be backported.

It could be a fun experiment if you want to give it a go, we can both
look at v6.8-rc1..v6.8 and compare notes?

>> When you couple that with commits that are stable tagged, nearly 85%
>> of commits in each -rc release actually address something that
>> happened prior to the merge window.
>
>Here you seem to be assuming that every commit with a Fixes tag for a
>prior release should be in stable ... I don't think that's true at all.

I really weren't. All I said that most commits with a Fixes tag in the
-rc cycles of a release don't fix an issue introduced in the merge
window of that release.

>> > So the clear rules look like they should be
>> >
>> >   1. every patch fixing something should have a fixes tag pointing
>> > to the fixed commit
>> >   2. Only patches with cc:stable should go automatically in to
>> > stable trees and as far back as the fixes tag allows
>> >   3. if a patch without cc:stable is later discovered to be a
>> > required fix, people can ask for it to be backported.
>>
>> These were the rules for a while, and the issue was that there were
>> so many commits without a stable tag that needed to be backported
>> that the model of "later discovered" simply overwhelmed the process.
>
>How can it?  The cc:stable is automation driven, surely we could
>automate the discovered after process as well.  Our control systems via
>email has rather dramatically improved over the years, just look at
>regzbot.

Stable tag is, but we don't have a nice story around everything else
yet. Maybe regzbot is indeed the solution to a lot of these issues in
the future, but right now we're not even close.

 From my experience, most issues tracked by regzbot and fixed upstream
don't actually have a stable tag. Here's one I just looked at a few days
ago: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f451fd97dd2b78f286379203a47d9d295c467255

And I'm actually happy to point to that one as an example because the
ext4 folks are usually great about stable tags.

Should we have not taken that commit?

-- 
Thanks,
Sasha

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Sasha Levin]

On Sun, Jul 14, 2024 at 12:27:55PM -0700, Linus Torvalds wrote:
>On Sun, 14 Jul 2024 at 11:47, Sasha Levin <sashal@kernel.org> wrote:
>>
>> I'm not trying to add an additional tag,
>
>What? You *literally* suggested exactly that - adding an "Improves:" tag.

I guess that in my mind I'm also taking away the cc:stable tag, so at
the end it's a net-zero :)

>I'm not going to use such an odd and pointless tag.

I'll jump into conclusions here and assume that since you weren't using
the stable tag, then it's also an odd and pointless tag, so here we are
trying to address the issue.

>I would hope that *all* commits improve on something. And if it's an
>actual fix to a previous commit, it should say so.
>
>If it's just a random improvement, it shouldn't refer to a previous
>commit at all.

This is *one* view. I've observed that both individuals and companies
started requiring a fixes tag unless they're writing a new feature.

Heck, back in Google you couldn't commit anything that is not a new
feature unless the commit message had a "Fixes:" tag to make the bot
happy.

>What you seem to want is some made-up distinction between "fix that
>wants backporting" and "fix that is not important for backporting".
>
>We have long been told that commits that have a "Fixes" tag don't need
>a "Cc: stable" because the stabl;e people already pick up on the
>fixes, so now you're complaining about the lack of stable tagging.

Who? Where?

This ended up happening because we couldn't get folks to consistently
add stable tags, so we've started pulling in more patches with just a
Fixes tag (and then introducing AUTOSEL for commits without a stable
tag).

I was looking to get away from that, as some maintainers would assume
what you did, and some would assume the exact opposite. I kept ending up
with people angry at me no matter which choice I'd make around the Fixes
tag policy.

If you think that anything with a Fixes tag should land in stable, then
great, we can change the formal policy and get it over with.

-- 
Thanks,
Sasha

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[James Bottomley]

On Sun, 2024-07-14 at 16:27 -0400, Sasha Levin wrote:
> On Sun, Jul 14, 2024 at 12:27:55PM -0700, Linus Torvalds wrote:
[...]
> > I would hope that *all* commits improve on something. And if it's
> > an actual fix to a previous commit, it should say so.
> > 
> > If it's just a random improvement, it shouldn't refer to a previous
> > commit at all.
> 
> This is *one* view. I've observed that both individuals and companies
> started requiring a fixes tag unless they're writing a new feature.
> 
> Heck, back in Google you couldn't commit anything that is not a new
> feature unless the commit message had a "Fixes:" tag to make the bot
> happy.

But we know how to enforce a unified view of process: we document it in
Documentaion/process ... and indeed we already have a handling-
regressions.rst which says:

    * Add a "Fixes:" tag to specify the commit causing the regression.
   
    * If the culprit was merged in an earlier development cycle,
   explicitly mark the fix for backporting using the ``Cc:
   stable@vger.kernel.org`` tag.

That doesn't seem ambiguous or unclear, why can't we just follow it? 
It does everything you want: distinguished patches that should be
backported from fixes that probably don't matter for backporting.
> 
> > What you seem to want is some made-up distinction between "fix that
> > wants backporting" and "fix that is not important for backporting".
> > 
> > We have long been told that commits that have a "Fixes" tag don't
> > need a "Cc: stable" because the stabl;e people already pick up on
> > the fixes, so now you're complaining about the lack of stable
> > tagging.
> 
> Who? Where?

I think the problem is there is a central document, saying what this
all means, but you're not pointing to it which means everyone has a
slightly different view because they haven't read it.  Let's just start
with pointing everyone to handling-regressions.rst and see if we get
better consistency before trying to invent new processes that give
everyone even more leeway to do things differently.

> This ended up happening because we couldn't get folks to consistently
> add stable tags, so we've started pulling in more patches with just a
> Fixes tag (and then introducing AUTOSEL for commits without a stable
> tag).
> 
> I was looking to get away from that, as some maintainers would assume
> what you did, and some would assume the exact opposite. I kept ending
> up with people angry at me no matter which choice I'd make around the
> Fixes tag policy.

But there doesn't seem to be an ambiguity: just point to the documents
and say "do that" (or "don't do that" if they deviate).

> If you think that anything with a Fixes tag should land in stable,
> then great, we can change the formal policy and get it over with.

Let's just start with trying to get everyone to unify around what's
already written.

Regards,

James

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Linus Torvalds]

On Sun, 14 Jul 2024 at 16:06, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> But we know how to enforce a unified view of process: we document it in
> Documentaion/process ...

Hahhahhahaahh.

Ahh, you're such a kidder, James.

              Linus

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Steven Rostedt]

On Sun, 14 Jul 2024 16:27:30 -0400
Sasha Levin <sashal@kernel.org> wrote:

> If you think that anything with a Fixes tag should land in stable, then
> great, we can change the formal policy and get it over with.

I guess it comes down to what exactly do we consider a fix worthy of
stable? Most of my commits I tag with Fixes have Cc stable too. But there's
some cases where I made an improvement in logic, or restructured something
that helps with performance and tag that with a Fixes. So, in these cases
when I add "Fixes" without a Cc stable, it does mean "improvement" more
than fix. Or I think the code change may be too risky to consider stable
(as it may introduce a bug, as the code change is too big).

Basically, it comes down to that I add Cc stable for anything I think is
worthy to send to Linus in an -rc release. Perhaps all commits that go to
Linus during -rc must have a Fixes tag, and Cc stable if the fixed commit
happened in a previous kernel version? As Linus has been pointing out that
he only wants fixes after -rc1.

Sometimes I find a behavior bug, that's been there since the code was
written. Like a new feature that works "some of the time", but then I find
a corner case that doesn't work. I'll add Fixes but not "stable" when I fix
the corner case. As Linus once criticized one of these types of fixes as
it's not a fix. But I still label it a fix for those that want the feature
to work in all cases.

Recently this happened with an update to the user events. We found a case
that doesn't work as expected, and I sent the "fix" to Linus in the -rc
release, but Linus refused it. That was because it was more of an extension
of an existing feature. That is, it wasn't a regression. The behavior never
worked that way in previous kernels, so it's not something to be added as a
fix. I still tagged it as Fixes, as for us, the feature wasn't working in
all the cases we wanted it to.

Things like this is questionable for adding to a "stable" release. In that
corner case, if you tried to do that feature, it would just fail. This
would not break any existing code. But if you wanted that full feature to
work, you needed the fix. Should this be considered a "stable" backport?
For any distro that wanted the full feature, the answer was "yes". For those
that just want stable not to have regressions, the answer is "no".

-- Steve

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[NeilBrown]

On Mon, 15 Jul 2024, James Bottomley wrote:
> On Sun, 2024-07-14 at 16:27 -0400, Sasha Levin wrote:
> > On Sun, Jul 14, 2024 at 12:27:55PM -0700, Linus Torvalds wrote:
> [...]
> > > I would hope that *all* commits improve on something. And if it's
> > > an actual fix to a previous commit, it should say so.
> > > 
> > > If it's just a random improvement, it shouldn't refer to a previous
> > > commit at all.
> > 
> > This is *one* view. I've observed that both individuals and companies
> > started requiring a fixes tag unless they're writing a new feature.
> > 
> > Heck, back in Google you couldn't commit anything that is not a new
> > feature unless the commit message had a "Fixes:" tag to make the bot
> > happy.
> 
> But we know how to enforce a unified view of process: we document it in
> Documentaion/process ... and indeed we already have a handling-
> regressions.rst which says:
> 
>     * Add a "Fixes:" tag to specify the commit causing the regression.

Interesting.  I don't think Fixes means that the target introduced a
regression, though that is one possibility.
Fixes could also mean that the target introduced a feature that didn't
quite work as intended.
Sometimes Fixes means that the target introduced a typo in a comment - I
wouldn't use it that way, but some people do and I find it hard to
disagree.
(and maybe all these targets do regress the "total perfection" score for
 Linux.....)

I'd generally be happy with any commit marked "Fixes" getting into a kernel that
contains the target.  In my mind the issues are more about:
 - how quickly should the fix land.  There is no easy answer.
 - Are Fixes focused on a backportable fix, or do they combine a fix with
   new functionality?  I think they are often quite focused, but it
   wouldn't hurt to often remind each other of the importance of focused
   fixes.

NeilBrown

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Mauro Carvalho Chehab]

Em Sun, 14 Jul 2024 14:38:57 -0400
Sasha Levin <sashal@kernel.org> escreveu:

> On Sun, Jul 14, 2024 at 09:35:26AM -0400, James Bottomley wrote:
> >On Sun, 2024-07-14 at 08:31 -0400, Sasha Levin wrote:  
> >> Hi folks,
> >>
> >> To address these shortcomings, I propose
> >> introducing an "Improves tag" (Improves: 012345678901 ("commit
> >> subject")) and altering the meaning of the Fixes tag.  

I don't think that a "Improves tag" would solve anything.

> All our documentation explicitly says that a stable tag is a *must*,
> we've been nagging folks to add it when they haven't, and we give them
> the spiel whenever we're asked why a certain fixes-only commit didn't
> make it into the stable trees.

Agreed.

> > So the clear rules look like they should be
> >
> >   1. every patch fixing something should have a fixes tag pointing to
> >      the fixed commit
> >   2. Only patches with cc:stable should go automatically in to stable
> >      trees and as far back as the fixes tag allows
> >   3. if a patch without cc:stable is later discovered to be a required
> >      fix, people can ask for it to be backported.  
> 
> These were the rules for a while, and the issue was that there were so
> many commits without a stable tag that needed to be backported that the
> model of "later discovered" simply overwhelmed the process.
> 
> We can't go back to that again.

In my case, I use to explicitly c/c stable when I feel the need of a
backport and I'm confident enough that the patch won't cause any
regressions. 

I use fixes on all sorts of fixes, including some that might
change behavior or eventually cause regressions. So, better not
hush backporting them.

There are some border cases though. For instance, patches adding new
PCI/USB IDs to drivers aren't technically fixes[1]. They're just usual
driver maintenance to support newer models (I'm considering the case
where the chipset is the same). Sometimes, those are not too trivial 
(just add new IDs to the id tables), as they may require some extra 
logic to properly setup resources like irqs, gpios, PM lines, etc.

If we had a new "improves" tag, I would be in doubt on such patches
about tagging them or not, as technically it is not an improvement 
either, as the driver itself didn't require any changes, except for 
the model-dependent resource allocation logic.

Still, they make sense to be backported on distros, as it may solve
use case problems of devices not being properly detected, thus closing
bugs at the distros bug tracking systems. (so, they're "Fixes" from
distro standpoint).

[1] some people still consider them fixes against the initial driver
    submission, even if the device model to be added didn't exist at
    the time the driver was written.

Thanks,
Mauro

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Greg KH]

On Sun, Jul 14, 2024 at 04:09:06PM -0700, Linus Torvalds wrote:
> On Sun, 14 Jul 2024 at 16:06, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> >
> > But we know how to enforce a unified view of process: we document it in
> > Documentaion/process ...
> 
> Hahhahhahaahh.
> 
> Ahh, you're such a kidder, James.

Yeah, and that's the issue here, we have whole subsystems and many many
maintainers and developers that do not follow this at all, hence our
need to pull into stable commits that only have a "Fixes:" tag on it.

Now we can start running a bot on all submissions that says something
like the one that I have been doing for the USB subsystem for the past
few months, here's the text I use:

	- You have marked a patch with a "Fixes:" tag for a commit that is in an
	  older released kernel, yet you do not have a cc: stable line in the
	  signed-off-by area at all, which means that the patch will not be
	  applied to any older kernel releases.  To properly fix this, please
	  follow the documented rules in the
	  Documentation/process/stable-kernel-rules.rst file for how to resolve
	  this.

But I doubt everyone wants to see that polute their mailing lists :(

Anyway, if people want to stick to the current, documented, process,
great, but as-is, trying to get people to follow that is rough and not
really working.

thanks,

greg k-h

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Mauro Carvalho Chehab]

Em Mon, 15 Jul 2024 10:02:54 +0200
Greg KH <gregkh@linuxfoundation.org> escreveu:

> On Sun, Jul 14, 2024 at 04:09:06PM -0700, Linus Torvalds wrote:
> > On Sun, 14 Jul 2024 at 16:06, James Bottomley
> > <James.Bottomley@hansenpartnership.com> wrote:  
> > >
> > > But we know how to enforce a unified view of process: we document it in
> > > Documentaion/process ...  
> > 
> > Hahhahhahaahh.
> > 
> > Ahh, you're such a kidder, James.  
> 
> Yeah, and that's the issue here, we have whole subsystems and many many
> maintainers and developers that do not follow this at all, hence our
> need to pull into stable commits that only have a "Fixes:" tag on it.
> 
> Now we can start running a bot on all submissions that says something
> like the one that I have been doing for the USB subsystem for the past
> few months, here's the text I use:
> 
> 	- You have marked a patch with a "Fixes:" tag for a commit that is in an
> 	  older released kernel, yet you do not have a cc: stable line in the
> 	  signed-off-by area at all, which means that the patch will not be
> 	  applied to any older kernel releases.  To properly fix this, please
> 	  follow the documented rules in the
> 	  Documentation/process/stable-kernel-rules.rst file for how to resolve
> 	  this.

Perhaps the text should also briefly describe how to make the bot ignore it
when there are good reasons for that. Something like:

	If the patch shall not be backported, please document it together
	with the rationale with:

	Cc: <stable+noautosel@kernel.org> # reason goes here, and must be present

> But I doubt everyone wants to see that polute their mailing lists :(

As people learn about that, the number of such emails will reduce.


Thanks,
Mauro

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Mimi Zohar]

On Mon, 2024-07-15 at 10:02 +0200, Greg KH wrote:
> On Sun, Jul 14, 2024 at 04:09:06PM -0700, Linus Torvalds wrote:
> > On Sun, 14 Jul 2024 at 16:06, James Bottomley
> > <James.Bottomley@hansenpartnership.com> wrote:
> > > But we know how to enforce a unified view of process: we document it in
> > > Documentaion/process ...
> > 
> > Hahhahhahaahh.
> > 
> > Ahh, you're such a kidder, James.
> 
> Yeah, and that's the issue here, we have whole subsystems and many many
> maintainers and developers that do not follow this at all, hence our
> need to pull into stable commits that only have a "Fixes:" tag on it.
> 
> Now we can start running a bot on all submissions that says something
> like the one that I have been doing for the USB subsystem for the past
> few months, here's the text I use:
> 
> 	- You have marked a patch with a "Fixes:" tag for a commit that is in an
> 	  older released kernel, yet you do not have a cc: stable line in the
> 	  signed-off-by area at all, which means that the patch will not be
> 	  applied to any older kernel releases.  To properly fix this, please
> 	  follow the documented rules in the
> 	  Documentation/process/stable-kernel-rules.rst file for how to resolve
> 	  this.
> 
> But I doubt everyone wants to see that polute their mailing lists :(
> 
> Anyway, if people want to stick to the current, documented, process,
> great, but as-is, trying to get people to follow that is rough and not
> really working.

Sigh, I'm one of them, but I'd like to move to backporting "stable" only.
Unfortunately, the more that you backport "Fixes:" the less likely maintainers
(me) will remember to Cc stable.  Other than email, is there a way of letting
you and Sasha know to only backport those with "Stable"?.  Instead of sending
the email above to everyone, only send it maintainers who don't Cc stable
properly/haven't notified you that they will.  The emails should get everyone
else's attention. :)

Mimi

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Mimi Zohar]

On Mon, 2024-07-15 at 08:48 -0400, Mimi Zohar wrote:
> On Mon, 2024-07-15 at 10:02 +0200, Greg KH wrote:
> > On Sun, Jul 14, 2024 at 04:09:06PM -0700, Linus Torvalds wrote:
> > > On Sun, 14 Jul 2024 at 16:06, James Bottomley
> > > <James.Bottomley@hansenpartnership.com> wrote:
> > > > But we know how to enforce a unified view of process: we document it in
> > > > Documentaion/process ...
> > > 
> > > Hahhahhahaahh.
> > > 
> > > Ahh, you're such a kidder, James.
> > 
> > Yeah, and that's the issue here, we have whole subsystems and many many
> > maintainers and developers that do not follow this at all, hence our
> > need to pull into stable commits that only have a "Fixes:" tag on it.
> > 
> > Now we can start running a bot on all submissions that says something
> > like the one that I have been doing for the USB subsystem for the past
> > few months, here's the text I use:
> > 
> > 	- You have marked a patch with a "Fixes:" tag for a commit that is in an
> > 	  older released kernel, yet you do not have a cc: stable line in the
> > 	  signed-off-by area at all, which means that the patch will not be
> > 	  applied to any older kernel releases.  To properly fix this, please
> > 	  follow the documented rules in the
> > 	  Documentation/process/stable-kernel-rules.rst file for how to resolve
> > 	  this.
> > 
> > But I doubt everyone wants to see that polute their mailing lists :(
> > 
> > Anyway, if people want to stick to the current, documented, process,
> > great, but as-is, trying to get people to follow that is rough and not
> > really working.
> 
> Sigh, I'm one of them, but I'd like to move to backporting "stable" only.
> Unfortunately, the more that you backport "Fixes:" the less likely maintainers
> (me) will remember to Cc stable.  Other than email, is there a way of letting
> you and Sasha know to only backport those with "Stable"?.  Instead of sending
> the email above to everyone, only send it maintainers who don't Cc stable
> properly/haven't notified you that they will.  The emails should get everyone
> else's attention. :)

Basically, reward good behavior, penalize bad behavior.

Mimi

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Alexandre Belloni]

On 15/07/2024 08:52:54-0400, Mimi Zohar wrote:
> On Mon, 2024-07-15 at 08:48 -0400, Mimi Zohar wrote:
> > On Mon, 2024-07-15 at 10:02 +0200, Greg KH wrote:
> > > On Sun, Jul 14, 2024 at 04:09:06PM -0700, Linus Torvalds wrote:
> > > > On Sun, 14 Jul 2024 at 16:06, James Bottomley
> > > > <James.Bottomley@hansenpartnership.com> wrote:
> > > > > But we know how to enforce a unified view of process: we document it in
> > > > > Documentaion/process ...
> > > > 
> > > > Hahhahhahaahh.
> > > > 
> > > > Ahh, you're such a kidder, James.
> > > 
> > > Yeah, and that's the issue here, we have whole subsystems and many many
> > > maintainers and developers that do not follow this at all, hence our
> > > need to pull into stable commits that only have a "Fixes:" tag on it.
> > > 
> > > Now we can start running a bot on all submissions that says something
> > > like the one that I have been doing for the USB subsystem for the past
> > > few months, here's the text I use:
> > > 
> > > 	- You have marked a patch with a "Fixes:" tag for a commit that is in an
> > > 	  older released kernel, yet you do not have a cc: stable line in the
> > > 	  signed-off-by area at all, which means that the patch will not be
> > > 	  applied to any older kernel releases.  To properly fix this, please
> > > 	  follow the documented rules in the
> > > 	  Documentation/process/stable-kernel-rules.rst file for how to resolve
> > > 	  this.
> > > 
> > > But I doubt everyone wants to see that polute their mailing lists :(
> > > 
> > > Anyway, if people want to stick to the current, documented, process,
> > > great, but as-is, trying to get people to follow that is rough and not
> > > really working.
> > 
> > Sigh, I'm one of them, but I'd like to move to backporting "stable" only.
> > Unfortunately, the more that you backport "Fixes:" the less likely maintainers
> > (me) will remember to Cc stable.  Other than email, is there a way of letting
> > you and Sasha know to only backport those with "Stable"?.  Instead of sending
> > the email above to everyone, only send it maintainers who don't Cc stable
> > properly/haven't notified you that they will.  The emails should get everyone
> > else's attention. :)
> 
> Basically, reward good behavior, penalize bad behavior.

I'm very surprised that it didn't cross anyone's mind yet that
contributors and maintainers don't bother using Cc: stable because they
don't care about stable for various reasons. So the behaviour is nether
good nor bad and doesn't have to be penalized.


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Greg KH]

On Mon, Jul 15, 2024 at 04:34:19PM +0200, Alexandre Belloni wrote:
> On 15/07/2024 08:52:54-0400, Mimi Zohar wrote:
> > On Mon, 2024-07-15 at 08:48 -0400, Mimi Zohar wrote:
> > > On Mon, 2024-07-15 at 10:02 +0200, Greg KH wrote:
> > > > On Sun, Jul 14, 2024 at 04:09:06PM -0700, Linus Torvalds wrote:
> > > > > On Sun, 14 Jul 2024 at 16:06, James Bottomley
> > > > > <James.Bottomley@hansenpartnership.com> wrote:
> > > > > > But we know how to enforce a unified view of process: we document it in
> > > > > > Documentaion/process ...
> > > > > 
> > > > > Hahhahhahaahh.
> > > > > 
> > > > > Ahh, you're such a kidder, James.
> > > > 
> > > > Yeah, and that's the issue here, we have whole subsystems and many many
> > > > maintainers and developers that do not follow this at all, hence our
> > > > need to pull into stable commits that only have a "Fixes:" tag on it.
> > > > 
> > > > Now we can start running a bot on all submissions that says something
> > > > like the one that I have been doing for the USB subsystem for the past
> > > > few months, here's the text I use:
> > > > 
> > > > 	- You have marked a patch with a "Fixes:" tag for a commit that is in an
> > > > 	  older released kernel, yet you do not have a cc: stable line in the
> > > > 	  signed-off-by area at all, which means that the patch will not be
> > > > 	  applied to any older kernel releases.  To properly fix this, please
> > > > 	  follow the documented rules in the
> > > > 	  Documentation/process/stable-kernel-rules.rst file for how to resolve
> > > > 	  this.
> > > > 
> > > > But I doubt everyone wants to see that polute their mailing lists :(
> > > > 
> > > > Anyway, if people want to stick to the current, documented, process,
> > > > great, but as-is, trying to get people to follow that is rough and not
> > > > really working.
> > > 
> > > Sigh, I'm one of them, but I'd like to move to backporting "stable" only.
> > > Unfortunately, the more that you backport "Fixes:" the less likely maintainers
> > > (me) will remember to Cc stable.  Other than email, is there a way of letting
> > > you and Sasha know to only backport those with "Stable"?.  Instead of sending
> > > the email above to everyone, only send it maintainers who don't Cc stable
> > > properly/haven't notified you that they will.  The emails should get everyone
> > > else's attention. :)
> > 
> > Basically, reward good behavior, penalize bad behavior.
> 
> I'm very surprised that it didn't cross anyone's mind yet that
> contributors and maintainers don't bother using Cc: stable because they
> don't care about stable for various reasons. So the behaviour is nether
> good nor bad and doesn't have to be penalized.

Totally true.  The first rule of the stable tree is "it will not put
additional burden on any developer or maintainer that doesn't want to
participate in it."  So if you don't want to deal with it, wonderful,
don't take anything and just don't worry about it.

Many subsystems have also opted-out of the autosel and Fixes: merges as
they don't want that to happen for them.  We gladly support that, anyone
can just email us and we will add them to the list, or they can send a
patch to:
	https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git/tree/ignore_list

Again, we do NOT want to penalize anyone here, that's not how kernel
development should work, our maintainers are overworked as it is.

thanks,

greg k-h

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Jonathan Corbet]

Greg KH <gregkh@linuxfoundation.org> writes:

> On Mon, Jul 15, 2024 at 04:34:19PM +0200, Alexandre Belloni wrote:
>> I'm very surprised that it didn't cross anyone's mind yet that
>> contributors and maintainers don't bother using Cc: stable because they
>> don't care about stable for various reasons. So the behaviour is nether
>> good nor bad and doesn't have to be penalized.
>
> Totally true.  The first rule of the stable tree is "it will not put
> additional burden on any developer or maintainer that doesn't want to
> participate in it."  So if you don't want to deal with it, wonderful,
> don't take anything and just don't worry about it.

I must confess I've been wondering about this, since I've seen the above
policy expressed a number of times over the years.  Stable participation
is entirely optional.  But then this conversation has included things
like:

> All our documentation explicitly says that a stable tag is a *must*,
> we've been nagging folks to add it when they haven't, and we give them
> the spiel whenever we're asked why a certain fixes-only commit didn't
> make it into the stable trees.

Sasha, https://lore.kernel.org/all/ZpQbQa-_8GkoiPhE@sashalap

> Anyway, if people want to stick to the current, documented, process,
> great, but as-is, trying to get people to follow that is rough and not
> really working.

Greg, https://lore.kernel.org/all/2024071528-cahoots-reacquire-9eab@gregkh

So, to me, the real question here is: what is the actual policy?  Are
developers and maintainers expected to put in stable tags the way they
are expected to add Signed-off-by, or is it a fully optional practice?
In the latter case, I'm not sure how much good messing with the tags
will do.

Thanks,

jon

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[James Bottomley]

On Mon, 2024-07-15 at 09:00 -0600, Jonathan Corbet wrote:
[...]
> So, to me, the real question here is: what is the actual policy?

This thread does demonstrate huge confusion around that.  I'd really
like to try to run with what's documented because that's the easiest
for Maintainers to enforce.

> Are developers and maintainers expected to put in stable tags the way
> they are expected to add Signed-off-by, or is it a fully optional
> practice? In the latter case, I'm not sure how much good messing with
> the tags will do.

So what's documented is

Fixes: is generally useful outside of stable (for bug tracing and the
like) and is thus not optional.  It simply means something about the
target was updated by the patch, but this could be spelling in comments
or white space and thus may not be a stable candidate.

cc:stable is only for trees opting in to the stable process to mark
commits that should be backported (i.e. is optional).

Backport requests for upstream commits that didn't get a cc:stable tag
go to the stable list, but I'll look into improving the automation
around it.

James

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Greg KH]

On Mon, Jul 15, 2024 at 09:00:01AM -0600, Jonathan Corbet wrote:
> Greg KH <gregkh@linuxfoundation.org> writes:
> 
> > On Mon, Jul 15, 2024 at 04:34:19PM +0200, Alexandre Belloni wrote:
> >> I'm very surprised that it didn't cross anyone's mind yet that
> >> contributors and maintainers don't bother using Cc: stable because they
> >> don't care about stable for various reasons. So the behaviour is nether
> >> good nor bad and doesn't have to be penalized.
> >
> > Totally true.  The first rule of the stable tree is "it will not put
> > additional burden on any developer or maintainer that doesn't want to
> > participate in it."  So if you don't want to deal with it, wonderful,
> > don't take anything and just don't worry about it.
> 
> I must confess I've been wondering about this, since I've seen the above
> policy expressed a number of times over the years.  Stable participation
> is entirely optional.  But then this conversation has included things
> like:
> 
> > All our documentation explicitly says that a stable tag is a *must*,
> > we've been nagging folks to add it when they haven't, and we give them
> > the spiel whenever we're asked why a certain fixes-only commit didn't
> > make it into the stable trees.
> 
> Sasha, https://lore.kernel.org/all/ZpQbQa-_8GkoiPhE@sashalap
> 
> > Anyway, if people want to stick to the current, documented, process,
> > great, but as-is, trying to get people to follow that is rough and not
> > really working.
> 
> Greg, https://lore.kernel.org/all/2024071528-cahoots-reacquire-9eab@gregkh

I say this as many maintainers/developers get the "Fixes:" tag stuff
mixed up.  We had a very senior/core developer email us about this just
this weekend, they thought they were following the proper process and
didn't realize that "Cc: stable@" was the proper way instead.

That's what I mean, for people who _WANT_ to participate, they get it
wrong, as Linus points out, none of us read the documentation we have :(

> So, to me, the real question here is: what is the actual policy?  Are
> developers and maintainers expected to put in stable tags the way they
> are expected to add Signed-off-by, or is it a fully optional practice?
> In the latter case, I'm not sure how much good messing with the tags
> will do.

It's optional, but if you DO want to do it right, please cc: stable@ as
that's the documented way AND you will get proper integration into the
process (i.e. emails when things fail to apply.)  If you don't cc:
stable@ but do just use "Fixes:" and haven't told us otherwise (see my
previous email here), then us stable maintainers just have to guess and
try to apply "Fixes:" commits as they obviously are fixing real problems
that people have and report and want resolved.

thanks,

greg k-h

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Sasha Levin]

On Mon, Jul 15, 2024 at 11:07:30AM -0400, James Bottomley wrote:
>On Mon, 2024-07-15 at 09:00 -0600, Jonathan Corbet wrote:
>> Are developers and maintainers expected to put in stable tags the way
>> they are expected to add Signed-off-by, or is it a fully optional
>> practice? In the latter case, I'm not sure how much good messing with
>> the tags will do.
>
>So what's documented is
>
>Fixes: is generally useful outside of stable (for bug tracing and the
>like) and is thus not optional.  It simply means something about the
>target was updated by the patch, but this could be spelling in comments
>or white space and thus may not be a stable candidate.

But then look at how folks interpert the Fixes tag:

On Sun, Jul 14, 2024 at 12:27:55PM -0700, Linus Torvalds wrote:
>I would hope that *all* commits improve on something. And if it's an
>actual fix to a previous commit, it should say so.
>
>If it's just a random improvement, it shouldn't refer to a previous
>commit at all.

Linus (and others) see the Fixes tag differently than how you (and
others) see it.

Which, I guess, is the issue I'm trying to resolve.

-- 
Thanks,
Sasha

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[James Bottomley]

On Mon, 2024-07-15 at 11:19 -0400, Sasha Levin wrote:
> On Mon, Jul 15, 2024 at 11:07:30AM -0400, James Bottomley wrote:
> > On Mon, 2024-07-15 at 09:00 -0600, Jonathan Corbet wrote:
> > > Are developers and maintainers expected to put in stable tags the
> > > way they are expected to add Signed-off-by, or is it a fully
> > > optional practice? In the latter case, I'm not sure how much good
> > > messing with the tags will do.
> > 
> > So what's documented is
> > 
> > Fixes: is generally useful outside of stable (for bug tracing and
> > the like) and is thus not optional.  It simply means something
> > about the target was updated by the patch, but this could be
> > spelling in comments or white space and thus may not be a stable
> > candidate.
> 
> But then look at how folks interpert the Fixes tag:
> 
> On Sun, Jul 14, 2024 at 12:27:55PM -0700, Linus Torvalds wrote:
> > I would hope that *all* commits improve on something. And if it's
> > an actual fix to a previous commit, it should say so.
> > 
> > If it's just a random improvement, it shouldn't refer to a previous
> > commit at all.
> 
> Linus (and others) see the Fixes tag differently than how you (and
> others) see it.
> 
> Which, I guess, is the issue I'm trying to resolve.

Right.  We have clear(ish) text in 5.Posting.rst and stable-kernel-
rules.rst that supports the above, so I think the only outcome might be
making this more clear in the documentation and actually trying to
enforce it.

James

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Dan Carpenter]

On Mon, Jul 15, 2024 at 11:19:31AM -0400, Sasha Levin wrote:
> On Mon, Jul 15, 2024 at 11:07:30AM -0400, James Bottomley wrote:
> > On Mon, 2024-07-15 at 09:00 -0600, Jonathan Corbet wrote:
> > > Are developers and maintainers expected to put in stable tags the way
> > > they are expected to add Signed-off-by, or is it a fully optional
> > > practice? In the latter case, I'm not sure how much good messing with
> > > the tags will do.
> > 
> > So what's documented is
> > 
> > Fixes: is generally useful outside of stable (for bug tracing and the
> > like) and is thus not optional.  It simply means something about the
> > target was updated by the patch, but this could be spelling in comments
> > or white space and thus may not be a stable candidate.
> 
> But then look at how folks interpert the Fixes tag:
> 
> On Sun, Jul 14, 2024 at 12:27:55PM -0700, Linus Torvalds wrote:
> > I would hope that *all* commits improve on something. And if it's an
> > actual fix to a previous commit, it should say so.
> > 
> > If it's just a random improvement, it shouldn't refer to a previous
> > commit at all.
> 
> Linus (and others) see the Fixes tag differently than how you (and
> others) see it.

I don't see how those two statement conflict.

Fixes tags are totally separate from stable.  It should never be a
thing where "This patch doesn't need to be backported so leave off the
Fixes tag."

To me the main ambiguity is in cleanups.  Is this where the Improves-on
tag would be used?  I tell people to only add Fixes tags for actual
fixes.  If a patch is just silencing a static checker warning like
"delete write-only variable" then I'll mention the commit which
introduced warning, but I won't add a Fixes tag.  "We stopped using
variable "foo" in commit 2342423 ("blah blah blah").  It's unnecessary
now.  Delete it."

In Steve's case there was a debate whether it was a Fix a or new
functionality.  That line can be blurry as well.

regards,
dan carpenter

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Mauro Carvalho Chehab]

Em Mon, 15 Jul 2024 17:10:46 +0200
Greg KH <gregkh@linuxfoundation.org> escreveu:

> as Linus points out, none of us read the documentation we have :(

I guess we all read some version of the documentation, but most likely
several years ago, only re-reading it when something like this comes
up.

Thanks,
Mauro

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Theodore Ts'o]

On Sun, Jul 14, 2024 at 04:18:00PM -0400, Sasha Levin wrote:
> From my experience, most issues tracked by regzbot and fixed upstream
> don't actually have a stable tag. Here's one I just looked at a few days
> ago: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f451fd97dd2b78f286379203a47d9d295c467255
> 
> And I'm actually happy to point to that one as an example because the
> ext4 folks are usually great about stable tags.
>
> Should we have not taken that commit?

Yep, that's just a mistake on our (my) part; you should have taken
that commit, and my thanks for taking it without asking us.

That being said, maybe one path forward is that the stable team
*should* be asking the subsystem maintainers about.  "Hey, the
following commits appear to be backported, but you didn't add a cc:
stable.  We plan to backport them unless you complain."  This has the
benefit of giving feedback to the subsystem maintainers that they they
missed tagging some number of commits, which might remind them to do
better, or make them decide that they want to do something more
explicit, such as have their own stable backports initiative ala XFS.

More generally, it seems to me that we are conflating multiple issues
here in this discussion which may be making it harder for us make
progress on the question.

1) There are some subsystems who don't care about tagging commits,
   either Fixes: or Cc: stable, or both;

2) There are subsystems which are trying to appropriately tag commits, but:
   a) Sometimes they will make a mistake, and forget to cc: stable
   b) Sometimes it's too hard (tm) to figure out what is the commit which
      introduces the regression, so they either make up something (e.g.,
      hmm, it looks like commit XYZ changes one of the line that is touched
      by the patch, so <shrug emoji>), or they will add a Cc: stable but
      not supply a Fixes: tag
   c) They don't want it to get immediately pulled into stable, but might
     be OK if it gets pulled in after some period of time, or if someone
     actually does the regression testing first.

3)  There might be subsystems who believe 2c) applies to all of their commits,
    but they've been too passive-aggressive to explicitly tell the stable
    team to put them on the opt-out list.  :-)

						- Ted

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 897 bytes --]

On Mon, Jul 15, 2024 at 04:40:01PM +0200, Greg KH wrote:
> On Mon, Jul 15, 2024 at 04:34:19PM +0200, Alexandre Belloni wrote:

> > I'm very surprised that it didn't cross anyone's mind yet that
> > contributors and maintainers don't bother using Cc: stable because they
> > don't care about stable for various reasons. So the behaviour is nether
> > good nor bad and doesn't have to be penalized.

> Totally true.  The first rule of the stable tree is "it will not put
> additional burden on any developer or maintainer that doesn't want to
> participate in it."  So if you don't want to deal with it, wonderful,
> don't take anything and just don't worry about it.

FWIW as we've discussed before I stopped adding explicit CCs for the
most part since AUTOSEL is picking up far more than I ever would so it
doesn't seem worth the effort to filter which is a variation on what
Alexandre mentioned.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 674 bytes --]

On Mon, Jul 15, 2024 at 02:00:00PM -0400, Theodore Ts'o wrote:

> That being said, maybe one path forward is that the stable team
> *should* be asking the subsystem maintainers about.  "Hey, the
> following commits appear to be backported, but you didn't add a cc:
> stable.  We plan to backport them unless you complain."  This has the
> benefit of giving feedback to the subsystem maintainers that they they
> missed tagging some number of commits, which might remind them to do
> better, or make them decide that they want to do something more
> explicit, such as have their own stable backports initiative ala XFS.

That's pretty much exactly what AUTOSEL already does.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Dan Carpenter]

On Mon, Jul 15, 2024 at 02:00:00PM -0400, Theodore Ts'o wrote:
> On Sun, Jul 14, 2024 at 04:18:00PM -0400, Sasha Levin wrote:
> > From my experience, most issues tracked by regzbot and fixed upstream
> > don't actually have a stable tag. Here's one I just looked at a few days
> > ago: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f451fd97dd2b78f286379203a47d9d295c467255
> > 
> > And I'm actually happy to point to that one as an example because the
> > ext4 folks are usually great about stable tags.
> >
> > Should we have not taken that commit?
> 
> Yep, that's just a mistake on our (my) part; you should have taken
> that commit, and my thanks for taking it without asking us.
> 
> That being said, maybe one path forward is that the stable team
> *should* be asking the subsystem maintainers about.  "Hey, the
> following commits appear to be backported, but you didn't add a cc:
> stable.  We plan to backport them unless you complain."  This has the
> benefit of giving feedback to the subsystem maintainers that they they
> missed tagging some number of commits, which might remind them to do
> better, or make them decide that they want to do something more
> explicit, such as have their own stable backports initiative ala XFS.
> 
> More generally, it seems to me that we are conflating multiple issues
> here in this discussion which may be making it harder for us make
> progress on the question.
> 
> 1) There are some subsystems who don't care about tagging commits,
>    either Fixes: or Cc: stable, or both;
> 
> 2) There are subsystems which are trying to appropriately tag commits, but:
>    a) Sometimes they will make a mistake, and forget to cc: stable
>    b) Sometimes it's too hard (tm) to figure out what is the commit which
>       introduces the regression, so they either make up something (e.g.,
>       hmm, it looks like commit XYZ changes one of the line that is touched
>       by the patch, so <shrug emoji>), or they will add a Cc: stable but
>       not supply a Fixes: tag

Too hard doesn't work as an excuse because someone has to figure it out,
and it may as well be the subsystem expert.

I've already added a checkpatch warning when people CC stable but don't
include a Fixes tag.  I also plan to start going back to maintainers
and manually asking them for Fixes tags.  This will be attached to the
patch.msgid.link URL so the stable tooling can pick up Fixes tags which
are added later.

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")".

regards,
dan carpenter

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Steven Rostedt]

On Mon, 15 Jul 2024 14:06:55 -0500
Dan Carpenter <dan.carpenter@linaro.org> 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")".

That commit is the most common Fixes tag in the git history:

$ git log | grep '^    Fixes:' | cut -d' ' -f6 | perl -e 'while (<>) { chomp; $cnt{$_}++ } foreach $c (sort { $cnt{$a} < $cnt{$b} } keys %cnt) { print "$c $cnt{$c}\n"; }' | head
1da177e4c3f4 528
76ad4f0ee747 40
e1eaea46bb40 39
9cb837480424 20
9af6bcb11e12 14
99cb0dbd47a1 11
848440544b41 9
f91696f2f053 8
89d44f0a6c73 7
4ed8eb6570a4 6

-- Steve

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[James Bottomley]

On Mon, 2024-07-15 at 14:06 -0500, Dan Carpenter 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")".

Actually, we have a history tree (based on Bitkeeper) that goes back
before that:

https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/

So I'd tag these as

Fixes: 3bc0a5f5c1 (history "ext3: reservation info cleanup")

Regards,

James

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Steven Rostedt]

On Mon, 15 Jul 2024 15:24:42 -0400
James Bottomley <James.Bottomley@HansenPartnership.com> wrote:

> On Mon, 2024-07-15 at 14:06 -0500, Dan Carpenter 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")".  
> 
> Actually, we have a history tree (based on Bitkeeper) that goes back
> before that:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/
> 
> So I'd tag these as
> 
> Fixes: 3bc0a5f5c1 (history "ext3: reservation info cleanup")
> 

I don't think that is necessary. If something is found to be broken
from before 2005, I don't believe it's worth the effort to search
another tree for the actual change that introduced the problem. When
you go that far, it becomes only an academic exercise.

-- Steve

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[James Bottomley]

On Mon, 2024-07-15 at 15:28 -0400, Steven Rostedt wrote:
> On Mon, 15 Jul 2024 15:24:42 -0400
> James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> 
> > On Mon, 2024-07-15 at 14:06 -0500, Dan Carpenter 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")".  
> > 
> > Actually, we have a history tree (based on Bitkeeper) that goes
> > back before that:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/
> > 
> > So I'd tag these as
> > 
> > Fixes: 3bc0a5f5c1 (history "ext3: reservation info cleanup")
> > 
> 
> I don't think that is necessary. If something is found to be broken
> from before 2005, I don't believe it's worth the effort to search
> another tree for the actual change that introduced the problem. When
> you go that far, it becomes only an academic exercise.

Some people are naturally curious, so it makes sense to have a format
for them even if we don't require everyone to do it.

James

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Dan Carpenter]

On Mon, Jul 15, 2024 at 03:24:42PM -0400, James Bottomley wrote:
> On Mon, 2024-07-15 at 14:06 -0500, Dan Carpenter 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")".
> 
> Actually, we have a history tree (based on Bitkeeper) that goes back
> before that:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/
> 
> So I'd tag these as
> 
> Fixes: 3bc0a5f5c1 (history "ext3: reservation info cleanup")

I don't think most people have the ancient history so would just confuse
people and break scripts.

regards,
dan carpenter

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Mauro Carvalho Chehab]

Em Mon, 15 Jul 2024 15:24:42 -0400
James Bottomley <James.Bottomley@HansenPartnership.com> escreveu:

> On Mon, 2024-07-15 at 14:06 -0500, Dan Carpenter 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")".  
> 
> Actually, we have a history tree (based on Bitkeeper) that goes back
> before that:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/
> 
> So I'd tag these as
> 
> Fixes: 3bc0a5f5c1 (history "ext3: reservation info cleanup")

The problem with such tags is that eventually conflicts may arise
with other checksum. Also, people may not notice that the fixes are
referring to a different tree.

IMO, if you want to reference older commits, the best would be to use
Link instead:

Link: https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=3bc0a5f5c1

and perhaps keep:

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")

Regards,
Mauro

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Dmitry Torokhov]

On Mon, Jul 15, 2024 at 02:06:55PM -0500, Dan Carpenter wrote:
> On Mon, Jul 15, 2024 at 02:00:00PM -0400, Theodore Ts'o wrote:
> > On Sun, Jul 14, 2024 at 04:18:00PM -0400, Sasha Levin wrote:
> > > From my experience, most issues tracked by regzbot and fixed upstream
> > > don't actually have a stable tag. Here's one I just looked at a few days
> > > ago: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f451fd97dd2b78f286379203a47d9d295c467255
> > > 
> > > And I'm actually happy to point to that one as an example because the
> > > ext4 folks are usually great about stable tags.
> > >
> > > Should we have not taken that commit?
> > 
> > Yep, that's just a mistake on our (my) part; you should have taken
> > that commit, and my thanks for taking it without asking us.
> > 
> > That being said, maybe one path forward is that the stable team
> > *should* be asking the subsystem maintainers about.  "Hey, the
> > following commits appear to be backported, but you didn't add a cc:
> > stable.  We plan to backport them unless you complain."  This has the
> > benefit of giving feedback to the subsystem maintainers that they they
> > missed tagging some number of commits, which might remind them to do
> > better, or make them decide that they want to do something more
> > explicit, such as have their own stable backports initiative ala XFS.
> > 
> > More generally, it seems to me that we are conflating multiple issues
> > here in this discussion which may be making it harder for us make
> > progress on the question.
> > 
> > 1) There are some subsystems who don't care about tagging commits,
> >    either Fixes: or Cc: stable, or both;
> > 
> > 2) There are subsystems which are trying to appropriately tag commits, but:
> >    a) Sometimes they will make a mistake, and forget to cc: stable
> >    b) Sometimes it's too hard (tm) to figure out what is the commit which
> >       introduces the regression, so they either make up something (e.g.,
> >       hmm, it looks like commit XYZ changes one of the line that is touched
> >       by the patch, so <shrug emoji>), or they will add a Cc: stable but
> >       not supply a Fixes: tag
> 
> Too hard doesn't work as an excuse because someone has to figure it out,
> and it may as well be the subsystem expert.
> 
> I've already added a checkpatch warning when people CC stable but don't
> include a Fixes tag.  I also plan to start going back to maintainers
> and manually asking them for Fixes tags.  This will be attached to the
> patch.msgid.link URL so the stable tooling can pick up Fixes tags which
> are added later.
> 
> 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.

Thanks.

-- 
Dmitry

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Dmitry Torokhov]

On Mon, Jul 15, 2024 at 07:04:34PM +0100, Mark Brown wrote:
> On Mon, Jul 15, 2024 at 04:40:01PM +0200, Greg KH wrote:
> > On Mon, Jul 15, 2024 at 04:34:19PM +0200, Alexandre Belloni wrote:
> 
> > > I'm very surprised that it didn't cross anyone's mind yet that
> > > contributors and maintainers don't bother using Cc: stable because they
> > > don't care about stable for various reasons. So the behaviour is nether
> > > good nor bad and doesn't have to be penalized.
> 
> > Totally true.  The first rule of the stable tree is "it will not put
> > additional burden on any developer or maintainer that doesn't want to
> > participate in it."  So if you don't want to deal with it, wonderful,
> > don't take anything and just don't worry about it.
> 
> FWIW as we've discussed before I stopped adding explicit CCs for the
> most part since AUTOSEL is picking up far more than I ever would so it
> doesn't seem worth the effort to filter which is a variation on what
> Alexandre mentioned.

I am still tagging cc: stable for fixes that I truly believe belong in
stable, but otherwise I let AUTOSEL do whatever it wants because I do
not own stable tree. I however also believe that AUTOSEL picks up much
more than it should. Maybe if "Fixes: " tags commit older than <age> (2
years?) it should ignore such fixes?

Thanks.

-- 
Dmitry

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Greg KH]

On Mon, Jul 15, 2024 at 07:04:34PM +0100, Mark Brown wrote:
> On Mon, Jul 15, 2024 at 04:40:01PM +0200, Greg KH wrote:
> > On Mon, Jul 15, 2024 at 04:34:19PM +0200, Alexandre Belloni wrote:
> 
> > > I'm very surprised that it didn't cross anyone's mind yet that
> > > contributors and maintainers don't bother using Cc: stable because they
> > > don't care about stable for various reasons. So the behaviour is nether
> > > good nor bad and doesn't have to be penalized.
> 
> > Totally true.  The first rule of the stable tree is "it will not put
> > additional burden on any developer or maintainer that doesn't want to
> > participate in it."  So if you don't want to deal with it, wonderful,
> > don't take anything and just don't worry about it.
> 
> FWIW as we've discussed before I stopped adding explicit CCs for the
> most part since AUTOSEL is picking up far more than I ever would so it
> doesn't seem worth the effort to filter which is a variation on what
> Alexandre mentioned.

If you want to, we can add your subsystems to the "do not run AUTOSEL or
Fixes:" list and then you can just mark whatever you feel is needed for
stable directly.  We already do that for many other subsystems (kvm, mm,
xfs, and others), just let us know and we will be glad to do so as it
reduces our load AND it lets you be in control of what is in stable.

thanks,

greg k-h

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Greg KH]

On Mon, Jul 15, 2024 at 01:47:14PM -0700, Dmitry Torokhov wrote:
> On Mon, Jul 15, 2024 at 02:06:55PM -0500, Dan Carpenter wrote:
> > On Mon, Jul 15, 2024 at 02:00:00PM -0400, Theodore Ts'o wrote:
> > > On Sun, Jul 14, 2024 at 04:18:00PM -0400, Sasha Levin wrote:
> > > > From my experience, most issues tracked by regzbot and fixed upstream
> > > > don't actually have a stable tag. Here's one I just looked at a few days
> > > > ago: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f451fd97dd2b78f286379203a47d9d295c467255
> > > > 
> > > > And I'm actually happy to point to that one as an example because the
> > > > ext4 folks are usually great about stable tags.
> > > >
> > > > Should we have not taken that commit?
> > > 
> > > Yep, that's just a mistake on our (my) part; you should have taken
> > > that commit, and my thanks for taking it without asking us.
> > > 
> > > That being said, maybe one path forward is that the stable team
> > > *should* be asking the subsystem maintainers about.  "Hey, the
> > > following commits appear to be backported, but you didn't add a cc:
> > > stable.  We plan to backport them unless you complain."  This has the
> > > benefit of giving feedback to the subsystem maintainers that they they
> > > missed tagging some number of commits, which might remind them to do
> > > better, or make them decide that they want to do something more
> > > explicit, such as have their own stable backports initiative ala XFS.
> > > 
> > > More generally, it seems to me that we are conflating multiple issues
> > > here in this discussion which may be making it harder for us make
> > > progress on the question.
> > > 
> > > 1) There are some subsystems who don't care about tagging commits,
> > >    either Fixes: or Cc: stable, or both;
> > > 
> > > 2) There are subsystems which are trying to appropriately tag commits, but:
> > >    a) Sometimes they will make a mistake, and forget to cc: stable
> > >    b) Sometimes it's too hard (tm) to figure out what is the commit which
> > >       introduces the regression, so they either make up something (e.g.,
> > >       hmm, it looks like commit XYZ changes one of the line that is touched
> > >       by the patch, so <shrug emoji>), or they will add a Cc: stable but
> > >       not supply a Fixes: tag
> > 
> > Too hard doesn't work as an excuse because someone has to figure it out,
> > and it may as well be the subsystem expert.
> > 
> > I've already added a checkpatch warning when people CC stable but don't
> > include a Fixes tag.  I also plan to start going back to maintainers
> > and manually asking them for Fixes tags.  This will be attached to the
> > patch.msgid.link URL so the stable tooling can pick up Fixes tags which
> > are added later.
> > 
> > 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.

But if there is the "Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")" line, I
_know_ it should also go to older kernels, and so I send the "FAILED"
email letting everyone know something went wrong and that allows those
developers who watch for them, to pick them up and potentially provide a
working fix.

So yes, it does bring a load of value to do this, please do it if you
know it.

thanks,

greg k-h

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Greg KH]

On Mon, Jul 15, 2024 at 02:39:59PM -0500, Dan Carpenter wrote:
> On Mon, Jul 15, 2024 at 03:24:42PM -0400, James Bottomley wrote:
> > On Mon, 2024-07-15 at 14:06 -0500, Dan Carpenter 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")".
> > 
> > Actually, we have a history tree (based on Bitkeeper) that goes back
> > before that:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/
> > 
> > So I'd tag these as
> > 
> > Fixes: 3bc0a5f5c1 (history "ext3: reservation info cleanup")
> 
> I don't think most people have the ancient history so would just confuse
> people and break scripts.

Yes, as an example, it breaks mine which I use to find out how far back
a commit should be backported to.  For the above, it gives:
	Could not get sha1 for 3bc0a5f5c1. Skipping.
as an output :(

thanks,

greg k-h

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Takashi Iwai]

On Tue, 16 Jul 2024 08:28:53 +0200,
Greg KH wrote:
> 
> On Mon, Jul 15, 2024 at 01:47:14PM -0700, Dmitry Torokhov wrote:
> > On Mon, Jul 15, 2024 at 02:06:55PM -0500, Dan Carpenter wrote:
> > > On Mon, Jul 15, 2024 at 02:00:00PM -0400, Theodore Ts'o wrote:
> > > > On Sun, Jul 14, 2024 at 04:18:00PM -0400, Sasha Levin wrote:
> > > > > From my experience, most issues tracked by regzbot and fixed upstream
> > > > > don't actually have a stable tag. Here's one I just looked at a few days
> > > > > ago: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f451fd97dd2b78f286379203a47d9d295c467255
> > > > > 
> > > > > And I'm actually happy to point to that one as an example because the
> > > > > ext4 folks are usually great about stable tags.
> > > > >
> > > > > Should we have not taken that commit?
> > > > 
> > > > Yep, that's just a mistake on our (my) part; you should have taken
> > > > that commit, and my thanks for taking it without asking us.
> > > > 
> > > > That being said, maybe one path forward is that the stable team
> > > > *should* be asking the subsystem maintainers about.  "Hey, the
> > > > following commits appear to be backported, but you didn't add a cc:
> > > > stable.  We plan to backport them unless you complain."  This has the
> > > > benefit of giving feedback to the subsystem maintainers that they they
> > > > missed tagging some number of commits, which might remind them to do
> > > > better, or make them decide that they want to do something more
> > > > explicit, such as have their own stable backports initiative ala XFS.
> > > > 
> > > > More generally, it seems to me that we are conflating multiple issues
> > > > here in this discussion which may be making it harder for us make
> > > > progress on the question.
> > > > 
> > > > 1) There are some subsystems who don't care about tagging commits,
> > > >    either Fixes: or Cc: stable, or both;
> > > > 
> > > > 2) There are subsystems which are trying to appropriately tag commits, but:
> > > >    a) Sometimes they will make a mistake, and forget to cc: stable
> > > >    b) Sometimes it's too hard (tm) to figure out what is the commit which
> > > >       introduces the regression, so they either make up something (e.g.,
> > > >       hmm, it looks like commit XYZ changes one of the line that is touched
> > > >       by the patch, so <shrug emoji>), or they will add a Cc: stable but
> > > >       not supply a Fixes: tag
> > > 
> > > Too hard doesn't work as an excuse because someone has to figure it out,
> > > and it may as well be the subsystem expert.
> > > 
> > > I've already added a checkpatch warning when people CC stable but don't
> > > include a Fixes tag.  I also plan to start going back to maintainers
> > > and manually asking them for Fixes tags.  This will be attached to the
> > > patch.msgid.link URL so the stable tooling can pick up Fixes tags which
> > > are added later.
> > > 
> > > 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.


thanks,

Takashi

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Jason Gunthorpe]

On Sun, Jul 14, 2024 at 03:20:48PM -0400, James Bottomley wrote:

> Here you seem to be assuming that every commit with a Fixes tag for a
> prior release should be in stable ... I don't think that's true at all.

I think that is the main issue here. Having talked to enough people in
the server/enterprise space there are a lot of different ideas about
what should be backported into the various kernel forks out there.

Just looking at the patch meta data there is a spectrum of:
   No-Fixes -> Fixes -> Fixes&Stable

As I maintainer I don't really have a stake in backporter's policies. I
feel like the best I can do is try hard to mark things with Fixes tags
and provide quality commit messages. I know the various forks will
make decisions based on that.

IMHO the cc stable is a nice way to provide an additional opinion that
the patch was thought to be critical when it was merged. People
running forks should really examine those patches closely.

Jason

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 868 bytes --]

On Tue, Jul 16, 2024 at 08:25:34AM +0200, Greg KH wrote:
> On Mon, Jul 15, 2024 at 07:04:34PM +0100, Mark Brown wrote:

> > FWIW as we've discussed before I stopped adding explicit CCs for the
> > most part since AUTOSEL is picking up far more than I ever would so it
> > doesn't seem worth the effort to filter which is a variation on what
> > Alexandre mentioned.

> If you want to, we can add your subsystems to the "do not run AUTOSEL or
> Fixes:" list and then you can just mark whatever you feel is needed for
> stable directly.  We already do that for many other subsystems (kvm, mm,
> xfs, and others), just let us know and we will be glad to do so as it
> reduces our load AND it lets you be in control of what is in stable.

Given that other bits of the stable tree are going with the looser
approach anyway it doesn't seem worth it to apply more filtering.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Dan Carpenter]

On Tue, Jul 16, 2024 at 11:51:23AM -0300, Jason Gunthorpe wrote:
> On Sun, Jul 14, 2024 at 03:20:48PM -0400, James Bottomley wrote:
> 
> > Here you seem to be assuming that every commit with a Fixes tag for a
> > prior release should be in stable ... I don't think that's true at all.
> 
> I think that is the main issue here. Having talked to enough people in
> the server/enterprise space there are a lot of different ideas about
> what should be backported into the various kernel forks out there.
> 
> Just looking at the patch meta data there is a spectrum of:
>    No-Fixes -> Fixes -> Fixes&Stable

(I've been looking at stable kernels for Linaro).

There are a few reasons to apply patches without Fixes tags.

1) Quite a few patches that should have Fixes tags don't.  For example
   syzbot stuff should always get a Fixes tag.
2) Sometimes we don't realize it's a fix until later.
3) Adding PCI IDs.
4) Other fixes depend on cleanups before they can be backported.
   For example, this patch:
   https://lore.kernel.org/all/20240429133832.9547-1-kabel@kernel.org/
   It's a one line fix.  But it's easier to apply if we apply it if we
   apply a couple other patches first.
   https://lore.kernel.org/all/20180319100523.24498-2-u.kleine-koenig@pengutronix.de/
   https://lore.kernel.org/all/20191105001301.27966-3-andrew@lunn.ch/
   The second patch doesn't do anything, but now the fix can be applied
   cleanly.

regards,
dan carpenter

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Dan Carpenter]

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.

Eventually, if patches don't apply then they're manually reviewed.  I
find it really frustrating to review stable patches without a Fixes tag.
When I see it I think that probably I wouldn't have had to manually
review the failed patch at all if they had used a Fixes tag.  (I just
naturally start assuming the worst about people after I've spent five
hours backporting patches.  I'm not going to investigate it either
because it's not security related.  I'm just going to assume the worse,
say some curse words, and move on.)

Of course, for security related stuff the Fixes tag is absolutely
essential.  For example, this patch does not backport cleanly to 4.14
kernels.
https://lore.kernel.org/all/20240501125448.896529-1-edumazet@google.com/
It's really nice that Eric Dumazet added a Fixes: 1da177e4c3f4
("Linux-2.6.12-rc2").

As a upstream reviewer, I think the Fixes tag is important as well.  I
don't think I've ever used the Fixes 1da177e4c3f4 ("Linux-2.6.12-rc2")
tag because I know it annoys people but I write that "This bug has been
there since before git" because the information is useful.

regards,
dan carpenter

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Takashi Iwai]

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.  It's a thing that can be
backported safely whenever it's cleanly applied and built, but
otherwise it can be simply skipped.  It's no "regression", per se, but
a new feature that didn't exist in the past, after all.

That said, if a change is important and should be inevitably
backported to stable, I'd point Fixes tag, too.


thanks,

Takashi

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Dan Carpenter]

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

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[James Bottomley]

On Thu, 2024-07-18 at 09:48 -0500, Dan Carpenter wrote:
> On Thu, Jul 18, 2024 at 09:34:04AM +0200, Takashi Iwai wrote:
[...]
> > 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.

Really, no, that's what got us into this issue in the first place: if
you only tag regressions with Fixes:, then we don't need cc:stable. 
Fixes: should be for anything that updated what was done in that prior
commit (including white space and spellings).  That way there's no
debate about whether it should apply and it's easy for Maintainers to
verify.

cc:stable should be for stuff that is backport worthy (regressions
etc), which does require some thought and a bit of guesswork.  And if
something later proves to be a bug fix that wasn't tagged cc:stable it
can be sent to stable later (and the Fixes: tag that wouldn't otherwise
have been applied under your proposed rule helps to locate what trees
it should be backported to).

James

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Dan Carpenter]

On Thu, Jul 18, 2024 at 10:56:14AM -0400, James Bottomley wrote:
> On Thu, 2024-07-18 at 09:48 -0500, Dan Carpenter wrote:
> > On Thu, Jul 18, 2024 at 09:34:04AM +0200, Takashi Iwai wrote:
> [...]
> > > 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.
> 
> Really, no, that's what got us into this issue in the first place: if
> you only tag regressions with Fixes:, then we don't need cc:stable. 
> Fixes: should be for anything that updated what was done in that prior
> commit (including white space and spellings).  That way there's no
> debate about whether it should apply and it's easy for Maintainers to
> verify.

I'm honestly surprised you would say this.  You're very much in the
minority view here.  I've reviewed over a thousand spelling mistake
fixes across the whole tree as part of kernel-janitors and I don't
remember anyone asking for a Fixes tag.

The one area where people debate is for harmless static checker fixes
such as deleting an unnecessary variable, but the majority of
maintainers say that doesn't qualify for a Fixes tag.

The majority opinion is that Fixes is only for bugs.

regards,
dan carpenter

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[NeilBrown]

On Fri, 19 Jul 2024, Dan Carpenter wrote:
> On Thu, Jul 18, 2024 at 10:56:14AM -0400, James Bottomley wrote:
> > On Thu, 2024-07-18 at 09:48 -0500, Dan Carpenter wrote:
> > > On Thu, Jul 18, 2024 at 09:34:04AM +0200, Takashi Iwai wrote:
> > [...]
> > > > 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.
> > 
> > Really, no, that's what got us into this issue in the first place: if
> > you only tag regressions with Fixes:, then we don't need cc:stable. 
> > Fixes: should be for anything that updated what was done in that prior
> > commit (including white space and spellings).  That way there's no
> > debate about whether it should apply and it's easy for Maintainers to
> > verify.
> 
> I'm honestly surprised you would say this.  You're very much in the
> minority view here.  I've reviewed over a thousand spelling mistake
> fixes across the whole tree as part of kernel-janitors and I don't
> remember anyone asking for a Fixes tag.
> 
> The one area where people debate is for harmless static checker fixes
> such as deleting an unnecessary variable, but the majority of
> maintainers say that doesn't qualify for a Fixes tag.
> 
> The majority opinion is that Fixes is only for bugs.

First you said "regressions".  Then you said "bugs".  Which is it?

If I add a new feature that doesn't work as documented, it is clearly a
bug.  I don't think it is a regression.  I think the patch that corrects
the bug (either the code or the documentation or both) is a fix and
should be marked as such.

NeilBrown

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Dan Carpenter]

On Fri, Jul 19, 2024 at 10:49:17AM +1000, NeilBrown wrote:
> On Fri, 19 Jul 2024, Dan Carpenter wrote:
> > On Thu, Jul 18, 2024 at 10:56:14AM -0400, James Bottomley wrote:
> > > On Thu, 2024-07-18 at 09:48 -0500, Dan Carpenter wrote:
> > > > On Thu, Jul 18, 2024 at 09:34:04AM +0200, Takashi Iwai wrote:
> > > [...]
> > > > > 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.
> > > 
> > > Really, no, that's what got us into this issue in the first place: if
> > > you only tag regressions with Fixes:, then we don't need cc:stable. 
> > > Fixes: should be for anything that updated what was done in that prior
> > > commit (including white space and spellings).  That way there's no
> > > debate about whether it should apply and it's easy for Maintainers to
> > > verify.
> > 
> > I'm honestly surprised you would say this.  You're very much in the
> > minority view here.  I've reviewed over a thousand spelling mistake
> > fixes across the whole tree as part of kernel-janitors and I don't
> > remember anyone asking for a Fixes tag.
> > 
> > The one area where people debate is for harmless static checker fixes
> > such as deleting an unnecessary variable, but the majority of
> > maintainers say that doesn't qualify for a Fixes tag.
> > 
> > The majority opinion is that Fixes is only for bugs.
> 
> First you said "regressions".  Then you said "bugs".  Which is it?
> 
> If I add a new feature that doesn't work as documented, it is clearly a
> bug.  I don't think it is a regression.  I think the patch that corrects
> the bug (either the code or the documentation or both) is a fix and
> should be marked as such.

Yeah.  I said that badly.  It should be for bugs.  Fixes tags mostly
point to "Add <support> for something".  They mostly aren't regressions.

regards,
dan carpenter

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Vegard Nossum]

On 19/07/2024 03:35, Dan Carpenter wrote:
> On Fri, Jul 19, 2024 at 10:49:17AM +1000, NeilBrown wrote:
>> On Fri, 19 Jul 2024, Dan Carpenter wrote:
>>> On Thu, Jul 18, 2024 at 10:56:14AM -0400, James Bottomley wrote:
>>> The majority opinion is that Fixes is only for bugs.
>>
>> First you said "regressions".  Then you said "bugs".  Which is it?
>>
>> If I add a new feature that doesn't work as documented, it is clearly a
>> bug.  I don't think it is a regression.  I think the patch that corrects
>> the bug (either the code or the documentation or both) is a fix and
>> should be marked as such.
> 
> Yeah.  I said that badly.  It should be for bugs.  Fixes tags mostly
> point to "Add <support> for something".  They mostly aren't regressions.

With my distro hat on, I definitely think Fixes: should be used for
anything that fixes a bug, regardless of whether it was a bug introduced
with a new feature or it was a regression.

Fixes: is incredibly useful in the following way: You can check whether
you need to apply/cherry-pick a patch simply by checking whether you
have the commit it fixes (or a backport of it) in your branch. As a
distro (or even as a user), if you have a buggy commit in your branch
and there's a known fix for it, you almost always want the fix.

I don't really like the "Cc: stable # version+" tag for the exact same
reason: Authors/maintainers may be looking at when the bug was
introduced and decide not to put that "Cc: stable" tag since the patch
that introduced the problem was in an earlier -rc of the same release or
never appeared in a stable kernel _at that point in time_. But that's
the wrong approach, as the fixed commit may be backported to stable (or
a distro kernel) many years after it was merged into mainline (just look
at all the commits with Stable-dep-of: in stable), and without the
Fixes: tag we have no way to know that the fixed commit has a bug and
needs to have subsequent patches applied.

IOW, for any patch that fixes an issue, the full determination of
whether something should go into stable can and should NOT be made by
the patch author/maintainer when the patch is authored/merged to
mainline; that decision is guaranteed to go stale very quickly.

 From my point of view, ensuring that people accurately annotate Fixes:
in all cases (bugs in new features, regressions, whatever it is) should
be the top priority in terms of working out and documenting the rules,
and also makes the rules simpler; "Cc: stable" can be reserved for all
the other categories of stable patches like new device IDs, improved
driver support, or whatever. And downstream forks (like stable and most
distro kernels) can then make the determination of whether they need a
patch simply by looking at Fixes: tags and whether those commits appear
in their branches.

Thanks,


Vegard

Re: Proposal: Enhancing Commit Tagging for Stable Kernel Branches

[Jiri Kosina]

On Fri, 19 Jul 2024, Vegard Nossum wrote:

> With my distro hat on, I definitely think Fixes: should be used for
> anything that fixes a bug, regardless of whether it was a bug introduced
> with a new feature or it was a regression.
> 
> Fixes: is incredibly useful in the following way: You can check whether
> you need to apply/cherry-pick a patch simply by checking whether you
> have the commit it fixes (or a backport of it) in your branch. As a
> distro (or even as a user), if you have a buggy commit in your branch
> and there's a known fix for it, you almost always want the fix.
> 
> I don't really like the "Cc: stable # version+" tag for the exact same
> reason: Authors/maintainers may be looking at when the bug was
> introduced and decide not to put that "Cc: stable" tag since the patch
> that introduced the problem was in an earlier -rc of the same release or
> never appeared in a stable kernel _at that point in time_. But that's
> the wrong approach, as the fixed commit may be backported to stable (or
> a distro kernel) many years after it was merged into mainline (just look
> at all the commits with Stable-dep-of: in stable), and without the
> Fixes: tag we have no way to know that the fixed commit has a bug and
> needs to have subsequent patches applied.

For me, the biggest question coming out of this discussion is: what are 
the groups / types of patches that should be getting "Cc: stable" despite 
not being fixes for some older code?

As "fixing bugs" should be all what the stable tree is supposed to be 
about, shouldn't it? (*)

One of the obvious categories is "fixing HW issue", being it either 
mitigations for HW security issues, or adding new device IDs, adding 
quirks, etc.

But what next?



(*) There is this famous defition at the beginning of

	Documentation/process/stable-kernel-rules.rst

but I guess it's obvious to everybody that it doesn't apply.

-- 
Jiri Kosina
SUSE Labs