diff mbox

[RFC,3/3] memory/qmp: add set-memory-merge command

Message ID 1340643332-5653-4-git-send-email-lcapitulino@redhat.com
State New
Headers show

Commit Message

Luiz Capitulino June 25, 2012, 4:55 p.m. UTC
Allow to enable/disable memory merging during run-time.

This is implemented by extending the qemu_set_mem_merge() function.

To test on HMP:

  (qemu) set_memory_merge on
  (qemu) set_memory_merge off

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 exec.c           | 15 +++++++++++++++
 hmp-commands.hx  | 14 ++++++++++++++
 hmp.c            |  6 ++++++
 hmp.h            |  1 +
 monitor.c        |  6 ++++++
 osdep.h          |  2 ++
 qapi-schema.json | 18 ++++++++++++++++++
 qmp-commands.hx  |  5 +++++
 8 files changed, 67 insertions(+)

Comments

Anthony Liguori June 25, 2012, 10:05 p.m. UTC | #1
On 06/25/2012 11:55 AM, Luiz Capitulino wrote:
> Allow to enable/disable memory merging during run-time.
>
> This is implemented by extending the qemu_set_mem_merge() function.
>
> To test on HMP:
>
>    (qemu) set_memory_merge on
>    (qemu) set_memory_merge off
>
> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>

Is there a requirement to tweak KSM dynamically on a per-VM basis or was this 
just a nice to have?

I can certainly understand wanting to run your VMs without KSM but not wanting 
to have to muck with global configuration values.

I would suspect that people would either overcommit or not overcommit as a 
global decision.  Once you overcommit even one machine, you potentially put your 
whole system in danger of swap.

Regards,

Anthony Liguori

> ---
>   exec.c           | 15 +++++++++++++++
>   hmp-commands.hx  | 14 ++++++++++++++
>   hmp.c            |  6 ++++++
>   hmp.h            |  1 +
>   monitor.c        |  6 ++++++
>   osdep.h          |  2 ++
>   qapi-schema.json | 18 ++++++++++++++++++
>   qmp-commands.hx  |  5 +++++
>   8 files changed, 67 insertions(+)
>
> diff --git a/exec.c b/exec.c
> index e29b27f..1fa82b4 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2504,6 +2504,21 @@ void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev)
>
>   void qemu_set_mem_merge(bool value)
>   {
> +    const RAMBlock *block;
> +
> +    if (ram_list.merge_memory == value) {
> +        /* do nothing */
> +        return;
> +    }
> +
> +    /* XXX: report errors? */
> +    QLIST_FOREACH(block,&ram_list.blocks, next) {
> +        if (block->flags&  RAM_MERGEABLE) {
> +            qemu_madvise(block->host, block->length,
> +                         value ? QEMU_MADV_MERGEABLE : QEMU_MADV_UNMERGEABLE);
> +        }
> +    }
> +
>       ram_list.merge_memory = value;
>   }
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index f5d9d91..6ab3852 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -366,6 +366,20 @@ Wakeup guest from suspend.
>   ETEXI
>
>       {
> +        .name       = "set_memory_merge",
> +        .args_type  = "status:b",
> +        .params     = "status",
> +        .help       = "enable/disable memory merge support",
> +        .mhandler.cmd = hmp_set_memory_merge,
> +    },
> +
> +STEXI
> +@item set_memory_merge
> +@findex set_memory_merge
> +Enable/disable memory merge support
> +ETEXI
> +
> +    {
>           .name       = "gdbserver",
>           .args_type  = "device:s?",
>           .params     = "[device]",
> diff --git a/hmp.c b/hmp.c
> index b9cec1d..063cb3e 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1000,3 +1000,9 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
>       qmp_netdev_del(id,&err);
>       hmp_handle_error(mon,&err);
>   }
> +
> +void hmp_set_memory_merge(Monitor *mon, const QDict *qdict)
> +{
> +    bool enable = qdict_get_bool(qdict, "status");
> +    qmp_set_memory_merge(enable, NULL);
> +}
> diff --git a/hmp.h b/hmp.h
> index 79d138d..d884733 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -64,5 +64,6 @@ void hmp_device_del(Monitor *mon, const QDict *qdict);
>   void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
>   void hmp_netdev_add(Monitor *mon, const QDict *qdict);
>   void hmp_netdev_del(Monitor *mon, const QDict *qdict);
> +void hmp_set_memory_merge(Monitor *mon, const QDict *qdict);
>
>   #endif
> diff --git a/monitor.c b/monitor.c
> index f6107ba..c7f9015 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4813,3 +4813,9 @@ int monitor_read_block_device_key(Monitor *mon, const char *device,
>
>       return monitor_read_bdrv_key_start(mon, bs, completion_cb, opaque);
>   }
> +
> +/* FIXME: move to qmp.c, although including memory.h doesn't work there */
> +void qmp_set_memory_merge(bool enable, Error **errp)
> +{
> +    memory_global_set_merge(enable);
> +}
> diff --git a/osdep.h b/osdep.h
> index 3ea4af0..f5563c2 100644
> --- a/osdep.h
> +++ b/osdep.h
> @@ -97,8 +97,10 @@ void qemu_vfree(void *ptr);
>   #endif
>   #ifdef MADV_MERGEABLE
>   #define QEMU_MADV_MERGEABLE MADV_MERGEABLE
> +#define QEMU_MADV_UNMERGEABLE MADV_UNMERGEABLE
>   #else
>   #define QEMU_MADV_MERGEABLE QEMU_MADV_INVALID
> +#define QEMU_MADV_UNMERGEABLE QEMU_MADV_INVALID
>   #endif
>
>   #elif defined(CONFIG_POSIX_MADVISE)
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 3b6e346..0b99bc0 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1862,3 +1862,21 @@
>   # Since: 0.14.0
>   ##
>   { 'command': 'netdev_del', 'data': {'id': 'str'} }
> +
> +##
> +# @set-memory-merge:
> +#
> +# Enable or disable memory merge support. On Linux systems, this enables
> +# or disables KSM support.
> +#
> +# @enable: true (enables memory merging) or false (disables memory merging)
> +#
> +# Returns: Nothing on success
> +#
> +# Note: this command never fails. The only possible reason for a failure
> +# is when the host doesn't support memory merging, but that's not reported
> +# by this command, it will just return success.
> +#
> +# Since: 1.2.0
> +##
> +{ 'command': 'set-memory-merge', 'data': { 'enable': 'bool' } }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 2e1a38e..74530b9 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2209,3 +2209,8 @@ EQMP
>           .args_type  = "implements:s?,abstract:b?",
>           .mhandler.cmd_new = qmp_marshal_input_qom_list_types,
>       },
> +    {
> +        .name       = "set-memory-merge",
> +        .args_type  = "enable:b",
> +        .mhandler.cmd_new = qmp_marshal_input_set_memory_merge,
> +    },
Daniel P. Berrangé June 26, 2012, 9:25 a.m. UTC | #2
On Mon, Jun 25, 2012 at 05:05:45PM -0500, Anthony Liguori wrote:
> On 06/25/2012 11:55 AM, Luiz Capitulino wrote:
> >Allow to enable/disable memory merging during run-time.
> >
> >This is implemented by extending the qemu_set_mem_merge() function.
> >
> >To test on HMP:
> >
> >   (qemu) set_memory_merge on
> >   (qemu) set_memory_merge off
> >
> >Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> 
> Is there a requirement to tweak KSM dynamically on a per-VM basis or
> was this just a nice to have?
> 
> I can certainly understand wanting to run your VMs without KSM but
> not wanting to have to muck with global configuration values.
> 
> I would suspect that people would either overcommit or not
> overcommit as a global decision.  Once you overcommit even one
> machine, you potentially put your whole system in danger of swap.

I tend to agree, this monitor commands feels like a feature without a
clear usecase.

Daniel
Luiz Capitulino June 26, 2012, 12:53 p.m. UTC | #3
On Tue, 26 Jun 2012 10:25:22 +0100
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> On Mon, Jun 25, 2012 at 05:05:45PM -0500, Anthony Liguori wrote:
> > On 06/25/2012 11:55 AM, Luiz Capitulino wrote:
> > >Allow to enable/disable memory merging during run-time.
> > >
> > >This is implemented by extending the qemu_set_mem_merge() function.
> > >
> > >To test on HMP:
> > >
> > >   (qemu) set_memory_merge on
> > >   (qemu) set_memory_merge off
> > >
> > >Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> > 
> > Is there a requirement to tweak KSM dynamically on a per-VM basis or
> > was this just a nice to have?
> > 
> > I can certainly understand wanting to run your VMs without KSM but
> > not wanting to have to muck with global configuration values.
> > 
> > I would suspect that people would either overcommit or not
> > overcommit as a global decision.  Once you overcommit even one
> > machine, you potentially put your whole system in danger of swap.
> 
> I tend to agree, this monitor commands feels like a feature without a
> clear usecase.

Well, the request for it came from libvirt and I implemented it without
wondering much about it.

But if you think it's not worth it, I'll just drop it.
Daniel P. Berrangé June 26, 2012, 1:26 p.m. UTC | #4
On Tue, Jun 26, 2012 at 09:53:32AM -0300, Luiz Capitulino wrote:
> On Tue, 26 Jun 2012 10:25:22 +0100
> "Daniel P. Berrange" <berrange@redhat.com> wrote:
> 
> > On Mon, Jun 25, 2012 at 05:05:45PM -0500, Anthony Liguori wrote:
> > > On 06/25/2012 11:55 AM, Luiz Capitulino wrote:
> > > >Allow to enable/disable memory merging during run-time.
> > > >
> > > >This is implemented by extending the qemu_set_mem_merge() function.
> > > >
> > > >To test on HMP:
> > > >
> > > >   (qemu) set_memory_merge on
> > > >   (qemu) set_memory_merge off
> > > >
> > > >Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> > > 
> > > Is there a requirement to tweak KSM dynamically on a per-VM basis or
> > > was this just a nice to have?
> > > 
> > > I can certainly understand wanting to run your VMs without KSM but
> > > not wanting to have to muck with global configuration values.
> > > 
> > > I would suspect that people would either overcommit or not
> > > overcommit as a global decision.  Once you overcommit even one
> > > machine, you potentially put your whole system in danger of swap.
> > 
> > I tend to agree, this monitor commands feels like a feature without a
> > clear usecase.
> 
> Well, the request for it came from libvirt and I implemented it without
> wondering much about it.

Hmm, I think that must have been a mis-understanding soemwhere, because
I'm not aware of having asked for a monitor command! All we desire for
libvirt is for the custom command line argument hack that was done in
RHEL KVM trees, to be turned into a upstream accepted command line arg.

> But if you think it's not worth it, I'll just drop it.

Sounds good :-)

Daniel
diff mbox

Patch

diff --git a/exec.c b/exec.c
index e29b27f..1fa82b4 100644
--- a/exec.c
+++ b/exec.c
@@ -2504,6 +2504,21 @@  void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev)
 
 void qemu_set_mem_merge(bool value)
 {
+    const RAMBlock *block;
+
+    if (ram_list.merge_memory == value) {
+        /* do nothing */
+        return;
+    }
+
+    /* XXX: report errors? */
+    QLIST_FOREACH(block, &ram_list.blocks, next) {
+        if (block->flags & RAM_MERGEABLE) {
+            qemu_madvise(block->host, block->length,
+                         value ? QEMU_MADV_MERGEABLE : QEMU_MADV_UNMERGEABLE);
+        }
+    }
+
     ram_list.merge_memory = value;
 }
 
diff --git a/hmp-commands.hx b/hmp-commands.hx
index f5d9d91..6ab3852 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -366,6 +366,20 @@  Wakeup guest from suspend.
 ETEXI
 
     {
+        .name       = "set_memory_merge",
+        .args_type  = "status:b",
+        .params     = "status",
+        .help       = "enable/disable memory merge support",
+        .mhandler.cmd = hmp_set_memory_merge,
+    },
+
+STEXI
+@item set_memory_merge
+@findex set_memory_merge
+Enable/disable memory merge support
+ETEXI
+
+    {
         .name       = "gdbserver",
         .args_type  = "device:s?",
         .params     = "[device]",
diff --git a/hmp.c b/hmp.c
index b9cec1d..063cb3e 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1000,3 +1000,9 @@  void hmp_netdev_del(Monitor *mon, const QDict *qdict)
     qmp_netdev_del(id, &err);
     hmp_handle_error(mon, &err);
 }
+
+void hmp_set_memory_merge(Monitor *mon, const QDict *qdict)
+{
+    bool enable = qdict_get_bool(qdict, "status");
+    qmp_set_memory_merge(enable, NULL);
+}
diff --git a/hmp.h b/hmp.h
index 79d138d..d884733 100644
--- a/hmp.h
+++ b/hmp.h
@@ -64,5 +64,6 @@  void hmp_device_del(Monitor *mon, const QDict *qdict);
 void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
 void hmp_netdev_add(Monitor *mon, const QDict *qdict);
 void hmp_netdev_del(Monitor *mon, const QDict *qdict);
+void hmp_set_memory_merge(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/monitor.c b/monitor.c
index f6107ba..c7f9015 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4813,3 +4813,9 @@  int monitor_read_block_device_key(Monitor *mon, const char *device,
 
     return monitor_read_bdrv_key_start(mon, bs, completion_cb, opaque);
 }
+
+/* FIXME: move to qmp.c, although including memory.h doesn't work there */
+void qmp_set_memory_merge(bool enable, Error **errp)
+{
+    memory_global_set_merge(enable);
+}
diff --git a/osdep.h b/osdep.h
index 3ea4af0..f5563c2 100644
--- a/osdep.h
+++ b/osdep.h
@@ -97,8 +97,10 @@  void qemu_vfree(void *ptr);
 #endif
 #ifdef MADV_MERGEABLE
 #define QEMU_MADV_MERGEABLE MADV_MERGEABLE
+#define QEMU_MADV_UNMERGEABLE MADV_UNMERGEABLE
 #else
 #define QEMU_MADV_MERGEABLE QEMU_MADV_INVALID
+#define QEMU_MADV_UNMERGEABLE QEMU_MADV_INVALID
 #endif
 
 #elif defined(CONFIG_POSIX_MADVISE)
diff --git a/qapi-schema.json b/qapi-schema.json
index 3b6e346..0b99bc0 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1862,3 +1862,21 @@ 
 # Since: 0.14.0
 ##
 { 'command': 'netdev_del', 'data': {'id': 'str'} }
+
+##
+# @set-memory-merge:
+#
+# Enable or disable memory merge support. On Linux systems, this enables
+# or disables KSM support.
+#
+# @enable: true (enables memory merging) or false (disables memory merging)
+#
+# Returns: Nothing on success
+#
+# Note: this command never fails. The only possible reason for a failure
+# is when the host doesn't support memory merging, but that's not reported
+# by this command, it will just return success.
+#
+# Since: 1.2.0
+##
+{ 'command': 'set-memory-merge', 'data': { 'enable': 'bool' } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 2e1a38e..74530b9 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2209,3 +2209,8 @@  EQMP
         .args_type  = "implements:s?,abstract:b?",
         .mhandler.cmd_new = qmp_marshal_input_qom_list_types,
     },
+    {
+        .name       = "set-memory-merge",
+        .args_type  = "enable:b",
+        .mhandler.cmd_new = qmp_marshal_input_set_memory_merge,
+    },