linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] Documentation, dt, numa: Add note to empty NUMA node
@ 2021-06-25  5:23 Gavin Shan
  2021-06-25  5:29 ` Randy Dunlap
  2021-06-25  7:02 ` Andrew Jones
  0 siblings, 2 replies; 9+ messages in thread
From: Gavin Shan @ 2021-06-25  5:23 UTC (permalink / raw)
  To: devicetree; +Cc: linux-kernel, rdunlap, robh+dt, drjones, shan.gavin

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@redhat.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.
+
 	memory@c00000 {
 		device_type = "memory";
 		reg = <0x0 0xc00000 0x0 0x80000000>;
-- 
2.23.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] Documentation, dt, numa: Add note to empty NUMA node
  2021-06-25  5:23 [PATCH v4] Documentation, dt, numa: Add note to empty NUMA node Gavin Shan
@ 2021-06-25  5:29 ` Randy Dunlap
  2021-06-25  7:02 ` Andrew Jones
  1 sibling, 0 replies; 9+ messages in thread
From: Randy Dunlap @ 2021-06-25  5:29 UTC (permalink / raw)
  To: Gavin Shan, devicetree; +Cc: linux-kernel, robh+dt, drjones, shan.gavin

On 6/24/21 10:23 PM, 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@redhat.com>

Acked-by: Randy Dunlap <rdunlap@infradead.org>

Thanks.

> ---
>  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.
> +
>  	memory@c00000 {
>  		device_type = "memory";
>  		reg = <0x0 0xc00000 0x0 0x80000000>;
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] Documentation, dt, numa: Add note to empty NUMA node
  2021-06-25  5:23 [PATCH v4] Documentation, dt, numa: Add note to empty NUMA node Gavin Shan
  2021-06-25  5:29 ` Randy Dunlap
@ 2021-06-25  7:02 ` Andrew Jones
  2021-06-25  7:06   ` Andrew Jones
  1 sibling, 1 reply; 9+ messages in thread
From: Andrew Jones @ 2021-06-25  7:02 UTC (permalink / raw)
  To: Gavin Shan; +Cc: devicetree, linux-kernel, rdunlap, robh+dt, shan.gavin

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@redhat.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
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.

Thanks,
drew

> +
>  	memory@c00000 {
>  		device_type = "memory";
>  		reg = <0x0 0xc00000 0x0 0x80000000>;
> -- 
> 2.23.0
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] Documentation, dt, numa: Add note to empty NUMA node
  2021-06-25  7:02 ` Andrew Jones
@ 2021-06-25  7:06   ` Andrew Jones
  2021-06-25  8:36     ` Gavin Shan
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Jones @ 2021-06-25  7:06 UTC (permalink / raw)
  To: Gavin Shan; +Cc: devicetree, linux-kernel, rdunlap, robh+dt, shan.gavin

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@redhat.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.
> 
> Thanks,
> drew
> 
> > +
> >  	memory@c00000 {
> >  		device_type = "memory";
> >  		reg = <0x0 0xc00000 0x0 0x80000000>;
> > -- 
> > 2.23.0
> > 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] Documentation, dt, numa: Add note to empty NUMA node
  2021-06-25  7:06   ` Andrew Jones
@ 2021-06-25  8:36     ` Gavin Shan
  2021-06-25 10:18       ` Andrew Jones
  0 siblings, 1 reply; 9+ messages in thread
From: Gavin Shan @ 2021-06-25  8:36 UTC (permalink / raw)
  To: Andrew Jones; +Cc: devicetree, linux-kernel, rdunlap, robh+dt, shan.gavin

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@redhat.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.

--- a/Documentation/devicetree/bindings/numa.txt
+++ b/Documentation/devicetree/bindings/numa.txt
@@ -103,7 +103,65 @@ Example:
  		};
  
  ==============================================================================
-4 - Example dts
+4 - Empty memory node
+==============================================================================
+
+Empty memory nodes, which no memory resides in, are allowed. The 'length'
+field of 'reg' property is zero, but 'base-address' is dummy and invalid
+for these empty memory nodes. However, the NUMA node IDs and distance maps
+for them are still valid, but memory may be added into them through hotplug
+afterwards.
+
+Example:
+
+	memory@0 {
+		device_type = "memory";
+		reg = <0x0 0x0 0x0 0x80000000>;
+		numa-node-id = <0>;
+	};
+
+	memory@0x80000000 {
+		device_type = "memory";
+		reg = <0x0 0x80000000 0x0 0x80000000>;
+		numa-node-id = <1>;
+	};
+
+	/* Empty memory node */
+	memory@0x100000000 {
+		device_type = "memory";
+		reg = <0x1 0x0 0x0 0x0>;
+		numa-node-id = <2>;
+	};
+
+	/* Empty memory node */
+	memory@0x180000000 {
+		device_type = "memory";
+		reg = <0x1 0x80000000 0x0 0x0>;
+		numa-node-id = <3>;
+	};
+
+	distance-map {
+		compatible = "numa-distance-map-v1";
+		distance-matrix = <0 0  10>,
+				  <0 1  20>,
+				  <0 2  40>,
+				  <0 3  20>,
+				  <1 0  20>,
+				  <1 1  10>,
+				  <1 2  20>,
+				  <1 3  40>,
+				  <2 0  40>,
+				  <2 1  20>,
+				  <2 2  10>,
+				  <2 3  20>,
+				  <3 0  20>,
+				  <3 1  40>,
+				  <3 2  20>,
+				  <3 3  10>;
+	};
+
+==============================================================================
+5 - Example dts
  ==============================================================================
  
  Dual socket system consists of 2 boards connected through ccn bus and


Thanks,
Gavin



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] Documentation, dt, numa: Add note to empty NUMA node
  2021-06-25  8:36     ` Gavin Shan
@ 2021-06-25 10:18       ` Andrew Jones
  2021-06-25 11:14         ` Gavin Shan
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Jones @ 2021-06-25 10:18 UTC (permalink / raw)
  To: Gavin Shan; +Cc: devicetree, linux-kernel, rdunlap, robh+dt, shan.gavin

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@redhat.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.

> 
> --- a/Documentation/devicetree/bindings/numa.txt
> +++ b/Documentation/devicetree/bindings/numa.txt
> @@ -103,7 +103,65 @@ Example:
>  		};
>  ==============================================================================
> -4 - Example dts
> +4 - Empty memory node

nodes

> +==============================================================================
> +
> +Empty memory nodes, which no memory resides in, are allowed. The 'length'
> +field of 'reg' property is zero, but 'base-address' is dummy and invalid

 is a dummy address and is invalid.

> +for these empty memory nodes.

Can drop this "for these empty memory nodes" that's clear from the
context.

> However, the NUMA node IDs and distance maps
> +for them are still valid, but memory may be added into them through hotplug
   ^ drop 'for them'         ^ Again, this should be "and". "but" is a
contrastive conjunction. We want to express that the nodes are valid *and*
may have memory hotplugged later.

> +afterwards.

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.

> +
> +Example:
> +
> +	memory@0 {
> +		device_type = "memory";
> +		reg = <0x0 0x0 0x0 0x80000000>;
> +		numa-node-id = <0>;
> +	};
> +
> +	memory@0x80000000 {
> +		device_type = "memory";
> +		reg = <0x0 0x80000000 0x0 0x80000000>;
> +		numa-node-id = <1>;
> +	};
> +
> +	/* Empty memory node */

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@0x100000000 {
> +		device_type = "memory";
> +		reg = <0x1 0x0 0x0 0x0>;
> +		numa-node-id = <2>;
> +	};
> +
> +	/* Empty memory node */
> +	memory@0x180000000 {
> +		device_type = "memory";
> +		reg = <0x1 0x80000000 0x0 0x0>;
> +		numa-node-id = <3>;
> +	};
> +
> +	distance-map {
> +		compatible = "numa-distance-map-v1";
> +		distance-matrix = <0 0  10>,
> +				  <0 1  20>,
> +				  <0 2  40>,
> +				  <0 3  20>,
> +				  <1 0  20>,
> +				  <1 1  10>,
> +				  <1 2  20>,
> +				  <1 3  40>,
> +				  <2 0  40>,
> +				  <2 1  20>,
> +				  <2 2  10>,
> +				  <2 3  20>,
> +				  <3 0  20>,
> +				  <3 1  40>,
> +				  <3 2  20>,
> +				  <3 3  10>;
> +	};
> +
> +==============================================================================
> +5 - Example dts
>  ==============================================================================
>  Dual socket system consists of 2 boards connected through ccn bus and
> 
> 
> Thanks,
> Gavin
> 
>

Thanks,
drew


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] Documentation, dt, numa: Add note to empty NUMA node
  2021-06-25 10:18       ` Andrew Jones
@ 2021-06-25 11:14         ` Gavin Shan
  2021-06-28  8:34           ` Andrew Jones
  0 siblings, 1 reply; 9+ messages in thread
From: Gavin Shan @ 2021-06-25 11:14 UTC (permalink / raw)
  To: Andrew Jones; +Cc: devicetree, linux-kernel, rdunlap, robh+dt, shan.gavin

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@redhat.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@0x100000000 {
>> +		device_type = "memory";
>> +		reg = <0x1 0x0 0x0 0x0>;
>> +		numa-node-id = <2>;
>> +	};
>> +
>> +	/* Empty memory node */
>> +	memory@0x180000000 {
>> +		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@0x80000000 {
                 device_type = "memory";
                 reg = <0x0 0x80000000 0x0 0x80000000>;
                 numa-node-id = <1>;
         };

         /* Empty memory node */
         memory@0x100000002 {
                 device_type = "memory";
                 reg = <0x1 0x2 0x0 0x0>;
                 numa-node-id = <2>;
         };

         /* Empty memory node */
         memory@0x100000003 {
                 device_type = "memory";
                 reg = <0x1 0x3 0x0 0x0>;
                 numa-node-id = <3>;
         };

         :

Thanks,
Gavin



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] Documentation, dt, numa: Add note to empty NUMA node
  2021-06-25 11:14         ` Gavin Shan
@ 2021-06-28  8:34           ` Andrew Jones
  2021-06-28  9:38             ` Gavin Shan
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Jones @ 2021-06-28  8:34 UTC (permalink / raw)
  To: Gavin Shan; +Cc: devicetree, linux-kernel, rdunlap, robh+dt, shan.gavin

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@redhat.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@0x100000000 {
> > > +		device_type = "memory";
> > > +		reg = <0x1 0x0 0x0 0x0>;
> > > +		numa-node-id = <2>;
> > > +	};
> > > +
> > > +	/* Empty memory node */
> > > +	memory@0x180000000 {
> > > +		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@0x80000000 {
>                 device_type = "memory";
>                 reg = <0x0 0x80000000 0x0 0x80000000>;
>                 numa-node-id = <1>;
>         };
> 
>         /* Empty memory node */
>         memory@0x100000002 {
>                 device_type = "memory";
>                 reg = <0x1 0x2 0x0 0x0>;
>                 numa-node-id = <2>;
>         };
> 
>         /* Empty memory node */
>         memory@0x100000003 {
>                 device_type = "memory";
>                 reg = <0x1 0x3 0x0 0x0>;
>                 numa-node-id = <3>;
>         };
> 
>         :
> 
> Thanks,
> Gavin
> 
>

Looks good to me.

Thanks,
drew 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] Documentation, dt, numa: Add note to empty NUMA node
  2021-06-28  8:34           ` Andrew Jones
@ 2021-06-28  9:38             ` Gavin Shan
  0 siblings, 0 replies; 9+ messages in thread
From: Gavin Shan @ 2021-06-28  9:38 UTC (permalink / raw)
  To: Andrew Jones; +Cc: devicetree, linux-kernel, rdunlap, robh+dt, shan.gavin

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@redhat.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@0x100000000 {
>>>> +		device_type = "memory";
>>>> +		reg = <0x1 0x0 0x0 0x0>;
>>>> +		numa-node-id = <2>;
>>>> +	};
>>>> +
>>>> +	/* Empty memory node */
>>>> +	memory@0x180000000 {
>>>> +		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@0x80000000 {
>>                  device_type = "memory";
>>                  reg = <0x0 0x80000000 0x0 0x80000000>;
>>                  numa-node-id = <1>;
>>          };
>>
>>          /* Empty memory node */
>>          memory@0x100000002 {
>>                  device_type = "memory";
>>                  reg = <0x1 0x2 0x0 0x0>;
>>                  numa-node-id = <2>;
>>          };
>>
>>          /* Empty memory node */
>>          memory@0x100000003 {
>>                  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


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-06-28  9:38 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-25  5:23 [PATCH v4] Documentation, dt, numa: Add note to empty NUMA node Gavin Shan
2021-06-25  5:29 ` Randy Dunlap
2021-06-25  7:02 ` Andrew Jones
2021-06-25  7:06   ` Andrew Jones
2021-06-25  8:36     ` Gavin Shan
2021-06-25 10:18       ` Andrew Jones
2021-06-25 11:14         ` Gavin Shan
2021-06-28  8:34           ` Andrew Jones
2021-06-28  9:38             ` Gavin Shan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).