2019-11-23 02:46:19

by Paul Walmsley

[permalink] [raw]
Subject: [PATCH] Documentation: riscv: add patch acceptance guidelines


Formalize, in kernel documentation, the patch acceptance policy for
arch/riscv. In summary, it states that as maintainers, we plan to only
accept patches for new modules or extensions that have been frozen or
ratified by the RISC-V Foundation.

We've been following these guidelines for the past few months. In the
meantime, we've received quite a bit of feedback that it would be
helpful to have these guidelines formally documented.

Signed-off-by: Paul Walmsley <[email protected]>
Cc: Palmer Dabbelt <[email protected]>
Cc: Albert Ou <[email protected]>
Cc: Krste Asanovic <[email protected]>
Cc: Andrew Waterman <[email protected]>
---
Documentation/riscv/patch-acceptance.rst | 32 ++++++++++++++++++++++++
1 file changed, 32 insertions(+)
create mode 100644 Documentation/riscv/patch-acceptance.rst

diff --git a/Documentation/riscv/patch-acceptance.rst b/Documentation/riscv/patch-acceptance.rst
new file mode 100644
index 000000000000..2e658353b53c
--- /dev/null
+++ b/Documentation/riscv/patch-acceptance.rst
@@ -0,0 +1,32 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+====================================================
+arch/riscv maintenance and the RISC-V specifications
+====================================================
+
+The RISC-V instruction set architecture is developed in the open:
+in-progress drafts are available for all to review and to experiment
+with implementations. New module or extension drafts can change
+during the development process - sometimes in ways that are
+incompatible with previous drafts. This flexibility can present a
+challenge for RISC-V Linux maintenance. Linux maintainers disapprove
+of churn, and the Linux development process prefers well-reviewed and
+tested code over experimental code. We wish to extend these same
+principles to the RISC-V-related code that will be accepted for
+inclusion in the kernel.
+
+Therefore, as maintainers, we'll only accept patches for new modules
+or extensions if the specifications for those modules or extensions
+are listed as being "Frozen" or "Ratified" by the RISC-V Foundation.
+(Developers may, of course, maintain their own Linux kernel trees that
+contain code for any draft extensions that they wish.)
+
+Additionally, the RISC-V specification allows implementors to create
+their own custom extensions. These custom extensions aren't required
+to go through any review or ratification process by the RISC-V
+Foundation. To avoid the maintenance complexity and potential
+performance impact of adding kernel code for implementor-specific
+RISC-V extensions, we'll only to accept patches for extensions that
+have been officially frozen or ratified by the RISC-V Foundation.
+(Implementors, may, of course, maintain their own Linux kernel trees
+containing code for any custom extensions that they wish.)
--
2.24.0.rc0


2019-11-23 04:08:49

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH] Documentation: riscv: add patch acceptance guidelines

On Fri, Nov 22, 2019 at 06:44:39PM -0800, Paul Walmsley wrote:
> Documentation/riscv/patch-acceptance.rst | 32 ++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
> create mode 100644 Documentation/riscv/patch-acceptance.rst

Should this be linked into the toctree somewhere so it's findable
on kernel.org? Maybe add a line to Documentation/process/index.rst
to include ../riscv/patch-acceptance.rst?

2019-11-23 16:40:54

by Jonathan Corbet

[permalink] [raw]
Subject: Re: [PATCH] Documentation: riscv: add patch acceptance guidelines

On Fri, 22 Nov 2019 18:44:39 -0800 (PST)
Paul Walmsley <[email protected]> wrote:

> Formalize, in kernel documentation, the patch acceptance policy for
> arch/riscv. In summary, it states that as maintainers, we plan to only
> accept patches for new modules or extensions that have been frozen or
> ratified by the RISC-V Foundation.
>
> We've been following these guidelines for the past few months. In the
> meantime, we've received quite a bit of feedback that it would be
> helpful to have these guidelines formally documented.

If at all possible, I would really love to have this be part of the
maintainer profile documentation:

https://lwn.net/ml/linux-kernel/156821692280.2951081.18036584954940423225.stgit@dwillia2-desk3.amr.corp.intel.com/

...if we could only (hint...CC'd...) get Dan to resubmit it with the
needed tweaks so it could be merged...

Thanks,

jon

2019-11-23 18:32:29

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH] Documentation: riscv: add patch acceptance guidelines

On Fri, 22 Nov 2019 18:44:39 PST (-0800), Paul Walmsley wrote:
>
> Formalize, in kernel documentation, the patch acceptance policy for
> arch/riscv. In summary, it states that as maintainers, we plan to only
> accept patches for new modules or extensions that have been frozen or
> ratified by the RISC-V Foundation.
>
> We've been following these guidelines for the past few months. In the
> meantime, we've received quite a bit of feedback that it would be
> helpful to have these guidelines formally documented.
>
> Signed-off-by: Paul Walmsley <[email protected]>
> Cc: Palmer Dabbelt <[email protected]>
> Cc: Albert Ou <[email protected]>
> Cc: Krste Asanovic <[email protected]>
> Cc: Andrew Waterman <[email protected]>
> ---
> Documentation/riscv/patch-acceptance.rst | 32 ++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
> create mode 100644 Documentation/riscv/patch-acceptance.rst
>
> diff --git a/Documentation/riscv/patch-acceptance.rst b/Documentation/riscv/patch-acceptance.rst
> new file mode 100644
> index 000000000000..2e658353b53c
> --- /dev/null
> +++ b/Documentation/riscv/patch-acceptance.rst
> @@ -0,0 +1,32 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +====================================================
> +arch/riscv maintenance and the RISC-V specifications
> +====================================================
> +
> +The RISC-V instruction set architecture is developed in the open:
> +in-progress drafts are available for all to review and to experiment
> +with implementations. New module or extension drafts can change
> +during the development process - sometimes in ways that are
> +incompatible with previous drafts. This flexibility can present a
> +challenge for RISC-V Linux maintenance. Linux maintainers disapprove
> +of churn, and the Linux development process prefers well-reviewed and
> +tested code over experimental code. We wish to extend these same
> +principles to the RISC-V-related code that will be accepted for
> +inclusion in the kernel.
> +
> +Therefore, as maintainers, we'll only accept patches for new modules
> +or extensions if the specifications for those modules or extensions
> +are listed as being "Frozen" or "Ratified" by the RISC-V Foundation.
> +(Developers may, of course, maintain their own Linux kernel trees that
> +contain code for any draft extensions that they wish.)
> +
> +Additionally, the RISC-V specification allows implementors to create
> +their own custom extensions. These custom extensions aren't required
> +to go through any review or ratification process by the RISC-V
> +Foundation. To avoid the maintenance complexity and potential
> +performance impact of adding kernel code for implementor-specific
> +RISC-V extensions, we'll only to accept patches for extensions that
> +have been officially frozen or ratified by the RISC-V Foundation.
> +(Implementors, may, of course, maintain their own Linux kernel trees
> +containing code for any custom extensions that they wish.)

Reviewed-by: Palmer Dabbelt <[email protected]>

2019-11-23 23:29:20

by Paul Walmsley

[permalink] [raw]
Subject: Re: [PATCH] Documentation: riscv: add patch acceptance guidelines

Hi Jon,

On Sat, 23 Nov 2019, Jonathan Corbet wrote:

> On Fri, 22 Nov 2019 18:44:39 -0800 (PST) Paul Walmsley
> <[email protected]> wrote:
>
> > Formalize, in kernel documentation, the patch acceptance policy for
> > arch/riscv. In summary, it states that as maintainers, we plan to only
> > accept patches for new modules or extensions that have been frozen or
> > ratified by the RISC-V Foundation.
> >
> > We've been following these guidelines for the past few months. In the
> > meantime, we've received quite a bit of feedback that it would be
> > helpful to have these guidelines formally documented.
>
> If at all possible, I would really love to have this be part of the
> maintainer profile documentation:
>
> https://lwn.net/ml/linux-kernel/156821692280.2951081.18036584954940423225.stgit@dwillia2-desk3.amr.corp.intel.com/
>
> ...if we could only (hint...CC'd...) get Dan to resubmit it with the
> needed tweaks so it could be merged...

It looks like the main thing that would be needed would be to add the P:
entry with the path to our patch-acceptance.rst file into the MAINTAINERS
file, after Dan's patches are merged.

Of course, we could also add more information about sparse cleanliness,
checkpatch warnings, etc., but we mostly try to follow the common kernel
guidelines there.

Is that summary accurate, or did I miss some additional steps?


- Paul

2019-11-23 23:39:49

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH] Documentation: riscv: add patch acceptance guidelines

On Sat, Nov 23, 2019 at 3:27 PM Paul Walmsley <[email protected]> wrote:
>
> Hi Jon,
>
> On Sat, 23 Nov 2019, Jonathan Corbet wrote:
>
> > On Fri, 22 Nov 2019 18:44:39 -0800 (PST) Paul Walmsley
> > <[email protected]> wrote:
> >
> > > Formalize, in kernel documentation, the patch acceptance policy for
> > > arch/riscv. In summary, it states that as maintainers, we plan to only
> > > accept patches for new modules or extensions that have been frozen or
> > > ratified by the RISC-V Foundation.
> > >
> > > We've been following these guidelines for the past few months. In the
> > > meantime, we've received quite a bit of feedback that it would be
> > > helpful to have these guidelines formally documented.
> >
> > If at all possible, I would really love to have this be part of the
> > maintainer profile documentation:
> >
> > https://lwn.net/ml/linux-kernel/156821692280.2951081.18036584954940423225.stgit@dwillia2-desk3.amr.corp.intel.com/
> >
> > ...if we could only (hint...CC'd...) get Dan to resubmit it with the
> > needed tweaks so it could be merged...
>
> It looks like the main thing that would be needed would be to add the P:
> entry with the path to our patch-acceptance.rst file into the MAINTAINERS
> file, after Dan's patches are merged.
>
> Of course, we could also add more information about sparse cleanliness,
> checkpatch warnings, etc., but we mostly try to follow the common kernel
> guidelines there.

Those could likely be automated to highlight warnings that a given
subsystem treats as errors, but wherever possible my expectation is
that the policy should be specified globally.

>
> Is that summary accurate, or did I miss some additional steps?
>

I'll go fixup and get the into patch submitted today then we can go from there.

2019-11-23 23:40:38

by Paul Walmsley

[permalink] [raw]
Subject: Re: [PATCH] Documentation: riscv: add patch acceptance guidelines

Hi Matthew,

On Fri, 22 Nov 2019, Matthew Wilcox wrote:

> On Fri, Nov 22, 2019 at 06:44:39PM -0800, Paul Walmsley wrote:
> > Documentation/riscv/patch-acceptance.rst | 32 ++++++++++++++++++++++++
> > 1 file changed, 32 insertions(+)
> > create mode 100644 Documentation/riscv/patch-acceptance.rst
>
> Should this be linked into the toctree somewhere so it's findable
> on kernel.org? Maybe add a line to Documentation/process/index.rst
> to include ../riscv/patch-acceptance.rst?

Does this updated patch contain what you had in mind?


- Paul

From: Paul Walmsley <[email protected]>
Date: Fri, 22 Nov 2019 18:33:28 -0800
Subject: [PATCH] Documentation: riscv: add patch acceptance guidelines

Formalize, in kernel documentation, the patch acceptance policy for
arch/riscv. In summary, it states that as maintainers, we plan to
only accept patches for new modules or extensions that have been
frozen or ratified by the RISC-V Foundation.

We've been following these guidelines for the past few months. In the
meantime, we've received quite a bit of feedback that it would be
helpful to have these guidelines formally documented.

Based on a suggestion from Matthew Wilcox, we also add a link to this
file to Documentation/process/index.rst, to make this document easier
to find.

Signed-off-by: Paul Walmsley <[email protected]>
Reviewed-by: Palmer Dabbelt <[email protected]>
Cc: Palmer Dabbelt <[email protected]>
Cc: Albert Ou <[email protected]>
Cc: Krste Asanovic <[email protected]>
Cc: Andrew Waterman <[email protected]>
Cc: Matthew Wilcox <[email protected]>
---
Documentation/process/index.rst | 1 +
Documentation/riscv/index.rst | 1 +
Documentation/riscv/patch-acceptance.rst | 32 ++++++++++++++++++++++++
3 files changed, 34 insertions(+)
create mode 100644 Documentation/riscv/patch-acceptance.rst

diff --git a/Documentation/process/index.rst b/Documentation/process/index.rst
index e2c9ffc682c5..9b8394eacea6 100644
--- a/Documentation/process/index.rst
+++ b/Documentation/process/index.rst
@@ -58,6 +58,7 @@ lack of a better place.
magic-number
volatile-considered-harmful
clang-format
+ ../riscv/patch-acceptance

.. only:: subproject and html

diff --git a/Documentation/riscv/index.rst b/Documentation/riscv/index.rst
index 215fd3c1f2d5..fa33bffd8992 100644
--- a/Documentation/riscv/index.rst
+++ b/Documentation/riscv/index.rst
@@ -7,6 +7,7 @@ RISC-V architecture

boot-image-header
pmu
+ patch-acceptance

.. only:: subproject and html

diff --git a/Documentation/riscv/patch-acceptance.rst b/Documentation/riscv/patch-acceptance.rst
new file mode 100644
index 000000000000..2e658353b53c
--- /dev/null
+++ b/Documentation/riscv/patch-acceptance.rst
@@ -0,0 +1,32 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+====================================================
+arch/riscv maintenance and the RISC-V specifications
+====================================================
+
+The RISC-V instruction set architecture is developed in the open:
+in-progress drafts are available for all to review and to experiment
+with implementations. New module or extension drafts can change
+during the development process - sometimes in ways that are
+incompatible with previous drafts. This flexibility can present a
+challenge for RISC-V Linux maintenance. Linux maintainers disapprove
+of churn, and the Linux development process prefers well-reviewed and
+tested code over experimental code. We wish to extend these same
+principles to the RISC-V-related code that will be accepted for
+inclusion in the kernel.
+
+Therefore, as maintainers, we'll only accept patches for new modules
+or extensions if the specifications for those modules or extensions
+are listed as being "Frozen" or "Ratified" by the RISC-V Foundation.
+(Developers may, of course, maintain their own Linux kernel trees that
+contain code for any draft extensions that they wish.)
+
+Additionally, the RISC-V specification allows implementors to create
+their own custom extensions. These custom extensions aren't required
+to go through any review or ratification process by the RISC-V
+Foundation. To avoid the maintenance complexity and potential
+performance impact of adding kernel code for implementor-specific
+RISC-V extensions, we'll only to accept patches for extensions that
+have been officially frozen or ratified by the RISC-V Foundation.
+(Implementors, may, of course, maintain their own Linux kernel trees
+containing code for any custom extensions that they wish.)
--
2.24.0.rc0

2019-11-23 23:51:35

by Paul Walmsley

[permalink] [raw]
Subject: Re: [PATCH] Documentation: riscv: add patch acceptance guidelines

On Sat, 23 Nov 2019, Dan Williams wrote:

> On Sat, Nov 23, 2019 at 3:27 PM Paul Walmsley <[email protected]> wrote:
>
> > It looks like the main thing that would be needed would be to add the P:
> > entry with the path to our patch-acceptance.rst file into the MAINTAINERS
> > file, after Dan's patches are merged.
> >
> > Of course, we could also add more information about sparse cleanliness,
> > checkpatch warnings, etc., but we mostly try to follow the common kernel
> > guidelines there.
>
> Those could likely be automated to highlight warnings that a given
> subsystem treats as errors, but wherever possible my expectation is
> that the policy should be specified globally.
>
> > Is that summary accurate, or did I miss some additional steps?
>
> I'll go fixup and get the into patch submitted today then we can go from
> there.

I guess I'm still looking for guidance along the lines of my earlier
question: what (if anything) would we need to change about the current
patch to have it work with the maintainer profile documentation (beyond
the "P:" entry in MAINTAINERS) ?


- Paul

2019-11-24 00:04:35

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH] Documentation: riscv: add patch acceptance guidelines

On Sat, Nov 23, 2019 at 3:50 PM Paul Walmsley <[email protected]> wrote:
>
> On Sat, 23 Nov 2019, Dan Williams wrote:
>
> > On Sat, Nov 23, 2019 at 3:27 PM Paul Walmsley <[email protected]> wrote:
> >
> > > It looks like the main thing that would be needed would be to add the P:
> > > entry with the path to our patch-acceptance.rst file into the MAINTAINERS
> > > file, after Dan's patches are merged.
> > >
> > > Of course, we could also add more information about sparse cleanliness,
> > > checkpatch warnings, etc., but we mostly try to follow the common kernel
> > > guidelines there.
> >
> > Those could likely be automated to highlight warnings that a given
> > subsystem treats as errors, but wherever possible my expectation is
> > that the policy should be specified globally.
> >
> > > Is that summary accurate, or did I miss some additional steps?
> >
> > I'll go fixup and get the into patch submitted today then we can go from
> > there.
>
> I guess I'm still looking for guidance along the lines of my earlier
> question: what (if anything) would we need to change about the current
> patch to have it work with the maintainer profile documentation (beyond
> the "P:" entry in MAINTAINERS) ?

Oh, sorry, I just reacted to Jon's comments. I took a look, and I
think the content would just need to be organized into the proposed
sections. The rules about what level of ratification a specification
needs to receive before a patch will be received sounds like an
extension to the Submit Checklist to me. So I'd say just format your
first paragraph into the Overview section and the other 2 into Submit
Checklist and call it good.

2019-11-24 00:44:52

by Paul Walmsley

[permalink] [raw]
Subject: Re: [PATCH] Documentation: riscv: add patch acceptance guidelines

On Sat, 23 Nov 2019, Dan Williams wrote:

> I took a look, and I think the content would just need to be organized
> into the proposed sections. The rules about what level of ratification a
> specification needs to receive before a patch will be received sounds
> like an extension to the Submit Checklist to me. So I'd say just format
> your first paragraph into the Overview section and the other 2 into
> Submit Checklist and call it good.

I'm fine with doing that for this patch.

Stepping back to the broader topic of the maintainer profile patches, one
comment there: unless you're planning to do automated processing on these
maintainer profile document sections, it's probably better to let
maintainers format their own profile documents as they wish.

Just to use the arch/riscv document as an example: the last two
paragraphs, to me, don't belong in a "submit checklist" section, since
that implies that the text there only needs to be read before patches are
submitted. We'd really prefer that developers understand what patches
we'll take before they even start developing them.

I imagine we wouldn't be the only ones that would prefer to create their
own section headings in this document, etc.


- Paul

2019-11-24 03:40:35

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH] Documentation: riscv: add patch acceptance guidelines

On Sat, Nov 23, 2019 at 4:42 PM Paul Walmsley <[email protected]> wrote:
>
> On Sat, 23 Nov 2019, Dan Williams wrote:
>
> > I took a look, and I think the content would just need to be organized
> > into the proposed sections. The rules about what level of ratification a
> > specification needs to receive before a patch will be received sounds
> > like an extension to the Submit Checklist to me. So I'd say just format
> > your first paragraph into the Overview section and the other 2 into
> > Submit Checklist and call it good.
>
> I'm fine with doing that for this patch.
>
> Stepping back to the broader topic of the maintainer profile patches, one
> comment there: unless you're planning to do automated processing on these
> maintainer profile document sections, it's probably better to let
> maintainers format their own profile documents as they wish.
>
> Just to use the arch/riscv document as an example: the last two
> paragraphs, to me, don't belong in a "submit checklist" section, since
> that implies that the text there only needs to be read before patches are
> submitted. We'd really prefer that developers understand what patches
> we'll take before they even start developing them.
>
> I imagine we wouldn't be the only ones that would prefer to create their
> own section headings in this document, etc.

I'm open to updating the headers to make a section heading that
matches what you're trying to convey, however that header definition
should be globally agreed upon. I don't want the document that tries
to clarify per-subsystem behaviours itself to have per-subsystem
permutations. I think we, subsystem maintainers, at least need to be
able to agree on the topics we disagree on. Would it be sufficient if
I just clarified that "Submit Checklist Addendum" also includes
guidance about which patches are out of scope for submission in the
first instance?

2019-11-25 02:51:41

by Paul Walmsley

[permalink] [raw]
Subject: Re: [PATCH] Documentation: riscv: add patch acceptance guidelines

On Sat, 23 Nov 2019, Dan Williams wrote:

> I'm open to updating the headers to make a section heading that
> matches what you're trying to convey, however that header definition
> should be globally agreed upon. I don't want the document that tries
> to clarify per-subsystem behaviours itself to have per-subsystem
> permutations. I think we, subsystem maintainers, at least need to be
> able to agree on the topics we disagree on.

Unless you're planning to, say, follow up with some kind of automated
process working across all of the profile documents in such a way that it
would make technical sense for the different sections to be standardized,
I personally don't see any need at all for profile document
standardization. As far as I can tell, these documents are meant for
humans, rather than computers, to read. And in the absence of a strong
technical rationale to limit how maintainers express themselves here, I
don't think it's justified.


- Paul

2019-11-25 03:25:48

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH] Documentation: riscv: add patch acceptance guidelines

On Sun, Nov 24, 2019 at 6:49 PM Paul Walmsley <[email protected]> wrote:
>
> On Sat, 23 Nov 2019, Dan Williams wrote:
>
> > I'm open to updating the headers to make a section heading that
> > matches what you're trying to convey, however that header definition
> > should be globally agreed upon. I don't want the document that tries
> > to clarify per-subsystem behaviours itself to have per-subsystem
> > permutations. I think we, subsystem maintainers, at least need to be
> > able to agree on the topics we disagree on.
>
> Unless you're planning to, say, follow up with some kind of automated
> process working across all of the profile documents in such a way that it
> would make technical sense for the different sections to be standardized,
> I personally don't see any need at all for profile document
> standardization. As far as I can tell, these documents are meant for
> humans, rather than computers, to read. And in the absence of a strong
> technical rationale to limit how maintainers express themselves here, I
> don't think it's justified.
>

It's just a template, you're free to make sub-headings of your own
choosing, but please try to give a contributor that is spanning
subsystems a chance to navigate similar information across profile
documents.

2019-11-25 18:48:38

by Jonathan Corbet

[permalink] [raw]
Subject: Re: [PATCH] Documentation: riscv: add patch acceptance guidelines

On Sun, 24 Nov 2019 18:48:54 -0800 (PST)
Paul Walmsley <[email protected]> wrote:

> On Sat, 23 Nov 2019, Dan Williams wrote:
>
> > I'm open to updating the headers to make a section heading that
> > matches what you're trying to convey, however that header definition
> > should be globally agreed upon. I don't want the document that tries
> > to clarify per-subsystem behaviours itself to have per-subsystem
> > permutations. I think we, subsystem maintainers, at least need to be
> > able to agree on the topics we disagree on.
>
> Unless you're planning to, say, follow up with some kind of automated
> process working across all of the profile documents in such a way that it
> would make technical sense for the different sections to be standardized,
> I personally don't see any need at all for profile document
> standardization. As far as I can tell, these documents are meant for
> humans, rather than computers, to read. And in the absence of a strong
> technical rationale to limit how maintainers express themselves here, I
> don't think it's justified.

Patch changelogs are (mostly) meant for humans to read too, but we have
some standards for how we want them formatted. I don't think the
maintainer profiles should be all that tightly specified, but it would be
a whole lot better if cross-subsystem developers knew where to look to
quickly find the information they need. So I'd prefer it if we could find
a way to conform to a set of loose guidelines for these files.

Thanks,

jon