We appear to have a gap in our process docs. We go into detail
on how to contribute code to the kernel, and how to be a subsystem
maintainer. I can't find any docs directed towards the thousands
of small scale maintainers, like folks maintaining a single driver
or a single network protocol.
Document our expectations and best practices. I'm hoping this doc
will be particularly useful to set expectations with HW vendors.
Signed-off-by: Jakub Kicinski <[email protected]>
---
Please consider this more of a draft than a statement of my opinion.
IOW prefer suggesting edits over arguing about correctness, hope
that makes sense.
---
.../feature-and-driver-maintainers.rst | 159 ++++++++++++++++++
Documentation/maintainer/index.rst | 1 +
2 files changed, 160 insertions(+)
create mode 100644 Documentation/maintainer/feature-and-driver-maintainers.rst
diff --git a/Documentation/maintainer/feature-and-driver-maintainers.rst b/Documentation/maintainer/feature-and-driver-maintainers.rst
new file mode 100644
index 000000000000..ee8ccc22b16a
--- /dev/null
+++ b/Documentation/maintainer/feature-and-driver-maintainers.rst
@@ -0,0 +1,159 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+==============================
+Feature and driver maintainers
+==============================
+
+The term "maintainer" spans a very wide range of levels of engagement
+from people handling patches and pull requests as almost a full time job
+to people responsible for a small feature or a driver.
+
+Unlike most of the chapter, this section is meant for the latter (more
+populous) group. It provides tips and describes the expectations and
+responsibilities of maintainers of a small(ish) section of the code.
+
+Driver and alike most often do not have their own mailing lists and
+git trees but instead send and review patches on the list of a larger
+subsystem.
+
+Responsibilities
+================
+
+The amount of maintenance work is usually proportional to the size
+and popularity of the code base. Small features and drivers should
+require relatively small amount of care and feeding. Nonetheless
+when the work does arrive (in form of patches which need review,
+user bug reports etc.) it has to be acted upon very promptly.
+Even when single driver only sees one patch a month, or a quarter,
+a subsystem could well have a hundred such drivers. Subsystem
+maintainers cannot afford to wait a long time to hear from reviewers.
+
+The exact expectations on the review time will vary by subsystem
+from 1 day (e.g. networking) to a week in smaller subsystems.
+
+Mailing list participation
+--------------------------
+
+Linux kernel uses mailing lists as the primary form of communication.
+Maintainers must be subscribed and follow the appropriate subsystem-wide
+mailing list. Either by subscribing to the whole list or using more
+modern, selective setup like
+`lei <https://people.kernel.org/monsieuricon/lore-lei-part-1-getting-started>`_.
+
+Maintainers must know how to communicate on the list (plain text, no invasive
+legal footers, no top posting, etc.)
+
+Reviews
+-------
+
+Maintainers must review *all* patches touching exclusively their drivers,
+no matter how trivial. If the patch is a tree wide change and modifies
+multiple drivers - whether to provide a review is left to the maintainer.
+
+There should be multiple maintainers for any piece of code, an ``Acked-by``
+or ``Reviewed-by`` tag (or review comments) from a single maintainer is
+enough to satisfy this requirement.
+
+If review process or validation for a particular change will take longer
+than the expected review timeline for the subsystem, maintainer should
+reply to the submission indicating that the work is being done, and when
+to expect full results.
+
+Refactoring and core changes
+----------------------------
+
+Occasionally core code needs to be changed to improve the maintainability
+of the kernel as a whole. Maintainers are expected to be present and
+help guide and test changes to their code to fit the new infrastructure.
+
+Bug reports
+-----------
+
+Maintainers must respond to and address bug reports. The bug reports
+range from users reporting real life crashes, thru errors discovered
+in fuzzing to reports of issues with the code found by static analysis
+tools and new compiler warnings.
+
+Volunteer maintainers are only required to address bugs and regressions.
+It is understood that due to lack of access to documentation and
+implementation details they may not be able to solve all problems.
+
+Commercial vendors are expected to address all issues, on any reasonable
+platform supported by the Linux kernel, as well as answer ordinary user
+questions. There is no concept of product End-of-Life in the Linux kernel,
+the support is required until the subsystem maintainer deletes the code.
+
+The volunteer vs commercial vendor distinction could be seen as roughly
+corresponding to the *Maintained* and *Supported* statuses of the codebase
+in the MAINTAINERS file.
+
+Selecting the maintainer
+========================
+
+The previous section described the expectations of the maintainer,
+this section provides guidance on selecting one and decribes common
+misconceptions.
+
+The author
+----------
+
+Most natural and common choice of a maintainer is the author of the code.
+The author is intimately familiar with the code, so it is the best person
+to take care of it on an ongoing basis.
+
+That said, being a maintainer is an active role. The MAINTAINERS file
+is not a list of credits (in fact a separate CREDITS file exists),
+it is a list of those who will actively help with the code.
+If the author does not have the time, interest or ability to maintain
+the code, a different maintainer must be selected.
+
+Multiple maintainers
+--------------------
+
+Modern best practices dictate that there should be at least two maintainers
+for any piece of code, no matter how trivial. It spreads the burden, helps
+people take vacations and prevents burnout, trains new members of
+the community etc. etc. Even when there is clearly one perfect candidate,
+another maintainer should be found.
+
+Maintainers must be human, however, it is not acceptable to add a mailing
+list or a group email as a maintainer. Trust and understanding are the
+foundation of kernel maintenance and one cannot build trust with a mailing
+list.
+
+Corporate structures
+--------------------
+
+To an outsider the Linux kernel may resemble a hierarchical organization
+with Linus as the CEO. While the code flows in a hierarchical fashion,
+the corporate template does not apply here. Linux is an anarchy held
+together by (rarely expressed) mutual respect, trust and convenience.
+
+All that is to say that managers almost never make good maintainers.
+The maintainer position more closely matches an on-call rotation
+than a position of power.
+
+The following characteristics of a person selected as a maintainer
+are clear red flags:
+
+ - unknown to the community, never sent an email to the list before
+ - did not author any of the code
+ - (when development is contracted) works for a company which paid
+ for the development rather than the company which did the work
+
+Non compliance
+==============
+
+Subsystem maintainers may remove inactive maintainers from the MAINTAINERS
+file. If the maintainer was a significant author or have played an important
+role in the development of the code they should be moved to the CREDITS file.
+
+Removing an inactive maintainer should not be seen as a punitive action.
+Having an inactive maintainer has a real cost as all developeres have
+to remember to include the maintainers in discussions and subsystem
+maintainers spend brain power figuring out how to solicit feedback.
+
+Subsystem maintainers may remove code for lacking maintenance.
+
+Subsystem maintainers may refuse accepting code from companies
+which repeatedly neglected their maintainership duties.
diff --git a/Documentation/maintainer/index.rst b/Documentation/maintainer/index.rst
index 3e03283c144e..eeee27f8b18c 100644
--- a/Documentation/maintainer/index.rst
+++ b/Documentation/maintainer/index.rst
@@ -9,6 +9,7 @@ additions to this manual.
.. toctree::
:maxdepth: 2
+ feature-and-driver-maintainers
configure-git
rebasing-and-merging
pull-requests
--
2.41.0
On 14/07/2023 00:34, Jakub Kicinski wrote:
> We appear to have a gap in our process docs. We go into detail
> on how to contribute code to the kernel, and how to be a subsystem
> maintainer. I can't find any docs directed towards the thousands
> of small scale maintainers, like folks maintaining a single driver
> or a single network protocol.
>
> Document our expectations and best practices. I'm hoping this doc
> will be particularly useful to set expectations with HW vendors.
>
> Signed-off-by: Jakub Kicinski <[email protected]>
> ---
> Please consider this more of a draft than a statement of my opinion.
> IOW prefer suggesting edits over arguing about correctness, hope
> that makes sense.
> ---
> .../feature-and-driver-maintainers.rst | 159 ++++++++++++++++++
> Documentation/maintainer/index.rst | 1 +
> 2 files changed, 160 insertions(+)
> create mode 100644 Documentation/maintainer/feature-and-driver-maintainers.rst
>
> diff --git a/Documentation/maintainer/feature-and-driver-maintainers.rst b/Documentation/maintainer/feature-and-driver-maintainers.rst
> new file mode 100644
> index 000000000000..ee8ccc22b16a
> --- /dev/null
> +++ b/Documentation/maintainer/feature-and-driver-maintainers.rst
> @@ -0,0 +1,159 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +==============================
> +Feature and driver maintainers
> +==============================
> +
> +The term "maintainer" spans a very wide range of levels of engagement
> +from people handling patches and pull requests as almost a full time job
> +to people responsible for a small feature or a driver.
> +
> +Unlike most of the chapter, this section is meant for the latter (more
> +populous) group. It provides tips and describes the expectations and
> +responsibilities of maintainers of a small(ish) section of the code.
> +
> +Driver and alike most often do not have their own mailing lists and
> +git trees but instead send and review patches on the list of a larger
> +subsystem.
> +
> +Responsibilities
> +================
> +
> +The amount of maintenance work is usually proportional to the size
> +and popularity of the code base. Small features and drivers should
> +require relatively small amount of care and feeding. Nonetheless
> +when the work does arrive (in form of patches which need review,
> +user bug reports etc.) it has to be acted upon very promptly.
> +Even when single driver only sees one patch a month, or a quarter,
> +a subsystem could well have a hundred such drivers. Subsystem
> +maintainers cannot afford to wait a long time to hear from reviewers.
> +
> +The exact expectations on the review time will vary by subsystem
> +from 1 day (e.g. networking) to a week in smaller subsystems.
Two weeks is the upper limit.
> +
> +Mailing list participation
> +--------------------------
> +
> +Linux kernel uses mailing lists as the primary form of communication.
> +Maintainers must be subscribed and follow the appropriate subsystem-wide
> +mailing list. Either by subscribing to the whole list or using more
> +modern, selective setup like
> +`lei <https://people.kernel.org/monsieuricon/lore-lei-part-1-getting-started>`_.
> +
> +Maintainers must know how to communicate on the list (plain text, no invasive
> +legal footers, no top posting, etc.)
> +
> +Reviews
> +-------
> +
> +Maintainers must review *all* patches touching exclusively their drivers,
I don't agree with this as a small driver maintainer. Several subsystem
maintainers take the patches much faster than I am able to check the
inbox. I can provide names if you need some proves. With such criteria I
should be removed from maintainers, because I am not able to review
within 24h.
Either give reasonable time, like two weeks, or don't require driver
maintainers to be 24/7 for subystem maintainer disposal. This is very
unfair rule.
> +no matter how trivial. If the patch is a tree wide change and modifies
> +multiple drivers - whether to provide a review is left to the maintainer.
> +
> +There should be multiple maintainers for any piece of code, an ``Acked-by``
> +or ``Reviewed-by`` tag (or review comments) from a single maintainer is
> +enough to satisfy this requirement.
> +
> +If review process or validation for a particular change will take longer
> +than the expected review timeline for the subsystem, maintainer should
> +reply to the submission indicating that the work is being done, and when
> +to expect full results.
> +
> +Refactoring and core changes
> +----------------------------
> +
> +Occasionally core code needs to be changed to improve the maintainability
> +of the kernel as a whole. Maintainers are expected to be present and
> +help guide and test changes to their code to fit the new infrastructure.
> +
> +Bug reports
> +-----------
> +
> +Maintainers must respond to and address bug reports. The bug reports
This is even more unreasonable than previous 1 day review. I don't have
capabilities to address bug reports for numerous drivers I am
maintaining. I don't have hardware, I don't have time, no one pays me
for it. I still need some life outside of working hours, so expecting
both reviews in 1 day and addressing bugs is way too much.
> +range from users reporting real life crashes, thru errors discovered
> +in fuzzing to reports of issues with the code found by static analysis
> +tools and new compiler warnings.
> +
> +Volunteer maintainers are only required to address bugs and regressions.
"Only required"? That's not "only" but a lot.
> +It is understood that due to lack of access to documentation and
> +implementation details they may not be able to solve all problems.
So how do I address? Say "Oh, that's bad"?
Jakub, with both of your criteria - reviewing and addressing - I should
be dropped from all the driver maintainership. If this document passes,
I will do it - drop myself - because:
1. No one pays me for it,
2. I barely have hardware,
3. I want to live a life and I am already working much more than 8h per day.
Best regards,
Krzysztof
On Fri, Jul 14, 2023 at 06:36:41AM +0200, Krzysztof Kozlowski wrote:
> On 14/07/2023 00:34, Jakub Kicinski wrote:
> > +Responsibilities
> > +================
> > +
> > +The amount of maintenance work is usually proportional to the size
> > +and popularity of the code base. Small features and drivers should
> > +require relatively small amount of care and feeding. Nonetheless
> > +when the work does arrive (in form of patches which need review,
> > +user bug reports etc.) it has to be acted upon very promptly.
> > +Even when single driver only sees one patch a month, or a quarter,
> > +a subsystem could well have a hundred such drivers. Subsystem
> > +maintainers cannot afford to wait a long time to hear from reviewers.
> > +
> > +The exact expectations on the review time will vary by subsystem
> > +from 1 day (e.g. networking) to a week in smaller subsystems.
>
> Two weeks is the upper limit.
Indeed. People need to be able to take holiday. Maybe this is
partially covered by "multiple maintainers", but even so, it is
unreasonable to expect this kind of responsiveness.
On 14.07.23 00:34, Jakub Kicinski wrote:
> We appear to have a gap in our process docs. We go into detail
> on how to contribute code to the kernel, and how to be a subsystem
> maintainer. I can't find any docs directed towards the thousands
> of small scale maintainers, like folks maintaining a single driver
> or a single network protocol.
>
> Document our expectations and best practices. I'm hoping this doc
> will be particularly useful to set expectations with HW vendors.
thx for working on this, much appreciated.
> [...]
> +Bug reports
> +-----------
> +
> +Maintainers must respond to and address bug reports. The bug reports
> +range from users reporting real life crashes, thru errors discovered
> +in fuzzing to reports of issues with the code found by static analysis
> +tools and new compiler warnings.
> +
> +Volunteer maintainers are only required to address bugs and regressions.
> +It is understood that due to lack of access to documentation and
> +implementation details they may not be able to solve all problems.
> +
> +Commercial vendors are expected to address all issues, on any reasonable
> +platform supported by the Linux kernel, as well as answer ordinary user
> +questions. There is no concept of product End-of-Life in the Linux kernel,
> +the support is required until the subsystem maintainer deletes the code.
> +
> +The volunteer vs commercial vendor distinction could be seen as roughly
> +corresponding to the *Maintained* and *Supported* statuses of the codebase
> +in the MAINTAINERS file.
The first sentence sets a pretty high bar -- one that afaics doesn't
match current practices, as I frequently see maintainers from commercial
vendors ignoring bad and some good bugs reports (like many reports from
CI systems or report that lack in quality). Without any consequences in
the community afaics, unless they ignore a lot of the good bug reports
or repeatedly ignore regressions reports that reached a certain quality
level (really bad ones are ignored as well and I don't really blame
anyone for that).
Also: It's totally normal that commercial vendor contribute basic
drivers with known problems and missing features (some of which will
never be implemented). The latter will be considered a "bug" for quite a
few users that read this. Those suddenly thus might becomes something
they now "must" fix, which leads to questions: how fast? just in
mainline, or in stable, too?
All this also opens questions like "what counts as bug report" -- I'd
assume users that find and read this will expect that a report in
bugzilla.kernel.org is one maintainers "must" respond to. But I assume
you only meant bugs reports by mail or in trackers the MAINTAINERS file
mentions?
And overall I don't really like the way how handling of regressions is
described in that section, as they afaics are expected to be handled
with a higher priority than bugs.
I considered writing something new, but I now feel a bit confused, as
I'm unsure if my world view is off and yours closer to the proper one.
FWIW, I recently published something[1] related that tries to explain to
ordinary users why their bug report might be ignored. It round about
shows my understanding of things:
```
Developers in the scope of the Linux kernel are considered volunteers
that don't owe you anything
---------------------------------------------------------------------
---------------------------
In the scope of upstream Linux kernel development all developers are
considered volunteers – and as such obviously free to decide what to
spend their time on.
That's because developers in the end fall into two groups:
Companies, universities, government agencies, and other institutions
contribute voluntarily through employees, contractors, students, et. al.
Individuals contribute voluntarily in their own time.
Not even Linus Torvalds has a handle to make those do something he
wishes, except his reputation and control over what is merged into
mainline. That allows him to motivate and occasionally even compel those
volunteers to do something he wants to see – but even for him that only
works up to some point, as those institutions and individuals otherwise
might stop contributing or even fork the Linux kernel.
That in principle is true even for regressions or severe bugs (e.g.
vulnerabilities, data loss, or hardware damage) – but developers or
maintainers will look into those to avoid a bad reputation, which would
cause trouble for future contributions or might cost them their rank.
That can also happen if developers regularly ignore decent bug reports –
which is among the reasons why developers usually help with them, too.
```
[1]
https://linux-regtracking.leemhuis.info/post/frequent-reasons-why-linux-kernel-bug-reports-are-ignored/
This was downplaying/ignoring the "commercial vendor" aspect on purpose.
I would do that differently for a document like this one.
Ciao, Thorsten
On Fri, 14 Jul 2023 06:36:41 +0200 Krzysztof Kozlowski wrote:
> On 14/07/2023 00:34, Jakub Kicinski wrote:
> > +The amount of maintenance work is usually proportional to the size
> > +and popularity of the code base. Small features and drivers should
> > +require relatively small amount of care and feeding. Nonetheless
> > +when the work does arrive (in form of patches which need review,
> > +user bug reports etc.) it has to be acted upon very promptly.
> > +Even when single driver only sees one patch a month, or a quarter,
> > +a subsystem could well have a hundred such drivers. Subsystem
> > +maintainers cannot afford to wait a long time to hear from reviewers.
> > +
> > +The exact expectations on the review time will vary by subsystem
> > +from 1 day (e.g. networking) to a week in smaller subsystems.
>
> Two weeks is the upper limit.
>
> > +Mailing list participation
> > +--------------------------
> > +
> > +Linux kernel uses mailing lists as the primary form of communication.
> > +Maintainers must be subscribed and follow the appropriate subsystem-wide
> > +mailing list. Either by subscribing to the whole list or using more
> > +modern, selective setup like
> > +`lei <https://people.kernel.org/monsieuricon/lore-lei-part-1-getting-started>`_.
> > +
> > +Maintainers must know how to communicate on the list (plain text, no invasive
> > +legal footers, no top posting, etc.)
> > +
> > +Reviews
> > +-------
> > +
> > +Maintainers must review *all* patches touching exclusively their drivers,
>
> I don't agree with this as a small driver maintainer. Several subsystem
> maintainers take the patches much faster than I am able to check the
> inbox. I can provide names if you need some proves. With such criteria I
> should be removed from maintainers, because I am not able to review
> within 24h.
>
> Either give reasonable time, like two weeks, or don't require driver
> maintainers to be 24/7 for subystem maintainer disposal. This is very
> unfair rule.
I think your concern is more about the timeline than what's quoted here,
so I rephrased that:
-The exact expectations on the review time will vary by subsystem
-from 1 day (e.g. networking) to a week in smaller subsystems.
+The exact expectations on the response time will vary by subsystem.
+The patch review SLA the subsystem had set for itself can sometimes
+be found in the subsystem documentation. Failing that as a rule of thumb
+reviewers should try to respond quicker than what is the usual patch
+review delay of the subsystem maintainer. The resulting expectations
+may range from two working days for fast-paced subsystems to two weeks
+in slower moving parts of the kernel.
To the point of reviewing "all" patches, I want to keep this. When
I ping vendors they often reply with "oh I didn't know I'm supposed
to respond, the change looks good". People confuse the review process
with a veto process, if they don't want to outright reject the change
they stay quiet :|
> > +no matter how trivial. If the patch is a tree wide change and modifies
> > +multiple drivers - whether to provide a review is left to the maintainer.
> > +
> > +There should be multiple maintainers for any piece of code, an ``Acked-by``
> > +or ``Reviewed-by`` tag (or review comments) from a single maintainer is
> > +enough to satisfy this requirement.
> > +
> > +If review process or validation for a particular change will take longer
> > +than the expected review timeline for the subsystem, maintainer should
> > +reply to the submission indicating that the work is being done, and when
> > +to expect full results.
> > +
> > +Refactoring and core changes
> > +----------------------------
> > +
> > +Occasionally core code needs to be changed to improve the maintainability
> > +of the kernel as a whole. Maintainers are expected to be present and
> > +help guide and test changes to their code to fit the new infrastructure.
> > +
> > +Bug reports
> > +-----------
> > +
> > +Maintainers must respond to and address bug reports. The bug reports
>
> This is even more unreasonable than previous 1 day review. I don't have
> capabilities to address bug reports for numerous drivers I am
> maintaining. I don't have hardware, I don't have time, no one pays me
> for it. I still need some life outside of working hours, so expecting
> both reviews in 1 day and addressing bugs is way too much.
>
> > +range from users reporting real life crashes, thru errors discovered
> > +in fuzzing to reports of issues with the code found by static analysis
> > +tools and new compiler warnings.
> > +
> > +Volunteer maintainers are only required to address bugs and regressions.
>
> "Only required"? That's not "only" but a lot.
I was trying to soften the paragraph for volunteers let me try to
soften it.. harder?
> > +It is understood that due to lack of access to documentation and
> > +implementation details they may not be able to solve all problems.
>
> So how do I address? Say "Oh, that's bad"?
How about:
Bug reports
-----------
Maintainers must respond to bug reports of reasonable quality. The bug reports
range from users reporting real life crashes, thru errors discovered
in fuzzing to reports of issues with the code found by static analysis
tools and new compiler warnings.
It is understood that the hands of volunteer maintainers can often be tied
by the lack of access to documentation, implementation details, hardware
platforms, etc.
I don't know how to phrase it better :( Obviously maintainers are
expected to look at bug reports. At the same time we all know the
feeling of being a maintainer of some crappy HW which sometimes
doesn't work and all we can do is say "thoughts and prayers".
IDK.
The doc would be incomplete without mentioning that bug reports are
part of maintainers' life :(
> Jakub, with both of your criteria - reviewing and addressing - I should
> be dropped from all the driver maintainership. If this document passes,
> I will do it - drop myself - because:
> 1. No one pays me for it,
> 2. I barely have hardware,
> 3. I want to live a life and I am already working much more than 8h per day.
It's really hard to codify the rules. I hope we can start somewhere
and chisel at the rules if/as we start getting feedback/complaints.
I can give you examples of bad vendor behavior or people who stopped
participating 10 years ago yet they still figure in MAINTAINERS all day.
Next time I see a rando manager added as a maintainer I want to be able
to point them at a document. If the document is too "soft" they will
just wave it off :(
On Fri, 14 Jul 2023 08:24:38 +0200 Thorsten Leemhuis wrote:
> Also: It's totally normal that commercial vendor contribute basic
> drivers with known problems and missing features (some of which will
> never be implemented). The latter will be considered a "bug" for quite a
> few users that read this. Those suddenly thus might becomes something
> they now "must" fix, which leads to questions: how fast? just in
> mainline, or in stable, too?
If we try to fend off anyone who doesn't understand common meaning
of words the document will be very long and painful to read.
> All this also opens questions like "what counts as bug report" -- I'd
> assume users that find and read this will expect that a report in
> bugzilla.kernel.org is one maintainers "must" respond to. But I assume
> you only meant bugs reports by mail or in trackers the MAINTAINERS file
> mentions?
I don't want to be too prescriptive, subsystems will vary.
> And overall I don't really like the way how handling of regressions is
> described in that section, as they afaics are expected to be handled
> with a higher priority than bugs.
Me neither, FWIW. I tried a couple of times to weave that information
in but I can't come up with a way of doing that without breaking the
logical flow. Could just be me. Edit to what I sent to Krzysztof would
be appreciated if you have one in mind.
On Fri, Jul 14, 2023 at 10:22:18AM -0700, Jakub Kicinski wrote:
> On Fri, 14 Jul 2023 08:24:38 +0200 Thorsten Leemhuis wrote:
> > Also: It's totally normal that commercial vendor contribute basic
> > drivers with known problems and missing features (some of which will
> > never be implemented). The latter will be considered a "bug" for quite a
> > few users that read this. Those suddenly thus might becomes something
> > they now "must" fix, which leads to questions: how fast? just in
> > mainline, or in stable, too?
> If we try to fend off anyone who doesn't understand common meaning
> of words the document will be very long and painful to read.
That's true, but "bug" is one of those things where there is a frequent
disconnect on definitions, and when coupled with the must respond bit I
can see things going wrong.
On Fri, 14 Jul 2023 18:59:08 +0100 Mark Brown wrote:
> > If we try to fend off anyone who doesn't understand common meaning
> > of words the document will be very long and painful to read.
>
> That's true, but "bug" is one of those things where there is a frequent
> disconnect on definitions, and when coupled with the must respond bit I
> can see things going wrong.
Right, I agree the "what's a bug" arguments often happen but they
happen primarily when people are trying to get Greg to pull patches
into stable. Or when people try to fast path their "oh, so important
feature" into Linus's tree skipping -next.
The simple way to put it would be if the resulting code goes to stable
or linux/master then it was a bug.
But we can't expect from the user to know if the problem is stable
material, or even whether their problem is a bug in the first place.
Simple example - WiFi cards which don't support AP mode. User may try
to run an AP, and report it doesn't work. They may not know whether
it's HW limitation or a bug. The maintainer responding with "it's not
supported, sorry" does not seem to me to be a high bar.
Also, in my defense, I do give a rough ballpark of what we consider to
be a problem we expect to be addressed:
... bug reports of reasonable quality. The bug reports range from
users reporting real life crashes, thru errors discovered in fuzzing
to reports of issues with the code found by static analysis tools
and new compiler warnings.
Just in case someone thought that maintainers are their tech support.
Then again, I don't want to completely exclude technical questions which
aren't straight up crashes because some of those are reasonable, should
be answered or even result in improving docs or error reporting.
It's a balancing act :(
On Fri, Jul 14, 2023 at 11:34:18AM -0700, Jakub Kicinski wrote:
> On Fri, 14 Jul 2023 18:59:08 +0100 Mark Brown wrote:
> > > If we try to fend off anyone who doesn't understand common meaning
> > > of words the document will be very long and painful to read.
> > That's true, but "bug" is one of those things where there is a frequent
> > disconnect on definitions, and when coupled with the must respond bit I
> > can see things going wrong.
...
> But we can't expect from the user to know if the problem is stable
> material, or even whether their problem is a bug in the first place.
> Simple example - WiFi cards which don't support AP mode. User may try
> to run an AP, and report it doesn't work. They may not know whether
> it's HW limitation or a bug. The maintainer responding with "it's not
> supported, sorry" does not seem to me to be a high bar.
Sure, there's cases where it's really clear and people ought to reply
but there's other things especially as you get into the automated
reports - for things like the fuzzers with automated reports and
sometimes janky bisection it's a lot more reasonable to just drop them
on the floor.
> Just in case someone thought that maintainers are their tech support.
> Then again, I don't want to completely exclude technical questions which
> aren't straight up crashes because some of those are reasonable, should
> be answered or even result in improving docs or error reporting.
> It's a balancing act :(
Honestly I think a lot of it is the "must" rather than "should", it
comes over as being a bit inflexible.
On Thu, Jul 13, 2023 at 03:34:32PM -0700, Jakub Kicinski wrote:
> We appear to have a gap in our process docs. We go into detail
> on how to contribute code to the kernel, and how to be a subsystem
> maintainer. I can't find any docs directed towards the thousands
> of small scale maintainers, like folks maintaining a single driver
> or a single network protocol.
>
> Document our expectations and best practices. I'm hoping this doc
> will be particularly useful to set expectations with HW vendors.
>
> Signed-off-by: Jakub Kicinski <[email protected]>
> ---
> Please consider this more of a draft than a statement of my opinion.
> IOW prefer suggesting edits over arguing about correctness, hope
> that makes sense.
This looks great to me, thanks for putting it together.
But I do have one objection on the timeline portion:
> ---
> .../feature-and-driver-maintainers.rst | 159 ++++++++++++++++++
> Documentation/maintainer/index.rst | 1 +
> 2 files changed, 160 insertions(+)
> create mode 100644 Documentation/maintainer/feature-and-driver-maintainers.rst
>
> diff --git a/Documentation/maintainer/feature-and-driver-maintainers.rst b/Documentation/maintainer/feature-and-driver-maintainers.rst
> new file mode 100644
> index 000000000000..ee8ccc22b16a
> --- /dev/null
> +++ b/Documentation/maintainer/feature-and-driver-maintainers.rst
> @@ -0,0 +1,159 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +==============================
> +Feature and driver maintainers
> +==============================
> +
> +The term "maintainer" spans a very wide range of levels of engagement
> +from people handling patches and pull requests as almost a full time job
> +to people responsible for a small feature or a driver.
> +
> +Unlike most of the chapter, this section is meant for the latter (more
> +populous) group. It provides tips and describes the expectations and
> +responsibilities of maintainers of a small(ish) section of the code.
> +
> +Driver and alike most often do not have their own mailing lists and
> +git trees but instead send and review patches on the list of a larger
> +subsystem.
> +
> +Responsibilities
> +================
> +
> +The amount of maintenance work is usually proportional to the size
> +and popularity of the code base. Small features and drivers should
> +require relatively small amount of care and feeding. Nonetheless
> +when the work does arrive (in form of patches which need review,
> +user bug reports etc.) it has to be acted upon very promptly.
> +Even when single driver only sees one patch a month, or a quarter,
> +a subsystem could well have a hundred such drivers. Subsystem
> +maintainers cannot afford to wait a long time to hear from reviewers.
> +
> +The exact expectations on the review time will vary by subsystem
> +from 1 day (e.g. networking) to a week in smaller subsystems.
"to a few weeks".
I can't do 1 day, or even 1 week for the subsystems I maintain
(especially during merge windows or vacations.) How about that line
being:
from 1 day (e.g. networking) to a few weeks for smaller subsystems.
And then add a link to "For specific subsystem response times, please
see the document in [insert link here to where we keep the subsystem
expectations]"
And yeah, I do need to go add some process/maintainer-* files for the
subsystems I maintain, it might be a good idea to also say that any new
subsystems also provide this so we can start catching up on that.
thanks,
greg k-h
[CCing other people in the thread]
On 14.07.23 19:10, Jakub Kicinski wrote:
> On Fri, 14 Jul 2023 06:36:41 +0200 Krzysztof Kozlowski wrote:
>> On 14/07/2023 00:34, Jakub Kicinski wrote:
> [...]
>>> +Bug reports
>>> +-----------
>>> +
>>> +Maintainers must respond to and address bug reports. The bug reports
>>
>> This is even more unreasonable than previous 1 day review. I don't have
>> capabilities to address bug reports for numerous drivers I am
>> maintaining. I don't have hardware, I don't have time, no one pays me
>> for it. I still need some life outside of working hours, so expecting
>> both reviews in 1 day and addressing bugs is way too much.
>>
>>> +range from users reporting real life crashes, thru errors discovered
>>> +in fuzzing to reports of issues with the code found by static analysis
>>> +tools and new compiler warnings.
>>> +
>>> +Volunteer maintainers are only required to address bugs and regressions.
>>
>> "Only required"? That's not "only" but a lot.
>
> I was trying to soften the paragraph for volunteers let me try to
> soften it.. harder?
>
>>> +It is understood that due to lack of access to documentation and
>>> +implementation details they may not be able to solve all problems.
>>
>> So how do I address? Say "Oh, that's bad"?
>
> How about:
>
> Bug reports
> -----------
>
> Maintainers must respond to bug reports of reasonable quality. The bug reports
> range from users reporting real life crashes, thru errors discovered
> in fuzzing to reports of issues with the code found by static analysis
> tools and new compiler warnings.
>
> It is understood that the hands of volunteer maintainers can often be tied
> by the lack of access to documentation, implementation details, hardware
> platforms, etc.
>
>
> I don't know how to phrase it better :( Obviously maintainers are
> expected to look at bug reports. At the same time we all know the
> feeling of being a maintainer of some crappy HW which sometimes
> doesn't work and all we can do is say "thoughts and prayers".
>
> IDK.
>
> The doc would be incomplete without mentioning that bug reports are
> part of maintainers' life :(
How about something like this:
```
Bug reports
-----------
Maintainers must ensure severe problems in their code reported to them
are resolved in a timely manner: security vulnerabilities, regressions,
compilation errors, data loss, kernel crashes, and bugs of similar scope.
Maintainers furthermore should respond to reports about other kind of
bugs as well, if the report is of reasonable quality or indicates a
problem that might be severe -- especially if they have *Supported*
status of the codebase in the MAINTAINERS file.
```
Ciao, Thorsten
On 14/07/2023 19:10, Jakub Kicinski wrote:
>>> +
>>> +Maintainers must review *all* patches touching exclusively their drivers,
>>
>> I don't agree with this as a small driver maintainer. Several subsystem
>> maintainers take the patches much faster than I am able to check the
>> inbox. I can provide names if you need some proves. With such criteria I
>> should be removed from maintainers, because I am not able to review
>> within 24h.
>>
>> Either give reasonable time, like two weeks, or don't require driver
>> maintainers to be 24/7 for subystem maintainer disposal. This is very
>> unfair rule.
>
> I think your concern is more about the timeline than what's quoted here,
> so I rephrased that:
My concerns are for both timeline and for wording which makes it
obligatory. I think we should not have stale maintainers in MAINTAINERS
file, thus if someone repeatedly does not match criteria, should be
dropped and moved to CREDITS. However I felt here your wording quite
strong, thus I would assume we will start dropping a lot, a lot of
driver maintainers. I am not sure if we really want it, because from
time to time, such maintainer might be actually active and helpful.
>
> -The exact expectations on the review time will vary by subsystem
> -from 1 day (e.g. networking) to a week in smaller subsystems.
>
> +The exact expectations on the response time will vary by subsystem.
> +The patch review SLA the subsystem had set for itself can sometimes
> +be found in the subsystem documentation. Failing that as a rule of thumb
> +reviewers should try to respond quicker than what is the usual patch
> +review delay of the subsystem maintainer. The resulting expectations
> +may range from two working days for fast-paced subsystems to two weeks
> +in slower moving parts of the kernel.
Sounds good. Thank you.
>
>
> To the point of reviewing "all" patches, I want to keep this. When
> I ping vendors they often reply with "oh I didn't know I'm supposed
> to respond, the change looks good". People confuse the review process
> with a veto process, if they don't want to outright reject the change
> they stay quiet :|
OK, I understand. That's the good point.
>
>>> +no matter how trivial. If the patch is a tree wide change and modifies
>>> +multiple drivers - whether to provide a review is left to the maintainer.
>>> +
>>> +There should be multiple maintainers for any piece of code, an ``Acked-by``
>>> +or ``Reviewed-by`` tag (or review comments) from a single maintainer is
>>> +enough to satisfy this requirement.
>>> +
>>> +If review process or validation for a particular change will take longer
>>> +than the expected review timeline for the subsystem, maintainer should
>>> +reply to the submission indicating that the work is being done, and when
>>> +to expect full results.
>>> +
>>> +Refactoring and core changes
>>> +----------------------------
>>> +
>>> +Occasionally core code needs to be changed to improve the maintainability
>>> +of the kernel as a whole. Maintainers are expected to be present and
>>> +help guide and test changes to their code to fit the new infrastructure.
>>> +
>>> +Bug reports
>>> +-----------
>>> +
>>> +Maintainers must respond to and address bug reports. The bug reports
>>
>> This is even more unreasonable than previous 1 day review. I don't have
>> capabilities to address bug reports for numerous drivers I am
>> maintaining. I don't have hardware, I don't have time, no one pays me
>> for it. I still need some life outside of working hours, so expecting
>> both reviews in 1 day and addressing bugs is way too much.
>>
>>> +range from users reporting real life crashes, thru errors discovered
>>> +in fuzzing to reports of issues with the code found by static analysis
>>> +tools and new compiler warnings.
>>> +
>>> +Volunteer maintainers are only required to address bugs and regressions.
>>
>> "Only required"? That's not "only" but a lot.
Thanks.
>
> I was trying to soften the paragraph for volunteers let me try to
> soften it.. harder?
>
>>> +It is understood that due to lack of access to documentation and
>>> +implementation details they may not be able to solve all problems.
>>
>> So how do I address? Say "Oh, that's bad"?
>
> How about:
>
> Bug reports
> -----------
>
> Maintainers must respond to bug reports of reasonable quality. The bug reports
> range from users reporting real life crashes, thru errors discovered
> in fuzzing to reports of issues with the code found by static analysis
> tools and new compiler warnings.
>
> It is understood that the hands of volunteer maintainers can often be tied
> by the lack of access to documentation, implementation details, hardware
> platforms, etc.
>
>
> I don't know how to phrase it better :( Obviously maintainers are
> expected to look at bug reports. At the same time we all know the
> feeling of being a maintainer of some crappy HW which sometimes
> doesn't work and all we can do is say "thoughts and prayers".
Yes, sounds better.
>
> IDK.
>
> The doc would be incomplete without mentioning that bug reports are
> part of maintainers' life :(
>
>> Jakub, with both of your criteria - reviewing and addressing - I should
>> be dropped from all the driver maintainership. If this document passes,
>> I will do it - drop myself - because:
>> 1. No one pays me for it,
>> 2. I barely have hardware,
>> 3. I want to live a life and I am already working much more than 8h per day.
>
> It's really hard to codify the rules. I hope we can start somewhere
> and chisel at the rules if/as we start getting feedback/complaints.
>
> I can give you examples of bad vendor behavior or people who stopped
> participating 10 years ago yet they still figure in MAINTAINERS all day.
Yep, I understand and I was cleaning such entries as well... :)
> Next time I see a rando manager added as a maintainer I want to be able
> to point them at a document. If the document is too "soft" they will
> just wave it off :(
Best regards,
Krzysztof
On 15/07/2023 12:31, Linux regression tracking (Thorsten Leemhuis) wrote:
> [CCing other people in the thread]
> How about something like this:
>
> ```
> Bug reports
> -----------
>
> Maintainers must ensure severe problems in their code reported to them
> are resolved in a timely manner: security vulnerabilities, regressions,
> compilation errors, data loss, kernel crashes, and bugs of similar scope.
>
> Maintainers furthermore should respond to reports about other kind of
> bugs as well, if the report is of reasonable quality or indicates a
> problem that might be severe -- especially if they have *Supported*
> status of the codebase in the MAINTAINERS file.
I like mentioning the "Supported" part. We should be a bit more
understanding to all folks who are not paid to do this.
Best regards,
Krzysztof
On Mon, Jul 17, 2023 at 09:49:09AM +0200, Krzysztof Kozlowski wrote:
> On 15/07/2023 12:31, Linux regression tracking (Thorsten Leemhuis) wrote:
> > [CCing other people in the thread]
> > How about something like this:
> >
> > ```
> > Bug reports
> > -----------
> >
> > Maintainers must ensure severe problems in their code reported to them
> > are resolved in a timely manner: security vulnerabilities, regressions,
> > compilation errors, data loss, kernel crashes, and bugs of similar scope.
> >
> > Maintainers furthermore should respond to reports about other kind of
> > bugs as well, if the report is of reasonable quality or indicates a
> > problem that might be severe -- especially if they have *Supported*
> > status of the codebase in the MAINTAINERS file.
>
> I like mentioning the "Supported" part. We should be a bit more
> understanding to all folks who are not paid to do this.
And, we should not be as understanding for companies who do NOT allow
their developers to do this on company time, so pointing out the
difference here is good, as most of the time it goes unnoticed as to
just how little companies allow their maintainers to do their work.
thanks,
greg k-h
On Sat, 15 Jul 2023 12:31:02 +0200 Linux regression tracking (Thorsten
Leemhuis) wrote:
> Maintainers must ensure severe problems in their code reported to them
> are resolved in a timely manner: security vulnerabilities, regressions,
> compilation errors, data loss, kernel crashes, and bugs of similar scope.
SG, thanks for the suggestion!
One edit - I'd like to remove "security vulnerabilities" from the list.
Security implications are an axis on which bug can be evaluated, one of
many. All kernel bugs have some security implications. Placing them as
a category like crashes, lockups or compiler errors could deepen the
confusion.
On 18.07.23 17:37, Jakub Kicinski wrote:
> On Sat, 15 Jul 2023 12:31:02 +0200 Linux regression tracking (Thorsten
> Leemhuis) wrote:
>> Maintainers must ensure severe problems in their code reported to them
BTW: I wonder if "reported to them" should be removed. Or maybe it
should be "they become aware of" instead, as they might be reported to
of the the contributors to the subsystem the maintainer handles. Not
sure. Currently I think removing might be better. Judge yourself.
>> are resolved in a timely manner: security vulnerabilities, regressions,
>> compilation errors, data loss, kernel crashes, and bugs of similar scope.
>
> SG, thanks for the suggestion!
+1
> One edit - I'd like to remove "security vulnerabilities" from the list.
> Security implications are an axis on which bug can be evaluated, one of
> many. All kernel bugs have some security implications. Placing them as
> a category like crashes, lockups or compiler errors could deepen the
> confusion.
I don't really care, but that could be avoided with something like
"security vulnerabilities known to be exploitable".
Cioa, Thorsten