Re: [PATCH v2 1/4] Revert"x86/acpi: Set persistent cpuid <-> nodeid mapping when booting"

[Thomas Gleixner]

On Mon, 20 Feb 2017, Dou Liyang wrote:

> Currently, We make the mapping of "cpuid <-> nodeid" fixed at the booting time.
> It keeps consistent with the WorkQueue and avoids some bugs which may be caused
> by the dynamic assignment.
> 
> But, The ACPI table is unreliable and it is very risky that we use the entity
> which isn't related to a physical device at booting time.
> 
> Now, we revert our patches. Do the last mapping of "cpuid <-> nodeid" at
> hot-plug time, not at booting time where we did some useless work.
> It also can make the mapping of "cpuid <-> nodeid" fixed and avoid excessive
> use of the ACPI table.
> 
> The patch revert the commit dc6db24d24:
>   "x86/acpi: Set persistent cpuid <-> nodeid mapping when booting".

That changelog needs some massaging. Something like this:

  The mapping of "cpuid <-> nodeid" is established at boot time via ACPI
  tables to keep associations of workqueues and other node related items
  consistent across cpu hotplug.

  But, ACPI tables are unreliable and failures with that boot time mapping
  have been reported on machines where the ACPI table and the physical
  information which is retrieved at actual hotplug is inconsistent.

  Revert the mapping implementation so it can be replaced with a less error
  prone approach.

This clearly describes:

  1) The context

  2) The problem

  3) The solution (revert)

You don't have to explain what the new solution will be in the changelog of
the revert. For the revert it's only relevant WHY we do the revert.

Please avoid writing changelogs in 'we' form. Write it pure technical, like
a manual.

Also avoid phrases like: "The patch/This patch". We all know already that
this is a patch, otherwise it wouldn't have been sent.

Documentation/process/submitting-patches.rst says:

  Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
  instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
  to do frotz", as if you are giving orders to the codebase to change
  its behaviour.

Thanks,

	tglx