docs: add instruction about handling regressions to our core texts for contributors
Subsystems (3)
Thorsten Leemhuis
maintainercover docs: add instruction about handling regressions to our core texts for contributors
Add some more detailed instruction on handling regressions to our core texts for contributors, e.g. Documentation/process/6.Followthrough.rst and Documentation/process/submitting-patches.rst. This was asked for at the last Maintainer Summit[1] and is meant to help clarifing what's expected.
The added instructions are somewhat inspired by existing and sometimes similar text in Documentation/process/handling-regressions.rst which is semoved during the series.
The patch-set starts with some basics, which are based on the existing text or recent statements from Linus. I tried to stay close to what was said, but used my own words here -- if I got something wrong just tell me so I can fix this.
The guidelines then are extended in multiple steps to cover more things and be more specific in some areas. Some of that is meant to work against problems I noticed during my regression tracking work. Most of these are my words. These thus need to be discussed properly and in the end ideally ACKed by Linus, so we all know what he wants. The parts I expect to get more discussion are found near the end of the patchset.
---
v1:
- initial version
Thorsten Leemhuis (6):
docs: more detailed instructions on handling regressions
docs: 6.Followthrough.rst: when to involved Linus in regressions
docs: 6.Followthrough.rst: interaction with stable wrt to regressions
docs: 6.Followthrough.rst: tags to use in regressions fixes
docs: 6.Followthrough.rst: more specific advice on fixing regressions
docs: 6.Followthrough.rst: advice on handling regressions fixes
Documentation/process/6.Followthrough.rst | 152 ++++++++++++++++-- .../process/handling-regressions.rst | 111 +------------ Documentation/process/submitting-patches.rst | 20 +++ 3 files changed, 157 insertions(+), 126 deletions(-)
base-commit: 010a08cbf0d63b246c4cd0d385c3f664add2322d
Signature · 1 line
2.45.0
Thorsten Leemhuis
maintainerPATCH 1/6 docs: more detailed instructions on handling regressions
Add a few more specific guidelines on handling regressions to the kernel's two most prominent guides about contributing to Linux, as developers apparently work with quite different interpretations of what Linus expects.
Changes like this were asked for during the Maintainers Summit 2024. The four rules of thumb spelled out are all closely based on statements Linus made there; LWN documented all except "Expedite fixing regressions that reached a release deemed for end users.." in their coverage [1].
This removes equivalent paragraphs from a section in Documentation/process/handling-regressions.rst, which will become mostly obsolete through this and follow-up changes.
--- Documentation/process/6.Followthrough.rst | 63 ++++++++++++++----- .../process/handling-regressions.rst | 12 +--- Documentation/process/submitting-patches.rst | 20 ++++++ 3 files changed, 68 insertions(+), 27 deletions(-)
Documentation/process/6.Followthrough.rst +46 -17
@@ -161,16 +161,8 @@ a driver for hardware which is not yet available, you will be surprised by how many people will build your code into their kernels. And, of course, where there are testers, there will be bug reports. -The worst sort of bug reports are regressions. If your patch causes a -regression, you'll find an uncomfortable number of eyes upon you; -regressions need to be fixed as soon as possible. If you are unwilling or -unable to fix the regression (and nobody else does it for you), your patch -will almost certainly be removed during the stabilization period. Beyond -negating all of the work you have done to get your patch into the mainline, -having a patch pulled as the result of a failure to fix a regression could -well make it harder for you to get work merged in the future. - -After any regressions have been dealt with, there may be other, ordinary +If the bug is a regression, you must fix it -- see 'Handling Regressions' +below. After any regressions have been dealt with, there may be other, ordinary bugs to deal with. The stabilization period is your best opportunity to fix these bugs and ensure that your code's debut in a mainline kernel release is as solid as possible. So, please, answer bug reports, and fix
@@ -180,13 +172,50 @@ ones have been taken care of. And don't forget that there are other milestones which may also create bug reports: the next mainline stable release, when prominent distributors pick -up a version of the kernel containing your patch, etc. Continuing to -respond to these reports is a matter of basic pride in your work. If that -is insufficient motivation, though, it's also worth considering that the -development community remembers developers who lose interest in their code -after it's merged. The next time you post a patch, they will be evaluating -it with the assumption that you will not be around to maintain it -afterward. +up a version of the kernel containing your patch, etc. Regressions reported +at this point again must be fixed. For other bugs, continuing to respond to +these reports is a matter of basic pride in your work. If that is insufficient +motivation, though, it's also worth considering that the development community +remembers developers who lose interest in their code after it's merged. The +next time you post a patch, they will be evaluating it with the assumption +that you will not be around to maintain it afterward. + + +Handling Regressions +-------------------- + +If one of your changes causes a regression, you must fix it, as "*we don't cause +regressions*" is the first rule of Linux kernel development. + +A bug is considered a regression when something that worked fine with one Linux +kernel works worse or not at all with a newer version. For a more detailed +definition, see Documentation/admin-guide/reporting-regressions.rst. + +You must fix regressions quickly while applying reasonable care to prevent +causing another regression. The balance and thus the appropriate approach +depends on the situation. Below rules of thumb serve as a guide. Be aware that +maintainers and other developers will take note if you fail to handle regression +appropriately, especially if they then have to fix the problem themselves: this +could well make it harder for you to incorporate future changes. + +On timing: + + - Expedite fixing regressions that reached releases deemed for end users + through new mainline releases or stable backports during the past year. + + - If the culprit was mainlined during the current development cycle and not + backported to stable, fix the regression before -rc6. + + - If a proper regression fix is unlikely to become ready in a reasonable + timeframe, resolve the regression by reverting the culprit. This is + considered an reputable approach, as it allows working out and merging an + improved variant of the change calmly. + +On procedure: + + - Regression fixes are not required to spend time in linux-next, but depending + on the fix and the alignment with pull requests it might be beneficial to + have them in there for a day or two. Other things that can happen
Documentation/process/handling-regressions.rst +2 -10
@@ -153,8 +153,8 @@ only these options: severe or affects many users -- either in general or in prevalent environments. -How to realize that in practice depends on various factors. Use the following -rules of thumb as a guide. +How to realize that in practice depends on various factors. Use the rules of +thumb outlined in Documentation/process/6.Followthrough.rst as a guide. In general:
@@ -162,9 +162,6 @@ In general: latter concerns a severe issue (e.g. acute security vulnerability, data loss, bricked hardware, ...). - * Expedite fixing mainline regressions that recently made it into a proper - mainline, stable, or longterm release (either directly or via backport). - * Do not consider regressions from the current cycle as something that can wait till the end of the cycle, as the issue might discourage or prevent users and CI systems from testing mainline now or generally.
@@ -194,11 +191,6 @@ On timing once the culprit of a regression is known: On procedure: - * Always consider reverting the culprit, as it's often the quickest and least - dangerous way to fix a regression. Don't worry about mainlining a fixed - variant later: that should be straight-forward, as most of the code went - through review once already. - * Try to resolve any regressions introduced in mainline during the past twelve months before the current development cycle ends: Linus wants such regressions to be handled like those from the current cycle, unless fixing
Documentation/process/submitting-patches.rst +20 -0
@@ -842,6 +842,26 @@ Make sure that base commit is in an official maintainer/mainline tree and not in some internal, accessible only to you tree - otherwise it would be worthless. + +Handling Regressions +-------------------- + +If one of your changes causes a regression, you must fix it, as "*we don't cause +regressions*" is the first rule of Linux kernel development. + +A bug is considered a regression when something that worked fine with one Linux +kernel works worse or not at all with a newer version. For a more detailed +definition, see Documentation/admin-guide/reporting-regressions.rst. + +You must fix regressions quickly while applying reasonable care to prevent +causing another regression. The balance and thus the appropriate approach +depends on the situation. See Documentation/process/6.Followthrough.rst for +more rules of thumb that serve as a guide. Be aware that maintainers and other +developers will take note if you fail to handle regression appropriately, +especially if they then have to fix the problem themselves: this could well +make it harder for you to incorporate future changes. + + Tooling -------
Signature · 1 line
2.45.0
Jonathan Corbet
maintainerRe: [PATCH v1 1/6] docs: more detailed instructions on handling regressions
Quoted 9 lines · Thorsten Leemhuis
Add a few more specific guidelines on handling regressions to the kernel's two most prominent guides about contributing to Linux, as developers apparently work with quite different interpretations of what Linus expects.
Changes like this were asked for during the Maintainers Summit 2024. The four rules of thumb spelled out are all closely based on statements Linus made there; LWN documented all except "Expedite fixing regressions that reached a release deemed for end users.." in their coverage [1].
"deemed" used in this way is a bit strange. "intended" perhaps?
Documentation/process/6.Followthrough.rst ones have been taken care of. · line 204
Quoted 4 lines
Documentation/process/6.Followthrough.rst +4
-0+On timing: + + - Expedite fixing regressions that reached releases deemed for end users + through new mainline releases or stable backports during the past year.
Here too.
That little nit is all I can find to complain about in this one, though :)
jon
Thorsten Leemhuis
maintainerRe: [PATCH v1 1/6] docs: more detailed instructions on handling regressions
Quoted 7 lines · Jonathan Corbet
Quoted 5 lines · Thorsten Leemhuis
Add a few more specific guidelines on handling regressions to the kernel's two most prominent guides about contributing to Linux, as developers apparently work with quite different interpretations of what Linus expects.
Quoted 6 lines
Quoted 4 lines
Changes like this were asked for during the Maintainers Summit 2024. The four rules of thumb spelled out are all closely based on statements Linus made there; LWN documented all except "Expedite fixing regressions that reached a release deemed for end users.." in their coverage [1].
"deemed" used in this way is a bit strange. "intended" perhaps?
Yeah, guess that is better. I had trouble finding something short that clearly excludes pre-releases.
Quoted 2 lines
That little nit is all I can find to complain about in this one, though :)
Sorry for replying so late and many thx for looking into this series. Will only reply to the stuff where it's worth spending a few more words and pick up the rest of your suggestions without mentioning it to save us all some time.
Thx again!
Ciao, Thoprsten
Thorsten Leemhuis
maintainerPATCH 2/6 docs: 6.Followthrough.rst: when to involved Linus in regressions
In reply to Thorsten LeemhuisAdd a few notes on when to involve Linus in regressions. Part of this spells out slightly obvious things infrequent developers might not be aware of, while others are based on a recent statement from Linus[1].
This removes equivalent paragraphs from a section in Documentation/process/handling-regressions.rst, which will become mostly obsolete through this and follow-up changes.
--- Documentation/process/6.Followthrough.rst | 17 +++++++++++++++++ Documentation/process/handling-regressions.rst | 17 ----------------- 2 files changed, 17 insertions(+), 17 deletions(-)
Documentation/process/6.Followthrough.rst +17 -0
@@ -217,6 +217,23 @@ On procedure: on the fix and the alignment with pull requests it might be beneficial to have them in there for a day or two. + - If a regression seems tangly, precarious, or urgent, consider CCing Linus on + discussions and patch reviews; do the same if the responsible maintainers + are suspected to be unavailable. + + - For urgent fixes, consider asking Linus to pick them up straight from the + mailing list: he is totally fine with that for occasional and uncontroversial + fixes. Such requests should ideally come directly from maintainers or happen + in accordance with them. + + - In case you are unsure if a fix is worth the risk applying just days before + a new mainline release, send Linus a mail with the usual lists, developers, + and maintainers in CC; in it, summarize the situation while asking him to + consider picking up the fix straight from the list as he sees fit. Linus + then can make the call and when appropriate even postpone the release. Such + requests again should ideally come directly from maintainers or happen in + accordance with them. + Other things that can happen -----------------------------
Documentation/process/handling-regressions.rst +0 -17
@@ -196,23 +196,6 @@ On procedure: regressions to be handled like those from the current cycle, unless fixing bears unusual risks. - * Consider CCing Linus on discussions or patch review, if a regression seems - tangly. Do the same in precarious or urgent cases -- especially if the - subsystem maintainer might be unavailable. Also CC the stable team, when you - know such a regression made it into a mainline, stable, or longterm release. - - * For urgent regressions, consider asking Linus to pick up the fix straight - from the mailing list: he is totally fine with that for uncontroversial - fixes. Ideally though such requests should happen in accordance with the - subsystem maintainers or come directly from them. - - * In case you are unsure if a fix is worth the risk applying just days before - a new mainline release, send Linus a mail with the usual lists and people in - CC; in it, summarize the situation while asking him to consider picking up - the fix straight from the list. He then himself can make the call and when - needed even postpone the release. Such requests again should ideally happen - in accordance with the subsystem maintainers or come directly from them. - Regarding stable and longterm kernels: * You are free to leave regressions to the stable team, if they at no point in
Signature · 1 line
2.45.0
Jonathan Corbet
maintainerRe: [PATCH v1 2/6] docs: 6.Followthrough.rst: when to involved Linus in regressions
Quoted 20 lines · Thorsten Leemhuis
Add a few notes on when to involve Linus in regressions. Part of this spells out slightly obvious things infrequent developers might not be aware of, while others are based on a recent statement from Linus[1].
This removes equivalent paragraphs from a section in
Documentation/process/handling-regressions.rst, which will become mostly obsolete through this and follow-up changes.Link https://lore.kernel.org/all/CAHk-=wis_qQy4oDNynNKi5b7Qhosmxtoj1jxo5wmB6SRUwQUBQ@mail.gmail.com/ [1] open hereSigned-off-by Thorsten Leemhuis <[email protected]>--- Documentation/process/6.Followthrough.rst | 17 +++++++++++++++++ Documentation/process/handling-regressions.rst | 17 ----------------- 2 files changed, 17 insertions(+), 17 deletions(-)Documentation/process/6.Followthrough.rst +0
-0
Documentation/process/6.Followthrough.rst On procedure: · line 222
Quoted 6 lines
Documentation/process/6.Followthrough.rst +3
-0@@ -217,6 +217,23 @@ On procedure: on the fix and the alignment with pull requests it might be beneficial to have them in there for a day or two. + - If a regression seems tangly, precarious, or urgent, consider CCing Linus on + discussions and patch reviews; do the same if the responsible maintainers + are suspected to be unavailable.
I'm not quite sure what "tangly" or "precarious" means in this case?
Documentation/process/6.Followthrough.rst On procedure: · line 231
Quoted 8 lines
Documentation/process/6.Followthrough.rst +8
-0+ - For urgent fixes, consider asking Linus to pick them up straight from the + mailing list: he is totally fine with that for occasional and uncontroversial + fixes. Such requests should ideally come directly from maintainers or happen + in accordance with them. + + - In case you are unsure if a fix is worth the risk applying just days before + a new mainline release, send Linus a mail with the usual lists, developers, + and maintainers in CC; in it, summarize the situation while asking him to
s/usual/appropriate/ ?
That's all.
jon
Thorsten Leemhuis
maintainerRe: [PATCH v1 2/6] docs: 6.Followthrough.rst: when to involved Linus in regressions
Quoted 25 lines · Jonathan Corbet
Quoted 23 lines · Thorsten Leemhuis
Add a few notes on when to involve Linus in regressions. Part of this spells out slightly obvious things infrequent developers might not be aware of, while others are based on a recent statement from Linus[1].
This removes equivalent paragraphs from a section in
Documentation/process/handling-regressions.rst, which will become mostly obsolete through this and follow-up changes.Link https://lore.kernel.org/all/CAHk-=wis_qQy4oDNynNKi5b7Qhosmxtoj1jxo5wmB6SRUwQUBQ@mail.gmail.com/ [1] open hereSigned-off-by Thorsten Leemhuis <[email protected]>--- Documentation/process/6.Followthrough.rst | 17 +++++++++++++++++ Documentation/process/handling-regressions.rst | 17 ----------------- 2 files changed, 17 insertions(+), 17 deletions(-)Documentation/process/6.Followthrough.rst +0
-0@@ -217,6 +217,23 @@ On procedure: on the fix and the alignment with pull requests it might be beneficial to have them in there for a day or two.
Quoted 5 lines
Documentation/process/6.Followthrough.rst On procedure: · line 222
Quoted 3 lines
Documentation/process/6.Followthrough.rst +3
-0+ - If a regression seems tangly, precarious, or urgent, consider CCing Linus on + discussions and patch reviews; do the same if the responsible maintainers + are suspected to be unavailable.I'm not quite sure what "tangly" or "precarious" means in this case?
Hmmm. Something like "complicated", but that didn't feel right and then like went too far. I'll to with "urgent, tricky, or controversial" instead.
Ciao, Thorsten
Thorsten Leemhuis
maintainerPATCH 3/6 docs: 6.Followthrough.rst: interaction with stable wrt to regressions
In reply to Thorsten LeemhuisAdd a few notes on how the interaction with the stable team works when it comes to mainline regressions that also affect stable series.
This removes equivalent paragraphs from a section in Documentation/process/handling-regressions.rst, which will become mostly obsolete through this and follow-up changes.
--- Documentation/process/6.Followthrough.rst | 22 +++++++++++++++++++ .../process/handling-regressions.rst | 19 ---------------- 2 files changed, 22 insertions(+), 19 deletions(-)
Documentation/process/6.Followthrough.rst +22 -0
@@ -234,6 +234,28 @@ On procedure: requests again should ideally come directly from maintainers or happen in accordance with them. +Regarding stable and longterm series: + + - You are free to leave handling regressions to the stable team if the problem + at no point in time occurred with mainline or was fixed there already. + + - When receiving reports about regressions in recent stable or longterm kernel + series, consider evaluating at least briefly if the issue might happen in + current mainline as well -- and if that seems likely, take hold of the + report. If in doubt, ask the reporter to check mainline. + + - Fix regressions quickly in mainline, whenever you want to swiftly resolve one + that recently made it into a mainline, stable, or longterm release; in urgent + cases hence involve Linus to fast-track fixes (see above). This route is + required, as the stable team normally does neither revert nor fix any changes + in their trees, as long as they cause the same problem in mainline. + + - In case of urgent fixes for regression affecting a recent mainline, stable, + or longterm release, you might want to ensure prompt backporting by dropping + the stable team a note once the fix was mainlined; this is especially + advisable during merge windows and shortly thereafter, as the fix otherwise + might land at the end of a huge patch queue. + Other things that can happen -----------------------------
Documentation/process/handling-regressions.rst +0 -19
@@ -198,30 +198,11 @@ On procedure: Regarding stable and longterm kernels: - * You are free to leave regressions to the stable team, if they at no point in - time occurred with mainline or were fixed there already. - * If a regression made it into a proper mainline release during the past twelve months, ensure to tag the fix with "Cc: [email protected]", as a "Fixes:" tag alone does not guarantee a backport. Please add the same tag, in case you know the culprit was backported to stable or longterm kernels. - * When receiving reports about regressions in recent stable or longterm kernel - series, please evaluate at least briefly if the issue might happen in current - mainline as well -- and if that seems likely, take hold of the report. If in - doubt, ask the reporter to check mainline. - - * Whenever you want to swiftly resolve a regression that recently also made it - into a proper mainline, stable, or longterm release, fix it quickly in - mainline; when appropriate thus involve Linus to fast-track the fix (see - above). That's because the stable team normally does neither revert nor fix - any changes that cause the same problems in mainline. - - * In case of urgent regression fixes you might want to ensure prompt - backporting by dropping the stable team a note once the fix was mainlined; - this is especially advisable during merge windows and shortly thereafter, as - the fix otherwise might land at the end of a huge patch queue. - On patch flow: * Developers, when trying to reach the time periods mentioned above, remember
Signature · 1 line
2.45.0
Jonathan Corbet
maintainerRe: [PATCH v1 3/6] docs: 6.Followthrough.rst: interaction with stable wrt to regressions
Quoted 24 lines · Thorsten Leemhuis
Add a few notes on how the interaction with the stable team works when it comes to mainline regressions that also affect stable series.
This removes equivalent paragraphs from a section in
Documentation/process/handling-regressions.rst, which will become mostly obsolete through this and follow-up changes.Signed-off-by Thorsten Leemhuis <[email protected]>--- Documentation/process/6.Followthrough.rst | 22 +++++++++++++++++++ .../process/handling-regressions.rst | 19 ---------------- 2 files changed, 22 insertions(+), 19 deletions(-)Documentation/process/6.Followthrough.rst +3
-0@@ -234,6 +234,28 @@ On procedure: requests again should ideally come directly from maintainers or happen in accordance with them. +Regarding stable and longterm series: + + - You are free to leave handling regressions to the stable team if the problem
Documentation/process/6.Followthrough.rst On procedure: · line 249
Quoted 10 lines
Documentation/process/6.Followthrough.rst +10
-0+ at no point in time occurred with mainline or was fixed there already. + + - When receiving reports about regressions in recent stable or longterm kernel + series, consider evaluating at least briefly if the issue might happen in + current mainline as well -- and if that seems likely, take hold of the + report. If in doubt, ask the reporter to check mainline. + + - Fix regressions quickly in mainline, whenever you want to swiftly resolve one + that recently made it into a mainline, stable, or longterm release; in urgent + cases hence involve Linus to fast-track fixes (see above). This route is
I'm not quite sure what that sentence is intended to say. Might something like s/quickly/first/ better convey the intent? Maybe also s/want/need/ ?
Thanks,
jon
Thorsten Leemhuis
maintainerRe: [PATCH v1 3/6] docs: 6.Followthrough.rst: interaction with stable wrt to regressions
Quoted 32 lines · Jonathan Corbet
Quoted 30 lines · Thorsten Leemhuis
Add a few notes on how the interaction with the stable team works when it comes to mainline regressions that also affect stable series.
This removes equivalent paragraphs from a section in
Documentation/process/handling-regressions.rst, which will become mostly obsolete through this and follow-up changes.Signed-off-by Thorsten Leemhuis <[email protected]>--- Documentation/process/6.Followthrough.rst | 22 +++++++++++++++++++ .../process/handling-regressions.rst | 19 ---------------- 2 files changed, 22 insertions(+), 19 deletions(-)Documentation/process/6.Followthrough.rst +9
-0@@ -234,6 +234,28 @@ On procedure: requests again should ideally come directly from maintainers or happen in accordance with them. +Regarding stable and longterm series: + + - You are free to leave handling regressions to the stable team if the problem + at no point in time occurred with mainline or was fixed there already. + + - When receiving reports about regressions in recent stable or longterm kernel + series, consider evaluating at least briefly if the issue might happen in + current mainline as well -- and if that seems likely, take hold of the + report. If in doubt, ask the reporter to check mainline.
Quoted 8 lines
Documentation/process/6.Followthrough.rst On procedure: · line 249
Quoted 4 lines
Documentation/process/6.Followthrough.rst +4
-0+ + - Fix regressions quickly in mainline, whenever you want to swiftly resolve one + that recently made it into a mainline, stable, or longterm release; in urgent + cases hence involve Linus to fast-track fixes (see above). This route isI'm not quite sure what that sentence is intended to say. Might something like s/quickly/first/ better convey the intent? Maybe also s/want/need/ ?
Hmmm. I went with this:
"""
- You must fix any regression in mainline first that also happens in
mainline, stable, or longterm releases due to the same change. That's because the stable team only addresses such problems once they were fixed in mainline. In urgent cases hence involve Linus to fast-track fixes (see above).
"""
I liked the old approach a bit better, but yeah, I guess it overloaded the first sentence. :-/
Ciao, Thorsten
Thorsten Leemhuis
maintainerPATCH 4/6 docs: 6.Followthrough.rst: tags to use in regressions fixes
In reply to Thorsten LeemhuisAdd a few notes on the appropriate tags to be used in changes that fix regressions.
This removes equivalent paragraphs from a section in Documentation/process/handling-regressions.rst, which will become mostly obsolete through this and follow-up changes.
---
Note:
* Not sure if the "add a second Fixes: tag for the change that exposed an earlier problem" is appropriate, but it results in the most reliable solution without much overhead.
* On a brief look it might seem like this changes the "participation in stable is optional for mainline developers" approach. But that is not the case, as the point is just kindly asking developers to take care of stable inclusion.
--- Documentation/process/6.Followthrough.rst | 16 ++++++++++++++++ Documentation/process/handling-regressions.rst | 7 ------- 2 files changed, 16 insertions(+), 7 deletions(-)
Documentation/process/6.Followthrough.rst +16 -0
@@ -234,6 +234,22 @@ On procedure: requests again should ideally come directly from maintainers or happen in accordance with them. +On tags in the patch description of regressions fixes: + + - Include the tags Documentation/process/5.Posting.rst mentions for + regressions; this usually means a "Reported-by:" tag followed by "Link:" or + "Closes:" tag pointing to the report as well as a "Fixes:" tag; if it's a + regression a later change exposed, add a "Fixes:" tag for that one, too. + + - Did the culprit make it into a proper mainline release during the past + twelve months? Or is it a recent mainline commit backported to stable or + longterm releases in the past few weeks? Then you are kindly asked to ensure + stable inclusion as described by Documentation/process/stable-kernel-rules.rst. + Usually you want to realized thos by adding a "Cc: [email protected]" to + the patch description. Note, a "Fixes:" tag alone does not guarantee a + backport, as the stable team does not pick up all such changes and might + silently drop them in case trouble arises. + Regarding stable and longterm series: - You are free to leave handling regressions to the stable team if the problem
Documentation/process/handling-regressions.rst +0 -7
@@ -196,13 +196,6 @@ On procedure: regressions to be handled like those from the current cycle, unless fixing bears unusual risks. -Regarding stable and longterm kernels: - - * If a regression made it into a proper mainline release during the past - twelve months, ensure to tag the fix with "Cc: [email protected]", as a - "Fixes:" tag alone does not guarantee a backport. Please add the same tag, - in case you know the culprit was backported to stable or longterm kernels. - On patch flow: * Developers, when trying to reach the time periods mentioned above, remember
Signature · 1 line
2.45.0
Jonathan Corbet
maintainerRe: [PATCH v1 4/6] docs: 6.Followthrough.rst: tags to use in regressions fixes
Quoted 30 lines · Thorsten Leemhuis
Add a few notes on the appropriate tags to be used in changes that fix regressions.
This removes equivalent paragraphs from a section in
Documentation/process/handling-regressions.rst, which will become mostly obsolete through this and follow-up changes.Signed-off-by Thorsten Leemhuis <[email protected]>---
Note:
* Not sure if the "add a second Fixes: tag for the change that exposed an earlier problem" is appropriate, but it results in the most reliable solution without much overhead.
* On a brief look it might seem like this changes the "participation in stable is optional for mainline developers" approach. But that is not the case, as the point is just kindly asking developers to take care of stable inclusion.
--- Documentation/process/6.Followthrough.rst | 16 ++++++++++++++++ Documentation/process/handling-regressions.rst | 7 ------- 2 files changed, 16 insertions(+), 7 deletions(-)Documentation/process/6.Followthrough.rst +0
-0
Documentation/process/6.Followthrough.rst On procedure: · line 237
Quoted 4 lines
Documentation/process/6.Followthrough.rst +1
-0@@ -234,6 +234,22 @@ On procedure: requests again should ideally come directly from maintainers or happen in accordance with them. +On tags in the patch description of regressions fixes:
"regression" (no "s")
Documentation/process/6.Followthrough.rst On procedure: · line 248
Quoted 10 lines
Documentation/process/6.Followthrough.rst +10
-0+ - Include the tags Documentation/process/5.Posting.rst mentions for + regressions; this usually means a "Reported-by:" tag followed by "Link:" or + "Closes:" tag pointing to the report as well as a "Fixes:" tag; if it's a + regression a later change exposed, add a "Fixes:" tag for that one, too. + + - Did the culprit make it into a proper mainline release during the past + twelve months? Or is it a recent mainline commit backported to stable or + longterm releases in the past few weeks? Then you are kindly asked to ensure + stable inclusion as described by Documentation/process/stable-kernel-rules.rst. + Usually you want to realized thos by adding a "Cc: [email protected]" to
Something went a bit astray in that sentence. "you want to do this" ?
Documentation/process/6.Followthrough.rst On procedure: · line 251
Quoted 3 lines
Documentation/process/6.Followthrough.rst +3
-0+ the patch description. Note, a "Fixes:" tag alone does not guarantee a + backport, as the stable team does not pick up all such changes and might + silently drop them in case trouble arises.
In the past we have had subsystem maintainers who didn't want people to put CC: stable tags into their own patches; not sure if that's still the case?
Thanks,
jon
Thorsten Leemhuis
maintainerRe: [PATCH v1 4/6] docs: 6.Followthrough.rst: tags to use in regressions fixes
Quoted 10 lines · Jonathan Corbet
Thorsten Leemhuis <[email protected]> writes:
[...]Quoted 7 lines
Documentation/process/6.Followthrough.rst +0
-0@@ -234,6 +234,22 @@ On procedure: requests again should ideally come directly from maintainers or happen in accordance with them.[...]
Quoted 7 lines
Documentation/process/6.Followthrough.rst On procedure: · line 248
Quoted 5 lines
+ - Did the culprit make it into a proper mainline release during the past
Documentation/process/6.Followthrough.rst +4
-0+ twelve months? Or is it a recent mainline commit backported to stable or + longterm releases in the past few weeks? Then you are kindly asked to ensure + stable inclusion as described by Documentation/process/stable-kernel-rules.rst. + Usually you want to realized thos by adding a "Cc: [email protected]" toSomething went a bit astray in that sentence. "you want to do this" ?
Uhhps, yeah, went with 'Many developers realize this by adding a "Cc:
[email protected]" to the patch description.'. That also leaves...
Quoted 6 lines
Documentation/process/6.Followthrough.rst On procedure: · line 251
Quoted 3 lines
Documentation/process/6.Followthrough.rst +3
-0+ the patch description. Note, a "Fixes:" tag alone does not guarantee a + backport, as the stable team does not pick up all such changes and might + silently drop them in case trouble arises.In the past we have had subsystem maintainers who didn't want people to put CC: stable tags into their own patches; not sure if that's still the case?
...it somewhat open for such subsystems, if there still are any.
Ciao, Thorsten
Thorsten Leemhuis
maintainerPATCH 5/6 docs: 6.Followthrough.rst: more specific advice on fixing regressions
In reply to Thorsten LeemhuisProvide something more concrete about fixing regressions in a few places, as telling people to "expedite" fixing those that reached a release deemed for end users is pretty vague. But every situation is different, so use the soft phrases like "aim for" and leave loopholes.
This removes equivalent paragraphs from a section in Documentation/process/handling-regressions.rst, which will become mostly obsolete through this and follow-up changes.
---
Note, the bits that remain like the "past year" aspect are based on statements from Linus. Other parts are made up and need to be discussed, especially the "past six weeks" thing followed by "aim to mainline a fix before the end of the week after the next" and the "[…] taking one more week is fine." shortly after.
Those targets are derived from experiences during regression tracking and the text currently in Documentation/process/handling-regressions.rst that was ACKed by Greg -- just like an earlier version which had even shorter time spans. They came into being from calculating backwards how long users would be exposed to regressions, as then you need to include "someone with the right set of skills needs to notice, bisect and report the problem to the right developer", which often will take at least two to four days days; you furthermore need to take into account how long it takes for the fix to reach regular users through a new stable release, which usually happen three to six days after it made it to a new mainline -rc release. That's why users might be exposed to a regression for three to four weeks in total, even if it's fixed after a bit more than two weeks in mainline. That "after a new mainline-rc release" is also one reason why the text uses the phrase "before the end of the week", as regression fixes that are mainlined on Monday might just miss a stable -rc, which often will delay pickup by a whole week (and this mean ~10+ days to reach users).
--- Documentation/process/6.Followthrough.rst | 30 ++++++++++---- .../process/handling-regressions.rst | 40 ------------------- 2 files changed, 23 insertions(+), 47 deletions(-)
Documentation/process/6.Followthrough.rst +23 -7
@@ -198,16 +198,32 @@ maintainers and other developers will take note if you fail to handle regression appropriately, especially if they then have to fix the problem themselves: this could well make it harder for you to incorporate future changes. -On timing: +On timing once the mainline change causing the regression became known: - - Expedite fixing regressions that reached releases deemed for end users - through new mainline releases or stable backports during the past year. + - If the regression is severe or reported by many people within a short + timeframe, aim to mainline a fix within two or three days and ideally before + the end of the week. - - If the culprit was mainlined during the current development cycle and not - backported to stable, fix the regression before -rc6. + - Expedite fixing regressions that recently reached releases deemed for end + users through new mainline releases or stable backports. If the culprit + reached it in the past six weeks, aim to mainline a fix before the end of the + week after the next; if it landed during the past year, taking one more week + is fine. Whenever possible, try to resolve the issue faster -- but it's also + okay to take more time if there are strong reasons and a revert no option. - - If a proper regression fix is unlikely to become ready in a reasonable - timeframe, resolve the regression by reverting the culprit. This is + - If the culprit was mainlined during the current development cycle and not + backported to stable, fix the regression before -rc6. But try to resolve it + faster whenever possible -- especially if the issue is either reported + multiple times or prevents CI systems or multiple users from testing, as that + might mask other bugs and drive testers away. + + - Try your best to mainline all regressions fixes before the current + development cycle ends, unless the culprit was committed more than a year + ago: then it is acceptable to queue a fix for the next merge window, which + is even advisable in case the change bears bigger risks. + + - If mainlining a proper fix within the timeframes outlined above looks + unlikely, resolve the regression by reverting the culprit. This is considered an reputable approach, as it allows working out and merging an improved variant of the change calmly.
Documentation/process/handling-regressions.rst +0 -40
@@ -156,46 +156,6 @@ only these options: How to realize that in practice depends on various factors. Use the rules of thumb outlined in Documentation/process/6.Followthrough.rst as a guide. -In general: - - * Prioritize work on regressions over all other Linux kernel work, unless the - latter concerns a severe issue (e.g. acute security vulnerability, data loss, - bricked hardware, ...). - - * Do not consider regressions from the current cycle as something that can wait - till the end of the cycle, as the issue might discourage or prevent users and - CI systems from testing mainline now or generally. - - * Work with the required care to avoid additional or bigger damage, even if - resolving an issue then might take longer than outlined below. - -On timing once the culprit of a regression is known: - - * Aim to mainline a fix within two or three days, if the issue is severe or - bothering many users -- either in general or in prevalent conditions like a - particular hardware environment, distribution, or stable/longterm series. - - * Aim to mainline a fix by Sunday after the next, if the culprit made it - into a recent mainline, stable, or longterm release (either directly or via - backport); if the culprit became known early during a week and is simple to - resolve, try to mainline the fix within the same week. - - * For other regressions, aim to mainline fixes before the hindmost Sunday - within the next three weeks. One or two Sundays later are acceptable, if the - regression is something people can live with easily for a while -- like a - mild performance regression. - - * It's strongly discouraged to delay mainlining regression fixes till the next - merge window, except when the fix is extraordinarily risky or when the - culprit was mainlined more than a year ago. - -On procedure: - - * Try to resolve any regressions introduced in mainline during the past - twelve months before the current development cycle ends: Linus wants such - regressions to be handled like those from the current cycle, unless fixing - bears unusual risks. - On patch flow: * Developers, when trying to reach the time periods mentioned above, remember
Signature · 1 line
2.45.0
Jonathan Corbet
maintainerRe: [PATCH v1 5/6] docs: 6.Followthrough.rst: more specific advice on fixing regressions
Quoted 13 lines · Thorsten Leemhuis
Provide something more concrete about fixing regressions in a few places, as telling people to "expedite" fixing those that reached a release deemed for end users is pretty vague. But every situation is different, so use the soft phrases like "aim for" and leave loopholes.
This removes equivalent paragraphs from a section in
Documentation/process/handling-regressions.rst, which will become mostly obsolete through this and follow-up changes.Signed-off-by Thorsten Leemhuis <[email protected]>---
- - If the culprit was mainlined during the current development cycle and not
Documentation/process/6.Followthrough.rst maintainers and other developers will take note if you fail to handle regression · line 208
Quoted 3 lines
Documentation/process/6.Followthrough.rst +2
-1- backported to stable, fix the regression before -rc6. + - Expedite fixing regressions that recently reached releases deemed for end + users through new mainline releases or stable backports. If the culprit
Again, I wouldn't used "deemed" this way.
Documentation/process/6.Followthrough.rst maintainers and other developers will take note if you fail to handle regression · line 210
Quoted 2 lines
Documentation/process/6.Followthrough.rst +2
-0+ reached it in the past six weeks, aim to mainline a fix before the end of the + week after the next; if it landed during the past year, taking one more week
Can we say "within ten days" or some such instead?
Documentation/process/6.Followthrough.rst maintainers and other developers will take note if you fail to handle regression · line 220
Quoted 12 lines
Documentation/process/6.Followthrough.rst +9
-2+ is fine. Whenever possible, try to resolve the issue faster -- but it's also + okay to take more time if there are strong reasons and a revert no option. - - If a proper regression fix is unlikely to become ready in a reasonable - timeframe, resolve the regression by reverting the culprit. This is + - If the culprit was mainlined during the current development cycle and not + backported to stable, fix the regression before -rc6. But try to resolve it + faster whenever possible -- especially if the issue is either reported + multiple times or prevents CI systems or multiple users from testing, as that + might mask other bugs and drive testers away. + + - Try your best to mainline all regressions fixes before the current
"regression" singular
Documentation/process/6.Followthrough.rst maintainers and other developers will take note if you fail to handle regression · line 223
Quoted 3 lines
Documentation/process/6.Followthrough.rst +3
-0+ development cycle ends, unless the culprit was committed more than a year + ago: then it is acceptable to queue a fix for the next merge window, which + is even advisable in case the change bears bigger risks.
Thanks,
jon
Thorsten Leemhuis
maintainerRe: [PATCH v1 5/6] docs: 6.Followthrough.rst: more specific advice on fixing regressions
Quoted 6 lines · Jonathan Corbet
Documentation/process/6.Followthrough.rst maintainers and other developers will take note if you fail to handle regression · line 210
Quoted 4 lines
+ - Expedite fixing regressions that recently reached releases deemed for end
Documentation/process/6.Followthrough.rst +3
-0+ users through new mainline releases or stable backports. If the culprit + reached it in the past six weeks, aim to mainline a fix before the end of the + week after the next; if it landed during the past year, taking one more weekCan we say "within ten days" or some such instead?
Well, I chose that approach due to various small reasons that add up:
- Many of us know that -rc releases are not special, but a lot of people seem to focus on them. For all of them it's not much of a differences if a fix is mainlined on Monday (Pacific Time) or Friday of the same week.
- Greg often prepares new stable releases early during a week, so for any fix that needs to land there it's ideal if it reaches a new -rc, as then Greg will often pick it automatically soon afterwards; anything merged on Monday or Tuesday will likely miss a new stable-rc and thus often take a week longer to reach users, unless it's one of those weeks where Greg does more than one release per week.
- I highly doubt that many subsystem maintainers will care much about such a "10 days for individual regressions" target, as that might easily mean they have to send additional PRs just to reach that goal. IOW: it will likely be ignored or even laughed at. And I don't blame them. That's why I went with something that developers and subsystem maintainers can actually implement, especially for the bigger subsystems where the weekdays when PRs to Linus are usually send are known.
Side note & reminder: overall the biggest problem I see wrt to fixing regressions is that fixes exists, just take a quite a while to make it through the ranks (review, merge to subsystem tree, PR to linux). Especially the last step. Which is why patch 6 has that "Furthermore try to avoid holding onto regression fixes over weekends -- especially when some are marked for backporting to stable series." in it.
Ciao, Thorsten
Thorsten Leemhuis
maintainerPATCH 6/6 docs: 6.Followthrough.rst: advice on handling regressions fixes
In reply to Thorsten LeemhuisAdd some advice on how to handle regressions as developer, reviewer, and maintainer, as resolving regression without unnecessary delays requires multiple people working hand in hand.
This removes equivalent paragraphs from a section in Documentation/process/handling-regressions.rst, which will become mostly obsolete through this and follow-up changes.
--- Documentation/process/6.Followthrough.rst | 24 ++++++++++++++++--- .../process/handling-regressions.rst | 16 ------------- 2 files changed, 21 insertions(+), 19 deletions(-)
Documentation/process/6.Followthrough.rst +21 -3
@@ -198,6 +198,11 @@ maintainers and other developers will take note if you fail to handle regression appropriately, especially if they then have to fix the problem themselves: this could well make it harder for you to incorporate future changes. +In general: + + - Prioritize work on providing, reviewing, and mainlining regression fixes over + other upstream Linux kernel work. + On timing once the mainline change causing the regression became known: - If the regression is severe or reported by many people within a short
@@ -229,9 +234,22 @@ On timing once the mainline change causing the regression became known: On procedure: - - Regression fixes are not required to spend time in linux-next, but depending - on the fix and the alignment with pull requests it might be beneficial to - have them in there for a day or two. + - Developers, when trying to reach the time periods mentioned above, remember + to account for the time it will take to test, review, commit, and mainline + fixes, ideally with them being in linux-next briefly. To achieve that, make + the appropriate urgency of a fix obvious in the submission and consider + opting for a revert to be on the safe side. + + - Reviewers, you are kindly asked to prefer reviewing regression fixes over + other changes. + + - Maintainers, you likewise are kindly asked to expedite the handling of + regression fixes. They for example are not required to spend time in + linux-next, but depending on the fix and the alignment with pull requests it + might be beneficial to have them in there for a day or two. When appropriate, + also consider sending an additional pull request. Furthermore try to avoid + holding onto regression fixes over weekends -- especially when some are + marked for backporting to stable series. - If a regression seems tangly, precarious, or urgent, consider CCing Linus on discussions and patch reviews; do the same if the responsible maintainers
Documentation/process/handling-regressions.rst +0 -16
@@ -156,22 +156,6 @@ only these options: How to realize that in practice depends on various factors. Use the rules of thumb outlined in Documentation/process/6.Followthrough.rst as a guide. -On patch flow: - - * Developers, when trying to reach the time periods mentioned above, remember - to account for the time it takes to get fixes tested, reviewed, and merged by - Linus, ideally with them being in linux-next at least briefly. Hence, if a - fix is urgent, make it obvious to ensure others handle it appropriately. - - * Reviewers, you are kindly asked to assist developers in reaching the time - periods mentioned above by reviewing regression fixes in a timely manner. - - * Subsystem maintainers, you likewise are encouraged to expedite the handling - of regression fixes. Thus evaluate if skipping linux-next is an option for - the particular fix. Also consider sending git pull requests more often than - usual when needed. And try to avoid holding onto regression fixes over - weekends -- especially when the fix is marked for backporting. - More aspects regarding regressions developers should be aware of ----------------------------------------------------------------
Signature · 1 line
2.45.0
Jonathan Corbet
maintainerRe: [PATCH v1 6/6] docs: 6.Followthrough.rst: advice on handling regressions fixes
Quoted 8 lines · Thorsten Leemhuis
Add some advice on how to handle regressions as developer, reviewer, and maintainer, as resolving regression without unnecessary delays requires multiple people working hand in hand.
This removes equivalent paragraphs from a section in
Documentation/process/handling-regressions.rst, which will become mostly obsolete through this and follow-up changes.
Quoted 5 lines
Signed-off-by Thorsten Leemhuis <[email protected]>--- Documentation/process/6.Followthrough.rst | 24 ++++++++++++++++--- .../process/handling-regressions.rst | 16 ------------- 2 files changed, 21 insertions(+), 19 deletions(-)
Nothing really to complain about here (though I do worry a bit about the idea that calling something a regression fix allows bypassing all of our usual testing).
Thanks,
jon
Thorsten Leemhuis
maintainerRe: [PATCH v1 6/6] docs: 6.Followthrough.rst: advice on handling regressions fixes
Quoted 16 lines · Jonathan Corbet
Quoted 8 lines · Thorsten Leemhuis
Add some advice on how to handle regressions as developer, reviewer, and maintainer, as resolving regression without unnecessary delays requires multiple people working hand in hand.
This removes equivalent paragraphs from a section in
Documentation/process/handling-regressions.rst, which will become mostly obsolete through this and follow-up changes.Quoted 5 lines
Signed-off-by Thorsten Leemhuis <[email protected]>--- Documentation/process/6.Followthrough.rst | 24 ++++++++++++++++--- .../process/handling-regressions.rst | 16 ------------- 2 files changed, 21 insertions(+), 19 deletions(-)
Quoted 3 lines
Nothing really to complain about here (though I do worry a bit about the idea that calling something a regression fix allows bypassing all of our usual testing).
Well, from Linus statements on the recent maintainers summit it seems he does not care too much about that. But I had a similar feeling, which is why I softened that bit somewhat into that "They for example are not required to spend time in linux-next, but depending on the fix and the alignment with pull requests it might be beneficial to have them in there for a day or two."
Ciao, Thorsten