[PATCH] [query-memory-size-summary] Report page size

Andrei Gudkov via posted 1 patch 11 months, 3 weeks ago
Failed in applying to current master (apply log)
Maintainers: Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
qapi/machine.json          | 8 ++++++--
hw/core/machine-qmp-cmds.c | 3 +++
2 files changed, 9 insertions(+), 2 deletions(-)
[PATCH] [query-memory-size-summary] Report page size
Posted by Andrei Gudkov via 11 months, 3 weeks ago
Some commands (query-migrate and calc-dirty-rate) report values
in units of pages. However, currently the only place where we can
get page size is through query-migrate and only after migration
has started. query-memory-size-summary seems like an appropritate
place where it should be reported instead.

Signed-off-by: Andrei Gudkov <gudkov.andrei@huawei.com>
---
 qapi/machine.json          | 8 ++++++--
 hw/core/machine-qmp-cmds.c | 3 +++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/qapi/machine.json b/qapi/machine.json
index 37660d8f2a..10b2d686da 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1125,10 +1125,13 @@
 #     field is omitted if target doesn't support memory hotplug (i.e.
 #     CONFIG_MEM_DEVICE not defined at build time).
 #
+# @page-size: the number of bytes per target page (since 8.1)
+#
 # Since: 2.11
 ##
 { 'struct': 'MemoryInfo',
-  'data'  : { 'base-memory': 'size', '*plugged-memory': 'size' } }
+  'data'  : { 'base-memory': 'size', '*plugged-memory': 'size',
+              'page-size': 'size' } }
 
 ##
 # @query-memory-size-summary:
@@ -1139,7 +1142,8 @@
 # Example:
 #
 # -> { "execute": "query-memory-size-summary" }
-# <- { "return": { "base-memory": 4294967296, "plugged-memory": 0 } }
+# <- { "return": { "page-size": 4096, "base-memory": 34359738368,
+#      "plugged-memory": 0 } }
 #
 # Since: 2.11
 ##
diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index 3860a50c3b..b768ff372f 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -27,6 +27,7 @@
 #include "sysemu/numa.h"
 #include "sysemu/runstate.h"
 #include "sysemu/sysemu.h"
+#include "exec/target_page.h"
 
 /*
  * fast means: we NEVER interrupt vCPU threads to retrieve
@@ -289,6 +290,8 @@ MemoryInfo *qmp_query_memory_size_summary(Error **errp)
     mem_info->has_plugged_memory =
         mem_info->plugged_memory != (uint64_t)-1;
 
+    mem_info->page_size = qemu_target_page_size();
+
     return mem_info;
 }
 
-- 
2.30.2
Re: [PATCH] [query-memory-size-summary] Report page size
Posted by Markus Armbruster 11 months, 2 weeks ago
Andrei Gudkov <gudkov.andrei@huawei.com> writes:

> Some commands (query-migrate and calc-dirty-rate) report values
> in units of pages. However, currently the only place where we can
> get page size is through query-migrate and only after migration
> has started. query-memory-size-summary seems like an appropritate
> place where it should be reported instead.
>
> Signed-off-by: Andrei Gudkov <gudkov.andrei@huawei.com>
> ---
>  qapi/machine.json          | 8 ++++++--
>  hw/core/machine-qmp-cmds.c | 3 +++
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 37660d8f2a..10b2d686da 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1125,10 +1125,13 @@
   ##
   # @MemoryInfo:
   #
   # Actual memory information in bytes.
   #
   # @base-memory: size of "base" memory specified with command line
   #     option -m.
   #
   # @plugged-memory: size of memory that can be hot-unplugged.  This
>  #     field is omitted if target doesn't support memory hotplug (i.e.
>  #     CONFIG_MEM_DEVICE not defined at build time).
>  #
> +# @page-size: the number of bytes per target page (since 8.1)

Recommend "size of target page in bytes" for consistency with the other
two sizes.

Machines may have multiple page sizes.  Should we discuss that here?

> +#
>  # Since: 2.11
>  ##
>  { 'struct': 'MemoryInfo',
> -  'data'  : { 'base-memory': 'size', '*plugged-memory': 'size' } }
> +  'data'  : { 'base-memory': 'size', '*plugged-memory': 'size',
> +              'page-size': 'size' } }
>  
>  ##
>  # @query-memory-size-summary:
> @@ -1139,7 +1142,8 @@
>  # Example:
>  #
>  # -> { "execute": "query-memory-size-summary" }
> -# <- { "return": { "base-memory": 4294967296, "plugged-memory": 0 } }
> +# <- { "return": { "page-size": 4096, "base-memory": 34359738368,
> +#      "plugged-memory": 0 } }
>  #
>  # Since: 2.11
>  ##
> diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
> index 3860a50c3b..b768ff372f 100644
> --- a/hw/core/machine-qmp-cmds.c
> +++ b/hw/core/machine-qmp-cmds.c
> @@ -27,6 +27,7 @@
>  #include "sysemu/numa.h"
>  #include "sysemu/runstate.h"
>  #include "sysemu/sysemu.h"
> +#include "exec/target_page.h"
>  
>  /*
>   * fast means: we NEVER interrupt vCPU threads to retrieve
> @@ -289,6 +290,8 @@ MemoryInfo *qmp_query_memory_size_summary(Error **errp)
>      mem_info->has_plugged_memory =
>          mem_info->plugged_memory != (uint64_t)-1;
>  
> +    mem_info->page_size = qemu_target_page_size();
> +
>      return mem_info;
>  }
Re: [PATCH] [query-memory-size-summary] Report page size
Posted by Markus Armbruster 11 months, 1 week ago
One more thing: the convention for patch subjects is

    subsystem: purpose

Suggest

    qmp: Report page size in query-memory-size-summary
Re: [PATCH] [query-memory-size-summary] Report page size
Posted by Eric Blake 11 months, 3 weeks ago
On Fri, May 19, 2023 at 11:10:30AM +0300, Andrei Gudkov wrote:
> Some commands (query-migrate and calc-dirty-rate) report values
> in units of pages. However, currently the only place where we can
> get page size is through query-migrate and only after migration
> has started. query-memory-size-summary seems like an appropritate

appropriate

> place where it should be reported instead.
> 
> Signed-off-by: Andrei Gudkov <gudkov.andrei@huawei.com>
> ---
>  qapi/machine.json          | 8 ++++++--
>  hw/core/machine-qmp-cmds.c | 3 +++
>  2 files changed, 9 insertions(+), 2 deletions(-)
>

Your reasoning for exposing this information in more than one place,
because the new place is lighter-weight to access, makes sense.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org