Series comparison

-[PATCH 0/2] overcommit: introduce mem-lock-onfault
+[PATCH v3 0/4] overcommit: introduce mem-lock-onfault
 Currently, passing mem-lock=on to QEMU causes memory usage to grow by
 huge amounts:
 no memlock:
-    $ qemu-system-x86_64 -overcommit mem-lock=off
+    $ ./qemu-system-x86_64 -overcommit mem-lock=off
     $ ps -p $(pidof ./qemu-system-x86_64) -o rss=
     $ ./qemu-system-x86_64 -overcommit mem-lock=off -enable-kvm
     $ ps -p $(pidof ./qemu-system-x86_64) -o rss=
 memlock:
-    $ qemu-system-x86_64 -overcommit mem-lock=on
+    $ ./qemu-system-x86_64 -overcommit mem-lock=on
     $ ps -p $(pidof ./qemu-system-x86_64) -o rss=
     1309876
     $ ./qemu-system-x86_64 -overcommit mem-lock=on -enable-kvm
     $ ps -p $(pidof ./qemu-system-x86_64) -o rss=
 ...
 active.
 mem-lock=on helps against this (given compact_unevictable_allowed is 0),
 but the memory overhead it introduces is an undesirable side effect,
 which we can completely avoid by passing MCL_ONFAULT to mlockall, which
-is what this series allows to do with a new command line option called
+is what this series allows to do with a new option for mem-lock called
-mem-lock-onfault.
+on-fault.
 memlock-onfault:
-    $ qemu-system-x86_64 -overcommit mem-lock-onfault=on
+    $ ./qemu-system-x86_64 -overcommit mem-lock=on-fault
     $ ps -p $(pidof ./qemu-system-x86_64) -o rss=
-    $ ./qemu-system-x86_64 -overcommit mem-lock-onfault=on -enable-kvm
+    $ ./qemu-system-x86_64 -overcommit mem-lock=on-fault -enable-kvm
     $ ps -p $(pidof ./qemu-system-x86_64) -o rss=
 You may notice the memory usage is still slightly higher, in this case
 by a few megabytes over the mem-lock=off case. I was able to trace this
 down to a bug in the linux kernel with MCL_ONFAULT not being honored for
 the early process heap (with brk(2) etc.) so it is still write-faulted in
 this case, but it's still way less than it was with just the mem-lock=on.
-Daniil Tatianin (2):
+Changes since v1:
     - Don't make a separate mem-lock-onfault, add an on-fault option to mem-lock instead
 Changes since v2:
     - Move overcommit option parsing out of line
     - Make enable_mlock an enum instead
 Daniil Tatianin (4):
   os: add an ability to lock memory on_fault
-  overcommit: introduce mem-lock-onfault
+  system/vl: extract overcommit option parsing into a helper
   sysemu: introduce a new MlockState enum
   overcommit: introduce mem-lock=on-fault
+ hw/virtio/virtio-mem.c    |  2 +-
  include/sysemu/os-posix.h |  2 +-
  include/sysemu/os-win32.h |  3 ++-
- include/sysemu/sysemu.h   |  1 +
+ include/sysemu/sysemu.h   | 12 ++++++++-
- migration/postcopy-ram.c  |  4 ++--
+ migration/postcopy-ram.c  |  4 +--
- os-posix.c                | 10 ++++++++--
+ os-posix.c                | 10 ++++++--
- qemu-options.hx           | 13 ++++++++++---
+ qemu-options.hx           | 14 +++++++----
- system/globals.c          |  1 +
+ system/globals.c          | 12 ++++++++-
- system/vl.c               | 18 ++++++++++++++++--
+ system/vl.c               | 52 +++++++++++++++++++++++++++++++--------
-files changed, 41 insertions(+), 11 deletions(-)
+files changed, 87 insertions(+), 24 deletions(-)
 --
 .34.1

-[PATCH 1/2] os: add an ability to lock memory on_fault
+[PATCH v3 1/4] os: add an ability to lock memory on_fault
 This will be used in the following commits to make it possible to only
 lock memory on fault instead of right away.
+Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
+Reviewed-by: Peter Xu <peterx@redhat.com>
 Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
 ---
  include/sysemu/os-posix.h |  2 +-
  include/sysemu/os-win32.h |  3 ++-
  migration/postcopy-ram.c  |  2 +-
 ...

-New patch
+[PATCH v3 2/4] system/vl: extract overcommit option parsing into a helper
+This will be extended in the future commits, let's move it out of line
+right away so that it's easier to read.
+Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
+---
+ system/vl.c | 21 ++++++++++++++-------
+file changed, 14 insertions(+), 7 deletions(-)
+diff --git a/system/vl.c b/system/vl.c
+index XXXXXXX..XXXXXXX 100644
+--- a/system/vl.c
++++ b/system/vl.c
+@@ -XXX,XX +XXX,XX @@ static void object_option_parse(const char *str)
+     visit_free(v);
+ }
++static void overcommit_parse(const char *str)
++{
++    QemuOpts *opts;
++
++    opts = qemu_opts_parse_noisily(qemu_find_opts("overcommit"),
++                                   str, false);
++    if (!opts) {
++        exit(1);
++    }
++    enable_mlock = qemu_opt_get_bool(opts, "mem-lock", enable_mlock);
++    enable_cpu_pm = qemu_opt_get_bool(opts, "cpu-pm", enable_cpu_pm);
++}
++
+ /*
+  * Very early object creation, before the sandbox options have been activated.
+  */
+@@ -XXX,XX +XXX,XX @@ void qemu_init(int argc, char **argv)
+                 object_option_parse(optarg);
+                 break;
+             case QEMU_OPTION_overcommit:
+-                opts = qemu_opts_parse_noisily(qemu_find_opts("overcommit"),
+-                                               optarg, false);
+-                if (!opts) {
+-                    exit(1);
+-                }
+-                enable_mlock = qemu_opt_get_bool(opts, "mem-lock", enable_mlock);
+-                enable_cpu_pm = qemu_opt_get_bool(opts, "cpu-pm", enable_cpu_pm);
++                overcommit_parse(optarg);
+                 break;
+             case QEMU_OPTION_compat:
+                 {
+--
+.34.1

-New patch
+[PATCH v3 3/4] sysemu: introduce a new MlockState enum
+Replace the boolean value enable_mlock with an enum and add a helper to
+decide whether we should be calling os_mlock.
+This is a stepping stone towards introducing a new mlock mode, which
+will be the third possible state of this enum.
+Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
+---
+ hw/virtio/virtio-mem.c   |  2 +-
+ include/sysemu/sysemu.h  | 10 +++++++++-
+ migration/postcopy-ram.c |  2 +-
+ system/globals.c         |  7 ++++++-
+ system/vl.c              |  9 +++++++--
+files changed, 24 insertions(+), 6 deletions(-)
+diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
+index XXXXXXX..XXXXXXX 100644
+--- a/hw/virtio/virtio-mem.c
++++ b/hw/virtio/virtio-mem.c
+@@ -XXX,XX +XXX,XX @@ static void virtio_mem_device_realize(DeviceState *dev, Error **errp)
+         return;
+     }
+-    if (enable_mlock) {
++    if (should_mlock(mlock_state)) {
+         error_setg(errp, "Incompatible with mlock");
+         return;
+     }
+diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
+index XXXXXXX..XXXXXXX 100644
+--- a/include/sysemu/sysemu.h
++++ b/include/sysemu/sysemu.h
+@@ -XXX,XX +XXX,XX @@ extern int display_opengl;
+ extern const char *keyboard_layout;
+ extern int old_param;
+ extern uint8_t *boot_splash_filedata;
+-extern bool enable_mlock;
+ extern bool enable_cpu_pm;
+ extern QEMUClockType rtc_clock;
++typedef enum {
++    MLOCK_OFF = 0,
++    MLOCK_ON,
++} MlockState;
++
++bool should_mlock(MlockState);
++
++extern MlockState mlock_state;
++
+ #define MAX_OPTION_ROMS 16
+ typedef struct QEMUOptionRom {
+     const char *name;
+diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
+index XXXXXXX..XXXXXXX 100644
+--- a/migration/postcopy-ram.c
++++ b/migration/postcopy-ram.c
+@@ -XXX,XX +XXX,XX @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
+         mis->have_fault_thread = false;
+     }
+-    if (enable_mlock) {
++    if (should_mlock(mlock_state)) {
+         if (os_mlock(false) < 0) {
+             error_report("mlock: %s", strerror(errno));
+             /*
+diff --git a/system/globals.c b/system/globals.c
+index XXXXXXX..XXXXXXX 100644
+--- a/system/globals.c
++++ b/system/globals.c
+@@ -XXX,XX +XXX,XX @@
+ #include "sysemu/cpus.h"
+ #include "sysemu/sysemu.h"
++bool should_mlock(MlockState state)
++{
++    return state == MLOCK_ON;
++}
++
+ enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
+ int display_opengl;
+ const char* keyboard_layout;
+-bool enable_mlock;
++MlockState mlock_state;
+ bool enable_cpu_pm;
+ int autostart = 1;
+ int vga_interface_type = VGA_NONE;
+diff --git a/system/vl.c b/system/vl.c
+index XXXXXXX..XXXXXXX 100644
+--- a/system/vl.c
++++ b/system/vl.c
+@@ -XXX,XX +XXX,XX @@ static QemuOptsList qemu_run_with_opts = {
+ static void realtime_init(void)
+ {
+-    if (enable_mlock) {
++    if (should_mlock(mlock_state)) {
+         if (os_mlock(false) < 0) {
+             error_report("locking memory failed");
+             exit(1);
+@@ -XXX,XX +XXX,XX @@ static void object_option_parse(const char *str)
+ static void overcommit_parse(const char *str)
+ {
+     QemuOpts *opts;
++    bool enable_mlock;
+     opts = qemu_opts_parse_noisily(qemu_find_opts("overcommit"),
+                                    str, false);
+     if (!opts) {
+         exit(1);
+     }
+-    enable_mlock = qemu_opt_get_bool(opts, "mem-lock", enable_mlock);
++
++    enable_mlock = qemu_opt_get_bool(opts, "mem-lock",
++                                     should_mlock(mlock_state));
++    mlock_state = enable_mlock ? MLOCK_ON : MLOCK_OFF;
++
+     enable_cpu_pm = qemu_opt_get_bool(opts, "cpu-pm", enable_cpu_pm);
+ }
+--
+.34.1

-[PATCH 2/2] overcommit: introduce mem-lock-onfault
+[PATCH v3 4/4] overcommit: introduce mem-lock=on-fault
 ...
 only lock pages lazily as they're faulted by the process by using
 MCL_ONFAULT if asked.
 Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
 ---
- include/sysemu/sysemu.h  |  1 +
+ include/sysemu/sysemu.h  |  2 ++
- migration/postcopy-ram.c |  4 ++--
+ migration/postcopy-ram.c |  2 +-
- qemu-options.hx          | 13 ++++++++++---
+ qemu-options.hx          | 14 +++++++++-----
- system/globals.c         |  1 +
+ system/globals.c         |  7 ++++++-
- system/vl.c              | 18 ++++++++++++++++--
+ system/vl.c              | 34 +++++++++++++++++++++++++++-------
-files changed, 30 insertions(+), 7 deletions(-)
+files changed, 45 insertions(+), 14 deletions(-)
 diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
 index XXXXXXX..XXXXXXX 100644
 --- a/include/sysemu/sysemu.h
 +++ b/include/sysemu/sysemu.h
-@@ -XXX,XX +XXX,XX @@ extern const char *keyboard_layout;
+@@ -XXX,XX +XXX,XX @@ extern QEMUClockType rtc_clock;
- extern int old_param;
+ typedef enum {
- extern uint8_t *boot_splash_filedata;
+     MLOCK_OFF = 0,
- extern bool enable_mlock;
+     MLOCK_ON,
-+extern bool enable_mlock_onfault;
++    MLOCK_ON_FAULT,
- extern bool enable_cpu_pm;
+ } MlockState;
- extern QEMUClockType rtc_clock;
  bool should_mlock(MlockState);
 +bool is_mlock_on_fault(MlockState);
  extern MlockState mlock_state;
 diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
 index XXXXXXX..XXXXXXX 100644
 --- a/migration/postcopy-ram.c
 +++ b/migration/postcopy-ram.c
 @@ -XXX,XX +XXX,XX @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
-         mis->have_fault_thread = false;
      }
--    if (enable_mlock) {
+     if (should_mlock(mlock_state)) {
 -        if (os_mlock(false) < 0) {
-+    if (enable_mlock || enable_mlock_onfault) {
++        if (os_mlock(is_mlock_on_fault(mlock_state)) < 0) {
 +        if (os_mlock(enable_mlock_onfault) < 0) {
              error_report("mlock: %s", strerror(errno));
              /*
               * It doesn't feel right to fail at this point, we have a valid
 diff --git a/qemu-options.hx b/qemu-options.hx
 index XXXXXXX..XXXXXXX 100644
 ...
 @@ -XXX,XX +XXX,XX @@ SRST
  ERST
  DEF("overcommit", HAS_ARG, QEMU_OPTION_overcommit,
 -    "-overcommit [mem-lock=on|off][cpu-pm=on|off]\n"
-+    "-overcommit [mem-lock=on|off][mem-lock-onfault=on|off][cpu-pm=on|off]\n"
++    "-overcommit [mem-lock=on|off|on-fault][cpu-pm=on|off]\n"
      "                run qemu with overcommit hints\n"
-     "                mem-lock=on|off controls memory lock support (default: off)\n"
+-    "                mem-lock=on|off controls memory lock support (default: off)\n"
-+    "                mem-lock-onfault=on|off controls memory lock on fault support (default: off)\n"
++    "                mem-lock=on|off|on-fault controls memory lock support (default: off)\n"
      "                cpu-pm=on|off controls cpu power management (default: off)\n",
      QEMU_ARCH_ALL)
  SRST
- ``-overcommit mem-lock=on|off``
+-``-overcommit mem-lock=on|off``
 +``-overcommit mem-lock=on|off|on-fault``
    \
-+``-overcommit mem-lock-onfault=on|off``
-+  \
  ``-overcommit cpu-pm=on|off``
      Run qemu with hints about host resource overcommit. The default is
      to assume that host overcommits all resources.
      Locking qemu and guest memory can be enabled via ``mem-lock=on``
 -    (disabled by default). This works when host memory is not
 -    overcommitted and reduces the worst-case latency for guest.
-+    or ``mem-lock-onfault=on`` (disabled by default). This works when
++    or ``mem-lock=on-fault`` (disabled by default). This works when
 +    host memory is not overcommitted and reduces the worst-case latency for
 +    guest. The on-fault option is better for reducing the memory footprint
 +    since it makes allocations lazy, but the pages still get locked in place
 +    once faulted by the guest or QEMU. Note that the two options are mutually
 +    exclusive.
 ...
      for other processes on the same host cpu, but decreasing latency for
 diff --git a/system/globals.c b/system/globals.c
 index XXXXXXX..XXXXXXX 100644
 --- a/system/globals.c
 +++ b/system/globals.c
-@@ -XXX,XX +XXX,XX @@ enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
+@@ -XXX,XX +XXX,XX @@
- int display_opengl;
- const char* keyboard_layout;
+ bool should_mlock(MlockState state)
- bool enable_mlock;
+ {
-+bool enable_mlock_onfault;
+-    return state == MLOCK_ON;
- bool enable_cpu_pm;
++    return state == MLOCK_ON || state == MLOCK_ON_FAULT;
- int autostart = 1;
++}
- int vga_interface_type = VGA_NONE;
++
 +bool is_mlock_on_fault(MlockState state)
 +{
 +    return state == MLOCK_ON_FAULT;
  }
  enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
 diff --git a/system/vl.c b/system/vl.c
 index XXXXXXX..XXXXXXX 100644
 --- a/system/vl.c
 +++ b/system/vl.c
 @@ -XXX,XX +XXX,XX @@ static QemuOptsList qemu_overcommit_opts = {
+     .desc = {
+         {
              .name = "mem-lock",
-             .type = QEMU_OPT_BOOL,
+-            .type = QEMU_OPT_BOOL,
 +            .type = QEMU_OPT_STRING,
          },
-+        {
-+            .name = "mem-lock-onfault",
-+            .type = QEMU_OPT_BOOL,
-+        },
          {
              .name = "cpu-pm",
-             .type = QEMU_OPT_BOOL,
 @@ -XXX,XX +XXX,XX @@ static QemuOptsList qemu_run_with_opts = {
  static void realtime_init(void)
  {
--    if (enable_mlock) {
+     if (should_mlock(mlock_state)) {
 -        if (os_mlock(false) < 0) {
-+    if (enable_mlock || enable_mlock_onfault) {
++        if (os_mlock(is_mlock_on_fault(mlock_state)) < 0) {
 +        if (os_mlock(enable_mlock_onfault) < 0) {
              error_report("locking memory failed");
              exit(1);
          }
-@@ -XXX,XX +XXX,XX @@ void qemu_init(int argc, char **argv)
+@@ -XXX,XX +XXX,XX @@ static void object_option_parse(const char *str)
-                 if (!opts) {
+ static void overcommit_parse(const char *str)
-                     exit(1);
+ {
-                 }
+     QemuOpts *opts;
 -    bool enable_mlock;
 +    const char *mem_lock_opt;
      opts = qemu_opts_parse_noisily(qemu_find_opts("overcommit"),
                                     str, false);
@@ -XXX,XX +XXX,XX @@ static void overcommit_parse(const char *str)
          exit(1);
      }
 -    enable_mlock = qemu_opt_get_bool(opts, "mem-lock",
 -                                     should_mlock(mlock_state));
 -    mlock_state = enable_mlock ? MLOCK_ON : MLOCK_OFF;
 -
      enable_cpu_pm = qemu_opt_get_bool(opts, "cpu-pm", enable_cpu_pm);
 +
-                 enable_mlock = qemu_opt_get_bool(opts, "mem-lock", enable_mlock);
++    mem_lock_opt = qemu_opt_get(opts, "mem-lock");
-+                enable_mlock_onfault = qemu_opt_get_bool(opts,
++    if (!mem_lock_opt) {
-+                                                         "mem-lock-onfault",
++        return;
-+                                                         enable_mlock_onfault);
++    }
 +                if (enable_mlock && enable_mlock_onfault) {
 +                    error_report("mem-lock and mem-lock-onfault are mutually"
 +                                 "exclusive");
 +                    exit(1);
 +                }
 +
-                 enable_cpu_pm = qemu_opt_get_bool(opts, "cpu-pm", enable_cpu_pm);
++    if (strcmp(mem_lock_opt, "on") == 0) {
-                 break;
++        mlock_state = MLOCK_ON;
-             case QEMU_OPTION_compat:
++        return;
 +    }
 +
 +    if (strcmp(mem_lock_opt, "off") == 0) {
 +        mlock_state = MLOCK_OFF;
 +        return;
 +    }
 +
 +    if (strcmp(mem_lock_opt, "on-fault") == 0) {
 +        mlock_state = MLOCK_ON_FAULT;
 +        return;
 +    }
 +
 +    error_report("parameter 'mem-lock' expects one of "
 +                 "'on', 'off', 'on-fault'");
 +    exit(1);
  }
  /*
 --
 .34.1

Currently, passing mem-lock=on to QEMU causes memory usage to grow by
huge amounts:

no memlock:
    $ qemu-system-x86_64 -overcommit mem-lock=off
    $ ps -p $(pidof ./qemu-system-x86_64) -o rss=
    45652

$ ./qemu-system-x86_64 -overcommit mem-lock=off -enable-kvm
    $ ps -p $(pidof ./qemu-system-x86_64) -o rss=
    39756

memlock:
    $ qemu-system-x86_64 -overcommit mem-lock=on
    $ ps -p $(pidof ./qemu-system-x86_64) -o rss=
    1309876

$ ./qemu-system-x86_64 -overcommit mem-lock=on -enable-kvm
    $ ps -p $(pidof ./qemu-system-x86_64) -o rss=
    259956

This is caused by the fact that mlockall(2) automatically
write-faults every existing and future anonymous mappings in the
process right away.

One of the reasons to enable mem-lock is to protect a QEMU process'
pages from being compacted and migrated by kcompactd (which does so
by messing with a live process page tables causing thousands of TLB
flush IPIs per second) basically stealing all guest time while it's
active.

mem-lock=on helps against this (given compact_unevictable_allowed is 0),
but the memory overhead it introduces is an undesirable side effect,
which we can completely avoid by passing MCL_ONFAULT to mlockall, which
is what this series allows to do with a new command line option called
mem-lock-onfault.

memlock-onfault:
    $ qemu-system-x86_64 -overcommit mem-lock-onfault=on
    $ ps -p $(pidof ./qemu-system-x86_64) -o rss=
    54004

$ ./qemu-system-x86_64 -overcommit mem-lock-onfault=on -enable-kvm
    $ ps -p $(pidof ./qemu-system-x86_64) -o rss=
    47772

You may notice the memory usage is still slightly higher, in this case
by a few megabytes over the mem-lock=off case. I was able to trace this
down to a bug in the linux kernel with MCL_ONFAULT not being honored for
the early process heap (with brk(2) etc.) so it is still write-faulted in
this case, but it's still way less than it was with just the mem-lock=on.

Daniil Tatianin (2):
  os: add an ability to lock memory on_fault
  overcommit: introduce mem-lock-onfault

-- 
2.34.1

This will be used in the following commits to make it possible to only
lock memory on fault instead of right away.

Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
---
 include/sysemu/os-posix.h |  2 +-
 include/sysemu/os-win32.h |  3 ++-
 migration/postcopy-ram.c  |  2 +-
 os-posix.c                | 10 ++++++++--
 system/vl.c               |  2 +-
 5 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index XXXXXXX..XXXXXXX 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -XXX,XX +XXX,XX @@ bool os_set_runas(const char *user_id);
 void os_set_chroot(const char *path);
 void os_setup_limits(void);
 void os_setup_post(void);
-int os_mlock(void);
+int os_mlock(bool on_fault);
 
 /**
  * qemu_alloc_stack:
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index XXXXXXX..XXXXXXX 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -XXX,XX +XXX,XX @@ static inline bool is_daemonized(void)
     return false;
 }
 
-static inline int os_mlock(void)
+static inline int os_mlock(bool on_fault)
 {
+    (void)on_fault;
     return -ENOSYS;
 }
 
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index XXXXXXX..XXXXXXX 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -XXX,XX +XXX,XX @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
     }
 
     if (enable_mlock) {
-        if (os_mlock() < 0) {
+        if (os_mlock(false) < 0) {
             error_report("mlock: %s", strerror(errno));
             /*
              * It doesn't feel right to fail at this point, we have a valid
diff --git a/os-posix.c b/os-posix.c
index XXXXXXX..XXXXXXX 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -XXX,XX +XXX,XX @@ void os_set_line_buffering(void)
     setvbuf(stdout, NULL, _IOLBF, 0);
 }
 
-int os_mlock(void)
+int os_mlock(bool on_fault)
 {
 #ifdef HAVE_MLOCKALL
     int ret = 0;
+    int flags = MCL_CURRENT | MCL_FUTURE;
 
-    ret = mlockall(MCL_CURRENT | MCL_FUTURE);
+    if (on_fault) {
+        flags |= MCL_ONFAULT;
+    }
+
+    ret = mlockall(flags);
     if (ret < 0) {
         error_report("mlockall: %s", strerror(errno));
     }
 
     return ret;
 #else
+    (void)on_fault;
     return -ENOSYS;
 #endif
 }
diff --git a/system/vl.c b/system/vl.c
index XXXXXXX..XXXXXXX 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -XXX,XX +XXX,XX @@ static QemuOptsList qemu_run_with_opts = {
 static void realtime_init(void)
 {
     if (enable_mlock) {
-        if (os_mlock() < 0) {
+        if (os_mlock(false) < 0) {
             error_report("locking memory failed");
             exit(1);
         }
-- 
2.34.1

Locking the memory without MCL_ONFAULT instantly prefaults any mmaped
anonymous memory with a write-fault, which introduces a lot of extra
overhead in terms of memory usage when all you want to do is to prevent
kcompactd from migrating and compacting QEMU pages. Add an option to
only lock pages lazily as they're faulted by the process by using
MCL_ONFAULT if asked.

Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
---
 include/sysemu/sysemu.h  |  1 +
 migration/postcopy-ram.c |  4 ++--
 qemu-options.hx          | 13 ++++++++++---
 system/globals.c         |  1 +
 system/vl.c              | 18 ++++++++++++++++--
 5 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index XXXXXXX..XXXXXXX 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -XXX,XX +XXX,XX @@ extern const char *keyboard_layout;
 extern int old_param;
 extern uint8_t *boot_splash_filedata;
 extern bool enable_mlock;
+extern bool enable_mlock_onfault;
 extern bool enable_cpu_pm;
 extern QEMUClockType rtc_clock;
 
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index XXXXXXX..XXXXXXX 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -XXX,XX +XXX,XX @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
         mis->have_fault_thread = false;
     }
 
-    if (enable_mlock) {
-        if (os_mlock(false) < 0) {
+    if (enable_mlock || enable_mlock_onfault) {
+        if (os_mlock(enable_mlock_onfault) < 0) {
             error_report("mlock: %s", strerror(errno));
             /*
              * It doesn't feel right to fail at this point, we have a valid
diff --git a/qemu-options.hx b/qemu-options.hx
index XXXXXXX..XXXXXXX 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -XXX,XX +XXX,XX @@ SRST
 ERST
 
 DEF("overcommit", HAS_ARG, QEMU_OPTION_overcommit,
-    "-overcommit [mem-lock=on|off][cpu-pm=on|off]\n"
+    "-overcommit [mem-lock=on|off][mem-lock-onfault=on|off][cpu-pm=on|off]\n"
     "                run qemu with overcommit hints\n"
     "                mem-lock=on|off controls memory lock support (default: off)\n"
+    "                mem-lock-onfault=on|off controls memory lock on fault support (default: off)\n"
     "                cpu-pm=on|off controls cpu power management (default: off)\n",
     QEMU_ARCH_ALL)
 SRST
 ``-overcommit mem-lock=on|off``
   \ 
+``-overcommit mem-lock-onfault=on|off``
+  \
 ``-overcommit cpu-pm=on|off``
     Run qemu with hints about host resource overcommit. The default is
     to assume that host overcommits all resources.
 
     Locking qemu and guest memory can be enabled via ``mem-lock=on``
-    (disabled by default). This works when host memory is not
-    overcommitted and reduces the worst-case latency for guest.
+    or ``mem-lock-onfault=on`` (disabled by default). This works when
+    host memory is not overcommitted and reduces the worst-case latency for
+    guest. The on-fault option is better for reducing the memory footprint
+    since it makes allocations lazy, but the pages still get locked in place
+    once faulted by the guest or QEMU. Note that the two options are mutually
+    exclusive.
 
     Guest ability to manage power state of host cpus (increasing latency
     for other processes on the same host cpu, but decreasing latency for
diff --git a/system/globals.c b/system/globals.c
index XXXXXXX..XXXXXXX 100644
--- a/system/globals.c
+++ b/system/globals.c
@@ -XXX,XX +XXX,XX @@ enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
 int display_opengl;
 const char* keyboard_layout;
 bool enable_mlock;
+bool enable_mlock_onfault;
 bool enable_cpu_pm;
 int autostart = 1;
 int vga_interface_type = VGA_NONE;
diff --git a/system/vl.c b/system/vl.c
index XXXXXXX..XXXXXXX 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -XXX,XX +XXX,XX @@ static QemuOptsList qemu_overcommit_opts = {
             .name = "mem-lock",
             .type = QEMU_OPT_BOOL,
         },
+        {
+            .name = "mem-lock-onfault",
+            .type = QEMU_OPT_BOOL,
+        },
         {
             .name = "cpu-pm",
             .type = QEMU_OPT_BOOL,
@@ -XXX,XX +XXX,XX @@ static QemuOptsList qemu_run_with_opts = {
 
 static void realtime_init(void)
 {
-    if (enable_mlock) {
-        if (os_mlock(false) < 0) {
+    if (enable_mlock || enable_mlock_onfault) {
+        if (os_mlock(enable_mlock_onfault) < 0) {
             error_report("locking memory failed");
             exit(1);
         }
@@ -XXX,XX +XXX,XX @@ void qemu_init(int argc, char **argv)
                 if (!opts) {
                     exit(1);
                 }
+
                 enable_mlock = qemu_opt_get_bool(opts, "mem-lock", enable_mlock);
+                enable_mlock_onfault = qemu_opt_get_bool(opts,
+                                                         "mem-lock-onfault",
+                                                         enable_mlock_onfault);
+                if (enable_mlock && enable_mlock_onfault) {
+                    error_report("mem-lock and mem-lock-onfault are mutually"
+                                 "exclusive");
+                    exit(1);
+                }
+
                 enable_cpu_pm = qemu_opt_get_bool(opts, "cpu-pm", enable_cpu_pm);
                 break;
             case QEMU_OPTION_compat:
-- 
2.34.1

Currently, passing mem-lock=on to QEMU causes memory usage to grow by
huge amounts:

no memlock:
    $ ./qemu-system-x86_64 -overcommit mem-lock=off
    $ ps -p $(pidof ./qemu-system-x86_64) -o rss=
    45652

$ ./qemu-system-x86_64 -overcommit mem-lock=off -enable-kvm
    $ ps -p $(pidof ./qemu-system-x86_64) -o rss=
    39756

memlock:
    $ ./qemu-system-x86_64 -overcommit mem-lock=on
    $ ps -p $(pidof ./qemu-system-x86_64) -o rss=
    1309876

$ ./qemu-system-x86_64 -overcommit mem-lock=on -enable-kvm
    $ ps -p $(pidof ./qemu-system-x86_64) -o rss=
    259956

This is caused by the fact that mlockall(2) automatically
write-faults every existing and future anonymous mappings in the
process right away.

memlock-onfault:
    $ ./qemu-system-x86_64 -overcommit mem-lock=on-fault
    $ ps -p $(pidof ./qemu-system-x86_64) -o rss=
    54004

$ ./qemu-system-x86_64 -overcommit mem-lock=on-fault -enable-kvm
    $ ps -p $(pidof ./qemu-system-x86_64) -o rss=
    47772

Changes since v1:
    - Don't make a separate mem-lock-onfault, add an on-fault option to mem-lock instead

Changes since v2:
    - Move overcommit option parsing out of line
    - Make enable_mlock an enum instead

Daniil Tatianin (4):
  os: add an ability to lock memory on_fault
  system/vl: extract overcommit option parsing into a helper
  sysemu: introduce a new MlockState enum
  overcommit: introduce mem-lock=on-fault

-- 
2.34.1

This will be used in the following commits to make it possible to only
lock memory on fault instead of right away.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
---
 include/sysemu/os-posix.h |  2 +-
 include/sysemu/os-win32.h |  3 ++-
 migration/postcopy-ram.c  |  2 +-
 os-posix.c                | 10 ++++++++--
 system/vl.c               |  2 +-
 5 files changed, 13 insertions(+), 6 deletions(-)

This will be extended in the future commits, let's move it out of line
right away so that it's easier to read.

Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
---
 system/vl.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/system/vl.c b/system/vl.c
index XXXXXXX..XXXXXXX 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -XXX,XX +XXX,XX @@ static void object_option_parse(const char *str)
     visit_free(v);
 }
 
+static void overcommit_parse(const char *str)
+{
+    QemuOpts *opts;
+
+    opts = qemu_opts_parse_noisily(qemu_find_opts("overcommit"),
+                                   str, false);
+    if (!opts) {
+        exit(1);
+    }
+    enable_mlock = qemu_opt_get_bool(opts, "mem-lock", enable_mlock);
+    enable_cpu_pm = qemu_opt_get_bool(opts, "cpu-pm", enable_cpu_pm);
+}
+
 /*
  * Very early object creation, before the sandbox options have been activated.
  */
@@ -XXX,XX +XXX,XX @@ void qemu_init(int argc, char **argv)
                 object_option_parse(optarg);
                 break;
             case QEMU_OPTION_overcommit:
-                opts = qemu_opts_parse_noisily(qemu_find_opts("overcommit"),
-                                               optarg, false);
-                if (!opts) {
-                    exit(1);
-                }
-                enable_mlock = qemu_opt_get_bool(opts, "mem-lock", enable_mlock);
-                enable_cpu_pm = qemu_opt_get_bool(opts, "cpu-pm", enable_cpu_pm);
+                overcommit_parse(optarg);
                 break;
             case QEMU_OPTION_compat:
                 {
-- 
2.34.1

Replace the boolean value enable_mlock with an enum and add a helper to
decide whether we should be calling os_mlock.

This is a stepping stone towards introducing a new mlock mode, which
will be the third possible state of this enum.

Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
---
 hw/virtio/virtio-mem.c   |  2 +-
 include/sysemu/sysemu.h  | 10 +++++++++-
 migration/postcopy-ram.c |  2 +-
 system/globals.c         |  7 ++++++-
 system/vl.c              |  9 +++++++--
 5 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -XXX,XX +XXX,XX @@ static void virtio_mem_device_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    if (enable_mlock) {
+    if (should_mlock(mlock_state)) {
         error_setg(errp, "Incompatible with mlock");
         return;
     }
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index XXXXXXX..XXXXXXX 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -XXX,XX +XXX,XX @@ extern int display_opengl;
 extern const char *keyboard_layout;
 extern int old_param;
 extern uint8_t *boot_splash_filedata;
-extern bool enable_mlock;
 extern bool enable_cpu_pm;
 extern QEMUClockType rtc_clock;
 
+typedef enum {
+    MLOCK_OFF = 0,
+    MLOCK_ON,
+} MlockState;
+
+bool should_mlock(MlockState);
+
+extern MlockState mlock_state;
+
 #define MAX_OPTION_ROMS 16
 typedef struct QEMUOptionRom {
     const char *name;
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index XXXXXXX..XXXXXXX 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -XXX,XX +XXX,XX @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
         mis->have_fault_thread = false;
     }
 
-    if (enable_mlock) {
+    if (should_mlock(mlock_state)) {
         if (os_mlock(false) < 0) {
             error_report("mlock: %s", strerror(errno));
             /*
diff --git a/system/globals.c b/system/globals.c
index XXXXXXX..XXXXXXX 100644
--- a/system/globals.c
+++ b/system/globals.c
@@ -XXX,XX +XXX,XX @@
 #include "sysemu/cpus.h"
 #include "sysemu/sysemu.h"
 
+bool should_mlock(MlockState state)
+{
+    return state == MLOCK_ON;
+}
+
 enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
 int display_opengl;
 const char* keyboard_layout;
-bool enable_mlock;
+MlockState mlock_state;
 bool enable_cpu_pm;
 int autostart = 1;
 int vga_interface_type = VGA_NONE;
diff --git a/system/vl.c b/system/vl.c
index XXXXXXX..XXXXXXX 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -XXX,XX +XXX,XX @@ static QemuOptsList qemu_run_with_opts = {
 
 static void realtime_init(void)
 {
-    if (enable_mlock) {
+    if (should_mlock(mlock_state)) {
         if (os_mlock(false) < 0) {
             error_report("locking memory failed");
             exit(1);
@@ -XXX,XX +XXX,XX @@ static void object_option_parse(const char *str)
 static void overcommit_parse(const char *str)
 {
     QemuOpts *opts;
+    bool enable_mlock;
 
     opts = qemu_opts_parse_noisily(qemu_find_opts("overcommit"),
                                    str, false);
     if (!opts) {
         exit(1);
     }
-    enable_mlock = qemu_opt_get_bool(opts, "mem-lock", enable_mlock);
+
+    enable_mlock = qemu_opt_get_bool(opts, "mem-lock",
+                                     should_mlock(mlock_state));
+    mlock_state = enable_mlock ? MLOCK_ON : MLOCK_OFF;
+
     enable_cpu_pm = qemu_opt_get_bool(opts, "cpu-pm", enable_cpu_pm);
 }
 
-- 
2.34.1

Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
---
 include/sysemu/sysemu.h  |  2 ++
 migration/postcopy-ram.c |  2 +-
 qemu-options.hx          | 14 +++++++++-----
 system/globals.c         |  7 ++++++-
 system/vl.c              | 34 +++++++++++++++++++++++++++-------
 5 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index XXXXXXX..XXXXXXX 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -XXX,XX +XXX,XX @@ extern QEMUClockType rtc_clock;
 typedef enum {
     MLOCK_OFF = 0,
     MLOCK_ON,
+    MLOCK_ON_FAULT,
 } MlockState;
 
 bool should_mlock(MlockState);
+bool is_mlock_on_fault(MlockState);
 
 extern MlockState mlock_state;
 
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index XXXXXXX..XXXXXXX 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -XXX,XX +XXX,XX @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
     }
 
     if (should_mlock(mlock_state)) {
-        if (os_mlock(false) < 0) {
+        if (os_mlock(is_mlock_on_fault(mlock_state)) < 0) {
             error_report("mlock: %s", strerror(errno));
             /*
              * It doesn't feel right to fail at this point, we have a valid
diff --git a/qemu-options.hx b/qemu-options.hx
index XXXXXXX..XXXXXXX 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -XXX,XX +XXX,XX @@ SRST
 ERST
 
 DEF("overcommit", HAS_ARG, QEMU_OPTION_overcommit,
-    "-overcommit [mem-lock=on|off][cpu-pm=on|off]\n"
+    "-overcommit [mem-lock=on|off|on-fault][cpu-pm=on|off]\n"
     "                run qemu with overcommit hints\n"
-    "                mem-lock=on|off controls memory lock support (default: off)\n"
+    "                mem-lock=on|off|on-fault controls memory lock support (default: off)\n"
     "                cpu-pm=on|off controls cpu power management (default: off)\n",
     QEMU_ARCH_ALL)
 SRST
-``-overcommit mem-lock=on|off``
+``-overcommit mem-lock=on|off|on-fault``
   \ 
 ``-overcommit cpu-pm=on|off``
     Run qemu with hints about host resource overcommit. The default is
     to assume that host overcommits all resources.
 
     Locking qemu and guest memory can be enabled via ``mem-lock=on``
-    (disabled by default). This works when host memory is not
-    overcommitted and reduces the worst-case latency for guest.
+    or ``mem-lock=on-fault`` (disabled by default). This works when
+    host memory is not overcommitted and reduces the worst-case latency for
+    guest. The on-fault option is better for reducing the memory footprint
+    since it makes allocations lazy, but the pages still get locked in place
+    once faulted by the guest or QEMU. Note that the two options are mutually
+    exclusive.
 
     Guest ability to manage power state of host cpus (increasing latency
     for other processes on the same host cpu, but decreasing latency for
diff --git a/system/globals.c b/system/globals.c
index XXXXXXX..XXXXXXX 100644
--- a/system/globals.c
+++ b/system/globals.c
@@ -XXX,XX +XXX,XX @@
 
 bool should_mlock(MlockState state)
 {
-    return state == MLOCK_ON;
+    return state == MLOCK_ON || state == MLOCK_ON_FAULT;
+}
+
+bool is_mlock_on_fault(MlockState state)
+{
+    return state == MLOCK_ON_FAULT;
 }
 
 enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
diff --git a/system/vl.c b/system/vl.c
index XXXXXXX..XXXXXXX 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -XXX,XX +XXX,XX @@ static QemuOptsList qemu_overcommit_opts = {
     .desc = {
         {
             .name = "mem-lock",
-            .type = QEMU_OPT_BOOL,
+            .type = QEMU_OPT_STRING,
         },
         {
             .name = "cpu-pm",
@@ -XXX,XX +XXX,XX @@ static QemuOptsList qemu_run_with_opts = {
 static void realtime_init(void)
 {
     if (should_mlock(mlock_state)) {
-        if (os_mlock(false) < 0) {
+        if (os_mlock(is_mlock_on_fault(mlock_state)) < 0) {
             error_report("locking memory failed");
             exit(1);
         }
@@ -XXX,XX +XXX,XX @@ static void object_option_parse(const char *str)
 static void overcommit_parse(const char *str)
 {
     QemuOpts *opts;
-    bool enable_mlock;
+    const char *mem_lock_opt;
 
     opts = qemu_opts_parse_noisily(qemu_find_opts("overcommit"),
                                    str, false);
@@ -XXX,XX +XXX,XX @@ static void overcommit_parse(const char *str)
         exit(1);
     }
 
-    enable_mlock = qemu_opt_get_bool(opts, "mem-lock",
-                                     should_mlock(mlock_state));
-    mlock_state = enable_mlock ? MLOCK_ON : MLOCK_OFF;
-
     enable_cpu_pm = qemu_opt_get_bool(opts, "cpu-pm", enable_cpu_pm);
+
+    mem_lock_opt = qemu_opt_get(opts, "mem-lock");
+    if (!mem_lock_opt) {
+        return;
+    }
+
+    if (strcmp(mem_lock_opt, "on") == 0) {
+        mlock_state = MLOCK_ON;
+        return;
+    }
+
+    if (strcmp(mem_lock_opt, "off") == 0) {
+        mlock_state = MLOCK_OFF;
+        return;
+    }
+
+    if (strcmp(mem_lock_opt, "on-fault") == 0) {
+        mlock_state = MLOCK_ON_FAULT;
+        return;
+    }
+
+    error_report("parameter 'mem-lock' expects one of "
+                 "'on', 'off', 'on-fault'");
+    exit(1);
 }
 
 /*
-- 
2.34.1