| Message ID | alpine.DEB.2.21.9999.1911221842200.14532@viisi.sifive.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Documentation: riscv: add patch acceptance guidelines | expand |
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?
On Fri, 22 Nov 2019 18:44:39 -0800 (PST) Paul Walmsley <paul.walmsley@sifive.com> 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
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 <paul.walmsley@sifive.com> > Cc: Palmer Dabbelt <palmer@dabbelt.com> > Cc: Albert Ou <aou@eecs.berkeley.edu> > Cc: Krste Asanovic <krste@berkeley.edu> > Cc: Andrew Waterman <waterman@eecs.berkeley.edu> > --- > 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 <palmerdabbelt@google.com>
Hi Jon, On Sat, 23 Nov 2019, Jonathan Corbet wrote: > On Fri, 22 Nov 2019 18:44:39 -0800 (PST) Paul Walmsley > <paul.walmsley@sifive.com> 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
On Sat, Nov 23, 2019 at 3:27 PM Paul Walmsley <paul.walmsley@sifive.com> wrote: > > Hi Jon, > > On Sat, 23 Nov 2019, Jonathan Corbet wrote: > > > On Fri, 22 Nov 2019 18:44:39 -0800 (PST) Paul Walmsley > > <paul.walmsley@sifive.com> 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.
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 <paul.walmsley@sifive.com> 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 <paul.walmsley@sifive.com> Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com> Cc: Palmer Dabbelt <palmer@dabbelt.com> Cc: Albert Ou <aou@eecs.berkeley.edu> Cc: Krste Asanovic <krste@berkeley.edu> Cc: Andrew Waterman <waterman@eecs.berkeley.edu> Cc: Matthew Wilcox <willy@infradead.org> --- 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.)
On Sat, 23 Nov 2019, Dan Williams wrote: > On Sat, Nov 23, 2019 at 3:27 PM Paul Walmsley <paul.walmsley@sifive.com> 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
On Sat, Nov 23, 2019 at 3:50 PM Paul Walmsley <paul.walmsley@sifive.com> wrote: > > On Sat, 23 Nov 2019, Dan Williams wrote: > > > On Sat, Nov 23, 2019 at 3:27 PM Paul Walmsley <paul.walmsley@sifive.com> 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.
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
On Sat, Nov 23, 2019 at 4:42 PM Paul Walmsley <paul.walmsley@sifive.com> 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?
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
On Sun, Nov 24, 2019 at 6:49 PM Paul Walmsley <paul.walmsley@sifive.com> 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.
On Sun, 24 Nov 2019 18:48:54 -0800 (PST) Paul Walmsley <paul.walmsley@sifive.com> 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
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.)
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 <paul.walmsley@sifive.com> Cc: Palmer Dabbelt <palmer@dabbelt.com> Cc: Albert Ou <aou@eecs.berkeley.edu> Cc: Krste Asanovic <krste@berkeley.edu> Cc: Andrew Waterman <waterman@eecs.berkeley.edu> --- Documentation/riscv/patch-acceptance.rst | 32 ++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 Documentation/riscv/patch-acceptance.rst