All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] A pair of HMAT docs fixes
@ 2020-06-10 13:17 Michal Privoznik
  2020-06-10 13:17 ` [PATCH v2 1/2] qemu-options.hx: Mark all hmat-cache attributes required Michal Privoznik
  2020-06-10 13:17 ` [PATCH v2 2/2] qemu-options.hx: Document hmat-lb and hmat-cache order Michal Privoznik
  0 siblings, 2 replies; 10+ messages in thread
From: Michal Privoznik @ 2020-06-10 13:17 UTC (permalink / raw)
  To: qemu-devel

Technically, this is a v2 of:

https://lists.nongnu.org/archive/html/qemu-devel/2020-05/msg08316.html

But as it turned out during the review of v1, we don't need to change
the code rather than documentation.

Michal Privoznik (2):
  qemu-options.hx: Mark all hmat-cache attributes required
  qemu-options.hx: Document hmat-lb and hmat-cache order

 qemu-options.hx | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

-- 
2.26.2



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

* [PATCH v2 1/2] qemu-options.hx: Mark all hmat-cache attributes required
  2020-06-10 13:17 [PATCH v2 0/2] A pair of HMAT docs fixes Michal Privoznik
@ 2020-06-10 13:17 ` Michal Privoznik
  2020-06-15  8:00   ` Markus Armbruster
  2020-06-16 12:00   ` Igor Mammedov
  2020-06-10 13:17 ` [PATCH v2 2/2] qemu-options.hx: Document hmat-lb and hmat-cache order Michal Privoznik
  1 sibling, 2 replies; 10+ messages in thread
From: Michal Privoznik @ 2020-06-10 13:17 UTC (permalink / raw)
  To: qemu-devel

The documentation to `-numa hmat-cache` says that @node-id, @size
and @level are the only required attributes. The rest
(@associativity, @policy and @line) is optional. Well, not quite
- if I try to start QEMU with only the three required attributes
defined the QAPI code is complaining about associativity missing.

According to QAPI all attributes are required. Make the docs
reflect that.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 qemu-options.hx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 93bde2bbc8..b1a399079a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -188,7 +188,7 @@ DEF("numa", HAS_ARG, QEMU_OPTION_numa,
     "-numa dist,src=source,dst=destination,val=distance\n"
     "-numa cpu,node-id=node[,socket-id=x][,core-id=y][,thread-id=z]\n"
     "-numa hmat-lb,initiator=node,target=node,hierarchy=memory|first-level|second-level|third-level,data-type=access-latency|read-latency|write-latency[,latency=lat][,bandwidth=bw]\n"
-    "-numa hmat-cache,node-id=node,size=size,level=level[,associativity=none|direct|complex][,policy=none|write-back|write-through][,line=size]\n",
+    "-numa hmat-cache,node-id=node,size=size,level=level,associativity=none|direct|complex,policy=none|write-back|write-through,line=size\n",
     QEMU_ARCH_ALL)
 SRST
 ``-numa node[,mem=size][,cpus=firstcpu[-lastcpu]][,nodeid=node][,initiator=initiator]``
@@ -201,7 +201,7 @@ SRST
   \ 
 ``-numa hmat-lb,initiator=node,target=node,hierarchy=hierarchy,data-type=tpye[,latency=lat][,bandwidth=bw]``
   \ 
-``-numa hmat-cache,node-id=node,size=size,level=level[,associativity=str][,policy=str][,line=size]``
+``-numa hmat-cache,node-id=node,size=size,level=level,associativity=str,policy=str,line=size``
     Define a NUMA node and assign RAM and VCPUs to it. Set the NUMA
     distance from a source node to a destination node. Set the ACPI
     Heterogeneous Memory Attributes for the given nodes.
-- 
2.26.2



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

* [PATCH v2 2/2] qemu-options.hx: Document hmat-lb and hmat-cache order
  2020-06-10 13:17 [PATCH v2 0/2] A pair of HMAT docs fixes Michal Privoznik
  2020-06-10 13:17 ` [PATCH v2 1/2] qemu-options.hx: Mark all hmat-cache attributes required Michal Privoznik
@ 2020-06-10 13:17 ` Michal Privoznik
  2020-06-15  8:02   ` Markus Armbruster
  2020-06-16 12:01   ` Igor Mammedov
  1 sibling, 2 replies; 10+ messages in thread
From: Michal Privoznik @ 2020-06-10 13:17 UTC (permalink / raw)
  To: qemu-devel

To simplify internal implementation the hmat-cache parsing code
expects hmat-lb to be already parsed. This means, that hmat-lb
arguments must come before hmat-cache. Document this restriction
so that management applications can follow it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 qemu-options.hx | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/qemu-options.hx b/qemu-options.hx
index b1a399079a..3fe9e6d6a0 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -319,6 +319,9 @@ SRST
     'none/direct(direct-mapped)/complex(complex cache indexing)'. policy
     is the write policy. line is the cache Line size in bytes.
 
+    Please note, that due to internal implementation, '\ ``hmat-cache``\ '
+    must be configured only after '\ ``hmat-lb``\ ' option.
+
     For example, the following options describe 2 NUMA nodes. Node 0 has
     2 cpus and a ram, node 1 has only a ram. The processors in node 0
     access memory in node 0 with access-latency 5 nanoseconds,
-- 
2.26.2



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

* Re: [PATCH v2 1/2] qemu-options.hx: Mark all hmat-cache attributes required
  2020-06-10 13:17 ` [PATCH v2 1/2] qemu-options.hx: Mark all hmat-cache attributes required Michal Privoznik
@ 2020-06-15  8:00   ` Markus Armbruster
  2020-06-16  6:52     ` Michal Privoznik
  2020-06-16 12:00   ` Igor Mammedov
  1 sibling, 1 reply; 10+ messages in thread
From: Markus Armbruster @ 2020-06-15  8:00 UTC (permalink / raw)
  To: Michal Privoznik
  Cc: Igor Mammedov, Liu Jingqi, Tao Xu, qemu-devel, Michael S. Tsirkin

Cc: the people involved in commit c412a48d4d "numa: Extend CLI to
provide memory side cache information".

Michal Privoznik <mprivozn@redhat.com> writes:

> The documentation to `-numa hmat-cache` says that @node-id, @size
> and @level are the only required attributes. The rest
> (@associativity, @policy and @line) is optional. Well, not quite
> - if I try to start QEMU with only the three required attributes
> defined the QAPI code is complaining about associativity missing.

Only because @associativity visited first.

> According to QAPI all attributes are required. Make the docs
> reflect that.

Correct.

> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  qemu-options.hx | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 93bde2bbc8..b1a399079a 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -188,7 +188,7 @@ DEF("numa", HAS_ARG, QEMU_OPTION_numa,
>      "-numa dist,src=source,dst=destination,val=distance\n"
>      "-numa cpu,node-id=node[,socket-id=x][,core-id=y][,thread-id=z]\n"
>      "-numa hmat-lb,initiator=node,target=node,hierarchy=memory|first-level|second-level|third-level,data-type=access-latency|read-latency|write-latency[,latency=lat][,bandwidth=bw]\n"
> -    "-numa hmat-cache,node-id=node,size=size,level=level[,associativity=none|direct|complex][,policy=none|write-back|write-through][,line=size]\n",
> +    "-numa hmat-cache,node-id=node,size=size,level=level,associativity=none|direct|complex,policy=none|write-back|write-through,line=size\n",
>      QEMU_ARCH_ALL)
>  SRST
>  ``-numa node[,mem=size][,cpus=firstcpu[-lastcpu]][,nodeid=node][,initiator=initiator]``
> @@ -201,7 +201,7 @@ SRST
>    \ 
>  ``-numa hmat-lb,initiator=node,target=node,hierarchy=hierarchy,data-type=tpye[,latency=lat][,bandwidth=bw]``
>    \ 
> -``-numa hmat-cache,node-id=node,size=size,level=level[,associativity=str][,policy=str][,line=size]``
> +``-numa hmat-cache,node-id=node,size=size,level=level,associativity=str,policy=str,line=size``
>      Define a NUMA node and assign RAM and VCPUs to it. Set the NUMA
>      distance from a source node to a destination node. Set the ACPI
>      Heterogeneous Memory Attributes for the given nodes.

Assuming non-optional is what we want:
Reviewed-by: Markus Armbruster <armbru@redhat.com>



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

* Re: [PATCH v2 2/2] qemu-options.hx: Document hmat-lb and hmat-cache order
  2020-06-10 13:17 ` [PATCH v2 2/2] qemu-options.hx: Document hmat-lb and hmat-cache order Michal Privoznik
@ 2020-06-15  8:02   ` Markus Armbruster
  2020-06-16  6:46     ` Michal Privoznik
  2020-06-16 12:01   ` Igor Mammedov
  1 sibling, 1 reply; 10+ messages in thread
From: Markus Armbruster @ 2020-06-15  8:02 UTC (permalink / raw)
  To: Michal Privoznik
  Cc: Igor Mammedov, Liu Jingqi, Tao Xu, qemu-devel, Michael S. Tsirkin

Michal Privoznik <mprivozn@redhat.com> writes:

> To simplify internal implementation the hmat-cache parsing code
> expects hmat-lb to be already parsed. This means, that hmat-lb
> arguments must come before hmat-cache. Document this restriction
> so that management applications can follow it.
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  qemu-options.hx | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index b1a399079a..3fe9e6d6a0 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -319,6 +319,9 @@ SRST
>      'none/direct(direct-mapped)/complex(complex cache indexing)'. policy
>      is the write policy. line is the cache Line size in bytes.
>  
> +    Please note, that due to internal implementation, '\ ``hmat-cache``\ '
> +    must be configured only after '\ ``hmat-lb``\ ' option.
> +
>      For example, the following options describe 2 NUMA nodes. Node 0 has
>      2 cpus and a ram, node 1 has only a ram. The processors in node 0
>      access memory in node 0 with access-latency 5 nanoseconds,

What happens when we do it wrong?

Can we catch doing it wrong somehow?  I figure that would be much better
than merely documenting it.



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

* Re: [PATCH v2 2/2] qemu-options.hx: Document hmat-lb and hmat-cache order
  2020-06-15  8:02   ` Markus Armbruster
@ 2020-06-16  6:46     ` Michal Privoznik
  2020-06-22  8:12       ` Markus Armbruster
  0 siblings, 1 reply; 10+ messages in thread
From: Michal Privoznik @ 2020-06-16  6:46 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Igor Mammedov, Liu Jingqi, Tao Xu, qemu-devel, Michael S. Tsirkin

On 6/15/20 10:02 AM, Markus Armbruster wrote:
> Michal Privoznik <mprivozn@redhat.com> writes:
> 
>> To simplify internal implementation the hmat-cache parsing code
>> expects hmat-lb to be already parsed. This means, that hmat-lb
>> arguments must come before hmat-cache. Document this restriction
>> so that management applications can follow it.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>   qemu-options.hx | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index b1a399079a..3fe9e6d6a0 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -319,6 +319,9 @@ SRST
>>       'none/direct(direct-mapped)/complex(complex cache indexing)'. policy
>>       is the write policy. line is the cache Line size in bytes.
>>   
>> +    Please note, that due to internal implementation, '\ ``hmat-cache``\ '
>> +    must be configured only after '\ ``hmat-lb``\ ' option.
>> +
>>       For example, the following options describe 2 NUMA nodes. Node 0 has
>>       2 cpus and a ram, node 1 has only a ram. The processors in node 0
>>       access memory in node 0 with access-latency 5 nanoseconds,
> 
> What happens when we do it wrong?
> 

We error out.

https://lists.nongnu.org/archive/html/qemu-devel/2020-05/msg08409.html

> Can we catch doing it wrong somehow?  I figure that would be much better
> than merely documenting it.
> 

Sure, but that would require some more code (according to Igor's e-mail 
and discussion linked from the linked document). And frankly, to libvirt 
it doesn't matter. For everybody else, let's document it at least and if 
somebody ever writes the missing piece we can remove the restriction 
from the docs.

Michal



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

* Re: [PATCH v2 1/2] qemu-options.hx: Mark all hmat-cache attributes required
  2020-06-15  8:00   ` Markus Armbruster
@ 2020-06-16  6:52     ` Michal Privoznik
  0 siblings, 0 replies; 10+ messages in thread
From: Michal Privoznik @ 2020-06-16  6:52 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Igor Mammedov, Liu Jingqi, Tao Xu, qemu-devel, Michael S. Tsirkin

On 6/15/20 10:00 AM, Markus Armbruster wrote:
> Cc: the people involved in commit c412a48d4d "numa: Extend CLI to
> provide memory side cache information".
> 
> Michal Privoznik <mprivozn@redhat.com> writes:
> 
>> The documentation to `-numa hmat-cache` says that @node-id, @size
>> and @level are the only required attributes. The rest
>> (@associativity, @policy and @line) is optional. Well, not quite
>> - if I try to start QEMU with only the three required attributes
>> defined the QAPI code is complaining about associativity missing.
> 
> Only because @associativity visited first.
> 
>> According to QAPI all attributes are required. Make the docs
>> reflect that.
> 
> Correct.
> 
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>   qemu-options.hx | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>

> 
> Assuming non-optional is what we want:
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 

Indeed, it is:

https://lists.nongnu.org/archive/html/qemu-devel/2020-05/msg08411.html

Thanks,
Michal



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

* Re: [PATCH v2 1/2] qemu-options.hx: Mark all hmat-cache attributes required
  2020-06-10 13:17 ` [PATCH v2 1/2] qemu-options.hx: Mark all hmat-cache attributes required Michal Privoznik
  2020-06-15  8:00   ` Markus Armbruster
@ 2020-06-16 12:00   ` Igor Mammedov
  1 sibling, 0 replies; 10+ messages in thread
From: Igor Mammedov @ 2020-06-16 12:00 UTC (permalink / raw)
  To: Michal Privoznik; +Cc: qemu-devel

On Wed, 10 Jun 2020 15:17:34 +0200
Michal Privoznik <mprivozn@redhat.com> wrote:

> The documentation to `-numa hmat-cache` says that @node-id, @size
> and @level are the only required attributes. The rest
> (@associativity, @policy and @line) is optional. Well, not quite
> - if I try to start QEMU with only the three required attributes
> defined the QAPI code is complaining about associativity missing.
> 
> According to QAPI all attributes are required. Make the docs
> reflect that.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  qemu-options.hx | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 93bde2bbc8..b1a399079a 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -188,7 +188,7 @@ DEF("numa", HAS_ARG, QEMU_OPTION_numa,
>      "-numa dist,src=source,dst=destination,val=distance\n"
>      "-numa cpu,node-id=node[,socket-id=x][,core-id=y][,thread-id=z]\n"
>      "-numa hmat-lb,initiator=node,target=node,hierarchy=memory|first-level|second-level|third-level,data-type=access-latency|read-latency|write-latency[,latency=lat][,bandwidth=bw]\n"
> -    "-numa hmat-cache,node-id=node,size=size,level=level[,associativity=none|direct|complex][,policy=none|write-back|write-through][,line=size]\n",
> +    "-numa hmat-cache,node-id=node,size=size,level=level,associativity=none|direct|complex,policy=none|write-back|write-through,line=size\n",
>      QEMU_ARCH_ALL)
>  SRST
>  ``-numa node[,mem=size][,cpus=firstcpu[-lastcpu]][,nodeid=node][,initiator=initiator]``
> @@ -201,7 +201,7 @@ SRST
>    \ 
>  ``-numa hmat-lb,initiator=node,target=node,hierarchy=hierarchy,data-type=tpye[,latency=lat][,bandwidth=bw]``
>    \ 
> -``-numa hmat-cache,node-id=node,size=size,level=level[,associativity=str][,policy=str][,line=size]``
> +``-numa hmat-cache,node-id=node,size=size,level=level,associativity=str,policy=str,line=size``
>      Define a NUMA node and assign RAM and VCPUs to it. Set the NUMA
>      distance from a source node to a destination node. Set the ACPI
>      Heterogeneous Memory Attributes for the given nodes.



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

* Re: [PATCH v2 2/2] qemu-options.hx: Document hmat-lb and hmat-cache order
  2020-06-10 13:17 ` [PATCH v2 2/2] qemu-options.hx: Document hmat-lb and hmat-cache order Michal Privoznik
  2020-06-15  8:02   ` Markus Armbruster
@ 2020-06-16 12:01   ` Igor Mammedov
  1 sibling, 0 replies; 10+ messages in thread
From: Igor Mammedov @ 2020-06-16 12:01 UTC (permalink / raw)
  To: Michal Privoznik; +Cc: qemu-devel

On Wed, 10 Jun 2020 15:17:35 +0200
Michal Privoznik <mprivozn@redhat.com> wrote:

> To simplify internal implementation the hmat-cache parsing code
> expects hmat-lb to be already parsed. This means, that hmat-lb
> arguments must come before hmat-cache. Document this restriction
> so that management applications can follow it.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  qemu-options.hx | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index b1a399079a..3fe9e6d6a0 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -319,6 +319,9 @@ SRST
>      'none/direct(direct-mapped)/complex(complex cache indexing)'. policy
>      is the write policy. line is the cache Line size in bytes.
>  
> +    Please note, that due to internal implementation, '\ ``hmat-cache``\ '
> +    must be configured only after '\ ``hmat-lb``\ ' option.
> +
>      For example, the following options describe 2 NUMA nodes. Node 0 has
>      2 cpus and a ram, node 1 has only a ram. The processors in node 0
>      access memory in node 0 with access-latency 5 nanoseconds,



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

* Re: [PATCH v2 2/2] qemu-options.hx: Document hmat-lb and hmat-cache order
  2020-06-16  6:46     ` Michal Privoznik
@ 2020-06-22  8:12       ` Markus Armbruster
  0 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2020-06-22  8:12 UTC (permalink / raw)
  To: Michal Privoznik
  Cc: Igor Mammedov, Tao Xu, Michael S. Tsirkin, qemu-devel, Liu Jingqi

Michal Privoznik <mprivozn@redhat.com> writes:

> On 6/15/20 10:02 AM, Markus Armbruster wrote:
>> Michal Privoznik <mprivozn@redhat.com> writes:
>>
>>> To simplify internal implementation the hmat-cache parsing code
>>> expects hmat-lb to be already parsed. This means, that hmat-lb
>>> arguments must come before hmat-cache. Document this restriction
>>> so that management applications can follow it.
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>>> ---
>>>   qemu-options.hx | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/qemu-options.hx b/qemu-options.hx
>>> index b1a399079a..3fe9e6d6a0 100644
>>> --- a/qemu-options.hx
>>> +++ b/qemu-options.hx
>>> @@ -319,6 +319,9 @@ SRST
>>>       'none/direct(direct-mapped)/complex(complex cache indexing)'. policy
>>>       is the write policy. line is the cache Line size in bytes.
>>>   +    Please note, that due to internal implementation, '\
>>> ``hmat-cache``\ '
>>> +    must be configured only after '\ ``hmat-lb``\ ' option.
>>> +
>>>       For example, the following options describe 2 NUMA nodes. Node 0 has
>>>       2 cpus and a ram, node 1 has only a ram. The processors in node 0
>>>       access memory in node 0 with access-latency 5 nanoseconds,
>>
>> What happens when we do it wrong?
>>
>
> We error out.
>
> https://lists.nongnu.org/archive/html/qemu-devel/2020-05/msg08409.html

Good.

>> Can we catch doing it wrong somehow?  I figure that would be much better
>> than merely documenting it.
>>
>
> Sure, but that would require some more code (according to Igor's
> e-mail and discussion linked from the linked document). And frankly,
> to libvirt it doesn't matter. For everybody else, let's document it at
> least and if somebody ever writes the missing piece we can remove the
> restriction from the docs.

Misunderstanding.  By "catch doing it wrong", I mean "error out when
hmat-cache is configured before hmat-lb".  I understand we do that.

Avoiding the restriction entirely would be even better, but the
maintainer obviously decided it was not worth the trouble.  I gladly
defer to the maintainer here.

Given the general undocumentedness of similar restrictions elsewhere, I
can't bring myself to care for documenting this one.  Up to the
maintainer.



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

end of thread, other threads:[~2020-06-22  8:13 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-10 13:17 [PATCH v2 0/2] A pair of HMAT docs fixes Michal Privoznik
2020-06-10 13:17 ` [PATCH v2 1/2] qemu-options.hx: Mark all hmat-cache attributes required Michal Privoznik
2020-06-15  8:00   ` Markus Armbruster
2020-06-16  6:52     ` Michal Privoznik
2020-06-16 12:00   ` Igor Mammedov
2020-06-10 13:17 ` [PATCH v2 2/2] qemu-options.hx: Document hmat-lb and hmat-cache order Michal Privoznik
2020-06-15  8:02   ` Markus Armbruster
2020-06-16  6:46     ` Michal Privoznik
2020-06-22  8:12       ` Markus Armbruster
2020-06-16 12:01   ` Igor Mammedov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.