[patch 0/2] Documentation/process: Add subsystem/tree handbook

[patch 0/2] Documentation/process: Add subsystem/tree handbook

[Thomas Gleixner]

Mark recently suggested in one of the ksummit discussions to add subsystem
or tree specific maintainer handbooks to document subsystem/tree specific
development process information.

The following series adds the general section and the tip tree specific
handbook.

Thanks,

	tglx

[patch 1/2] Documentation/process: Add maintainer handbooks section

[Thomas Gleixner]

[-- Attachment #1: Documentation-process--Add-maintainer-handbooks-section.patch --]
[-- Type: text/plain, Size: 2189 bytes --]

General rules for patch submission, coding style and related details are
available, but most subsystems have their sub-system specific extra rules
which differ or go beyond the common rules.

Mark suggested to add a subsystem/maintainer handbook section, where
subsystem maintainers can explain their specific quirks.

Add the section and link to it from the submitting-patches document.

Suggested-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 Documentation/process/index.rst                |    1 +
 Documentation/process/maintainer-handbooks.rst |   14 ++++++++++++++
 Documentation/process/submitting-patches.rst   |    4 ++++
 3 files changed, 19 insertions(+)

--- a/Documentation/process/index.rst
+++ b/Documentation/process/index.rst
@@ -27,6 +27,7 @@ Below are the essential guides that ever
    submitting-patches
    programming-language
    coding-style
+   maintainer-handbooks
    maintainer-pgp-guide
    email-clients
    kernel-enforcement-statement
--- /dev/null
+++ b/Documentation/process/maintainer-handbooks.rst
@@ -0,0 +1,14 @@
+.. _maintainer_handbooks_main:
+
+Subsystem and maintainer tree specific development process notes
+================================================================
+
+The purpose of this document is to provide subsystem specific information
+which is supplementary to the general development process handbook
+:ref:`Documentation/process <development_process_main>`.
+
+Contents:
+
+.. toctree::
+   :numbered:
+   :maxdepth: 2
--- a/Documentation/process/submitting-patches.rst
+++ b/Documentation/process/submitting-patches.rst
@@ -18,6 +18,10 @@ submitting code.  If you are submitting
 for device tree binding patches, read
 Documentation/devicetree/bindings/submitting-patches.txt.
 
+Some subsystems and maintainer trees have additional information about
+their workflow and expectations, see :ref:`Documentation/process
+<maintainer_handbooks_main>`.
+
 Many of these steps describe the default behavior of the ``git`` version
 control system; if you use ``git`` to prepare your patches, you'll find much
 of the mechanical work done for you, though you'll still need to prepare

[patch 2/2] Documentation/process: Add tip tree handbook

[Thomas Gleixner]

[-- Attachment #1: Documentation-process--Add-tip-tree-handbook.patch --]
[-- Type: text/plain, Size: 27746 bytes --]

Add a document to the subsystem/maintainer handbook section, which explains
what the tip tree is, how it operates and what rules and expectations it
has.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 Documentation/process/maintainer-handbooks.rst |    2 
 Documentation/process/maintainer-tip.rst       |  796 +++++++++++++++++++++++++
 2 files changed, 798 insertions(+)

--- a/Documentation/process/maintainer-handbooks.rst
+++ b/Documentation/process/maintainer-handbooks.rst
@@ -12,3 +12,5 @@ which is supplementary to the general de
 .. toctree::
    :numbered:
    :maxdepth: 2
+
+   maintainer-tip
--- /dev/null
+++ b/Documentation/process/maintainer-tip.rst
@@ -0,0 +1,796 @@
+The tip tree handbook
+=====================
+
+What is the tip tree?
+---------------------
+
+The tip tree is a collection of several subsystems and areas of
+development. The tip tree is both a direct development tree and a
+aggregation tree for several sub-maintainer trees. The tip tree gitweb URL
+is: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
+
+The tip tree contains the following subsystems:
+
+   - **x86 architecture**
+
+     The x86 architecture development takes place in the tip tree except
+     for the x86 KVM and XEN specific parts which are maintained in the
+     corresponding subsystems and routed directly to mainline from
+     there. It's still good practice to Cc the x86 maintainers on
+     x86-specific KVM and XEN patches.
+
+     Some x86 subsystems have their own maintainers in addition to the
+     overall x86 maintainers.  Please Cc the overall x86 maintainers on
+     patches touching files in arch/x86 even when they are not called out
+     by the MAINTAINER file.
+
+     Note, that ``x86@kernel.org`` is not a mailing list. It is merely a
+     mail alias which distributes mails to the x86 top-level maintainer
+     team. Please always Cc the Linux Kernel mailing list (LKML)
+     ``linux-kernel@vger.kernel.org``, otherwise your mail ends up only in
+     the private inboxes of the maintainers.
+
+   - **Scheduler**
+
+   - **Locking and atomics**
+
+   - **Generic interrupt subsystem and interrupt chip drivers**:
+
+     - interrupt core development happens in the irq/core branch
+
+     - interrupt chip driver development happens also in the irq/core
+       branch, but the patches are mostly applied in a separate maintainer
+       tree and then aggregated into irq/core
+
+   - **Time, timers, timekeeping, NOHZ and related chip drivers**:
+
+     - timekeeping, clocksource core, NTP and alarmtimer development
+       happens in the timers/core branch, but patches are mostly applied in
+       a separate maintainer tree and then aggregated into timers/core
+
+     - clocksource/event driver development happens in the timers/core
+       branch, but patches are mostly applied in a separate maintainer tree
+       and then aggregated into timers/core
+
+   - **Performance counters core, architecture support and tooling**:
+
+     - perf core and architecture support development happens in the
+       perf/core branch
+
+     - perf tooling development happens in the perf tools maintainer
+       tree and is aggregated into the tip tree.
+
+   - **CPU hotplug core**
+
+   - **RAS core**
+
+   - **EFI core**
+
+     EFI development in the efi git tree. The collected patches are
+     aggregated in the tip efi/core branch.
+
+   - **RCU**
+
+     RCU development happens in the linux-rcu tree. The resulting changes
+     are aggregated into the tip core/rcu branch.
+
+   - **Various core code components**:
+
+       - debugobjects
+
+       - objtool
+
+       - random bits and pieces
+
+
+Patch submission notes
+----------------------
+
+Selecting the tree/branch
+^^^^^^^^^^^^^^^^^^^^^^^^^
+
+In general, development against the head of the tip tree master branch is
+fine, but for the subsystems which are maintained separately, have their
+own git tree and are only aggregated into the tip tree, development should
+take place against the relevant subsystem tree or branch.
+
+Bug fixes which target mainline should always be applicable against the
+mainline kernel tree. Potential conflicts against changes which are already
+queued in the tip tree are handled by the maintainers.
+
+Patch subject
+^^^^^^^^^^^^^
+
+The tip tree preferred format for patch subject prefixes is
+'subsys/component:', e.g. 'x86/apic:', 'x86/mm/fault:', 'sched/fair:',
+'genirq/core:'. Please do not use file names or complete file paths as
+prefix. 'git log path/to/file' should give you a reasonable hint in most
+cases.
+
+The condensed patch description in the subject line should start with a
+uppercase letter and should be written in imperative tone.
+
+
+Changelog
+^^^^^^^^^
+
+The general rules about changelogs in the process documentation, see
+:ref:`Documentation/process/ <submittingpatches>`, apply.
+
+The tip tree maintainers set value on following these rules, especially on
+the request to write changelogs in imperative mood and not impersonating
+code or the execution of it. This is not just a whim of the
+maintainers. Changelogs written in abstract words are more precise and
+tend to be less confusing than those written in the form of novels.
+
+It's also useful to structure the changelog into several paragraphs and not
+lump everything together into a single one. A good structure is to explain
+the context, the problem and the solution in separate paragraphs and this
+order.
+
+Examples for illustration:
+
+  Example 1::
+
+    x86/intel_rdt/mbm: Fix MBM overflow handler during hot cpu
+
+    When a CPU is dying, we cancel the worker and schedule a new worker on a
+    different CPU on the same domain. But if the timer is already about to
+    expire (say 0.99s) then we essentially double the interval.
+
+    We modify the hot cpu handling to cancel the delayed work on the dying
+    cpu and run the worker immediately on a different cpu in same domain. We
+    donot flush the worker because the MBM overflow worker reschedules the
+    worker on same CPU and scans the domain->cpu_mask to get the domain
+    pointer.
+
+  Improved version::
+
+    x86/intel_rdt/mbm: Fix MBM overflow handler during CPU hotplug
+
+    When a CPU is dying, the overflow worker is canceled and rescheduled on a
+    different CPU in the same domain. But if the timer is already about to
+    expire this essentially doubles the interval which might result in a non
+    detected overflow.
+
+    Cancel the overflow worker and reschedule it immediately on a different CPU
+    in the same domain. The work could be flushed as well, but that would
+    reschedule it on the same CPU.
+
+  Example 2::
+
+    time: POSIX CPU timers: Ensure that variable is initialized
+
+    If cpu_timer_sample_group returns -EINVAL, it will not have written into
+    *sample. Checking for cpu_timer_sample_group's return value precludes the
+    potential use of an uninitialized value of now in the following block.
+    Given an invalid clock_idx, the previous code could otherwise overwrite
+    *oldval in an undefined manner. This is now prevented. We also exploit
+    short-circuiting of && to sample the timer only if the result will
+    actually be used to update *oldval.
+
+  Improved version::
+
+    posix-cpu-timers: Make set_process_cpu_timer() more robust
+
+    Because the return value of cpu_timer_sample_group() is not checked,
+    compilers and static checkers can legitimately warn about a potential use
+    of the uninitialized variable 'now'. This is not a runtime issue as all
+    call sites hand in valid clock ids.
+
+    Also cpu_timer_sample_group() is invoked unconditionally even when the
+    result is not used because *oldval is NULL.
+
+    Make the invocation conditional and check the return value.
+
+  Example 3::
+
+    The entity can also be used for other purposes.
+
+    Let's rename it to be more generic.
+
+  Improved version::
+
+    The entity can also be used for other purposes.
+
+    Rename it to be more generic.
+
+
+For complex scenarios, especially race conditions and memory ordering
+issues, it is valuable to depict the scenario with a table which shows
+the parallelism and the temporal order of events. Here is an example::
+
+    CPU0                            CPU1
+    free_irq(X)                     interrupt X
+                                    spin_lock(desc->lock)
+                                    wake irq thread()
+                                    spin_unlock(desc->lock)
+    spin_lock(desc->lock)
+    remove action()
+    shutdown_irq()
+    release_resources()             thread_handler()
+    spin_unlock(desc->lock)           access released resources.
+                                      ^^^^^^^^^^^^^^^^^^^^^^^^^
+    synchronize_irq()
+
+Lockdep provides similar useful output to depict a possible deadlock
+scenario::
+
+    CPU0                                    CPU1
+    rtmutex_lock(&rcu->rt_mutex)
+      spin_lock(&rcu->rt_mutex.wait_lock)
+                                            local_irq_disable()
+                                            spin_lock(&timer->it_lock)
+                                            spin_lock(&rcu->mutex.wait_lock)
+    --> Interrupt
+        spin_lock(&timer->it_lock)
+
+
+Function references in changelogs
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+When a function is mentioned in the changelog, either the text body or the
+subject line, please use the format 'function_name()'. Omitting the
+brackets after the function name can be ambiguous::
+
+  Subject: subsys/component: Make reservation_count static
+
+  reservation_count is only used in reservation_stats. Make it static.
+
+The variant with brackets is more precise::
+
+  Subject: subsys/component: Make reservation_count() static
+
+  reservation_count() is only called from reservation_stats(). Make it
+  static.
+
+
+Backtraces in changelogs
+^^^^^^^^^^^^^^^^^^^^^^^^
+
+Backtraces can be useful to document the call chain which led to a
+problem. Though not all back traces are really valuable because the call
+chain is unique and obvious, e.g. in early boot code. Just copying the full
+dmesg output is adding a lot of distracting information like timestamps,
+module lists, register and stack dumps etc.
+
+Reducing the backtrace to the real relevant data helps to concentrate on
+the issue and not being distracted by destilling the relevant information
+out of the dump. Here is an example of a well trimmed backtrace::
+
+  unchecked MSR access error: WRMSR to 0xd51 (tried to write 0x0000
+  000000000064) at rIP: 0xffffffffae059994 (native_write_msr+0x4/0x20)
+  Call Trace:
+  mba_wrmsr+0x41/0x80
+  update_domains+0x125/0x130
+  rdtgroup_mkdir+0x270/0x500
+
+
+Ordering of commit tags
+^^^^^^^^^^^^^^^^^^^^^^^
+
+To have a uniform view of the commit tags, the tip maintainers use the
+following tag ordering scheme:
+
+ - Fixes: 12char-SHA1 ("sub/sys: Original subject line")
+
+   A Fixes tag should be added even for changes which do not need to be
+   backported to stable kernels, i.e. when addressing a recently introduced
+   issue which only affects tip or the current head of mainline. These tags
+   are helpful to identify the original commit and are much more valuable
+   than prominently mentioning the commit which introduced a problem in the
+   text of the changelog itself because they can be automatically
+   extracted.
+
+   The following example illustrates the difference::
+
+     Commit
+
+       abcdef012345678 ("x86/xxx: Replace foo with bar")
+
+     left an unused instance of variable foo around. Remove it.
+
+     Signed-off-by: J.Dev <j.dev@mail>
+
+   Please say instead::
+
+     The recent replacement of foo with bar left an unused instance of
+     variable foo around. Remove it.
+
+     Fixes: abcdef012345678 ("x86/xxx: Replace foo with bar")
+     Signed-off-by: J.Dev <j.dev@mail>
+
+   The latter puts the information about the patch into the focus and
+   amends it with the reference to the commit which introduced the issue
+   rather than putting the focus on the original commit in the first place.
+
+ - Reported-by: ``Reporter <reporter@mail>``
+
+ - Originally-by: ``Original author <original-author@mail>``
+
+ - Suggested-by: ``Suggester <suggester@mail>``
+
+ - Co-developed-by: ``Co-author <co-author@mail>``
+
+   Signed-off: ``Co-author <co-author@mail>``
+
+   Note, that Co-developed-by and Signed-off-by of the co-author(s) must
+   come in pairs.
+
+ - Signed-off-by: ``Author <author@mail>``
+
+   The first Signed-off-by (SOB) after the last Co-developed-by/SOB pair is the
+   author SOB, i.e. the person flagged as author by git.
+
+ - Signed-off-by: ``Patch handler <handler@mail>``
+
+   SOBs after the author SOB are from people handling and transporting the
+   patch, but were not involved in development. If the handler made
+   modifications to the patch or the changelog, then this should be
+   mentioned **after** the changelog text and **above** all commit tags in
+   the following format::
+
+     ... changelog text ends.
+
+     [ handler: Replaced foo by bar and updated changelog ]
+
+     First-tag: .....
+
+   Note the two empty new lines which separate the changelog text and the
+   commit tags from that notice.
+
+   If a patch is sent to the mailing list by a handler then the author has
+   to be noted in the first line of the changelog with::
+
+     From: ``Author <author@mail>``
+
+     Changelog text starts here....
+
+   so the authorship is preserved. The 'From:' line has to be followed by a
+   empty newline. If that 'From:' line is missing, then the patch would be
+   attributed to the person who sent (transported) it. The 'From:' line is
+   automatically removed when the patch is applied and does not show up in
+   the final git changelog. It merely affects the authorship information of
+   the resulting git commit.
+
+ - Tested-by: ``Tester <tester@mail>``
+
+ - Reviewed-by: ``Reviewer <reviewer@mail>``
+
+ - Acked-by: ``Acker <acker@mail>``
+
+ - Cc: ``cc-ed-person <person@mail>``
+
+   If the patch should be backported to stable, then please add a '``Cc:
+   stable@vger.kernel.org``' tag, but do not Cc stable when sending your
+   mail.
+
+ - Link: ``https://link/to/information``
+
+   For referring to email on LKML or other kernel mailing lists, please use
+   the lkml.kernel.org redirector URL::
+
+     https://lkml.kernel.org/r/email-message@id
+
+   The kernel.org redirector is considered a stable URL unlike other email
+   archives.
+
+   Maintainers will add a Link tag referencing the email of the patch
+   submission when they apply a patch to the tip tree. This tag is useful
+   for later reference and is also used for commit notifications.
+
+Please do not use combined tags, e.g. Reported-and-tested-by, as they just
+complicate automated extraction of tags.
+
+
+Links to documentation
+^^^^^^^^^^^^^^^^^^^^^^
+
+Providing links to documentation in the changelog is a great help to later
+debugging and analysis.  Unfortunately, URLs often break very quickly
+because companies restructure their websites frequently.  Non-'volatile'
+exceptions include the Intel SDM and the AMD APM.
+
+Therefore, for 'volatile' documents, please create an entry in the kernel
+bugzilla https://bugzilla.kernel.org and attach a copy of these documents
+to the bugzilla entry. Finally, provide the URL of the bugzilla entry in
+the changelog.
+
+
+Patch version information
+^^^^^^^^^^^^^^^^^^^^^^^^^
+
+When sending a new modified version of a patch or a patch series please put
+a version number into the '[PATCH]' part of the subject line::
+
+  Subject: [PATCH V2] sub/sys: Condensed patch summary
+
+  Subject: [PATCH V2 M/N] sub/sys: Condensed patch summary
+
+It's good practice to include information about the changes between
+versions of patches into the cover letter and the changelogs of the
+individual patches.
+
+Please put this information **after** the '- - -' line which separates the
+changelog from the rest of the patch. The version information is not part
+of the changelog which gets committed to the git tree. It is additional
+information for the reviewers. If it's placed above the commit tags, it
+needs manual interaction to remove it. If it is below the separator line it
+gets automatically stripped off when applying the patch::
+
+  ...
+  Signed-off-by: Author <author@mail>
+  ---
+  V2 -> V3: Removed redundant helper function
+  V1 -> V2: Cleaned up coding style and addressed review comments
+
+  path/to/file | 5+++--
+  ...
+
+Patch resend or reminders
+^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The maintainers try to process patches in a timely manner, although it is
+not always possible to respond within a short time period.  Furthermore,
+from time to time patches can be lost or misplaced, so that a reminder or a
+resend is appropriate.
+
+Please wait at least a week or two before replying to your patch or cover
+letter with a gentle reminder on the mailing list.
+
+It's also ok to resend the patch or the patch series after a couple of
+weeks with the word 'RESEND' added to the subject line::
+
+   [PATCH Vx RESEND] sub/sys: Condensed patch summary
+
+Don't add RESEND when you are submitting a modified version of a patch or a
+patch series. RESEND only applies for resubmission of a non-modified patch
+or patch series.
+
+
+Merge window
+^^^^^^^^^^^^
+
+Please do not expect large patch series to be handled during the merge
+window or even during the week before.  Such patches should be submitted in
+mergeable state *at* *least* a week before the merge window opens.
+Exceptions are made for bug fixes and *sometimes* for small standalone
+drivers for new hardware or minimally invasive patches for hardware
+enablement.
+
+During the merge window, the maintainers instead focus on following the
+upstream changes, fixing merge window fallout, collecting bug fixes, and
+allowing themselves a breath. Please respect that.
+
+The release candidate -rc1 is the starting point for new patches to be
+applied which are targeted for the next merge window.
+
+
+Git
+^^^
+
+The tip maintainers accept git pull requests from maintainers who provide
+subsystem changes for aggregation in the tip tree.
+
+Pull requests for new patch submissions are usually not accepted and do not
+replace proper patch submission to the mailing list. The main reason for
+this is that the review workflow is email based.
+
+If you submit a larger patch series it is helpful to provide a git branch
+in a private repository which allows interested people to easily pull the
+series for testing. The usual way to offer this is a git URL in the cover
+letter of the patch series.
+
+
+Coding style notes
+------------------
+
+Comment style
+^^^^^^^^^^^^^
+
+Sentences in comments start with a uppercase letter.
+
+Single line comments::
+
+	/* This is a single line comment */
+
+Multi-line comments::
+
+	/*
+	 * This is a properly formatted
+	 * multi-line comment.
+	 *
+	 * Larger multi-line comments should be split into paragraphs.
+	 */
+
+No tail comments:
+
+  Please refrain from using tail comments. Tail comments disturb the
+  reading flow in almost all contexts, but especially in code::
+
+  	if (somecondition_is_true) /* Don't put a comment here */
+		dostuff(); /* Neither here */
+
+	seed = MAGIC_CONSTANT; /* Nor here */
+
+  Use freestanding comments instead::
+
+	/* This condition is not obvious without a comment */
+	if (somecondition_is_true) {
+		/* This really needs to be documented */
+		dostuff();
+	}
+
+	/* This magic initialization needs a comment. Maybe not? */
+	seed = MAGIC_CONSTANT;
+
+Comment the important things:
+
+  Comments should be added where the operation is not obvious. Documenting
+  the obvious is just a distraction::
+
+  	/* Decrement refcount and check for zero */
+	if (refcount_dec_and_test(&p->refcnt)) {
+		do;
+		lots;
+		of;
+		magic;
+		things;
+	}
+
+  Instead comments should explain the non-obvious details and document
+  constraints::
+
+	if (refcount_dec_and_test(&p->refcnt)) {
+		/*
+		 * Really good explanation why the magic things below
+		 * need to be done, ordering and locking constraints,
+		 * etc..
+		 */
+		do;
+		lots;
+		of;
+		magic;
+		/* Needs to be the last operation because ... */
+		things;
+	}
+
+Function documentation comments:
+
+  To document functions and their arguments please use kernel-doc format
+  and not free form comments::
+
+  	/**
+	 * magic_function - Do lots of magic stuff
+	 * @magic:	Pointer to the magic data to operate on
+	 * @offset:	Offset in the data array of @magic
+	 *
+	 * Deep explanation of mysterious things done with @magic along
+         * with documentation of the return values.
+	 *
+	 * Note, that the argument descriptors above are arranged
+	 * in a tabular fashion.
+	 */
+
+  This applies especially for globally visible functions and inline
+  functions in public header files. It might be overkill to use kernel-doc
+  format for every (static) function which needs a tiny explanation. The
+  usage of descriptive function names often replaces these tiny comments.
+  Apply common sense as always.
+
+
+Bracket rules
+^^^^^^^^^^^^^
+
+Brackets should be omitted only if the statement which follows 'if', 'for',
+'while' etc. is truly a single line::
+
+	if (foo)
+		do_something();
+
+The following is not considered to be a single line statement even
+though C does not require brackets::
+
+	for (i = 0; i < end; i++)
+		if (foo[i])
+			do_something(foo[i]);
+
+Adding brackets around the outer loop enhances the reading flow::
+
+	for (i = 0; i < end; i++) {
+		if (foo[i])
+			do_something(foo[i]);
+	}
+
+
+Variable declarations
+^^^^^^^^^^^^^^^^^^^^^
+
+The preferred ordering of variable declarations at the beginning of a
+function is reverse fir tree order::
+
+	struct long_struct_name *descriptive_name;
+	unsigned long foo, bar;
+	unsigned int tmp;
+	int ret;
+
+The above is faster to parse than the reverse ordering::
+
+	int ret;
+	unsigned int tmp;
+	unsigned long foo, bar;
+	struct long_struct_name *descriptive_name;
+
+And even more so than random ordering::
+
+	unsigned long foo, bar;
+	int ret;
+	struct long_struct_name *descriptive_name;
+	unsigned int tmp;
+
+Also please try to aggregate variables of the same type into a single
+line. There is no point in wasting screen space::
+
+	unsigned long a;
+	unsigned long b;
+	unsigned long c;
+	unsigned long d;
+
+It's really sufficient to do::
+
+	unsigned long a, b, c, d;
+
+Please also refrain from introducing line splits in variable declarations::
+
+	struct long_struct_name *descriptive_name = container_of(bar,
+						      struct long_struct_name,
+	                                              member);
+	struct foobar foo;
+
+It's way better to move the initialization to a separate line after the
+declarations::
+
+	struct long_struct_name *descriptive_name;
+	struct foobar foo;
+
+	descriptive_name = container_of(bar, struct long_struct_name, member);
+
+
+Variable types
+^^^^^^^^^^^^^^
+
+Please use the proper u8, u16, u32, u64 types for variables which are meant
+to describe hardware or are used as arguments for functions which access
+hardware. These types are clearly defining the bit width and avoid
+truncation, expansion and 32/64 bit confusion.
+
+u64 is also recommended in code which would become ambiguous for 32bit when
+'unsigned long' would be used instead. While in such situations 'unsigned
+long long' could be used as well, u64 is shorter and also clearly shows
+that the operation is required to be 64bit wide independent of the target
+CPU.
+
+Please use 'unsigned int' instead of 'unsigned'.
+
+
+Constants
+^^^^^^^^^
+
+Please do not use literal (hexa)decimal numbers in code or initializers.
+Either use proper defines which have descriptive names or consider using
+an enum.
+
+
+Struct declarations and initializers
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Struct declarations should align the struct member names in a tabular
+fashion::
+
+	struct bar_order {
+		unsigned int	guest_id;
+		int		ordered_item;
+		struct menu	*menu;
+	};
+
+Please avoid documenting struct members within the declaration, because
+this often results in strangely formatted comments and the struct members
+become obfuscated::
+
+ 	struct bar_order {
+  		unsigned int	guest_id; /* Unique guest id */
+		int		ordered_item;
+		/* Pointer to a menu instance which contains all the drinks */
+		struct menu	*menu;
+	};
+
+Instead, please consider using the kernel-doc format in a comment preceding
+the struct declaration, which is easier to read and has the added advantage
+of including the information in the kernel documentation, for example, as
+follows::
+
+
+	/**
+	 * struct bar_order - Description of a bar order
+	 * @guest_id:		Unique guest id
+	 * @ordered_item:	The item number from the menu
+	 * @menu:		Pointer to the menu from which the item
+	 *  			was ordered
+	 *
+	 * Supplementary information for using the struct.
+	 *
+	 * Note, that the struct member descriptors above are arranged
+	 * in a tabular fashion.
+	 */
+	struct bar_order {
+		unsigned int	guest_id;
+		int		ordered_item;
+		struct menu	*menu;
+	};
+
+Static struct initializers must use C99 initializers and should also be
+aligned in a tabular fashion::
+
+	static struct foo statfoo = {
+		.a		= 0,
+		.plain_integer	= CONSTANT_DEFINE_OR_ENUM,
+		.bar		= &statbar,
+	};
+
+
+Line breaks
+^^^^^^^^^^^
+
+Restricting line length to 80 characters makes deeply indented code hard to
+read.  Consider breaking out code into helper functions to avoid excessive
+line breaking.
+
+The 80 character rule is not a strict rule, so please use common sense when
+breaking lines. Especially format strings should never be broken up.
+
+When splitting function declarations or function calls, then please align
+the first argument in the second line with the first argument in the first
+line::
+
+  static int long_function_name(struct foobar *barfoo, unsigned int id,
+  				unsigned int offset)
+  {
+
+  	if (!id) {
+		ret = longer_function_name(barfoo, DEFAULT_BARFOO_ID,
+					   offset);
+	...
+
+Namespaces
+^^^^^^^^^^
+
+To improve readability and to allow easy grepping for information the usage
+of consistent namespaces is important. The namespace should be used as a
+prefix for globally visible (inline) functions and variables. A simple rule
+for chosing a namespace prefix is to combine the subsystem and the
+component name, e.g. 'x86_comp\_', 'sched\_', 'irq\_', 'mutex\_', etc. For
+static functions and variables the namespace prefix can be omitted.
+
+Also be careful when using vendor names in namespaces. There is no value of
+having 'xxx_vendor\_' or 'vendor_xxx_` as prefix for all static functions
+of a vendor specific file as it is already clear that the code is vendor
+specific. Aside of that vendor names should only be used when it is really
+vendor specific functionality.
+
+As always apply common sense and aim for consistency and readability.
+
+
+Commit notifications
+--------------------
+
+The tip tree is monitored by a bot for new commits. The bot sends an email
+for each new commit to a dedicated mailing list
+(``linux-tip-commits@vger.kernel.org``) and Cc's all people who are
+mentioned in one of the commit tags. It uses the email message id from the
+Link tag at the end of the tag list to set the In-Reply-To email header so
+the message is properly threaded with the patch submission email.
+
+The maintainers try to reply to the submitter when merging a patch, but
+they sometimes forget or it does not fit the workflow of the moment. While
+the bot message is purely mechanical assume it implies a 'Thank you!
+Applied.'.

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Thomas Gleixner]

On Wed, 7 Nov 2018, Thomas Gleixner wrote:

> Add a document to the subsystem/maintainer handbook section, which explains
> what the tip tree is, how it operates and what rules and expectations it
> has.

Peter asked me to add a section about locking comments. I added it and
forgot to refresh the patch before sending. Delta patch below.

Thanks,

	tglx
---
--- a/Documentation/process/maintainer-tip.rst
+++ b/Documentation/process/maintainer-tip.rst
@@ -578,6 +578,29 @@ Sentences in comments start with a upper
   usage of descriptive function names often replaces these tiny comments.
   Apply common sense as always.
 
+ 
+Documenting locking requirements
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  Documenting locking requirements is a good thing, but comments are not
+  necessarily the best choice. Instead of writing::
+
+    	/* Caller must hold foo->lock */
+	void func(struct foo *foo)
+	{
+		...
+	}
+
+  Please use::
+
+	void func(struct foo *foo)
+	{
+		lockdep_assert_held(&foo->lock);
+		...
+	}
+
+  The latter enables run time debugging when lockdep is enabled which
+  verifies that all callers hold the lock. Comments can't do that.
+
 
 Bracket rules
 ^^^^^^^^^^^^^

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Paul E. McKenney]

On Wed, Nov 07, 2018 at 06:10:12PM +0100, Thomas Gleixner wrote:
> Add a document to the subsystem/maintainer handbook section, which explains
> what the tip tree is, how it operates and what rules and expectations it
> has.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

A few more suggestions below, again to new text.  I have, admittedly
uncharacteristically, trimmed the patch.  ;-)

Reviewed-by: Paul E. McKenney <paulmck@linux.ibm.com>

> ---
>  Documentation/process/maintainer-handbooks.rst |    2 
>  Documentation/process/maintainer-tip.rst       |  796 +++++++++++++++++++++++++
>  2 files changed, 798 insertions(+)
> 
> --- a/Documentation/process/maintainer-handbooks.rst
> +++ b/Documentation/process/maintainer-handbooks.rst
> @@ -12,3 +12,5 @@ which is supplementary to the general de
>  .. toctree::
>     :numbered:
>     :maxdepth: 2
> +
> +   maintainer-tip
> --- /dev/null
> +++ b/Documentation/process/maintainer-tip.rst
> @@ -0,0 +1,796 @@
> +The tip tree handbook
> +=====================
> +
> +What is the tip tree?
> +---------------------
> +
> +The tip tree is a collection of several subsystems and areas of
> +development. The tip tree is both a direct development tree and a
> +aggregation tree for several sub-maintainer trees. The tip tree gitweb URL
> +is: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
> +
> +The tip tree contains the following subsystems:
> +
> +   - **x86 architecture**
> +
> +     The x86 architecture development takes place in the tip tree except
> +     for the x86 KVM and XEN specific parts which are maintained in the
> +     corresponding subsystems and routed directly to mainline from
> +     there. It's still good practice to Cc the x86 maintainers on
> +     x86-specific KVM and XEN patches.
> +
> +     Some x86 subsystems have their own maintainers in addition to the
> +     overall x86 maintainers.  Please Cc the overall x86 maintainers on
> +     patches touching files in arch/x86 even when they are not called out
> +     by the MAINTAINER file.
> +
> +     Note, that ``x86@kernel.org`` is not a mailing list. It is merely a
> +     mail alias which distributes mails to the x86 top-level maintainer
> +     team. Please always Cc the Linux Kernel mailing list (LKML)
> +     ``linux-kernel@vger.kernel.org``, otherwise your mail ends up only in
> +     the private inboxes of the maintainers.
> +
> +   - **Scheduler**
> +
> +   - **Locking and atomics**
> +
> +   - **Generic interrupt subsystem and interrupt chip drivers**:
> +
> +     - interrupt core development happens in the irq/core branch
> +
> +     - interrupt chip driver development happens also in the irq/core
> +       branch, but the patches are mostly applied in a separate maintainer

        - interrupt chip driver development also happens in the irq/core
          branch, but the patches are usually applied in a separate maintainer

> +       tree and then aggregated into irq/core
> +
> +   - **Time, timers, timekeeping, NOHZ and related chip drivers**:
> +
> +     - timekeeping, clocksource core, NTP and alarmtimer development
> +       happens in the timers/core branch, but patches are mostly applied in

          happens in the timers/core branch, but patches are usually applied in

> +       a separate maintainer tree and then aggregated into timers/core
> +
> +     - clocksource/event driver development happens in the timers/core
> +       branch, but patches are mostly applied in a separate maintainer tree
> +       and then aggregated into timers/core
> +
> +   - **Performance counters core, architecture support and tooling**:
> +
> +     - perf core and architecture support development happens in the
> +       perf/core branch
> +
> +     - perf tooling development happens in the perf tools maintainer
> +       tree and is aggregated into the tip tree.
> +
> +   - **CPU hotplug core**
> +
> +   - **RAS core**
> +
> +   - **EFI core**
> +
> +     EFI development in the efi git tree. The collected patches are
> +     aggregated in the tip efi/core branch.
> +
> +   - **RCU**
> +
> +     RCU development happens in the linux-rcu tree. The resulting changes
> +     are aggregated into the tip core/rcu branch.
> +
> +   - **Various core code components**:
> +
> +       - debugobjects
> +
> +       - objtool
> +
> +       - random bits and pieces

[ . . . ]

> +Backtraces in changelogs
> +^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Backtraces can be useful to document the call chain which led to a
> +problem. Though not all back traces are really valuable because the call
> +chain is unique and obvious, e.g. in early boot code. Just copying the full
> +dmesg output is adding a lot of distracting information like timestamps,
> +module lists, register and stack dumps etc.

   Backtraces help document the call chain leading to a problem. However,
   not all back traces are helpful, for example. early boot call chains
   are unique and obvious. Furthermore, copying the full dmesg output
   adds distracting information like timestamps, module lists, register
   and stack dumps.

> +Reducing the backtrace to the real relevant data helps to concentrate on
> +the issue and not being distracted by destilling the relevant information
> +out of the dump. Here is an example of a well trimmed backtrace::

   In constrast, the most useful backtraces distill the relevant
   information from the dump, which makes it easier to focus on the
   real issue.  Here is an example of a well-trimmed backtrace::

> +  unchecked MSR access error: WRMSR to 0xd51 (tried to write 0x0000
> +  000000000064) at rIP: 0xffffffffae059994 (native_write_msr+0x4/0x20)
> +  Call Trace:
> +  mba_wrmsr+0x41/0x80
> +  update_domains+0x125/0x130
> +  rdtgroup_mkdir+0x270/0x500

[ . . . ]

> +Namespaces
> +^^^^^^^^^^
> +
> +To improve readability and to allow easy grepping for information the usage
> +of consistent namespaces is important. The namespace should be used as a
> +prefix for globally visible (inline) functions and variables. A simple rule
> +for chosing a namespace prefix is to combine the subsystem and the
> +component name, e.g. 'x86_comp\_', 'sched\_', 'irq\_', 'mutex\_', etc. For
> +static functions and variables the namespace prefix can be omitted.

   Function/variable namespaces improve readability and allow easy
   grepping.  These namespaces are prefixes for globally visible function
   and variable names, including inlines.  These prefixes should combine
   the subsystem and the component name such as 'x86_comp\_', 'sched\_',
   'irq\_', and 'mutex\_'.  Namespace prefixes may be omitted for local
   static functions and variables.

> +Also be careful when using vendor names in namespaces. There is no value of
> +having 'xxx_vendor\_' or 'vendor_xxx_` as prefix for all static functions
> +of a vendor specific file as it is already clear that the code is vendor
> +specific. Aside of that vendor names should only be used when it is really
> +vendor specific functionality.

   Please note that 'xxx_vendor\_' and 'vendor_xxx_` prefixes are not
   helpful for static functions in vendor-specific files. After all,
   it is already clear that the code is vendor specific. In addition,
   vendor names should only be for truly vendor-specific functionality.

> +As always apply common sense and aim for consistency and readability.
> +
> +
> +Commit notifications
> +--------------------
> +
> +The tip tree is monitored by a bot for new commits. The bot sends an email
> +for each new commit to a dedicated mailing list
> +(``linux-tip-commits@vger.kernel.org``) and Cc's all people who are
> +mentioned in one of the commit tags. It uses the email message id from the
> +Link tag at the end of the tag list to set the In-Reply-To email header so
> +the message is properly threaded with the patch submission email.
> +
> +The maintainers try to reply to the submitter when merging a patch, but
> +they sometimes forget or it does not fit the workflow of the moment. While
> +the bot message is purely mechanical assume it implies a 'Thank you!
> +Applied.'.
> 
> 

Re: [patch 0/2] Documentation/process: Add subsystem/tree handbook

[Jonathan Corbet]

On Wed, 07 Nov 2018 18:10:10 +0100
Thomas Gleixner <tglx@linutronix.de> wrote:

> Mark recently suggested in one of the ksummit discussions to add subsystem
> or tree specific maintainer handbooks to document subsystem/tree specific
> development process information.
> 
> The following series adds the general section and the tip tree specific
> handbook.

So this is an idea that has gone around for a while; developers often get
into trouble when wandering into an unfamiliar part of the kernel, so
documenting the quaint local customs might help.  Assuming people actually
read the documentation, of course.

What's here seems generally good, but I do have an overall worry that we
may want to consider:

  - How much do we want to support and document subsystem-specific quirks
    vs. promoting reasonable and consistent rules kernel-wide?

There is a *lot* of stuff in the new tip manual.  Much of it, regarding
coding style and the writing of changelogs, really seems like it should be
global; if we need better documentation of that stuff, I'd really rather
see that advice folded into the central documents.  Having two (or more)
extensive coding-style documents doesn't seem like it's going to help us.

The stuff that is truly specific to tip seems fairly minimal:

  - what goes into tip
  - the reverse fir tree thing
  - tail comments, or the distaste thereabouts
  - subject-line prefixes

Having a tip-specific document that contains only those (plus whatever
else I forgot to list) would, IMO, make it much more likely that readers
would actually notice (and follow) the stuff that's specific to tip.

See what I'm getting at here?  Am I totally out to lunch on this?

Thanks,

jon

P.S. There is the separate issue of whether having subsystem-specific
     coding-style rules is a good thing overall.  I think we should try to
     be one big kernel project rather than 100 small ones, but perhaps
     that's just me.

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Paul E. McKenney]

On Wed, Nov 07, 2018 at 06:44:07PM +0100, Thomas Gleixner wrote:
> On Wed, 7 Nov 2018, Thomas Gleixner wrote:
> 
> > Add a document to the subsystem/maintainer handbook section, which explains
> > what the tip tree is, how it operates and what rules and expectations it
> > has.
> 
> Peter asked me to add a section about locking comments. I added it and
> forgot to refresh the patch before sending. Delta patch below.
> 
> Thanks,
> 
> 	tglx
> ---
> --- a/Documentation/process/maintainer-tip.rst
> +++ b/Documentation/process/maintainer-tip.rst
> @@ -578,6 +578,29 @@ Sentences in comments start with a upper
>    usage of descriptive function names often replaces these tiny comments.
>    Apply common sense as always.
> 
> + 
> +Documenting locking requirements
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +  Documenting locking requirements is a good thing, but comments are not
> +  necessarily the best choice. Instead of writing::
> +
> +    	/* Caller must hold foo->lock */
> +	void func(struct foo *foo)
> +	{
> +		...
> +	}
> +
> +  Please use::
> +
> +	void func(struct foo *foo)
> +	{
> +		lockdep_assert_held(&foo->lock);
> +		...
> +	}
> +
> +  The latter enables run time debugging when lockdep is enabled which
> +  verifies that all callers hold the lock. Comments can't do that.

     In PROVE_LOCKING kernels, lockdep_assert_held() emits a warning
     if the caller doesn't hold the lock.  Comments can't do that.

							Thanx, Paul

Re: [patch 0/2] Documentation/process: Add subsystem/tree handbook

[Dan Williams]

On Wed, Nov 7, 2018 at 11:49 AM Jonathan Corbet <corbet@lwn.net> wrote:
>
> On Wed, 07 Nov 2018 18:10:10 +0100
> Thomas Gleixner <tglx@linutronix.de> wrote:
>
> > Mark recently suggested in one of the ksummit discussions to add subsystem
> > or tree specific maintainer handbooks to document subsystem/tree specific
> > development process information.
> >
> > The following series adds the general section and the tip tree specific
> > handbook.
>
> So this is an idea that has gone around for a while; developers often get
> into trouble when wandering into an unfamiliar part of the kernel, so
> documenting the quaint local customs might help.  Assuming people actually
> read the documentation, of course.
>
> What's here seems generally good, but I do have an overall worry that we
> may want to consider:
>
>   - How much do we want to support and document subsystem-specific quirks
>     vs. promoting reasonable and consistent rules kernel-wide?
>
> There is a *lot* of stuff in the new tip manual.  Much of it, regarding
> coding style and the writing of changelogs, really seems like it should be
> global; if we need better documentation of that stuff, I'd really rather
> see that advice folded into the central documents.  Having two (or more)
> extensive coding-style documents doesn't seem like it's going to help us.
>
> The stuff that is truly specific to tip seems fairly minimal:
>
>   - what goes into tip
>   - the reverse fir tree thing
>   - tail comments, or the distaste thereabouts
>   - subject-line prefixes
>
> Having a tip-specific document that contains only those (plus whatever
> else I forgot to list) would, IMO, make it much more likely that readers
> would actually notice (and follow) the stuff that's specific to tip.
>
> See what I'm getting at here?  Am I totally out to lunch on this?

Not at all, and this is one of the thrusts of my talk next week at
Plumbers. I *do* want to propose that sub-systems document all their
local quirks. Then we can refactor the common ones into a global
document, have some discussion fodder if some sub-system specific
rules can be unified, and otherwise leave the freedom for individual
sub-systems to be different as long as it's documented.

Re: [patch 0/2] Documentation/process: Add subsystem/tree handbook

[Thomas Gleixner]

Dan,

On Wed, 7 Nov 2018, Dan Williams wrote:
> On Wed, Nov 7, 2018 at 11:49 AM Jonathan Corbet <corbet@lwn.net> wrote:
> > The stuff that is truly specific to tip seems fairly minimal:
> >
> >   - what goes into tip
> >   - the reverse fir tree thing
> >   - tail comments, or the distaste thereabouts
> >   - subject-line prefixes
> >
> > Having a tip-specific document that contains only those (plus whatever
> > else I forgot to list) would, IMO, make it much more likely that readers
> > would actually notice (and follow) the stuff that's specific to tip.
> >
> > See what I'm getting at here?  Am I totally out to lunch on this?
> 
> Not at all, and this is one of the thrusts of my talk next week at
> Plumbers. I *do* want to propose that sub-systems document all their
> local quirks. Then we can refactor the common ones into a global
> document, have some discussion fodder if some sub-system specific
> rules can be unified, and otherwise leave the freedom for individual
> sub-systems to be different as long as it's documented.

That was the intention here. I know that there is quite some stuff in that
document which could/should go into common documentation, but I have no
idea how much of it can be agreed on and how much different other
subsystems are looking at this. It's a braindump of all the things which
nag me/us in the daily business of patch juggling.

Jon, I agree that we should aim for as much consistency as we can. Though
if you look e.g. at comment style to begin with, then there is already
quite some disagreement. Changelog styles are a similar thing. Even if it's
somehow documented in the general documentation, some maintainers care and
others do not. The tip tree - and I have to admit especially myself - cares
a lot. I truly hate it when I have to look at useless, misleading or
uncomprehensible changelogs written by that Gleixner dude 10+ years ago.

We surely can spend a lot of time in bike shedding discussions how the
generally agreed guidelines should look like. But that would still leave
the contributors standing in the rain for a long time.

So I agree with Dan, that we should collect as much documentation from
subsystems/maintainers and get it into the tree so we can:

   - give contributors immediate access to subsystem/maintainer specific
     quirks

   - extract common views and move them into the general guidelines

   - have a clear idea which kind of things need to be discussed and agreed
     on and what might be left in the susbsystem/maintainer specific
     interpretation/expectation realm.

Thanks,

	tglx

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Ingo Molnar]

* Thomas Gleixner <tglx@linutronix.de> wrote:

> +Coding style notes
> +------------------
> +
> +Comment style
> +^^^^^^^^^^^^^
> +
> +Sentences in comments start with a uppercase letter.
> +
> +Single line comments::
> +
> +	/* This is a single line comment */
> +
> +Multi-line comments::
> +
> +	/*
> +	 * This is a properly formatted
> +	 * multi-line comment.
> +	 *
> +	 * Larger multi-line comments should be split into paragraphs.
> +	 */
> +
> +No tail comments:
> +
> +  Please refrain from using tail comments. Tail comments disturb the
> +  reading flow in almost all contexts, but especially in code::
> +
> +  	if (somecondition_is_true) /* Don't put a comment here */
> +		dostuff(); /* Neither here */
> +
> +	seed = MAGIC_CONSTANT; /* Nor here */
> +
> +  Use freestanding comments instead::
> +
> +	/* This condition is not obvious without a comment */
> +	if (somecondition_is_true) {
> +		/* This really needs to be documented */
> +		dostuff();
> +	}
> +
> +	/* This magic initialization needs a comment. Maybe not? */
> +	seed = MAGIC_CONSTANT;

Yeah, so I think a better way to visualize and explain the 'no tail 
comments' guideline in -tip is not these more complex code flows, but the 
totally simple linear flows of statements.

With tail comments the code looks like this:

	res = dostuff(); /* We explain something here. */

	seed = 1; /* Another explanation. */

	mod_timer(&our_object->our_timer, jiffies + OUR_INTERVAL); /* We like to talk */

	res = check_stuff(our_object); /* We explain something here. */
	if (res)
		return -EINVAL;

	interval = nontrivial_calculation(); /* Another explanation. */

	mod_timer(&our_object->our_timer, jiffies + interval); /* This doesn't race, because. */

... while with freestanding comments it's:

	/* We explain something here: */
	res = check_stuff(our_object);
	if (res)
		return -EINVAL;

	/* Another explanation: */
	interval = nontrivial_calculation();

	/* This doesn't race with init_our_stuff(), because: */
	mod_timer(&our_object->our_timer, jiffies + interval);

This comment placement style has several advantages:

  - Comments precede actual code - while in tail comments it's exactly
    the wrong way around.

  - We don't create over-long lines nor artificially short tail comments 
    just because we were trying to stay within the col80 limit with the 
    "this doesn't race" comment. The full-line comment was able to 
    explain that the race is with init_our_stuff().

  - Freestanding oneliner comments are much better aligned as well: note 
    how ever comment starts at the exact same column, making it very easy 
    to read (or not to read) these comments.

  - In short: predictable visual parsing rules and proper semantic 
    ordering of information is good, random joining of typographical 
    elements just to create marginally more compact source code is bad.

  - Just *look* at the tail comments example: it's a visually diffuse, 
    jumble of statements and misaligned comments with good structure.

Do you want me to send a delta patch, or an edited document?

Thanks,

	Ingo

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Ingo Molnar]

* Ingo Molnar <mingo@kernel.org> wrote:

> With tail comments the code looks like this:
> 
> 	res = dostuff(); /* We explain something here. */
> 
> 	seed = 1; /* Another explanation. */
> 
> 	mod_timer(&our_object->our_timer, jiffies + OUR_INTERVAL); /* We like to talk */
> 
> 	res = check_stuff(our_object); /* We explain something here. */
> 	if (res)
> 		return -EINVAL;
> 
> 	interval = nontrivial_calculation(); /* Another explanation. */
> 
> 	mod_timer(&our_object->our_timer, jiffies + interval); /* This doesn't race, because. */
> 
> ... while with freestanding comments it's:
> 
> 	/* We explain something here: */
> 	res = check_stuff(our_object);
> 	if (res)
> 		return -EINVAL;
> 
> 	/* Another explanation: */
> 	interval = nontrivial_calculation();
> 
> 	/* This doesn't race with init_our_stuff(), because: */
> 	mod_timer(&our_object->our_timer, jiffies + interval);
> 
> This comment placement style has several advantages:
> 
>   - Comments precede actual code - while in tail comments it's exactly
>     the wrong way around.
> 
>   - We don't create over-long lines nor artificially short tail comments 
>     just because we were trying to stay within the col80 limit with the 
>     "this doesn't race" comment. The full-line comment was able to 
>     explain that the race is with init_our_stuff().
> 
>   - Freestanding oneliner comments are much better aligned as well: note 
>     how ever comment starts at the exact same column, making it very easy 
>     to read (or not to read) these comments.
> 
>   - In short: predictable visual parsing rules and proper semantic 
>     ordering of information is good, random joining of typographical 
>     elements just to create marginally more compact source code is bad.
> 
>   - Just *look* at the tail comments example: it's a visually diffuse, 
>     jumble of statements and misaligned comments with good structure.

Forgot to mention the role of punctuation:

    - Also note how punctuation actually *helps* the integration of 
      comments and code. The ":" will direct attention to the code that 
      follows, making it clear that the sentence explains the next 
      statement. There are exceptions for when different type of 
      punctuation is a better choice, for example:

	/* TODO: convert the code to our newest calc API ASAP. */
	interval = nontrivial_calculation();

      Here we don't explain the statement but document a TODO entry. 

      Also, it's frequent practice to use multi-line comments to explain 
      the next section of code, with a particular note about the first 
      statement. Proper punctuation helps there too:

	/*
	 * Establish the timeout interval and use it to set up
	 * the timer of our object. The object is going to be
	 * freed when the last reference is released.
	 *
	 * Note, the initial calculation is non-trivial, because
	 * our timeout rules have complexity A), B) and C):
	 */
	interval = nontrivial_calculation();

      Note how part of the multi-line comment describes overall 
      principles of the code to follow, while the last sentence 
      describes a noteworthy aspect of the next C statement.

Thanks,

	Ingo

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Ingo Molnar]

* Thomas Gleixner <tglx@linutronix.de> wrote:

> +Backtraces in changelogs
> +^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Backtraces can be useful to document the call chain which led to a
> +problem. Though not all back traces are really valuable because the call
> +chain is unique and obvious, e.g. in early boot code. Just copying the full
> +dmesg output is adding a lot of distracting information like timestamps,
> +module lists, register and stack dumps etc.
> +
> +Reducing the backtrace to the real relevant data helps to concentrate on
> +the issue and not being distracted by destilling the relevant information
> +out of the dump. Here is an example of a well trimmed backtrace::
> +
> +  unchecked MSR access error: WRMSR to 0xd51 (tried to write 0x0000
> +  000000000064) at rIP: 0xffffffffae059994 (native_write_msr+0x4/0x20)
> +  Call Trace:
> +  mba_wrmsr+0x41/0x80
> +  update_domains+0x125/0x130
> +  rdtgroup_mkdir+0x270/0x500

Yeah, so I frequently simplify such backtraces even more, i.e.:

> +  unchecked MSR access error: WRMSR to 0xd51 (tried to write 0x0000 000000000064) at rIP: 0xffffffffae059994 (native_write_msr())
> +
> +  Call Trace:
> +    mba_wrmsr()
> +    update_domains()
> +    rdtgroup_mkdir()

Note how the actual MSR contents and the attempted operation's parameters 
are important, the actual hexadecimal offsets of the function call 
backtrace are not. They are useful when trying to do fuzzy version 
matching and in the occasional case when there's a question about which 
exact call chain it is - but those are 0.01% cases really.

See for example this recent commit:

 commit e4a02ed2aaf447fa849e3254bfdb3b9b01e1e520 (origin/locking-urgent-for-linus, locking-urgent-for-linus)
 Author: Guenter Roeck <linux@roeck-us.net>
 Date:   Tue Oct 2 14:48:49 2018 -0700

    locking/ww_mutex: Fix runtime warning in the WW mutex selftest
    
    If CONFIG_WW_MUTEX_SELFTEST=y is enabled, booting an image
    in an arm64 virtual machine results in the following
    traceback if 8 CPUs are enabled:
    
      DEBUG_LOCKS_WARN_ON(__owner_task(owner) != current)
      WARNING: CPU: 2 PID: 537 at kernel/locking/mutex.c:1033 __mutex_unlock_slowpath+0x1a8/0x2e0
      ...
      Call trace:
       __mutex_unlock_slowpath()
       ww_mutex_unlock()
       test_cycle_work()
       process_one_work()
       worker_thread()
       kthread()
       ret_from_fork()
    
    If requesting b_mutex fails with -EDEADLK, the error variable
    is reassigned to the return value from calling ww_mutex_lock
    on a_mutex again. If this call fails, a_mutex is not locked.
    It is, however, unconditionally unlocked subsequently, causing
    the reported warning. Fix the problem by using two error variables.
    
    With this change, the selftest still fails as follows:
    
      cyclic deadlock not resolved, ret[7/8] = -35
    
    However, the traceback is gone.

The C-style writing of the backtrace is more readable than listing the 
offsets.

Thanks,

	Ingo

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Ingo Molnar]

* Thomas Gleixner <tglx@linutronix.de> wrote:

> + - Fixes: 12char-SHA1 ("sub/sys: Original subject line")
> +
> +   A Fixes tag should be added even for changes which do not need to be
> +   backported to stable kernels, i.e. when addressing a recently introduced
> +   issue which only affects tip or the current head of mainline. These tags
> +   are helpful to identify the original commit and are much more valuable
> +   than prominently mentioning the commit which introduced a problem in the
> +   text of the changelog itself because they can be automatically
> +   extracted.
> +
> +   The following example illustrates the difference::
> +
> +     Commit
> +
> +       abcdef012345678 ("x86/xxx: Replace foo with bar")
> +
> +     left an unused instance of variable foo around. Remove it.
> +
> +     Signed-off-by: J.Dev <j.dev@mail>
> +
> +   Please say instead::
> +
> +     The recent replacement of foo with bar left an unused instance of
> +     variable foo around. Remove it.
> +
> +     Fixes: abcdef012345678 ("x86/xxx: Replace foo with bar")
> +     Signed-off-by: J.Dev <j.dev@mail>

Let me extend this policy element, I frequently write out commits in the 
changelog itself *as well*, because that's where I utilize it myself when 
reading other people's changelogs.

I.e. I would convert this:

     The recent replacement of left with right left an unused instance of
     variable left around. Remove it.

     Fixes: abcdef012345678 ("x86/xxx: Replace 'left' with 'right')
     Signed-off-by: J.Dev <j.dev@mail>

... to the following form:

     Two years ago the following commit:

       abcdef012345678 ("x86/xxx: Replace foo with bar")

     ... left an unused instance of the variable 'left' around. Remove it.

     Fixes: abcdef012345678 ("x86/xxx: Replace 'left' with 'right')
     Signed-off-by: J.Dev <j.dev@mail>

This changelog style, while more verbose, has a couple of advantages:

 - It's a bad practice to force the reader to go the tags sections, fish
   out a commit ID, just to be able to see the original commit. 
   Especially with longer changelogs and with changelogs mentioning 
   multiple source commits in-lining the commit ID is useful.

 - Also note how this style allows for human-readable time information to
   be inserted - which can be important to backporters. While an unused
   variable warning might not be backported, in other cases the time
   information can be useful in prioritizing the backporting.

 - Also note another pet peeve of mine: the quotation marks around the 
   variable names 'left' and 'right'. I changed the variable names to 
   English words that are ambiguous in free-flowing changelog text, just
   to illustrate how important it can be to escape them for better
   readability.

The 'Fixes' tag is mainly a standard tag that backporter tooling can 
search for - otherwise for human readers the in-line explanation is more 
useful.

I really trivial cases the inlining can be skipped and only a 'Fixes' tag 
is perfectly sufficient.

Thanks,

	Ingo

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Ingo Molnar]

* Thomas Gleixner <tglx@linutronix.de> wrote:

> + - Signed-off-by: ``Patch handler <handler@mail>``
> +
> +   SOBs after the author SOB are from people handling and transporting the
> +   patch, but were not involved in development. If the handler made
> +   modifications to the patch or the changelog, then this should be
> +   mentioned **after** the changelog text and **above** all commit tags in
> +   the following format::
> +
> +     ... changelog text ends.
> +
> +     [ handler: Replaced foo by bar and updated changelog ]
> +
> +     First-tag: .....
> +
> +   Note the two empty new lines which separate the changelog text and the
> +   commit tags from that notice.

Even after a decade of introducing Git I still see Signed-off-by used as 
an Acked-by or Reviewed-by substitutes, so I'd suggest adding this small 
explanation as well:

  +   SOB chains should reflect the *real* route a patch took as it was 
  +   propagated to us, with the first SOB entry signalling primary
  +   authorship of a single author. Acks should be given as Acked-by 
  +   lines and review approvals as Reviewed-by lines.


> +   If a patch is sent to the mailing list by a handler then the author has
> +   to be noted in the first line of the changelog with::
> +
> +     From: ``Author <author@mail>``
> +
> +     Changelog text starts here....
> +
> +   so the authorship is preserved. The 'From:' line has to be followed by a
> +   empty newline. If that 'From:' line is missing, then the patch would be
> +   attributed to the person who sent (transported) it. The 'From:' line is
> +   automatically removed when the patch is applied and does not show up in
> +   the final git changelog. It merely affects the authorship information of
> +   the resulting git commit.

s/(transported)
 /(transported, handled)

to connect the text with the whole 'handler' language used before?

and since we are not talking about the 'git command', maybe also:

s/git
 /Git

?

> + - Cc: ``cc-ed-person <person@mail>``
> +
> +   If the patch should be backported to stable, then please add a '``Cc:
> +   stable@vger.kernel.org``' tag, but do not Cc stable when sending your
> +   mail.

Can I suggest a more canonical form:

	Cc: <stable@vger.kernel.org> # v4.18 and later kernels

It would be nice if people adding Cc: stable lines would actually try to 
figure out which exact kernel versions are affected.

Also the '<>' form makes it easier to read and my email client will also 
syntax highlight it in that case. ;-)


> + - Link: ``https://link/to/information``
> +
> +   For referring to email on LKML or other kernel mailing lists, please use
> +   the lkml.kernel.org redirector URL::

s/referring to email
 /referring to an email

> +
> +     https://lkml.kernel.org/r/email-message@id
> +
> +   The kernel.org redirector is considered a stable URL unlike other email
> +   archives.

s/URL unlike
 /URL, unlike

?

Thanks,

	Ingo

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Ingo Molnar]

Lemme fill in the scheduler and locking/atomics bits as well:

> +The tip tree contains the following subsystems:
> +
> +   - **x86 architecture**
> +
> +     The x86 architecture development takes place in the tip tree except
> +     for the x86 KVM and XEN specific parts which are maintained in the
> +     corresponding subsystems and routed directly to mainline from
> +     there. It's still good practice to Cc the x86 maintainers on
> +     x86-specific KVM and XEN patches.
> +
> +     Some x86 subsystems have their own maintainers in addition to the
> +     overall x86 maintainers.  Please Cc the overall x86 maintainers on
> +     patches touching files in arch/x86 even when they are not called out
> +     by the MAINTAINER file.
> +
> +     Note, that ``x86@kernel.org`` is not a mailing list. It is merely a
> +     mail alias which distributes mails to the x86 top-level maintainer
> +     team. Please always Cc the Linux Kernel mailing list (LKML)
> +     ``linux-kernel@vger.kernel.org``, otherwise your mail ends up only in
> +     the private inboxes of the maintainers.
> +
> +   - **Scheduler**

        Scheduler development takes place in the -tip tree, in the 
        sched/core branch - with occasional sub-topic trees for 
        work-in-progress patch-sets.

> +
> +   - **Locking and atomics**

        Locking development (including atomics and other synchronization 
        primitives that are connected to locking) takes place in the -tip 
        tree, in the locking/core branch - with occasional sub-topic 
        trees for work-in-progress patch-sets.

Thanks,

	Ingo

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Ingo Molnar]

* Thomas Gleixner <tglx@linutronix.de> wrote:

> +Variable types
> +^^^^^^^^^^^^^^
> +
> +Please use the proper u8, u16, u32, u64 types for variables which are meant
> +to describe hardware or are used as arguments for functions which access
> +hardware. These types are clearly defining the bit width and avoid
> +truncation, expansion and 32/64 bit confusion.
> +
> +u64 is also recommended in code which would become ambiguous for 32bit when
> +'unsigned long' would be used instead. While in such situations 'unsigned
> +long long' could be used as well, u64 is shorter and also clearly shows
> +that the operation is required to be 64bit wide independent of the target
> +CPU.
> +
> +Please use 'unsigned int' instead of 'unsigned'.

s/for 32bit
 /for 32-bit kernels

s/64bit wide
 /64 bits wide

> +Constants
> +^^^^^^^^^
> +
> +Please do not use literal (hexa)decimal numbers in code or initializers.
> +Either use proper defines which have descriptive names or consider using
> +an enum.

I believe there should be an exception for 'obvious' literal values like 
0 and 1.

I.e. the above is mostly a rule that is intended to cover undocumented 
'magic' numbers.

I.e. how about this wording:

  +Constants
  +^^^^^^^^^
  +
  +Please do not use magic literal (hexa)decimal numbers when interfacing
  +with hardware where the number has an unclear origin in code or 
  +initializers. I.e. "no magic numbers".
  +
  +Either use proper defines which have descriptive names or use an enum.
  +
  +Using obvious 0/1 literal values is fine in most cases.

?

> +
> +
> +Struct declarations and initializers
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Struct declarations should align the struct member names in a tabular
> +fashion::
> +
> +	struct bar_order {
> +		unsigned int	guest_id;
> +		int		ordered_item;
> +		struct menu	*menu;
> +	};
> +
> +Please avoid documenting struct members within the declaration, because
> +this often results in strangely formatted comments and the struct members
> +become obfuscated::
> +
> + 	struct bar_order {
> +  		unsigned int	guest_id; /* Unique guest id */

[ Sidenote: there's whitespace damage (extra spaces) in the text here. ]

> +		int		ordered_item;
> +		/* Pointer to a menu instance which contains all the drinks */
> +		struct menu	*menu;
> +	};
> +
> +Instead, please consider using the kernel-doc format in a comment preceding
> +the struct declaration, which is easier to read and has the added advantage
> +of including the information in the kernel documentation, for example, as
> +follows::

I disagree slightly here. While adding kernel-doc format is fine of 
course, so are in-line comments which I frequently use.

This form is particularly helpful for more complex structures. Have a 
look at 'struct fpu' for example:


/*
 * Highest level per task FPU state data structure that
 * contains the FPU register state plus various FPU
 * state fields:
 */
struct fpu {
	/*
	 * @last_cpu:
	 *
	 * Records the last CPU on which this context was loaded into
	 * FPU registers. (In the lazy-restore case we might be
	 * able to reuse FPU registers across multiple context switches
	 * this way, if no intermediate task used the FPU.)
	 *
	 * A value of -1 is used to indicate that the FPU state in context
	 * memory is newer than the FPU state in registers, and that the
	 * FPU state should be reloaded next time the task is run.
	 */
	unsigned int			last_cpu;

	/*
	 * @initialized:
	 *
	 * This flag indicates whether this context is initialized: if the task
	 * is not running then we can restore from this context, if the task
	 * is running then we should save into this context.
	 */
	unsigned char			initialized;

	/*
	 * @state:
	 *
	 * In-memory copy of all FPU registers that we save/restore
	 * over context switches. If the task is using the FPU then
	 * the registers in the FPU are more recent than this state
	 * copy. If the task context-switches away then they get
	 * saved here and represent the FPU state.
	 */
	union fpregs_state		state;
	/*
	 * WARNING: 'state' is dynamically-sized.  Do not put
	 * anything after it here.
	 */
};

The in-line freestanding comments is perfectly structured and readable as 
well, and this is analogous to the 'freestanding comments' style for C 
statements.

We also have occasional examples where tail comments are fine, such as:

/*
 * The legacy x87 FPU state format, as saved by FSAVE and
 * restored by the FRSTOR instructions:
 */
struct fregs_state {
	u32			cwd;	/* FPU Control Word		*/
	u32			swd;	/* FPU Status Word		*/
	u32			twd;	/* FPU Tag Word			*/
	u32			fip;	/* FPU IP Offset		*/
	u32			fcs;	/* FPU IP Selector		*/
	u32			foo;	/* FPU Operand Pointer Offset	*/
	u32			fos;	/* FPU Operand Pointer Selector	*/

	/* 8*10 bytes for each FP-reg = 80 bytes:			*/
	u32			st_space[20];

	/* Software status information [not touched by FSAVE]:		*/
	u32			status;
};

But I'd not complicate the style guide with that.

> +Static struct initializers must use C99 initializers and should also be
> +aligned in a tabular fashion::
> +
> +	static struct foo statfoo = {
> +		.a		= 0,
> +		.plain_integer	= CONSTANT_DEFINE_OR_ENUM,
> +		.bar		= &statbar,
> +	};
> +

Yeah, and maybe also add a note about the final comma:

  + Note that while C99 syntax allows the omission of the final comma, we 
  + recommend the use of a comma on the last line because it makes 
  + reordering and addition of new lines easier, and makes such future 
  + patches slightly easier to read as well.

?

Thanks,

	Ingo

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Ingo Molnar]

* Thomas Gleixner <tglx@linutronix.de> wrote:

> +Namespaces
> +^^^^^^^^^^
> +
> +To improve readability and to allow easy grepping for information the usage
> +of consistent namespaces is important. The namespace should be used as a
> +prefix for globally visible (inline) functions and variables. A simple rule
> +for chosing a namespace prefix is to combine the subsystem and the
> +component name, e.g. 'x86_comp\_', 'sched\_', 'irq\_', 'mutex\_', etc. For
> +static functions and variables the namespace prefix can be omitted.
> +
> +Also be careful when using vendor names in namespaces. There is no value of
> +having 'xxx_vendor\_' or 'vendor_xxx_` as prefix for all static functions
> +of a vendor specific file as it is already clear that the code is vendor
> +specific. Aside of that vendor names should only be used when it is really
> +vendor specific functionality.
> +
> +As always apply common sense and aim for consistency and readability.

I'd also suggest adding:

> +This also includes static file scope functions that are immediately put into 
> +globally visible driver templates - it's useful for those symbols to carry a
> +good prefix as well, for backtrace readability.
> +
> +Truly local functions, only called by other local functions, can have 
> +shorter descriptive names - our primary concern is greppability and 
> +backtrace readability.

Beyond the driver-template aspect also note the backtrace readability 
argument I added: good namespaces are not just about greppability.

Thanks,

	Ingo

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Ingo Molnar]

* Thomas Gleixner <tglx@linutronix.de> wrote:

> +Commit notifications
> +--------------------
> +
> +The tip tree is monitored by a bot for new commits. The bot sends an email
> +for each new commit to a dedicated mailing list
> +(``linux-tip-commits@vger.kernel.org``) and Cc's all people who are
> +mentioned in one of the commit tags. It uses the email message id from the
> +Link tag at the end of the tag list to set the In-Reply-To email header so
> +the message is properly threaded with the patch submission email.

s/id
 /ID

> +
> +The maintainers try to reply to the submitter when merging a patch, but
> +they sometimes forget or it does not fit the workflow of the moment. While
> +the bot message is purely mechanical assume it implies a 'Thank you!
> +Applied.'.

s/The maintainers
 /The -tip maintainers and submaintainers

s/is purely mechanical assume it implies a 'Thank you! Applied.'
 /is purely mechanical, it also implies a 'Thank you! Applied.' message.

... added a missing comma, plus there's nothing to assume, that the 
thank-you note is implied is a given! :-)

Thanks,

	Ingo

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Ingo Molnar]

* Thomas Gleixner <tglx@linutronix.de> wrote:

> +Line breaks
> +^^^^^^^^^^^
> +
> +Restricting line length to 80 characters makes deeply indented code hard to
> +read.  Consider breaking out code into helper functions to avoid excessive
> +line breaking.
> +
> +The 80 character rule is not a strict rule, so please use common sense when
> +breaking lines. Especially format strings should never be broken up.

Might make sense to explain that:

  + The reason for that rule is that if for example we have this printk line:
  +
  +		if (uh_oh()) {
  +			pr_info("Something really bad happened, danger"
  +				"danger, blue smoke reported!\n");
  +		}
  +
  + People would see this message in the syslog:
  +
  +   Thu Nov  8 09:22:33: Something really bad happened, dangerdanger, blue smoke reported!
  +
  + And chances are that in sheer panic they'd type the most distinctive 
  + part of that text as a search pattern for the kernel source tree:
  +
  +   $ git grep -i 'dangerdanger'
  +   $
  +
  + ... and they'd get absolutely no match on that string due to the 
  + col80 broken format string, and confusion and frustration would rise, 
  + in addition to growing amounts of blue smoke.
  +
  + We don't want that, so just write out the single line:

  +		if (uh_oh())
  +			pr_info("Something really bad happened, danger danger, blue smoke reported!\n");
  +
  + Also note two other advantages of writing it like this:
  +
  +  - We saved two curly braces.
  +  - We also added a proper space to 'danger danger' which was the original intended message.

?

> +
> +When splitting function declarations or function calls, then please align
> +the first argument in the second line with the first argument in the first
> +line::
> +
> +  static int long_function_name(struct foobar *barfoo, unsigned int id,
> +  				unsigned int offset)
> +  {
> +
> +  	if (!id) {
> +		ret = longer_function_name(barfoo, DEFAULT_BARFOO_ID,
> +					   offset);

BTW., in this particular case I think small violations of col80 rule are 
even more readable, i.e.:

> +		ret = longer_function_name(barfoo, DEFAULT_BARFOO_ID, offset);

And note that in this example we used 78 colums so we didn't even violate 
the col80 rule. ;-)

Thanks,

	Ingo

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Peter Zijlstra]

On Thu, Nov 08, 2018 at 08:40:12AM +0100, Ingo Molnar wrote:
> > + - Cc: ``cc-ed-person <person@mail>``
> > +
> > +   If the patch should be backported to stable, then please add a '``Cc:
> > +   stable@vger.kernel.org``' tag, but do not Cc stable when sending your
> > +   mail.
> 
> Can I suggest a more canonical form:
> 
> 	Cc: <stable@vger.kernel.org> # v4.18 and later kernels
> 
> It would be nice if people adding Cc: stable lines would actually try to 
> figure out which exact kernel versions are affected.

I think Greg actually prefers we use stable@kernel.org, which is a
/dev/null target. His bot will then pick up on the tag once it hits
Linus' tree.

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Greg Kroah-Hartman]

On Thu, Nov 08, 2018 at 10:12:51AM +0100, Peter Zijlstra wrote:
> On Thu, Nov 08, 2018 at 08:40:12AM +0100, Ingo Molnar wrote:
> > > + - Cc: ``cc-ed-person <person@mail>``
> > > +
> > > +   If the patch should be backported to stable, then please add a '``Cc:
> > > +   stable@vger.kernel.org``' tag, but do not Cc stable when sending your
> > > +   mail.
> > 
> > Can I suggest a more canonical form:
> > 
> > 	Cc: <stable@vger.kernel.org> # v4.18 and later kernels
> > 
> > It would be nice if people adding Cc: stable lines would actually try to 
> > figure out which exact kernel versions are affected.
> 
> I think Greg actually prefers we use stable@kernel.org, which is a
> /dev/null target. His bot will then pick up on the tag once it hits
> Linus' tree.

I really do not care either way what address is used.  Both work fine.

thanks,

greg k-h

Re: [patch 0/2] Documentation/process: Add subsystem/tree handbook

[Jonathan Corbet]

On Wed, 7 Nov 2018 21:51:38 +0100 (CET)
Thomas Gleixner <tglx@linutronix.de> wrote:

> So I agree with Dan, that we should collect as much documentation from
> subsystems/maintainers and get it into the tree so we can:
> 
>    - give contributors immediate access to subsystem/maintainer specific
>      quirks
> 
>    - extract common views and move them into the general guidelines
> 
>    - have a clear idea which kind of things need to be discussed and agreed
>      on and what might be left in the susbsystem/maintainer specific
>      interpretation/expectation realm.

Ah, but Dan said:

> Not at all, and this is one of the thrusts of my talk next week at
> Plumbers. I *do* want to propose that sub-systems document all their
> local quirks. 

My concern is that you're going far beyond "local quirks" here, and
actually hiding the real quirks in the process. Just one example from
Ingo's email storm this (US/Mountain) morning:

> Even after a decade of introducing Git I still see Signed-off-by used as 
> an Acked-by or Reviewed-by substitutes, so I'd suggest adding this small 
> explanation as well:
> 
>   +   SOB chains should reflect the *real* route a patch took as it was 
>   +   propagated to us, with the first SOB entry signalling primary
>   +   authorship of a single author. Acks should be given as Acked-by 
>   +   lines and review approvals as Reviewed-by lines.

If SOB means anything like what it's supposed to mean, this *can't* be a
"local quirk" - we have to agree on it globally.

If you want to push this into the tree in something like its current form,
I'm not going to resist too hard - far be it from me to say we don't want
more documentation!  But allow me to complain a little.

Suppose I came along with my nifty new architecture, and it dragged in a
whole new set of timer and interrupt subsystems that duplicated a lot of
what's in the kernel now, but buried a few "local quirks" deep in the
middle.  "Don't worry", I say, "we'll factor out the common stuff later
once we figure out what it is; I'd rather not deal with the bikeshedding
now". Correct me if I'm wrong, but I suspect I might just get a response
back from you.  That's not how we normally do things.

This proposal takes a similar approach to the documentation.  Changelog
rules, your comment rules (other than tail comments), brace rules, line
breaks, etc. are common stuff; if they are not well-enough documented in
the global docs, the fix should really be applied there.  If it lands in
the current form, you know as well as I do that it will almost certainly
stay there for years, if not indefinitely.

IMO, the subsystem-specific documentation should be something that an
existing kernel developer can use to quickly learn how to avoid surprises
when wandering into a different subsystem.  So it should be concise and
strongly focused on the local customs.  If we don't start that way, I'm
afraid we'll never have that.  Then developers will miss the important
information, and we'll reinforce the image of the kernel project as a
collection of little fiefdoms that one wanders into at one's own risk.
And Documentation/ will continue to be a painful mess.

</soapbox>

Might it be worth asking Ted for a kernel summit slot to talk about this
next week?

(And thanks again for doing this!  I like the material and think we
definitely want it.)

jon

Re: [patch 0/2] Documentation/process: Add subsystem/tree handbook

[Peter Zijlstra]

On Thu, Nov 08, 2018 at 07:49:20AM -0700, Jonathan Corbet wrote:
> Might it be worth asking Ted for a kernel summit slot to talk about this
> next week?

Ah, on that, let me complain :-)

My plumbers schedule is already 100% booked with MCs and other things.
There is no kernel-summit schedule details available as of yet, but it
is already almost certain that I will not have time for anything in that
track anyway :/

And I'm fairly sure I'm not the only one in that boat.

Re: [patch 0/2] Documentation/process: Add subsystem/tree handbook

[Jonathan Corbet]

On Thu, 8 Nov 2018 16:05:17 +0100
Peter Zijlstra <peterz@infradead.org> wrote:

> On Thu, Nov 08, 2018 at 07:49:20AM -0700, Jonathan Corbet wrote:
> > Might it be worth asking Ted for a kernel summit slot to talk about this
> > next week?  
> 
> Ah, on that, let me complain :-)
> 
> My plumbers schedule is already 100% booked with MCs and other things.
> There is no kernel-summit schedule details available as of yet, but it
> is already almost certain that I will not have time for anything in that
> track anyway :/

But surely you can find a few minutes to drop by and throw catchboxes at
the docs maintainer? :)

I do understand, though, it's going to be a crazy week.

jon

Re: [patch 0/2] Documentation/process: Add subsystem/tree handbook

[Thomas Gleixner]

Jon,

On Thu, 8 Nov 2018, Jonathan Corbet wrote:
> On Wed, 7 Nov 2018 21:51:38 +0100 (CET)
> Thomas Gleixner <tglx@linutronix.de> wrote:
> >   +   SOB chains should reflect the *real* route a patch took as it was 
> >   +   propagated to us, with the first SOB entry signalling primary
> >   +   authorship of a single author. Acks should be given as Acked-by 
> >   +   lines and review approvals as Reviewed-by lines.
> 
> If SOB means anything like what it's supposed to mean, this *can't* be a
> "local quirk" - we have to agree on it globally.

Agreed.

> If you want to push this into the tree in something like its current form,
> I'm not going to resist too hard - far be it from me to say we don't want
> more documentation!  But allow me to complain a little.

Please ask for allowance next time _before_ complaining :)

> Suppose I came along with my nifty new architecture, and it dragged in a
> whole new set of timer and interrupt subsystems that duplicated a lot of
> what's in the kernel now, but buried a few "local quirks" deep in the
> middle.  "Don't worry", I say, "we'll factor out the common stuff later
> once we figure out what it is; I'd rather not deal with the bikeshedding
> now". Correct me if I'm wrong, but I suspect I might just get a response
> back from you.  That's not how we normally do things.

Darn. Not much I can argue about.

> This proposal takes a similar approach to the documentation.  Changelog
> rules, your comment rules (other than tail comments), brace rules, line
> breaks, etc. are common stuff; if they are not well-enough documented in
> the global docs, the fix should really be applied there.  If it lands in
> the current form, you know as well as I do that it will almost certainly
> stay there for years, if not indefinitely.
> 
> IMO, the subsystem-specific documentation should be something that an
> existing kernel developer can use to quickly learn how to avoid surprises
> when wandering into a different subsystem.  So it should be concise and
> strongly focused on the local customs.  If we don't start that way, I'm
> afraid we'll never have that.  Then developers will miss the important
> information, and we'll reinforce the image of the kernel project as a
> collection of little fiefdoms that one wanders into at one's own risk.
> And Documentation/ will continue to be a painful mess.

Fair enough. TBH, I picked up Marks idea and it started out small and then
all the stuff which itches me/us got dumped into it. Let me try to split
that into pieces.

> Might it be worth asking Ted for a kernel summit slot to talk about this
> next week?

Aside of the scheduling conflicts, definitely yes.

> (And thanks again for doing this!  I like the material and think we
> definitely want it.)

At least it was not complete waste of time then :)

Thanks,

	tglx

Re: [patch 0/2] Documentation/process: Add subsystem/tree handbook

[Paul E. McKenney]

On Thu, Nov 08, 2018 at 08:19:57AM -0700, Jonathan Corbet wrote:
> On Thu, 8 Nov 2018 16:05:17 +0100
> Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > On Thu, Nov 08, 2018 at 07:49:20AM -0700, Jonathan Corbet wrote:
> > > Might it be worth asking Ted for a kernel summit slot to talk about this
> > > next week?  
> > 
> > Ah, on that, let me complain :-)
> > 
> > My plumbers schedule is already 100% booked with MCs and other things.
> > There is no kernel-summit schedule details available as of yet, but it
> > is already almost certain that I will not have time for anything in that
> > track anyway :/
> 
> But surely you can find a few minutes to drop by and throw catchboxes at
> the docs maintainer? :)
> 
> I do understand, though, it's going to be a crazy week.

How about one of the Wednesday evening slots?

							Thanx, Paul

Re: [patch 0/2] Documentation/process: Add subsystem/tree handbook

[Theodore Y. Ts'o]

On Thu, Nov 08, 2018 at 04:05:17PM +0100, Peter Zijlstra wrote:
> On Thu, Nov 08, 2018 at 07:49:20AM -0700, Jonathan Corbet wrote:
> > Might it be worth asking Ted for a kernel summit slot to talk about this
> > next week?
> 
> Ah, on that, let me complain :-)
> 
> My plumbers schedule is already 100% booked with MCs and other things.
> There is no kernel-summit schedule details available as of yet, but it
> is already almost certain that I will not have time for anything in that
> track anyway :/
> 

I thought there was a slot already scheduled on the refereed track,
"Towards a Linux Kernel Mainainer Handbook" (Tuesday at 4:45pm) for
this purpose?

A draft kernel summit schedule was released yesterday on the
kernel-summit mailing list.  I haven't populated the web site yet
because I haven't had the time, and it's a lot easier to edit a text
file than to make changes using Indico.  :-)

Please take a look at it and let me know if there are conflicts; there
is still time to make changes, although trying to satisfy all of the
*other* scheduling constraints I know of is going to be tricky, so I
make no promises that I can satisfy any additional scheduling
requests/constraints.

Cheers,

						- Ted

Re: [patch 0/2] Documentation/process: Add subsystem/tree handbook

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 710 bytes --]

On Thu, Nov 08, 2018 at 11:21:33AM -0500, Theodore Y. Ts'o wrote:
> On Thu, Nov 08, 2018 at 04:05:17PM +0100, Peter Zijlstra wrote:

> > My plumbers schedule is already 100% booked with MCs and other things.
> > There is no kernel-summit schedule details available as of yet, but it
> > is already almost certain that I will not have time for anything in that
> > track anyway :/

> I thought there was a slot already scheduled on the refereed track,
> "Towards a Linux Kernel Mainainer Handbook" (Tuesday at 4:45pm) for
> this purpose?

I'm not 100% sure that people will pick up that the topic is about a
handbook for working with maintainers rather than a handbook for being a
maintainer from that title...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [patch 0/2] Documentation/process: Add subsystem/tree handbook

[Jonathan Corbet]

On Thu, 8 Nov 2018 11:21:33 -0500
"Theodore Y. Ts'o" <tytso@mit.edu> wrote:

> I thought there was a slot already scheduled on the refereed track,
> "Towards a Linux Kernel Mainainer Handbook" (Tuesday at 4:45pm) for
> this purpose?

My expectation is that this will be an actual talk; it seemed rude to
assume that Dan had penciled in a block of time for open arguments about
the tip-tree handbook :)  That said, I'm not feeling a groundswell of
support for scheduling another session at this point...

jon

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Dan Williams]

On Thu, Nov 8, 2018 at 1:13 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, Nov 08, 2018 at 08:40:12AM +0100, Ingo Molnar wrote:
> > > + - Cc: ``cc-ed-person <person@mail>``
> > > +
> > > +   If the patch should be backported to stable, then please add a '``Cc:
> > > +   stable@vger.kernel.org``' tag, but do not Cc stable when sending your
> > > +   mail.
> >
> > Can I suggest a more canonical form:
> >
> >       Cc: <stable@vger.kernel.org> # v4.18 and later kernels
> >
> > It would be nice if people adding Cc: stable lines would actually try to
> > figure out which exact kernel versions are affected.

I know at least StGit mail does not grok that "#"notation. I've
stopped using it in favor of a "Fixes:" tag. I would think "Fixes:" is
preferred over "# <KVER>" if only because it can be used to track
fixes to commits that have been backported to stable. Is there any
reason for "# <KVER>" to continue in a world where we have "Fixes:"?

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Borislav Petkov]

On Thu, Nov 08, 2018 at 09:19:33AM -0800, Dan Williams wrote:
> I know at least StGit mail does not grok that "#"notation. I've
> stopped using it in favor of a "Fixes:" tag. I would think "Fixes:" is
> preferred over "# <KVER>" if only because it can be used to track
> fixes to commits that have been backported to stable.

Yeah, FWIW, we do that for our SLES kernels.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [patch 0/2] Documentation/process: Add subsystem/tree handbook

[Dan Williams]

On Thu, Nov 8, 2018 at 8:32 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Thu, Nov 08, 2018 at 11:21:33AM -0500, Theodore Y. Ts'o wrote:
> > On Thu, Nov 08, 2018 at 04:05:17PM +0100, Peter Zijlstra wrote:
>
> > > My plumbers schedule is already 100% booked with MCs and other things.
> > > There is no kernel-summit schedule details available as of yet, but it
> > > is already almost certain that I will not have time for anything in that
> > > track anyway :/
>
> > I thought there was a slot already scheduled on the refereed track,
> > "Towards a Linux Kernel Mainainer Handbook" (Tuesday at 4:45pm) for
> > this purpose?
>
> I'm not 100% sure that people will pick up that the topic is about a
> handbook for working with maintainers rather than a handbook for being a
> maintainer from that title...

The intent is a handbook for being a maintainer. However, in the
process of describing how a given sub-system is maintained one also
needs to describe the local rules. So contributors should be able to
glean what matters to a sub-system from a description of how that
sub-system is maintained. In other words the direct audience is
maintainers, but it also hopefully makes the process more transparent
for contributors.

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Paul E. McKenney]

On Thu, Nov 08, 2018 at 09:19:33AM -0800, Dan Williams wrote:
> On Thu, Nov 8, 2018 at 1:13 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Thu, Nov 08, 2018 at 08:40:12AM +0100, Ingo Molnar wrote:
> > > > + - Cc: ``cc-ed-person <person@mail>``
> > > > +
> > > > +   If the patch should be backported to stable, then please add a '``Cc:
> > > > +   stable@vger.kernel.org``' tag, but do not Cc stable when sending your
> > > > +   mail.
> > >
> > > Can I suggest a more canonical form:
> > >
> > >       Cc: <stable@vger.kernel.org> # v4.18 and later kernels
> > >
> > > It would be nice if people adding Cc: stable lines would actually try to
> > > figure out which exact kernel versions are affected.
> 
> I know at least StGit mail does not grok that "#"notation. I've
> stopped using it in favor of a "Fixes:" tag. I would think "Fixes:" is
> preferred over "# <KVER>" if only because it can be used to track
> fixes to commits that have been backported to stable. Is there any
> reason for "# <KVER>" to continue in a world where we have "Fixes:"?

I sometimes have fixes that need to be different for different past
releases.  And there have been cases where RCU patches would apply and
build cleanly against releases for which it was not appropriate, but
would have some low-probability failure.  Which meant that it could be
expected to pass light testing.  :-/

So I sometimes need a way of saying which versions a given patch applies
to, independent of the version into which the bug was introduced.

							Thanx, Paul

Re: [patch 0/2] Documentation/process: Add subsystem/tree handbook

[Theodore Y. Ts'o]

On Thu, Nov 08, 2018 at 09:33:47AM -0700, Jonathan Corbet wrote:
> 
> > I thought there was a slot already scheduled on the refereed track,
> > "Towards a Linux Kernel Mainainer Handbook" (Tuesday at 4:45pm) for
> > this purpose?
> 
> My expectation is that this will be an actual talk; it seemed rude to
> assume that Dan had penciled in a block of time for open arguments about
> the tip-tree handbook :)  That said, I'm not feeling a groundswell of
> support for scheduling another session at this point...

Agreed, my apologies to Dan for making that presumption.  My reasoning
for suggesting that his session might be a good one is that many of
the things which are listed as "tip-tree quirks" are things which I
suspect we are discovering are actually things that should go into the
Maintainer Handbook.

I don't know how much time Dan was planning on presenting versus
having a discussion.  If there is a lot of discussion that gets kicked
up, that's why we have TBD/Unconference slots.  (And yes, there may be
conflicts; sorry about that.  On the other hand, better that people
are frustrated that there are too many high quality sessions they want
to attend, as opposed to twiddling their fingers and saying, "well
*this* afternoon is a complete waste of my time."  :-)

I suspect the main places where there will be divergencies between
different trees might be in how individual subsystems weigh various
recommendations when they come into conflict with each other.  We've
seen that already with checkpatch.pl, where in fact the choices about
"lines > 80 characters always BAD BAD BAD" versus "continuation
strings in printk EVEN WORSE" have flipped over time (and so now we
have cleanup patches being submitted which undo the work of different
cleanup patches years earlier :-).  And I have no doubt that this will
be true for things that might go into tree-specific quirks versus
Maintainer Handbook.

In fact, what might make sense is for Maintainer Handbook to have the
general recommendations, and then maybe tree-specific quirks document
might simply say that in terms of the Frobozz tree weighs the
suggestions of guideline Foo over guideline Quux, where as the
Frobnozz tree has a the opposite weighting.

Cheers,

						- Ted

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Thomas Gleixner]

On Thu, 8 Nov 2018, Paul E. McKenney wrote:
> On Thu, Nov 08, 2018 at 09:19:33AM -0800, Dan Williams wrote:
> > On Thu, Nov 8, 2018 at 1:13 AM Peter Zijlstra <peterz@infradead.org> wrote:
> > >
> > > On Thu, Nov 08, 2018 at 08:40:12AM +0100, Ingo Molnar wrote:
> > > > > + - Cc: ``cc-ed-person <person@mail>``
> > > > > +
> > > > > +   If the patch should be backported to stable, then please add a '``Cc:
> > > > > +   stable@vger.kernel.org``' tag, but do not Cc stable when sending your
> > > > > +   mail.
> > > >
> > > > Can I suggest a more canonical form:
> > > >
> > > >       Cc: <stable@vger.kernel.org> # v4.18 and later kernels
> > > >
> > > > It would be nice if people adding Cc: stable lines would actually try to
> > > > figure out which exact kernel versions are affected.
> > 
> > I know at least StGit mail does not grok that "#"notation. I've
> > stopped using it in favor of a "Fixes:" tag. I would think "Fixes:" is
> > preferred over "# <KVER>" if only because it can be used to track
> > fixes to commits that have been backported to stable. Is there any
> > reason for "# <KVER>" to continue in a world where we have "Fixes:"?
> 
> I sometimes have fixes that need to be different for different past
> releases.  And there have been cases where RCU patches would apply and
> build cleanly against releases for which it was not appropriate, but
> would have some low-probability failure.  Which meant that it could be
> expected to pass light testing.  :-/
> 
> So I sometimes need a way of saying which versions a given patch applies
> to, independent of the version into which the bug was introduced.

I can understand that you want to limit the scope of automatic backports.

But we really should try to always use of the Fixes: tag. In most cases the
SHA1 of the commit in the fixes tag defines the backport scope. 

For the rare cases where the buggy commit is really old, but you want to
limit the backport scope for a reason then I really like to avoid to
overload the Cc stable tag and have a dedicated tag instead. Something
like:

    Fixes: 1234567890AB ("subsys/comp: Short summary")
    Backport-to: 4.14

and have that backport tag right under the Fixes tag. If the Backport-to
tag is ommitted, the SHA1 defines the scope, but I'm fine with making it
mandatory.

If there is really the special RCU case where each and every stable version
needs some special treatment then say:

    Backport-to: Manual

or whatever sensible word would express it correctly.

The Fixes tag is really valuable when you need to make connections and I
know that the people who are looking into safety-critical Linux value the
tag because it can be used for tracking and for metrics.

Thanks,

	tglx

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Paul E. McKenney]

On Thu, Nov 08, 2018 at 08:58:32PM +0100, Thomas Gleixner wrote:
> On Thu, 8 Nov 2018, Paul E. McKenney wrote:
> > On Thu, Nov 08, 2018 at 09:19:33AM -0800, Dan Williams wrote:
> > > On Thu, Nov 8, 2018 at 1:13 AM Peter Zijlstra <peterz@infradead.org> wrote:
> > > >
> > > > On Thu, Nov 08, 2018 at 08:40:12AM +0100, Ingo Molnar wrote:
> > > > > > + - Cc: ``cc-ed-person <person@mail>``
> > > > > > +
> > > > > > +   If the patch should be backported to stable, then please add a '``Cc:
> > > > > > +   stable@vger.kernel.org``' tag, but do not Cc stable when sending your
> > > > > > +   mail.
> > > > >
> > > > > Can I suggest a more canonical form:
> > > > >
> > > > >       Cc: <stable@vger.kernel.org> # v4.18 and later kernels
> > > > >
> > > > > It would be nice if people adding Cc: stable lines would actually try to
> > > > > figure out which exact kernel versions are affected.
> > > 
> > > I know at least StGit mail does not grok that "#"notation. I've
> > > stopped using it in favor of a "Fixes:" tag. I would think "Fixes:" is
> > > preferred over "# <KVER>" if only because it can be used to track
> > > fixes to commits that have been backported to stable. Is there any
> > > reason for "# <KVER>" to continue in a world where we have "Fixes:"?
> > 
> > I sometimes have fixes that need to be different for different past
> > releases.  And there have been cases where RCU patches would apply and
> > build cleanly against releases for which it was not appropriate, but
> > would have some low-probability failure.  Which meant that it could be
> > expected to pass light testing.  :-/
> > 
> > So I sometimes need a way of saying which versions a given patch applies
> > to, independent of the version into which the bug was introduced.
> 
> I can understand that you want to limit the scope of automatic backports.
> 
> But we really should try to always use of the Fixes: tag. In most cases the
> SHA1 of the commit in the fixes tag defines the backport scope. 
> 
> For the rare cases where the buggy commit is really old, but you want to
> limit the backport scope for a reason then I really like to avoid to
> overload the Cc stable tag and have a dedicated tag instead. Something
> like:
> 
>     Fixes: 1234567890AB ("subsys/comp: Short summary")
>     Backport-to: 4.14
> 
> and have that backport tag right under the Fixes tag. If the Backport-to
> tag is ommitted, the SHA1 defines the scope, but I'm fine with making it
> mandatory.
> 
> If there is really the special RCU case where each and every stable version
> needs some special treatment then say:
> 
>     Backport-to: Manual
> 
> or whatever sensible word would express it correctly.
> 
> The Fixes tag is really valuable when you need to make connections and I
> know that the people who are looking into safety-critical Linux value the
> tag because it can be used for tracking and for metrics.

Indeed, I do need to get my act together with the Fixes tag.  And I am
happy with whatever format would limit backports appropriately.

							Thanx, Paul

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Theodore Y. Ts'o]

On Thu, Nov 08, 2018 at 09:19:33AM -0800, Dan Williams wrote:
> 
> I know at least StGit mail does not grok that "#"notation. I've
> stopped using it in favor of a "Fixes:" tag. I would think "Fixes:" is
> preferred over "# <KVER>" if only because it can be used to track
> fixes to commits that have been backported to stable. Is there any
> reason for "# <KVER>" to continue in a world where we have "Fixes:"?

The main annoyance I have with Fixes is because it can be a pain to
figure out what the "# <KVER>" would be.  Something like:

% tag --contains DEADBEEF | grep ^v | head

doesn't work because kernel version numbers don't sort obviously.  So
v4.10 comes before v4.3 using a normal sort, and even sort -n doesn't
do the right.

I suppose it wouldn't be that hard to write a perl/python script that
correctly sorts kernel version numbers, but when the "# <KVER>" is
present, I find it convenient.

(Also note that even with fast SSD's and/or everything in page cache,
runnning "tag --contains <COMMITID>" will take a good 3-4 seconds, and
if the git packs are not in the page cache, and/or you're unfortunate
enough to have your git trees on an HDD.... it's not pretty.)

      	       	    			    	     - Ted

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Thomas Gleixner]

On Thu, 8 Nov 2018, Theodore Y. Ts'o wrote:

> On Thu, Nov 08, 2018 at 09:19:33AM -0800, Dan Williams wrote:
> > 
> > I know at least StGit mail does not grok that "#"notation. I've
> > stopped using it in favor of a "Fixes:" tag. I would think "Fixes:" is
> > preferred over "# <KVER>" if only because it can be used to track
> > fixes to commits that have been backported to stable. Is there any
> > reason for "# <KVER>" to continue in a world where we have "Fixes:"?
> 
> The main annoyance I have with Fixes is because it can be a pain to
> figure out what the "# <KVER>" would be.  Something like:
> 
> % tag --contains DEADBEEF | grep ^v | head
> 
> doesn't work because kernel version numbers don't sort obviously.  So
> v4.10 comes before v4.3 using a normal sort, and even sort -n doesn't
> do the right.
> 
> I suppose it wouldn't be that hard to write a perl/python script that
> correctly sorts kernel version numbers, but when the "# <KVER>" is
> present, I find it convenient.
> 
> (Also note that even with fast SSD's and/or everything in page cache,
> runnning "tag --contains <COMMITID>" will take a good 3-4 seconds, and
> if the git packs are not in the page cache, and/or you're unfortunate
> enough to have your git trees on an HDD.... it's not pretty.)

Fair enough. But as I said before we please let us have both the fixes tag
and a decicated backport-to one and make the latter mandatory.

It if's not there and the patch has a 'Cc: stable' tag it's easy enough to
reject it. The the submitter or the maintainer who decides that a patch
needs to be backported to stable has to wait the 4 seconds and add that
information.

Thanks,

	tglx

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Greg KH]

On Thu, Nov 08, 2018 at 03:14:25PM -0500, Theodore Y. Ts'o wrote:
> On Thu, Nov 08, 2018 at 09:19:33AM -0800, Dan Williams wrote:
> > 
> > I know at least StGit mail does not grok that "#"notation. I've
> > stopped using it in favor of a "Fixes:" tag. I would think "Fixes:" is
> > preferred over "# <KVER>" if only because it can be used to track
> > fixes to commits that have been backported to stable. Is there any
> > reason for "# <KVER>" to continue in a world where we have "Fixes:"?
> 
> The main annoyance I have with Fixes is because it can be a pain to
> figure out what the "# <KVER>" would be.  Something like:
> 
> % tag --contains DEADBEEF | grep ^v | head
> 
> doesn't work because kernel version numbers don't sort obviously.  So
> v4.10 comes before v4.3 using a normal sort, and even sort -n doesn't
> do the right.
> 
> I suppose it wouldn't be that hard to write a perl/python script that
> correctly sorts kernel version numbers, but when the "# <KVER>" is
> present, I find it convenient.

'sort -V' should help you out here, no need to write anything new :)

> (Also note that even with fast SSD's and/or everything in page cache,
> runnning "tag --contains <COMMITID>" will take a good 3-4 seconds, and
> if the git packs are not in the page cache, and/or you're unfortunate
> enough to have your git trees on an HDD.... it's not pretty.)

I recommend the "static cache" or whatever that thing is called, that
helps out a _LOT_ with stuff like this.  For the kernel tree, which is
never rebased, it speeds up this so much.

thanks,

greg k-h

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Greg KH]

On Thu, Nov 08, 2018 at 08:58:32PM +0100, Thomas Gleixner wrote:
> On Thu, 8 Nov 2018, Paul E. McKenney wrote:
> > On Thu, Nov 08, 2018 at 09:19:33AM -0800, Dan Williams wrote:
> > > On Thu, Nov 8, 2018 at 1:13 AM Peter Zijlstra <peterz@infradead.org> wrote:
> > > >
> > > > On Thu, Nov 08, 2018 at 08:40:12AM +0100, Ingo Molnar wrote:
> > > > > > + - Cc: ``cc-ed-person <person@mail>``
> > > > > > +
> > > > > > +   If the patch should be backported to stable, then please add a '``Cc:
> > > > > > +   stable@vger.kernel.org``' tag, but do not Cc stable when sending your
> > > > > > +   mail.
> > > > >
> > > > > Can I suggest a more canonical form:
> > > > >
> > > > >       Cc: <stable@vger.kernel.org> # v4.18 and later kernels
> > > > >
> > > > > It would be nice if people adding Cc: stable lines would actually try to
> > > > > figure out which exact kernel versions are affected.
> > > 
> > > I know at least StGit mail does not grok that "#"notation. I've
> > > stopped using it in favor of a "Fixes:" tag. I would think "Fixes:" is
> > > preferred over "# <KVER>" if only because it can be used to track
> > > fixes to commits that have been backported to stable. Is there any
> > > reason for "# <KVER>" to continue in a world where we have "Fixes:"?
> > 
> > I sometimes have fixes that need to be different for different past
> > releases.  And there have been cases where RCU patches would apply and
> > build cleanly against releases for which it was not appropriate, but
> > would have some low-probability failure.  Which meant that it could be
> > expected to pass light testing.  :-/
> > 
> > So I sometimes need a way of saying which versions a given patch applies
> > to, independent of the version into which the bug was introduced.
> 
> I can understand that you want to limit the scope of automatic backports.
> 
> But we really should try to always use of the Fixes: tag. In most cases the
> SHA1 of the commit in the fixes tag defines the backport scope. 
> 
> For the rare cases where the buggy commit is really old, but you want to
> limit the backport scope for a reason then I really like to avoid to
> overload the Cc stable tag and have a dedicated tag instead. Something
> like:
> 
>     Fixes: 1234567890AB ("subsys/comp: Short summary")
>     Backport-to: 4.14

Ick, no.  Just stick to the "Fixes:" tag.  My scripts can now track when
a patch is backported to a stable tree so that I know to apply it to
older ones despite the original patch showing up in a newer release.

And yes, those scripts are new, as Sasha is about to point out all of
the places where I missed this in the past :)

thanks,

greg k-h

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Greg KH]

On Thu, Nov 08, 2018 at 01:06:15PM -0800, Greg KH wrote:
> On Thu, Nov 08, 2018 at 08:58:32PM +0100, Thomas Gleixner wrote:
> > On Thu, 8 Nov 2018, Paul E. McKenney wrote:
> > > On Thu, Nov 08, 2018 at 09:19:33AM -0800, Dan Williams wrote:
> > > > On Thu, Nov 8, 2018 at 1:13 AM Peter Zijlstra <peterz@infradead.org> wrote:
> > > > >
> > > > > On Thu, Nov 08, 2018 at 08:40:12AM +0100, Ingo Molnar wrote:
> > > > > > > + - Cc: ``cc-ed-person <person@mail>``
> > > > > > > +
> > > > > > > +   If the patch should be backported to stable, then please add a '``Cc:
> > > > > > > +   stable@vger.kernel.org``' tag, but do not Cc stable when sending your
> > > > > > > +   mail.
> > > > > >
> > > > > > Can I suggest a more canonical form:
> > > > > >
> > > > > >       Cc: <stable@vger.kernel.org> # v4.18 and later kernels
> > > > > >
> > > > > > It would be nice if people adding Cc: stable lines would actually try to
> > > > > > figure out which exact kernel versions are affected.
> > > > 
> > > > I know at least StGit mail does not grok that "#"notation. I've
> > > > stopped using it in favor of a "Fixes:" tag. I would think "Fixes:" is
> > > > preferred over "# <KVER>" if only because it can be used to track
> > > > fixes to commits that have been backported to stable. Is there any
> > > > reason for "# <KVER>" to continue in a world where we have "Fixes:"?
> > > 
> > > I sometimes have fixes that need to be different for different past
> > > releases.  And there have been cases where RCU patches would apply and
> > > build cleanly against releases for which it was not appropriate, but
> > > would have some low-probability failure.  Which meant that it could be
> > > expected to pass light testing.  :-/
> > > 
> > > So I sometimes need a way of saying which versions a given patch applies
> > > to, independent of the version into which the bug was introduced.
> > 
> > I can understand that you want to limit the scope of automatic backports.
> > 
> > But we really should try to always use of the Fixes: tag. In most cases the
> > SHA1 of the commit in the fixes tag defines the backport scope. 
> > 
> > For the rare cases where the buggy commit is really old, but you want to
> > limit the backport scope for a reason then I really like to avoid to
> > overload the Cc stable tag and have a dedicated tag instead. Something
> > like:
> > 
> >     Fixes: 1234567890AB ("subsys/comp: Short summary")
> >     Backport-to: 4.14
> 
> Ick, no.  Just stick to the "Fixes:" tag.  My scripts can now track when
> a patch is backported to a stable tree so that I know to apply it to
> older ones despite the original patch showing up in a newer release.
> 
> And yes, those scripts are new, as Sasha is about to point out all of
> the places where I missed this in the past :)

Here's the script if others are curious:
	https://github.com/gregkh/gregkh-linux/blob/master/scripts/fix_in_what_release

Yes, I know it's horrid, I abuse the fact that 'git grep' is very fast
on the stable-queue repo :)

thanks,

greg k-h

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Theodore Y. Ts'o]

On Thu, Nov 08, 2018 at 01:04:49PM -0800, Greg KH wrote:
> > (Also note that even with fast SSD's and/or everything in page cache,
> > runnning "tag --contains <COMMITID>" will take a good 3-4 seconds, and
> > if the git packs are not in the page cache, and/or you're unfortunate
> > enough to have your git trees on an HDD.... it's not pretty.)
> 
> I recommend the "static cache" or whatever that thing is called, that
> helps out a _LOT_ with stuff like this.  For the kernel tree, which is
> never rebased, it speeds up this so much.

At the risk of asking a stupid question, which cache is this?  I don't
think it's the untrackedCache; is it the BitmapHashCache?

Thanks,

      	       		       	     - Ted

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Greg KH]

On Thu, Nov 08, 2018 at 05:19:47PM -0500, Theodore Y. Ts'o wrote:
> On Thu, Nov 08, 2018 at 01:04:49PM -0800, Greg KH wrote:
> > > (Also note that even with fast SSD's and/or everything in page cache,
> > > runnning "tag --contains <COMMITID>" will take a good 3-4 seconds, and
> > > if the git packs are not in the page cache, and/or you're unfortunate
> > > enough to have your git trees on an HDD.... it's not pretty.)
> > 
> > I recommend the "static cache" or whatever that thing is called, that
> > helps out a _LOT_ with stuff like this.  For the kernel tree, which is
> > never rebased, it speeds up this so much.
> 
> At the risk of asking a stupid question, which cache is this?  I don't
> think it's the untrackedCache; is it the BitmapHashCache?

It's the 'commitGraph', here's the article I was trying to remember I
learned this from:
	https://blogs.msdn.microsoft.com/devops/2018/06/25/supercharging-the-git-commit-graph/

greg k-h

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Thomas Gleixner]

On Thu, 8 Nov 2018, Greg KH wrote:
> On Thu, Nov 08, 2018 at 08:58:32PM +0100, Thomas Gleixner wrote:
> >     Fixes: 1234567890AB ("subsys/comp: Short summary")
> >     Backport-to: 4.14
> 
> Ick, no.  Just stick to the "Fixes:" tag.  My scripts can now track when
> a patch is backported to a stable tree so that I know to apply it to
> older ones despite the original patch showing up in a newer release.

Fine with me.

I just did a few stats.

commits between 4.19-rc1 and 4.19-final:

Total commits:				2098

Total Cc: stable		         334
Cc: stable             			 119	35%
Cc: stable       #kver 		 	  10     3%
Cc: stable	       Revert		   0	 0%
Cc: stable Fixes       		   	 123    37%
Cc: stable Fixes #kver		 	  81    24%

Total      Fixes only  	       		 584
	   Fixes only >  4.18	   	 227    39%
	   Fixes only <= 4.18		 357    61%

and between 4.19 and todays top of tree:

Total commits:			       12947

Total Cc: stable		         251
Cc: stable             			  85    34%
Cc: stable       #kver 		 	  28    11%
Cc: stable	       reverts		   2     1%
Cc: stable Fixes       		   	  79    31%
Cc: stable Fixes #kver		 	  55    22%

Total      Fixes only  	       		 835
	   Fixes only >  4.19	   	 830    99%
	   Fixes only <= 4.19		   5     1%

Not much change in the 'cc stable' ratio for those without any information
and for those with Fixes tag and #kver.

The cc stable + kver and the cc stable + Fixes differ, but their sum is
roughly the same.

But the real interesting change is that between 4.19-rc1 and 4.19-final the
number of Fixes only (no CC stable) commits which fix a commit in 4.18 or
earlier is rather high, while the same category on post 4.19 is minimal.

I'll run a larger and more detailed scan to figure out whether there is a
trend or this is just random.

Thanks,

	tglx

Re: [patch 2/2] Documentation/process: Add tip tree handbook

[Dan Williams]

On Thu, Nov 8, 2018 at 12:14 PM Theodore Y. Ts'o <tytso@mit.edu> wrote:
>
> On Thu, Nov 08, 2018 at 09:19:33AM -0800, Dan Williams wrote:
> >
> > I know at least StGit mail does not grok that "#"notation. I've
> > stopped using it in favor of a "Fixes:" tag. I would think "Fixes:" is
> > preferred over "# <KVER>" if only because it can be used to track
> > fixes to commits that have been backported to stable. Is there any
> > reason for "# <KVER>" to continue in a world where we have "Fixes:"?
>
> The main annoyance I have with Fixes is because it can be a pain to
> figure out what the "# <KVER>" would be.  Something like:
>
> % tag --contains DEADBEEF | grep ^v | head
>
> doesn't work because kernel version numbers don't sort obviously.  So
> v4.10 comes before v4.3 using a normal sort, and even sort -n doesn't
> do the right.

Unless I'm misunderstanding, I think you want:

    git describe --contains $COMMIT --match=v[345]*

...which should give you the latest tagged kernel according to that match spec.

Re: [patch 0/2] Documentation/process: Add subsystem/tree handbook

[Ingo Molnar]

* Jonathan Corbet <corbet@lwn.net> wrote:

> > Even after a decade of introducing Git I still see Signed-off-by used as 
> > an Acked-by or Reviewed-by substitutes, so I'd suggest adding this small 
> > explanation as well:
> > 
> >   +   SOB chains should reflect the *real* route a patch took as it was 
> >   +   propagated to us, with the first SOB entry signalling primary
> >   +   authorship of a single author. Acks should be given as Acked-by 
> >   +   lines and review approvals as Reviewed-by lines.
> 
> If SOB means anything like what it's supposed to mean, this *can't* be a
> "local quirk" - we have to agree on it globally.

Yeah, I didn't intend this paragraph to be a local quirk - more like a 
reiteration of what the global rule already is (regardless of enforcement 
...), given the context.

I presume the cross section of readers of *both* the global documents and 
the -tip documents will be even smaller than just the readers of the -tip 
document - so some redundancy is beneficial. ;-)

In that sense the 'local' rules can also be indicators about which parts 
of the myriads of global rules the maintainers feel most strongly about, 
and are also a list of the most common problems we see in practice. Just 
repeating the very large set of global rules is counter-productive in 
that fashion. Maybe we should re-phrase and re-structure it into such a 
format, with references back to the original rules where we are just 
reiterating global rules? That would also allow the eventual inclusion of 
any clarification into the global rule.

Another aspect is that in -tip we are pretty strict about certain 
stylistic and not so stylistic details - this is partly a personal 
preference of the maintainers, and partly a maintainer workload reduction 
measure.

But level of enforcement matters and I think other trees can legitimately 
be more relaxed: when there's a lack of contributors then you *really* 
don't want to be the maintainer-from-hell who wants all i's dotted and 
all t's crossed, and you might not have the bandwidth to do it all 
yourself ...

Also I think there are and should be a lot of shades of grey when it 
comes to accepting patches, to find the right balance between pushing 
back on patches to improve quality and pulling in patches to move the 
kernel forward.

> If you want to push this into the tree in something like its current 
> form, I'm not going to resist too hard - far be it from me to say we 
> don't want more documentation!  But allow me to complain a little.
> 
> Suppose I came along with my nifty new architecture, and it dragged in 
> a whole new set of timer and interrupt subsystems that duplicated a lot 
> of what's in the kernel now, but buried a few "local quirks" deep in 
> the middle.  "Don't worry", I say, "we'll factor out the common stuff 
> later once we figure out what it is; I'd rather not deal with the 
> bikeshedding now". Correct me if I'm wrong, but I suspect I might just 
> get a response back from you.  That's not how we normally do things.
> 
> This proposal takes a similar approach to the documentation.  Changelog 
> rules, your comment rules (other than tail comments), brace rules, line 
> breaks, etc. are common stuff; if they are not well-enough documented 
> in the global docs, the fix should really be applied there.  If it 
> lands in the current form, you know as well as I do that it will almost 
> certainly stay there for years, if not indefinitely.
>
> IMO, the subsystem-specific documentation should be something that an
> existing kernel developer can use to quickly learn how to avoid surprises
> when wandering into a different subsystem.  So it should be concise and
> strongly focused on the local customs.  If we don't start that way, I'm
> afraid we'll never have that.  Then developers will miss the important
> information, and we'll reinforce the image of the kernel project as a
> collection of little fiefdoms that one wanders into at one's own risk.
> And Documentation/ will continue to be a painful mess.
> 
> </soapbox>

Yeah, so in -tip we don't do "local customs": we genuinely believe that 
*all* our rules where they go beyond the current development process 
would improve the generic kernel as well. (In fact if anyone can give 
reasons for why a particular rule is not an absolute advantage to the 
kernel we'd reconsider changing the rule.)

So your complaint is legitimate - how would you propose we handle this?

Thanks,

	Ingo

Re: [patch 0/2] Documentation/process: Add subsystem/tree handbook

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 836 bytes --]

On Thu, Nov 08, 2018 at 09:32:18AM -0800, Dan Williams wrote:
> On Thu, Nov 8, 2018 at 8:32 AM Mark Brown <broonie@kernel.org> wrote:

> > I'm not 100% sure that people will pick up that the topic is about a
> > handbook for working with maintainers rather than a handbook for being a
> > maintainer from that title...

> The intent is a handbook for being a maintainer. However, in the
> process of describing how a given sub-system is maintained one also
> needs to describe the local rules. So contributors should be able to
> glean what matters to a sub-system from a description of how that
> sub-system is maintained. In other words the direct audience is
> maintainers, but it also hopefully makes the process more transparent
> for contributors.

To me that sounds like a document aimed at contributors rather than
maintainers.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [patch 0/2] Documentation/process: Add subsystem/tree handbook

[Borislav Petkov]

On Thu, Nov 08, 2018 at 04:49:04PM +0100, Thomas Gleixner wrote:
> > Suppose I came along with my nifty new architecture, and it dragged in a
> > whole new set of timer and interrupt subsystems that duplicated a lot of
> > what's in the kernel now, but buried a few "local quirks" deep in the
> > middle.  "Don't worry", I say, "we'll factor out the common stuff later
> > once we figure out what it is; I'd rather not deal with the bikeshedding
> > now". Correct me if I'm wrong, but I suspect I might just get a response
> > back from you.  That's not how we normally do things.
> 
> Darn. Not much I can argue about.

So, that thing.

I have this ontop of 5.10 along with most comments integrated.

Now, I'm thinking if I start sending those pieces which belong into the
main process documentation, the bikeshedding that is going to ensue is
going to be insane. And we have day jobs too, you know. :)

Thus, I'm also thinking that I should do this piecemeal and once we've
all agreed on one aspect and you've applied it, Jon, I'll carve out and
send the next. Rinse and repeat.

How does that sound, makes sense?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [patch 0/2] Documentation/process: Add subsystem/tree handbook

[Jonathan Corbet]

On Thu, 17 Dec 2020 16:05:37 +0100
Borislav Petkov <bp@alien8.de> wrote:

> So, that thing.
> 
> I have this ontop of 5.10 along with most comments integrated.
> 
> Now, I'm thinking if I start sending those pieces which belong into the
> main process documentation, the bikeshedding that is going to ensue is
> going to be insane. And we have day jobs too, you know. :)
> 
> Thus, I'm also thinking that I should do this piecemeal and once we've
> all agreed on one aspect and you've applied it, Jon, I'll carve out and
> send the next. Rinse and repeat.
> 
> How does that sound, makes sense?

Gee...a response from a two-year-old thread...it's taking me a while to
page all of that back in :)

I'd love to see this work get in, I still feel bad about my part in
stalling it before.

Piecemeal could certainly work, or we could just try the whole thing and
see what happens.  We got Thorsten's reporting-issues tome in without much
trouble, after all, and that's bikeshed territory if anything is.  But
whatever works is fine; send stuff and I'll gladly look at it.

Thanks,

jon

Re: [patch 0/2] Documentation/process: Add subsystem/tree handbook

[Borislav Petkov]

On Thu, Dec 17, 2020 at 10:53:23AM -0700, Jonathan Corbet wrote:
> Gee...a response from a two-year-old thread...

You know how we love to document stuff, right? :-)

> it's taking me a while to page all of that back in :)

Here's the gist of your concern:

https://lkml.kernel.org/r/20181108074920.4c601ee3@lwn.net

> I'd love to see this work get in, I still feel bad about my part in
> stalling it before.

Why? It makes sense - you want the generic stuff which holds true for
the whole tree to be in generic docs. Read your example again and you'll
persuade yourself again. :-)

> Piecemeal could certainly work, or we could just try the whole thing and
> see what happens.  We got Thorsten's reporting-issues tome in without much
> trouble, after all, and that's bikeshed territory if anything is.  But
> whatever works is fine; send stuff and I'll gladly look at it.

Yeah, let's start slowly and small. We'll get there eventually... I
hope.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette