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