This patch series amends the kernel development process to reduce the
load on key maintainers during peak periods, by discouraging the submission
of non-urgent patches while the merge window is open.
Original discussion here:
https://lkml.org/lkml/2014/12/12/772
(As a non-subsystem-maintainer I don't really have a horse in this race,
but if everyone else agrees that it's the right thing to do, we might as
well make it the official policy and start teaching it to new developers.)
Kevin Cernekee (4):
Documentation: Change policy on sending patches during merge window
Documentation/SubmitChecklist: Remind submitters to check the merge
window
Documentation: Add cutoff periods for patch acceptance
Documentation: Provide suggestions on when to repost patches
Documentation/SubmitChecklist | 5 +++++
Documentation/development-process/5.Posting | 25 +++++++++++++++++++++++
Documentation/development-process/6.Followthrough | 5 ++++-
3 files changed, 34 insertions(+), 1 deletion(-)
--
2.1.1
Give submitters a rough idea of how long to wait before reposting, to
help avoid situations where a series is reposted before the original
submission is fully reviewed.
Suggested-by: Thomas Gleixner <[email protected]>
Signed-off-by: Kevin Cernekee <[email protected]>
---
Documentation/development-process/6.Followthrough | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/Documentation/development-process/6.Followthrough b/Documentation/development-process/6.Followthrough
index 41d324a..6cefb6c 100644
--- a/Documentation/development-process/6.Followthrough
+++ b/Documentation/development-process/6.Followthrough
@@ -74,7 +74,10 @@ around.
One fatal mistake is to ignore review comments in the hope that they will
go away. They will not go away. If you repost code without having
responded to the comments you got the time before, you're likely to find
-that your patches go nowhere.
+that your patches go nowhere. On the flipside, reposting an updated patch
+before the original has been fully reviewed can be a source of frustration
+too, so consider giving the reviewers ~3-7 calendar days (depending on
+patch complexity) before posting V2.
Speaking of reposting code: please bear in mind that reviewers are not
going to remember all the details of the code you posted the last time
--
2.1.1
This has been a recurring source of confusion for the new submitters who
I've helped; let's see if adding a small illustration improves the
situation.
Signed-off-by: Kevin Cernekee <[email protected]>
---
Documentation/development-process/5.Posting | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/Documentation/development-process/5.Posting b/Documentation/development-process/5.Posting
index 14f8b01..50cf4dc 100644
--- a/Documentation/development-process/5.Posting
+++ b/Documentation/development-process/5.Posting
@@ -35,6 +35,22 @@ merge window has closed. You can visit https://www.kernel.org/ to check
the merge window status; it is closed if the "mainline:" entry shows a
version number containing "-rc", and open if a non-rc version number appears.
+In many cases, feature additions received and accepted somewhere in the range
+of [N-rc1, N-rc5) will be merged into the -next tree targeting the (N+1) Linux
+release. For example:
+
+ kernel.org "mainline:" | Patch may appear
+ field when posted | in released kernel
+ ------------------------+--------------------------------
+ 3.18-rc[1-4] | 3.19
+ 3.18-rc[5-9] | 3.20
+ 3.18 | Merge window open - don't post
+ 3.19-rc[1-4] | 3.20
+ 3.19-rc[5-9] | 3.21
+ 3.19 | Merge window open - don't post
+
+Bug fixes can typically be accepted at any time.
+
5.2: BEFORE CREATING PATCHES
--
2.1.1
Currently the checklist does not provide an indication of appropriate
times to send patches; add a brief note on the topic.
Signed-off-by: Kevin Cernekee <[email protected]>
---
Documentation/SubmitChecklist | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/SubmitChecklist b/Documentation/SubmitChecklist
index 2b7e32d..8b23f9c 100644
--- a/Documentation/SubmitChecklist
+++ b/Documentation/SubmitChecklist
@@ -107,3 +107,8 @@ kernel patches.
CONFIG_SMP, CONFIG_SYSFS, CONFIG_PROC_FS, CONFIG_INPUT, CONFIG_PCI,
CONFIG_BLOCK, CONFIG_PM, CONFIG_MAGIC_SYSRQ,
CONFIG_NET, CONFIG_INET=n (but latter with CONFIG_NET=y)
+
+27: For patches which are not urgent fixes for bugs in the current tree,
+ double-check https://www.kernel.org/ to make sure the merge window is
+ CLOSED ("mainline:" showing an -rc kernel) before sending. For more
+ information on the release cycle, see Documentation/development-process/.
--
2.1.1
Ask patch submitters to avoid sending non-critical patches when the
merge window is open. This basically extends the net-next policy in
netdev-FAQ.txt to the entire kernel.
Suggested-by: Thomas Gleixner <[email protected]>
Signed-off-by: Kevin Cernekee <[email protected]>
---
Documentation/development-process/5.Posting | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Documentation/development-process/5.Posting b/Documentation/development-process/5.Posting
index 8a48c9b..14f8b01 100644
--- a/Documentation/development-process/5.Posting
+++ b/Documentation/development-process/5.Posting
@@ -26,6 +26,15 @@ which remains to be done and any known problems. Fewer people will look at
patches which are known to be half-baked, but those who do will come in
with the idea that they can help you drive the work in the right direction.
+Some maintainers prefer to receive only urgent patches when the merge
+window is open, so that critical fixes do not get lost in the noise during
+times of peak activity. If you are posting an actual [PATCH] (not something
+that is obviously low-priority such as an [RFC] or [RANT]) and you aren't sure
+of your maintainer's stance, the safest thing to do is hold off until the
+merge window has closed. You can visit https://www.kernel.org/ to check
+the merge window status; it is closed if the "mainline:" entry shows a
+version number containing "-rc", and open if a non-rc version number appears.
+
5.2: BEFORE CREATING PATCHES
--
2.1.1
On Sun, Dec 14, 2014 at 10:09:47PM -0800, Kevin Cernekee wrote:
> Ask patch submitters to avoid sending non-critical patches when the
> merge window is open. This basically extends the net-next policy in
> netdev-FAQ.txt to the entire kernel.
FYI, I very mich disagree with that. Merge window isn't really special,
and patches can easily be reviewed and queued up for the next merge
window in that time. If it said you shouldn't expect replies and not
_resend_ during the merge window that seems like a much saner policy.
On Sun, Dec 14, 2014 at 10:09:50PM -0800, Kevin Cernekee wrote:
> Give submitters a rough idea of how long to wait before reposting, to
> help avoid situations where a series is reposted before the original
> submission is fully reviewed.
>
> Suggested-by: Thomas Gleixner <[email protected]>
> Signed-off-by: Kevin Cernekee <[email protected]>
> ---
> Documentation/development-process/6.Followthrough | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/development-process/6.Followthrough b/Documentation/development-process/6.Followthrough
> index 41d324a..6cefb6c 100644
> --- a/Documentation/development-process/6.Followthrough
> +++ b/Documentation/development-process/6.Followthrough
> @@ -74,7 +74,10 @@ around.
> One fatal mistake is to ignore review comments in the hope that they will
> go away. They will not go away. If you repost code without having
> responded to the comments you got the time before, you're likely to find
> -that your patches go nowhere.
> +that your patches go nowhere. On the flipside, reposting an updated patch
> +before the original has been fully reviewed can be a source of frustration
> +too,
I'd like to make that aspect stronger/more explicit:
"Please do repost only after the reviewers have finished going through
your submission and you have collected, addressed and/or incorporated
their full feedback. You can use the time while waiting to test and
hammer more on your code. Any non-trivial submission of a patchset
should be resent after a full week (7 days) the earliest in order not to
spam people unnecessarily and to give them a chance to at least finish
reviewing."
Anyway, something like that, formulation might need more cleaning up - I
was just trying to express the sentiment...
> so consider giving the reviewers ~3-7 calendar days (depending on
> +patch complexity) before posting V2.
>
> Speaking of reposting code: please bear in mind that reviewers are not
> going to remember all the details of the code you posted the last time
Thanks for doing this, btw!
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
On Sun, Dec 14, 2014 at 10:09:49PM -0800, Kevin Cernekee wrote:
> This has been a recurring source of confusion for the new submitters who
> I've helped; let's see if adding a small illustration improves the
> situation.
>
> Signed-off-by: Kevin Cernekee <[email protected]>
> ---
> Documentation/development-process/5.Posting | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/Documentation/development-process/5.Posting b/Documentation/development-process/5.Posting
> index 14f8b01..50cf4dc 100644
> --- a/Documentation/development-process/5.Posting
> +++ b/Documentation/development-process/5.Posting
> @@ -35,6 +35,22 @@ merge window has closed. You can visit https://www.kernel.org/ to check
> the merge window status; it is closed if the "mainline:" entry shows a
> version number containing "-rc", and open if a non-rc version number appears.
>
> +In many cases, feature additions received and accepted somewhere in the range
> +of [N-rc1, N-rc5) will be merged into the -next tree targeting the (N+1) Linux
> +release. For example:
> +
> + kernel.org "mainline:" | Patch may appear
> + field when posted | in released kernel
> + ------------------------+--------------------------------
> + 3.18-rc[1-4] | 3.19
> + 3.18-rc[5-9] | 3.20
> + 3.18 | Merge window open - don't post
> + 3.19-rc[1-4] | 3.20
> + 3.19-rc[5-9] | 3.21
> + 3.19 | Merge window open - don't post
> +
> +Bug fixes can typically be accepted at any time.
Acked-by: Borislav Petkov <[email protected]>
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
On Sun, Dec 14, 2014 at 10:09:48PM -0800, Kevin Cernekee wrote:
> Currently the checklist does not provide an indication of appropriate
> times to send patches; add a brief note on the topic.
>
> Signed-off-by: Kevin Cernekee <[email protected]>
> ---
> Documentation/SubmitChecklist | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/SubmitChecklist b/Documentation/SubmitChecklist
> index 2b7e32d..8b23f9c 100644
> --- a/Documentation/SubmitChecklist
> +++ b/Documentation/SubmitChecklist
> @@ -107,3 +107,8 @@ kernel patches.
> CONFIG_SMP, CONFIG_SYSFS, CONFIG_PROC_FS, CONFIG_INPUT, CONFIG_PCI,
> CONFIG_BLOCK, CONFIG_PM, CONFIG_MAGIC_SYSRQ,
> CONFIG_NET, CONFIG_INET=n (but latter with CONFIG_NET=y)
> +
> +27: For patches which are not urgent fixes for bugs in the current tree,
> + double-check https://www.kernel.org/ to make sure the merge window is
> + CLOSED ("mainline:" showing an -rc kernel) before sending. For more
> + information on the release cycle, see Documentation/development-process/.
Acked-by: Borislav Petkov <[email protected]>
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
> +
> + kernel.org "mainline:" | Patch may appear
> + field when posted | in released kernel
> + ------------------------+--------------------------------
> + 3.18-rc[1-4] | 3.19
> + 3.18-rc[5-9] | 3.20
> + 3.18 | Merge window open - don't post
> + 3.19-rc[1-4] | 3.20
> + 3.19-rc[5-9] | 3.21
> + 3.19 | Merge window open - don't post
> +
> +Bug fixes can typically be accepted at any time.
That's exactly what was needed I think
Acked-by: Alan Cox <[email protected]>
On Mon, Dec 15, 2014 at 6:59 AM, One Thousand Gnomes
<[email protected]> wrote:
>> +
>> + kernel.org "mainline:" | Patch may appear
>> + field when posted | in released kernel
>> + ------------------------+--------------------------------
>> + 3.18-rc[1-4] | 3.19
>> + 3.18-rc[5-9] | 3.20
>> + 3.18 | Merge window open - don't post
>> + 3.19-rc[1-4] | 3.20
>> + 3.19-rc[5-9] | 3.21
>> + 3.19 | Merge window open - don't post
>> +
>> +Bug fixes can typically be accepted at any time.
>
> That's exactly what was needed I think
So I don't think this is wrong per se, but I do think that it should
also have a clarification that
(a) the above "may appear" is basically "best possible timings", and
things can get delayed by various issues
(b) the exact timing is maintainer-specific.
For that (b) in particular, for fairly simple subsystems in
particular, some maintainers basically have their pull requests for
the merge window open *before* the merge window even starts, and for
them, the merge window itself isn't actually all that busy, it's often
the week before that is the busy one. So the exact timing can vary by
maintainership, and while I think the above is a reasonable example,
it should perhaps be documented as such. An *example*, not a "this is
how it always works". If you are preparing a 50-patch monster, I
suspect you may want to talk to the maintainer you're planning on
spamming before you send it out.
Linus
Kevin Cernekee <[email protected]> writes:
> +27: For patches which are not urgent fixes for bugs in the current tree,
> + double-check https://www.kernel.org/ to make sure the merge window is
> + CLOSED ("mainline:" showing an -rc kernel) before sending. For more
> + information on the release cycle, see Documentation/development-process/.
Maybe the crystal ball should be extended to show if the merge is open
or closed? And if it's closed, an estimate when the next merge window
starts would be nice ("Merge window is closed, estimate for it to open:
21 days").
http://phb-crystal-ball.org/
I'm sure Johannes would be happy to take patches.
--
Kalle Valo
On Sun, 14 Dec 2014 22:09:46 -0800
Kevin Cernekee <[email protected]> wrote:
> This patch series amends the kernel development process to reduce the
> load on key maintainers during peak periods, by discouraging the submission
> of non-urgent patches while the merge window is open.
You do understand the irony of posting this during the merge window,
right? :)
In general, I worry about trying to codify things too much just because
different maintainers have different expectations. As Linus noted, some
maintainers have their work done by the time the merge window starts and
can take patches just fine — until something catches fire, at least.
I'll await a revision taking account the feedback you've gotten, just
adding my own suggestion to phrase things as guidelines rather than
hard-and-fast rules.
Thanks,
jon
On Tue, Dec 16, 2014 at 01:09:39PM -0500, Jonathan Corbet wrote:
>
> In general, I worry about trying to codify things too much just because
> different maintainers have different expectations. As Linus noted, some
> maintainers have their work done by the time the merge window starts and
> can take patches just fine — until something catches fire, at least.
Yeah, I think it's going to be really different pretty much all
maintainers.
Speaking for myself, that's what patchwork is for. There are plenty
of other times besides merge window when I'm going to be super busy
--- say, because I'm travelling to a conference or for business
reasons. And those times are far more likely to be hard on me as a
maintainer compared to the merge window. So everyone's mileage going
to vary widely.
And having a strict schedule like this:
+ kernel.org "mainline:" | Patch may appear
+ field when posted | in released kernel
+ ------------------------+--------------------------------
+ 3.18-rc[1-4] | 3.19
+ 3.18-rc[5-9] | 3.20
+ 3.18 | Merge window open - don't post
+ 3.19-rc[1-4] | 3.20
+ 3.19-rc[5-9] | 3.21
+ 3.19 | Merge window open - don't post
+
+Bug fixes can typically be accepted at any time.
Seems like it's very, VERY bad ju-ju. I'll take some feature patches
as late as -rc6, if they are simple and I'm confident that regression
testing will catch any problems (because for file systems, very often
xfstests is far more likely to find problems than soak time in
linux-next).
On the flip side, a feature patch, submitted between -rc2 and -rc3,
might not make it because we want more people to look at it, better
benchmarks, etc., etc. Even a bug fix submitted at that point might
not make it, especially if the bug is extremely rare (and we may have
lived with it for years or decades), and the change is especially
risky/hairy.
So having a schedule like this is very likely going to set all sorts
of false expectations, and may end up causing just as many problems as
it purports to solve.
I wonder if this sort of thing is better placed in various unofficial
documentations which help new users acculturate. For example, see
some of the slides in the last half of Sarah Sharp's presentation
here:
https://plus.google.com/+SarahSharp/posts/4GSqqGpg8cg
That has the advantage of significantly reducing the risk that things
that originally started out as preferences by a few maintainers
getting turned into bureaucratic rules.
- Ted
On Tue, Dec 16, 2014 at 10:09 AM, Jonathan Corbet <[email protected]> wrote:
> On Sun, 14 Dec 2014 22:09:46 -0800
> Kevin Cernekee <[email protected]> wrote:
>
>> This patch series amends the kernel development process to reduce the
>> load on key maintainers during peak periods, by discouraging the submission
>> of non-urgent patches while the merge window is open.
>
> You do understand the irony of posting this during the merge window,
> right? :)
I specifically exempted [RFC] from the rules because these threads are
readily distinguishable from urgent patches. One could envision
setting up a procmail rule to move all [RFC] and [.*-next] messages
into a "save for later" folder.
> In general, I worry about trying to codify things too much just because
> different maintainers have different expectations. As Linus noted, some
> maintainers have their work done by the time the merge window starts and
> can take patches just fine — until something catches fire, at least.
Do you think it might make sense to list the (stricter?) patch
acceptance policies in MAINTAINERS?
In the current process, many submitters do not know their maintainer's
policy until they get in trouble for violating it. This is not a very
efficient way of educating people. It might work if a submitter is
focusing most of his efforts on a small number of subsystems, with
which he becomes familiar over time. But it doesn't work if somebody
has to touch many different areas of the kernel just once to e.g. add
support for a new SoC (including its syscon devices, irqchip drivers,
regulators, clocks, buses, and peripherals).
On Tue, Dec 16, 2014 at 02:19:02PM -0800, Kevin Cernekee wrote:
> In the current process, many submitters do not know their maintainer's
> policy until they get in trouble for violating it.
Just say what Christoph suggested: people should not expect any feedback
during the merge window. If they get some, fine. It can't get any
simpler than that.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
On Tue, Dec 16, 2014 at 11:31:33PM +0100, Borislav Petkov wrote:
> On Tue, Dec 16, 2014 at 02:19:02PM -0800, Kevin Cernekee wrote:
> > In the current process, many submitters do not know their maintainer's
> > policy until they get in trouble for violating it.
>
> Just say what Christoph suggested: people should not expect any feedback
> during the merge window. If they get some, fine. It can't get any
> simpler than that.
Or during the week of Plumber's Conference... or LSF/MM... or the
during the Kernel Summit... or if they are in transit to
Linux.conf.au.... sometimes a patch submitter won't get feedback
right away. It's nothing personal.
With all due respect to Steven, I think he over-reacted a little with
his "maintainer abuse" reply. Most of the time people won't get
"yelled" at if they send patches at the wrong time. And what's wrong
for one maintainer will be right for another, and vice versa.
- Ted
On Wed, Dec 17, 2014 at 12:14:24AM -0500, Theodore Ts'o wrote:
> And what's wrong for one maintainer will be right for another, and
> vice versa.
Ok, so what's wrong with "should not expect any feedback during the
merge window"?
If they get it, then that's fine. The formulation is loose on purpose.
And besides, when one starts working with maintainers, one soon learns
when they are the busiest and can refrain from sending patchsets then.
Or should we have subsystem maintainers each state whether they wanna
do patchsets during the merge window or not? Or are we bikeshedding
already?
I see your point that different maintainers can be busy at different
times but you also have to acknowledge the desire of some maintainers
not to get new patchsets during the merge window. So we have to have a
way to communicate that to submitters so that no explosions happen.
Oh, and also the
not-resend-in-under-a-week-and-wait-out-complete-review-first rule.
That's my personal pet peeve.
Ha, this sounds like a KS topic :-P
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
On 16/12/2014 19:09, Jonathan Corbet wrote:
> In general, I worry about trying to codify things too much just because
> different maintainers have different expectations. As Linus noted, some
> maintainers have their work done by the time the merge window starts and
> can take patches just fine — until something catches fire, at least.
As one of the recipients of the original evidence ("[v3 00/26] Add VT-d
Posted-Interrupts support"), I can only agree.
The timing of the series wasn't optimal for me, either: I do _not_ take
patches during the merge window and generally would like people that are
not KVM submaintainers to leave me alone.
On the other hand, the series is complex and spans four maintainers
(KVM, VFIO, IOMMU, x86), and it would be weird to blame people for
posting early. In this case the x86 parts are just one patch out of 26.
When x86 maintainers see something like this, they can expect the KVM
maintainer (me) to tell them "can you review the approach" or just "can
you give your Acked-by" once the rest of the series is mature.
As an example of a different workflow, generally things go like this for me:
window Put up fires, otherwise do non-kernel stuff
rc1-rc2 Test large features, gather small patches in a rebased queue
rc2-rc3 Big testing week: open next tree
rc3-rc6 Merge small features
rc6- Merge bugfixes only, do non-kernel stuff, start bugging
submaintainers
So for me the worst time to send patches is actually the week between
rc2 and rc3. You're pretty much guaranteed to be ignored at that time,
unless of course the patch is for current mainline.
If you send during the merge window, chances are I'll be busy doing
something else and I won't have a fast turnaround. But a mail from your
manager asking to merge a large feature after rc6 will definitely make
me more grumpy.
Paolo
On Thu, Dec 18, 2014 at 11:35:40AM +0100, Paolo Bonzini wrote:
> But a mail from your manager asking to merge a large feature after rc6
> will definitely make me more grumpy.
I sincerely hope it'll never be the case where managers dictate the
development on lkml. If this happens, we're terminally screwed.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
On 18/12/2014 11:42, Borislav Petkov wrote:
> > But a mail from your manager asking to merge a large feature after rc6
> > will definitely make me more grumpy.
>
> I sincerely hope it'll never be the case where managers dictate the
> development on lkml. If this happens, we're terminally screwed.
They don't; they just try. :)
paolo
On Wed, Dec 17, 2014 at 10:52:27AM +0100, Borislav Petkov wrote:
> On Wed, Dec 17, 2014 at 12:14:24AM -0500, Theodore Ts'o wrote:
> > And what's wrong for one maintainer will be right for another, and
> > vice versa.
> Ok, so what's wrong with "should not expect any feedback during the
> merge window"?
AFAICT the original complaint wasn't about people expecting feedback
during the merge window, it was about people sending things at all which
is a different thing.
> And besides, when one starts working with maintainers, one soon learns
> when they are the busiest and can refrain from sending patchsets then.
Or if that even makes a difference of course.
> I see your point that different maintainers can be busy at different
> times but you also have to acknowledge the desire of some maintainers
> not to get new patchsets during the merge window. So we have to have a
> way to communicate that to submitters so that no explosions happen.
I think it's important to be clear what we're talking about when we
advise people; the advice about allowing for people being busy or
otherwise unavailable applies pretty much all the time - one of the most
common process problems I see is people expecting quick turnaround
times, it'd be really good to set expecations there and it seems hard to
go wrong.
Not posting at all is a bit different, though, and is much more
maintainer specific - personally I'm in the opposite camp to Thomas and
would rather people just sent things whenever so I can get round to them
as I have time rather than getting everyone sending things at once.