| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <60a2b930-40cc-ea32-878f-d063523b5fec@redhat.com>
Date: Mon, 28 Jun 2021 19:38:11 +1000
From: Gavin Shan <gshan@...hat.com>
To: Andrew Jones <drjones@...hat.com>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
rdunlap@...radead.org, robh+dt@...nel.org, shan.gavin@...il.com
Subject: Re: [PATCH v4] Documentation, dt, numa: Add note to empty NUMA node
On 6/28/21 6:34 PM, Andrew Jones wrote:
> On Fri, Jun 25, 2021 at 09:14:49PM +1000, Gavin Shan wrote:
>> On 6/25/21 8:18 PM, Andrew Jones wrote:
>>> On Fri, Jun 25, 2021 at 06:36:48PM +1000, Gavin Shan wrote:
>>>> On 6/25/21 5:06 PM, Andrew Jones wrote:
>>>>> On Fri, Jun 25, 2021 at 09:02:17AM +0200, Andrew Jones wrote:
>>>>>> On Fri, Jun 25, 2021 at 01:23:38PM +0800, Gavin Shan wrote:
>>>>>>> The empty memory nodes, where no memory resides in, are allowed.
>>>>>>> For these empty memory nodes, the 'len' of 'reg' property is zero.
>>>>>>> The NUMA node IDs are still valid and parsed, but memory can be
>>>>>>> added to them through hotplug afterwards. I finds difficulty to
>>>>>>> get where it's properly documented.
>>>>>>>
>>>>>>> So lets add note to empty memory nodes in the NUMA binding doc.
>>>>>>>
>>>>>>> Signed-off-by: Gavin Shan <gshan@...hat.com>
>>>>>>> ---
>>>>>>> Documentation/devicetree/bindings/numa.txt | 4 ++++
>>>>>>> 1 file changed, 4 insertions(+)
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/numa.txt b/Documentation/devicetree/bindings/numa.txt
>>>>>>> index 21b35053ca5a..0fc882e44270 100644
>>>>>>> --- a/Documentation/devicetree/bindings/numa.txt
>>>>>>> +++ b/Documentation/devicetree/bindings/numa.txt
>>>>>>> @@ -109,6 +109,10 @@ Example:
>>>>>>> Dual socket system consists of 2 boards connected through ccn bus and
>>>>>>> each board having one socket/soc of 8 cpus, memory and pci bus.
>>>>>>> +Note that empty memory nodes, which no memory resides in, are allowed.
>>>>>>> +The NUMA node IDs in these empty memory nodes are still valid, but
>>>>>>> +memory can be added into them through hotplug afterwards.
>>>>>>
>>>>>> Please change the second sentence to:
>>>>>>
>>>>>> The NUMA node IDs in these empty memory nodes are still valid and
>>>>>> memory may be added into them through hotplug afterwards.
>>>>>>
>>>>>> But, this doesn't look like the right place for this paragraph. You're
>>>>>> adding the paragraph to the example section, but the example doesn't have
>>>>>> any empty memory nodes.
>>>>>>
>>>>>> I think the paragraph should be added to section "2 - numa-node-id" and an
>>>>>
>>>>> Or maybe even create a new section for it.
>>>>>
>>>>>> example empty memory node should be provided. Also, the commit message
>>>>>> talks about the length of 'reg' being zero, which is an important
>>>>>> distinction which should also be documented.
>>>>>>
>>>>
>>>> Drew, thanks for your comments. Yeah, it sounds sensible to create
>>>> a new section for it and an example would be more helpful. Please
>>>> check if below changes are fine to you. I probably need Randy's review
>>>> again.
>>>>
>>>> I'm trying to avoid too many revisions for this sort of trivial patch,
>>>> even though I already had. However, it's time frame for v5.14 and I'm
>>>> pushing this to be merged during the cycle.
>>>
>>> We must revise until we're satisfied... Also, I wouldn't call writing
>>> specifications trivial.
>>>
>>
>> [...]
>>
>>> I'll just rewrite it:
>>>
>>> Empty memory nodes, which no memory resides in, are allowed. The 'length'
>>> field of the 'reg' property is zero, but the 'base-address' is a dummy
>>> address and invalid. However, the NUMA node IDs and distance maps are
>>> still valid and memory may be added into them through hotplug afterwards.
>>>
>>
>> [...]
>>
>>>
>>> You should write how you've selected the dummy addresses for the empty
>>> memory nodes, perhaps here in the /* Empty memory node */ comments or
>>> above in the paragraph or both.
>>>
>>>> + memory@...00000000 {
>>>> + device_type = "memory";
>>>> + reg = <0x1 0x0 0x0 0x0>;
>>>> + numa-node-id = <2>;
>>>> + };
>>>> +
>>>> + /* Empty memory node */
>>>> + memory@...80000000 {
>>>> + device_type = "memory";
>>>> + reg = <0x1 0x80000000 0x0 0x0>;
>>>> + numa-node-id = <3>;
>>>> + };
>>>> +
>>
>> [...]
>>
>> Thanks, Drew. As 'base-address' is dummy address and invalid. So we
>> would note it as suggestion. With your suggestion integrated, how about
>> something like this?
>>
>> ==============================================================================
>> 4 - Empty memory nodes
>> ==============================================================================
>>
>> Empty memory nodes, which no memory resides in, are allowed. The 'length'
>> field of the 'reg' property is zero, but the 'base-address' is a dummy
>> address and invalid. The 'base-address' could be the summation of highest
>> memory address plus the NUMA node ID. However, the NUMA node IDs and
>> distance maps are still valid and memory may be added into them through
>> hotplug afterwards.
>>
>> :
>>
>> memory@0 {
>> device_type = "memory";
>> reg = <0x0 0x0 0x0 0x80000000>;
>> numa-node-id = <0>;
>> };
>>
>> memory@...0000000 {
>> device_type = "memory";
>> reg = <0x0 0x80000000 0x0 0x80000000>;
>> numa-node-id = <1>;
>> };
>>
>> /* Empty memory node */
>> memory@...00000002 {
>> device_type = "memory";
>> reg = <0x1 0x2 0x0 0x0>;
>> numa-node-id = <2>;
>> };
>>
>> /* Empty memory node */
>> memory@...00000003 {
>> device_type = "memory";
>> reg = <0x1 0x3 0x0 0x0>;
>> numa-node-id = <3>;
>> };
>>
>> :
>>
>> Thanks,
>> Gavin
>>
>>
>
> Looks good to me.
>
Thanks, Drew. v5 was posted to reflect changes, but I don't
include Randy's r-b because too much changes introduced.
Hopefully, Randy can take a look on v5 when getting a chance.
Thanks,
Gavin
Powered by blists - more mailing lists