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

Dou Liyang
2017-03-02T08:12:09Z

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
>
>
>
>