While going through the submit checklist, the list order seemed rather
random, probably just by historical coincidences of always adding yet the
next point someone thought of at the end of the list.
Structure and order them by the category of such activity,
reviewing, documenting, checking with tools, building and testing.
As the diff of the reordering is large:
Review code now includes previous points 1, 5 and 22.
Review Kconfig includes previous 6, 7 and 8.
Documenting includes previous 11, 15, 16, 17, 18 and 23.
Checking with tools includes previous 5, 9 and 10.
Building includes previous 2, 3, 20 and 24.
Testing includes previous 12, 13, 14, 19 and 21.
Previous point 4 (compile for ppc64) was merged into point 3 (build for
many architectures), as it was just a further note to cross-compiling.
Previous point 5 was split into one in review and one in checking
to have every previous point in the right category.
Point 11 was shortened, as building documentation is mentioned already
in Build your code, 1d.
A note that was presented visually much too aggressive in the HTML view was
turned into a simple "Note that..." sentence in the enumeration.
The recommendation to test with the -mm patchset (previous 21, now
testing, point 5) was updated to the current state of affairs to test with
a recent tag of linux-next.
Note that the previous first point still remains the first list even after
reordering. Based on some vague memory, the first point was important to
Randy to stay the first one in any reordering.
While at it, the reference to CONFIG_SLUB_DEBUG was replaced by
CONFIG_DEBUG_SLAB.
Signed-off-by: Lukas Bulwahn <[email protected]>
---
So far, no point disappeared and nothing new was added.
Points/Ideas for further improvements (based on my knowledge and judgement):
- The Review Kconfig changes makes sense, but I am not sure if they are
so central during review. If we keep it, let us see if there are other
parts for review that are also important similar to Kconfig changes.
- Concerning checking with tools, checkpatch probably still makes sense;
it pointed out in several places. If sparse and checkstack are really
the next two tools to point out, I am not so sure about.
sparse has a lot of false positives nowadays, and many things are not
fixed just because sparse complains about it.
And I have never used make checkstack and have not found much
documentation about it.
So, maybe other tools deserve to be mentioned here instead?
I am happy to get feedback---I will work through submitting-patches next
and do some clean-up there. While doing that, I might learn what really
should go into a better future 'submit-checklist' documentation.
Documentation/process/submit-checklist.rst | 157 +++++++++++----------
1 file changed, 84 insertions(+), 73 deletions(-)
diff --git a/Documentation/process/submit-checklist.rst b/Documentation/process/submit-checklist.rst
index b1bc2d37bd0a..7d8dba942fe8 100644
--- a/Documentation/process/submit-checklist.rst
+++ b/Documentation/process/submit-checklist.rst
@@ -11,110 +11,121 @@ These are all above and beyond the documentation that is provided in
and elsewhere regarding submitting Linux kernel patches.
+*Review your code:*
+
1) If you use a facility then #include the file that defines/declares
that facility. Don't depend on other header files pulling in ones
that you use.
-2) Builds cleanly:
+2) Check your patch for general style as detailed in
+ :ref:`Documentation/process/coding-style.rst <codingstyle>`.
- a) with applicable or modified ``CONFIG`` options ``=y``, ``=m``, and
- ``=n``. No ``gcc`` warnings/errors, no linker warnings/errors.
+3) All memory barriers {e.g., ``barrier()``, ``rmb()``, ``wmb()``} need a
+ comment in the source code that explains the logic of what they are doing
+ and why.
- b) Passes ``allnoconfig``, ``allmodconfig``
- c) Builds successfully when using ``O=builddir``
+*Review Kconfig changes:*
- d) Any Documentation/ changes build successfully without new warnings/errors.
- Use ``make htmldocs`` or ``make pdfdocs`` to check the build and
- fix any issues.
+1) Any new or modified ``CONFIG`` options do not muck up the config menu and
+ default to off unless they meet the exception criteria documented in
+ ``Documentation/kbuild/kconfig-language.rst`` Menu attributes: default value.
-3) Builds on multiple CPU architectures by using local cross-compile tools
- or some other build farm.
+2) All new ``Kconfig`` options have help text.
-4) ppc64 is a good architecture for cross-compilation checking because it
- tends to use ``unsigned long`` for 64-bit quantities.
+3) Has been carefully reviewed with respect to relevant ``Kconfig``
+ combinations. This is very hard to get right with testing---brainpower
+ pays off here.
-5) Check your patch for general style as detailed in
- :ref:`Documentation/process/coding-style.rst <codingstyle>`.
- Check for trivial violations with the patch style checker prior to
- submission (``scripts/checkpatch.pl``).
- You should be able to justify all violations that remain in
- your patch.
+*Provide documentation:*
-6) Any new or modified ``CONFIG`` options do not muck up the config menu and
- default to off unless they meet the exception criteria documented in
- ``Documentation/kbuild/kconfig-language.rst`` Menu attributes: default value.
+1) Include :ref:`kernel-doc <kernel_doc>` to document global kernel APIs.
+ (Not required for static functions, but OK there also.)
-7) All new ``Kconfig`` options have help text.
+2) All new ``/proc`` entries are documented under ``Documentation/``
-8) Has been carefully reviewed with respect to relevant ``Kconfig``
- combinations. This is very hard to get right with testing -- brainpower
- pays off here.
+3) All new kernel boot parameters are documented in
+ ``Documentation/admin-guide/kernel-parameters.rst``.
+
+4) All new module parameters are documented with ``MODULE_PARM_DESC()``
-9) Check cleanly with sparse.
+5) All new userspace interfaces are documented in ``Documentation/ABI/``.
+ See ``Documentation/ABI/README`` for more information.
+ Patches that change userspace interfaces should be CCed to
+ [email protected].
-10) Use ``make checkstack`` and fix any problems that it finds.
+6) If any ioctl's are added by the patch, then also update
+ ``Documentation/userspace-api/ioctl/ioctl-number.rst``.
- .. note::
- ``checkstack`` does not point out problems explicitly,
- but any one function that uses more than 512 bytes on the stack is a
- candidate for change.
+*Check your code with tools:*
-11) Include :ref:`kernel-doc <kernel_doc>` to document global kernel APIs.
- (Not required for static functions, but OK there also.) Use
- ``make htmldocs`` or ``make pdfdocs`` to check the
- :ref:`kernel-doc <kernel_doc>` and fix any issues.
+1) Check for trivial violations with the patch style checker prior to
+ submission (``scripts/checkpatch.pl``).
+ You should be able to justify all violations that remain in
+ your patch.
-12) Has been tested with ``CONFIG_PREEMPT``, ``CONFIG_DEBUG_PREEMPT``,
- ``CONFIG_DEBUG_SLAB``, ``CONFIG_DEBUG_PAGEALLOC``, ``CONFIG_DEBUG_MUTEXES``,
- ``CONFIG_DEBUG_SPINLOCK``, ``CONFIG_DEBUG_ATOMIC_SLEEP``,
- ``CONFIG_PROVE_RCU`` and ``CONFIG_DEBUG_OBJECTS_RCU_HEAD`` all
- simultaneously enabled.
+2) Check cleanly with sparse.
-13) Has been build- and runtime tested with and without ``CONFIG_SMP`` and
- ``CONFIG_PREEMPT.``
+3) Use ``make checkstack`` and fix any problems that it finds.
+ Note that ``checkstack`` does not point out problems explicitly,
+ but any one function that uses more than 512 bytes on the stack is a
+ candidate for change.
-14) All codepaths have been exercised with all lockdep features enabled.
-15) All new ``/proc`` entries are documented under ``Documentation/``
+*Build your code:*
+
+1) Builds cleanly:
+
+ a) with applicable or modified ``CONFIG`` options ``=y``, ``=m``, and
+ ``=n``. No ``gcc`` warnings/errors, no linker warnings/errors.
+
+ b) Passes ``allnoconfig``, ``allmodconfig``
+
+ c) Builds successfully when using ``O=builddir``
+
+ d) Any Documentation/ changes build successfully without new warnings/errors.
+ Use ``make htmldocs`` or ``make pdfdocs`` to check the build and
+ fix any issues.
-16) All new kernel boot parameters are documented in
- ``Documentation/admin-guide/kernel-parameters.rst``.
+2) Builds on multiple CPU architectures by using local cross-compile tools
+ or some other build farm. Note that ppc64 is a good architecture for
+ cross-compilation checking because it tends to use ``unsigned long`` for
+ 64-bit quantities.
-17) All new module parameters are documented with ``MODULE_PARM_DESC()``
+3) Newly-added code has been compiled with ``gcc -W`` (use
+ ``make KCFLAGS=-W``). This will generate lots of noise, but is good
+ for finding bugs like "warning: comparison between signed and unsigned".
-18) All new userspace interfaces are documented in ``Documentation/ABI/``.
- See ``Documentation/ABI/README`` for more information.
- Patches that change userspace interfaces should be CCed to
- [email protected].
+4) If your modified source code depends on or uses any of the kernel
+ APIs or features that are related to the following ``Kconfig`` symbols,
+ then test multiple builds with the related ``Kconfig`` symbols disabled
+ and/or ``=m`` (if that option is available) [not all of these at the
+ same time, just various/random combinations of them]:
-19) Has been checked with injection of at least slab and page-allocation
- failures. See ``Documentation/fault-injection/``.
+ ``CONFIG_SMP``, ``CONFIG_SYSFS``, ``CONFIG_PROC_FS``, ``CONFIG_INPUT``,
+ ``CONFIG_PCI``, ``CONFIG_BLOCK``, ``CONFIG_PM``, ``CONFIG_MAGIC_SYSRQ``,
+ ``CONFIG_NET``, ``CONFIG_INET=n`` (but latter with ``CONFIG_NET=y``).
- If the new code is substantial, addition of subsystem-specific fault
- injection might be appropriate.
-20) Newly-added code has been compiled with ``gcc -W`` (use
- ``make KCFLAGS=-W``). This will generate lots of noise, but is good
- for finding bugs like "warning: comparison between signed and unsigned".
+*Test your code:*
-21) Tested after it has been merged into the -mm patchset to make sure
- that it still works with all of the other queued patches and various
- changes in the VM, VFS, and other subsystems.
+1) Has been tested with ``CONFIG_PREEMPT``, ``CONFIG_DEBUG_PREEMPT``,
+ ``CONFIG_SLUB_DEBUG``, ``CONFIG_DEBUG_PAGEALLOC``, ``CONFIG_DEBUG_MUTEXES``,
+ ``CONFIG_DEBUG_SPINLOCK``, ``CONFIG_DEBUG_ATOMIC_SLEEP``,
+ ``CONFIG_PROVE_RCU`` and ``CONFIG_DEBUG_OBJECTS_RCU_HEAD`` all
+ simultaneously enabled.
-22) All memory barriers {e.g., ``barrier()``, ``rmb()``, ``wmb()``} need a
- comment in the source code that explains the logic of what they are doing
- and why.
+2) Has been build- and runtime tested with and without ``CONFIG_SMP`` and
+ ``CONFIG_PREEMPT.``
-23) If any ioctl's are added by the patch, then also update
- ``Documentation/userspace-api/ioctl/ioctl-number.rst``.
+3) All codepaths have been exercised with all lockdep features enabled.
-24) If your modified source code depends on or uses any of the kernel
- APIs or features that are related to the following ``Kconfig`` symbols,
- then test multiple builds with the related ``Kconfig`` symbols disabled
- and/or ``=m`` (if that option is available) [not all of these at the
- same time, just various/random combinations of them]:
+4) Has been checked with injection of at least slab and page-allocation
+ failures. See ``Documentation/fault-injection/``.
+ If the new code is substantial, addition of subsystem-specific fault
+ injection might be appropriate.
- ``CONFIG_SMP``, ``CONFIG_SYSFS``, ``CONFIG_PROC_FS``, ``CONFIG_INPUT``, ``CONFIG_PCI``, ``CONFIG_BLOCK``, ``CONFIG_PM``, ``CONFIG_MAGIC_SYSRQ``,
- ``CONFIG_NET``, ``CONFIG_INET=n`` (but latter with ``CONFIG_NET=y``).
+5) Tested with the most recent tag of linux-next to make sure that it still
+ works with all of the other queued patches and various changes in the VM,
+ VFS, and other subsystems.
--
2.43.2
On Mon, 26 Feb 2024, Lukas Bulwahn <[email protected]> wrote:
> diff --git a/Documentation/process/submit-checklist.rst b/Documentation/process/submit-checklist.rst
> index b1bc2d37bd0a..7d8dba942fe8 100644
> --- a/Documentation/process/submit-checklist.rst
> +++ b/Documentation/process/submit-checklist.rst
> @@ -11,110 +11,121 @@ These are all above and beyond the documentation that is provided in
> and elsewhere regarding submitting Linux kernel patches.
>
>
> +*Review your code:*
If you're adding subheadings, maybe consider making them actual
subheadings instead of just italicizing them.
The top heading should probably be modified to follow the guidelines in
Documentation/doc-guide/sphinx.rst. This should be a separate change.
> +
> 1) If you use a facility then #include the file that defines/declares
> that facility. Don't depend on other header files pulling in ones
> that you use.
>
> -2) Builds cleanly:
> +2) Check your patch for general style as detailed in
> + :ref:`Documentation/process/coding-style.rst <codingstyle>`.
>
> - a) with applicable or modified ``CONFIG`` options ``=y``, ``=m``, and
> - ``=n``. No ``gcc`` warnings/errors, no linker warnings/errors.
> +3) All memory barriers {e.g., ``barrier()``, ``rmb()``, ``wmb()``} need a
> + comment in the source code that explains the logic of what they are doing
> + and why.
I think we should just remove all the manually updated bullet
numbering. Either make them bulleted lists with "*" or autonumbered
lists with "#.". See [1]. This should be a separate change.
BR,
Jani.
[1] https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#lists-and-quote-like-blocks
--
Jani Nikula, Intel
Hi Lukas,
I'll review the file changes separately. This is just replying
to the patch description comments.
On 2/26/24 02:46, Lukas Bulwahn wrote:
> While going through the submit checklist, the list order seemed rather
> random, probably just by historical coincidences of always adding yet the
> next point someone thought of at the end of the list.
Probably.
> Structure and order them by the category of such activity,
> reviewing, documenting, checking with tools, building and testing.
>
> As the diff of the reordering is large:
> Review code now includes previous points 1, 5 and 22.
> Review Kconfig includes previous 6, 7 and 8.
> Documenting includes previous 11, 15, 16, 17, 18 and 23.
> Checking with tools includes previous 5, 9 and 10.
> Building includes previous 2, 3, 20 and 24.
> Testing includes previous 12, 13, 14, 19 and 21.
>
..
>
> The recommendation to test with the -mm patchset (previous 21, now
> testing, point 5) was updated to the current state of affairs to test with
> a recent tag of linux-next.
ack.
> Note that the previous first point still remains the first list even after
> reordering. Based on some vague memory, the first point was important to
> Randy to stay the first one in any reordering.
Yes, I have said that. Stephen Rothwell wanted it to be first in the list.
> While at it, the reference to CONFIG_SLUB_DEBUG was replaced by
> CONFIG_DEBUG_SLAB.
I don't understand this comment. DEBUG_SLAB is gone.
I think those 2 symbols might be reversed in your comments. ?
> Signed-off-by: Lukas Bulwahn <[email protected]>
> ---
> So far, no point disappeared and nothing new was added.
>
That's a good start IMO.
> Points/Ideas for further improvements (based on my knowledge and judgement):
>
> - The Review Kconfig changes makes sense, but I am not sure if they are
> so central during review. If we keep it, let us see if there are other
> parts for review that are also important similar to Kconfig changes.
>
> - Concerning checking with tools, checkpatch probably still makes sense;
> it pointed out in several places. If sparse and checkstack are really
> the next two tools to point out, I am not so sure about.
I doubt that ckeckstack is important since gcc & clang warn us about
stack usage.
> sparse has a lot of false positives nowadays, and many things are not
> fixed just because sparse complains about it.
> And I have never used make checkstack and have not found much
> documentation about it.
> So, maybe other tools deserve to be mentioned here instead?
>
> I am happy to get feedback---I will work through submitting-patches next
> and do some clean-up there. While doing that, I might learn what really
> should go into a better future 'submit-checklist' documentation.
>
> Documentation/process/submit-checklist.rst | 157 +++++++++++----------
> 1 file changed, 84 insertions(+), 73 deletions(-)
--
#Randy
Hi Jani,
On Mon, Feb 26, 2024 at 1:48 PM Jani Nikula <[email protected]> wrote:
>
> On Mon, 26 Feb 2024, Lukas Bulwahn <[email protected]> wrote:
> > diff --git a/Documentation/process/submit-checklist.rst b/Documentation/process/submit-checklist.rst
> > index b1bc2d37bd0a..7d8dba942fe8 100644
> > --- a/Documentation/process/submit-checklist.rst
> > +++ b/Documentation/process/submit-checklist.rst
> > @@ -11,110 +11,121 @@ These are all above and beyond the documentation that is provided in
> > and elsewhere regarding submitting Linux kernel patches.
> >
> >
> > +*Review your code:*
>
> If you're adding subheadings, maybe consider making them actual
> subheadings instead of just italicizing them.
>
> The top heading should probably be modified to follow the guidelines in
> Documentation/doc-guide/sphinx.rst. This should be a separate change.
>
I have done that. In my humble personal opinion, at the moment, the
subheadings look a bit too large in the HTML view compared to the few
points below.
However, I am planning to add more points to the checklist anyway when
I understand and have summarized the essence of the other documents
for patch submissions (submitting-patches and howto).
So, let us make them subheadings.
> > +
> > 1) If you use a facility then #include the file that defines/declares
> > that facility. Don't depend on other header files pulling in ones
> > that you use.
> >
> > -2) Builds cleanly:
> > +2) Check your patch for general style as detailed in
> > + :ref:`Documentation/process/coding-style.rst <codingstyle>`.
> >
> > - a) with applicable or modified ``CONFIG`` options ``=y``, ``=m``, and
> > - ``=n``. No ``gcc`` warnings/errors, no linker warnings/errors.
> > +3) All memory barriers {e.g., ``barrier()``, ``rmb()``, ``wmb()``} need a
> > + comment in the source code that explains the logic of what they are doing
> > + and why.
>
> I think we should just remove all the manually updated bullet
> numbering. Either make them bulleted lists with "*" or autonumbered
> lists with "#.". See [1]. This should be a separate change.
>
Done that. I used "#." to still have the numbering in place.
The two changes are straightforward, and I will send them out as a v2
series, once Randy has had time to provide his feedback on the content
of the v1 patch and I have included his review remarks.
Lukas
On Tue, Feb 27, 2024 at 1:41 AM Randy Dunlap <[email protected]> wrote:
>
> Hi Lukas,
>
> I'll review the file changes separately. This is just replying
> to the patch description comments.
>
>
> On 2/26/24 02:46, Lukas Bulwahn wrote:
(snipped)
>
> > Note that the previous first point still remains the first list even after
> > reordering. Based on some vague memory, the first point was important to
> > Randy to stay the first one in any reordering.
>
> Yes, I have said that. Stephen Rothwell wanted it to be first in the list.
>
I have adjusted my commit message:
Note that the previous first point still remains the first list even after
reordering. Randy confirmed that it was important to Stephen Rothwell to
keep 'include what you use' to be the first in the list.
>
> > While at it, the reference to CONFIG_SLUB_DEBUG was replaced by
> > CONFIG_DEBUG_SLAB.
>
> I don't understand this comment. DEBUG_SLAB is gone.
> I think those 2 symbols might be reversed in your comments. ?
>
I must have mixed them up while writing down the commit message; so
now it reads:
While at it, replace the reference to the obsolete CONFIG_DEBUG_SLAB with
CONFIG_SLUB_DEBUG.
That is what is happening in the file.
>
> > Signed-off-by: Lukas Bulwahn <[email protected]>
> > ---
> > So far, no point disappeared and nothing new was added.
> >
>
> That's a good start IMO.
>
> > Points/Ideas for further improvements (based on my knowledge and judgement):
> >
> > - The Review Kconfig changes makes sense, but I am not sure if they are
> > so central during review. If we keep it, let us see if there are other
> > parts for review that are also important similar to Kconfig changes.
> >
> > - Concerning checking with tools, checkpatch probably still makes sense;
> > it pointed out in several places. If sparse and checkstack are really
> > the next two tools to point out, I am not so sure about.
>
> I doubt that ckeckstack is important since gcc & clang warn us about
> stack usage.
>
So, I might drop this later on and replace it with something more
important to ask.
I have put it on my todo list (but others are welcome as well to pick it up):
KTODO: Investigate if the make checkstack tool is really obsolete, as
gcc and clang are already set up to warn about large stack usage just
as well as make checkstack does.
Present how it was investigated and which kind of "benchmark" was set
up and how the results were evaluated. If make checkstack is really
obsolete, create a patch to remove the tool from the repository, and
add some documentation to explain how kernel developers can check for
large stack usage.
> > sparse has a lot of false positives nowadays, and many things are not
> > fixed just because sparse complains about it.
> > And I have never used make checkstack and have not found much
> > documentation about it.
> > So, maybe other tools deserve to be mentioned here instead?
> >
If make checkstack is removed from the list, this might give rise to
another linting/static analysis tool worth mentioning. The candidates
that come to my mind are clang-tidy or smatch. I need to check,
though, if the installation guides for those tools and the setup for
the kernel sources are clear enough to actually promote running these
tools.
But maybe there is another tool worth mentioning. I know about the
coverity setup, but this is not really suitable for checking
individual patches.
Randy, I will wait for your review and feedback on the file changes
and then send out a v2 patch. So far, the changes are only changes to
the commit message.
Lukas
Hi Randy,
On Tue, Feb 27, 2024 at 1:41 AM Randy Dunlap <[email protected]> wrote:
> > - Concerning checking with tools, checkpatch probably still makes sense;
> > it pointed out in several places. If sparse and checkstack are really
> > the next two tools to point out, I am not so sure about.
>
> I doubt that ckeckstack is important since gcc & clang warn us about
> stack usage.
True, but that would leave you without a tool to get figures when
there is no excess stack usage detected by the compiler.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Tue, Feb 27, 2024 at 9:57 AM Geert Uytterhoeven <[email protected]> wrote:
>
> Hi Randy,
>
> On Tue, Feb 27, 2024 at 1:41 AM Randy Dunlap <[email protected]> wrote:
> > > - Concerning checking with tools, checkpatch probably still makes sense;
> > > it pointed out in several places. If sparse and checkstack are really
> > > the next two tools to point out, I am not so sure about.
> >
> > I doubt that ckeckstack is important since gcc & clang warn us about
> > stack usage.
>
> True, but that would leave you without a tool to get figures when
> there is no excess stack usage detected by the compiler.
>
Geert,
possibly, we can configure the compiler to report/warn on any stack
usage from every invocation and then turn all those warnings into a
readable format or some format that further visualization and analysis
tools can process.
If that works, we can remove the checkstack tool. It is not a
massively large script, but it is certainly written with a very
special purpose. I mean it basically does object-code
reverse-engineering with a magic set of regular expressions in Perl.
If our current compilers can emit the same information, we are
probably better off just using the output from a compiler and
postprocessing that.
Anyways, I think it is worth investigating all that and then see if
the checkstack.pl tool still has a unique functionality, or if there
are other better ways to get this kind of information---well, it is
marked as todo, so anyone is free to pick it up.
Lukas
Lukas
Hi Lukas,
On Tue, Feb 27, 2024 at 12:04 PM Lukas Bulwahn <lukas.bulwahn@gmailcom> wrote:
> On Tue, Feb 27, 2024 at 9:57 AM Geert Uytterhoeven <[email protected]> wrote:
> > On Tue, Feb 27, 2024 at 1:41 AM Randy Dunlap <rdunlap@infradeadorg> wrote:
> > > > - Concerning checking with tools, checkpatch probably still makes sense;
> > > > it pointed out in several places. If sparse and checkstack are really
> > > > the next two tools to point out, I am not so sure about.
> > >
> > > I doubt that ckeckstack is important since gcc & clang warn us about
> > > stack usage.
> >
> > True, but that would leave you without a tool to get figures when
> > there is no excess stack usage detected by the compiler.
>
> possibly, we can configure the compiler to report/warn on any stack
> usage from every invocation and then turn all those warnings into a
> readable format or some format that further visualization and analysis
> tools can process.
"possibly"
> If that works, we can remove the checkstack tool. It is not a
> massively large script, but it is certainly written with a very
> special purpose. I mean it basically does object-code
> reverse-engineering with a magic set of regular expressions in Perl.
> If our current compilers can emit the same information, we are
> probably better off just using the output from a compiler and
> postprocessing that.
I'm fully aware how it works.
And I have used Linux' checkstack.pl tool for non-Linux projects, too.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Lukas Bulwahn <[email protected]> writes:
> While going through the submit checklist, the list order seemed rather
> random, probably just by historical coincidences of always adding yet the
> next point someone thought of at the end of the list.
>
> Structure and order them by the category of such activity,
> reviewing, documenting, checking with tools, building and testing.
So this is clearly a step in the right direction, so I'm not opposed to
it. But I do have a couple of thoughts:
- This document is old and unloved. Its age shows in a lot of ways
(wmb() rather than the sorts of barriers that are socially acceptable
in 2024, for example). It makes no mention of the CI systems that
should get their say for a lot of subsystems; nor does it mention the
subsystem-specific maintainer profiles that should also be
consulted. And so on. It needs a lot of work rather than a
reshuffling. (But, as I say, the reshuffling is an improvement, so
I'll take it).
- It's a bit of an awkward fit with submitting-patches.rst. Someday
we'll have a set of coherent docs, maybe.
Anyway, I'm done grumbling now...:) I'll look forward to v2 -
preferably soon; I have travel coming up and may need to cut things off
for 6.9 a bit earlier than usual.
Thanks,
jon
Hi Lukas,
Sorry about the (my) delay.
On 2/26/24 02:46, Lukas Bulwahn wrote:
>
> Documentation/process/submit-checklist.rst | 157 +++++++++++----------
> 1 file changed, 84 insertions(+), 73 deletions(-)
>
> diff --git a/Documentation/process/submit-checklist.rst b/Documentation/process/submit-checklist.rst
> index b1bc2d37bd0a..7d8dba942fe8 100644
> --- a/Documentation/process/submit-checklist.rst
> +++ b/Documentation/process/submit-checklist.rst
> @@ -11,110 +11,121 @@ These are all above and beyond the documentation that is provided in
> and elsewhere regarding submitting Linux kernel patches.
>
>
> +*Review your code:*
These "headings" (?) shouldn't have ending ':'s IMO.
Maybe they should be real headings?
Otherwise the patch is a very good & welcome improvement, although as
Jon said, the file needs some TLC. (after this patch is applied)
Reviewed-by: Randy Dunlap <[email protected]>
Tested-by: Randy Dunlap <[email protected]>
Thanks.
--
#Randy