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

[Dou Liyang]

Hi tglx,

Thank you very much for your guidance! It makes me more profound
understanding of the changelog. And you also rewrote my changelog for
giving me an example.

I am so grateful that you can help me so carefully.
Once I heard the charm of the open source community, Now i can
really feel it. I love it so much.

I will try to improve myself and help others.  :)

Thanks,
	Liyang.

At 03/01/2017 06:51 PM, Thomas Gleixner wrote:
> 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