* [PATCH v2 0/2] overcommit: introduce mem-lock-onfault
@ 2024-12-11 0:04 Daniil Tatianin
2024-12-11 0:04 ` [PATCH v2 1/2] os: add an ability to lock memory on_fault Daniil Tatianin
2024-12-11 0:04 ` [PATCH v2 2/2] overcommit: introduce mem-lock=on-fault Daniil Tatianin
0 siblings, 2 replies; 6+ messages in thread
From: Daniil Tatianin @ 2024-12-11 0:04 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Daniil Tatianin, Stefan Weil, Peter Xu, Fabiano Rosas, qemu-devel
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 option for mem-lock called
on-fault.
memlock=on-fault:
$ ./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
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.
Changes since v1:
- Don't make a separate mem-lock-onfault, add an on-fault option to mem-lock instead
Daniil Tatianin (2):
os: add an ability to lock memory on_fault
overcommit: introduce mem-lock=on-fault
include/sysemu/os-posix.h | 2 +-
include/sysemu/os-win32.h | 3 ++-
include/sysemu/sysemu.h | 1 +
migration/postcopy-ram.c | 4 ++--
os-posix.c | 10 +++++++--
qemu-options.hx | 14 +++++++-----
system/globals.c | 1 +
system/vl.c | 46 +++++++++++++++++++++++++++++++--------
8 files changed, 61 insertions(+), 20 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] os: add an ability to lock memory on_fault
2024-12-11 0:04 [PATCH v2 0/2] overcommit: introduce mem-lock-onfault Daniil Tatianin
@ 2024-12-11 0:04 ` Daniil Tatianin
2024-12-11 21:44 ` Peter Xu
2024-12-11 0:04 ` [PATCH v2 2/2] overcommit: introduce mem-lock=on-fault Daniil Tatianin
1 sibling, 1 reply; 6+ messages in thread
From: Daniil Tatianin @ 2024-12-11 0:04 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Daniil Tatianin, Stefan Weil, Peter Xu, Fabiano Rosas, qemu-devel
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 b881ac6c6f..ce5b3bccf8 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -53,7 +53,7 @@ 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 b82a5d3ad9..cd61d69e10 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -123,8 +123,9 @@ 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 a535fd2e30..36ec6a3d75 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -652,7 +652,7 @@ 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 43f9a43f3f..0948128134 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -327,18 +327,24 @@ 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 54998fdbc7..03819a80ef 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -793,7 +793,7 @@ 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] overcommit: introduce mem-lock=on-fault
2024-12-11 0:04 [PATCH v2 0/2] overcommit: introduce mem-lock-onfault Daniil Tatianin
2024-12-11 0:04 ` [PATCH v2 1/2] os: add an ability to lock memory on_fault Daniil Tatianin
@ 2024-12-11 0:04 ` Daniil Tatianin
2024-12-11 21:52 ` Peter Xu
1 sibling, 1 reply; 6+ messages in thread
From: Daniil Tatianin @ 2024-12-11 0:04 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Daniil Tatianin, Stefan Weil, Peter Xu, Fabiano Rosas, qemu-devel
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 | 14 +++++++-----
system/globals.c | 1 +
system/vl.c | 46 ++++++++++++++++++++++++++++++++--------
5 files changed, 50 insertions(+), 16 deletions(-)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 7ec419ce13..b6519c3c1e 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -44,6 +44,7 @@ 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 36ec6a3d75..8ff8c73a27 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -651,8 +651,8 @@ 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 dacc9790a4..6c8360e62e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4566,21 +4566,25 @@ 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 84ce943ac9..43501fe690 100644
--- a/system/globals.c
+++ b/system/globals.c
@@ -35,6 +35,7 @@ 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 03819a80ef..4e2efd3ad4 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -347,7 +347,7 @@ static QemuOptsList qemu_overcommit_opts = {
.desc = {
{
.name = "mem-lock",
- .type = QEMU_OPT_BOOL,
+ .type = QEMU_OPT_STRING,
},
{
.name = "cpu-pm",
@@ -792,8 +792,8 @@ 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);
}
@@ -3532,14 +3532,42 @@ 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) {
+ {
+ const char *mem_lock_opt;
+
+ opts = qemu_opts_parse_noisily(qemu_find_opts("overcommit"),
+ optarg, false);
+ if (!opts) {
+ exit(1);
+ }
+
+ 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) {
+ break;
+ }
+
+ if (strcmp(mem_lock_opt, "on") == 0) {
+ enable_mlock = true;
+ break;
+ }
+
+ if (strcmp(mem_lock_opt, "off") == 0) {
+ enable_mlock = false;
+ enable_mlock_onfault = false;
+ break;
+ }
+
+ if (strcmp(mem_lock_opt, "on-fault") == 0) {
+ enable_mlock_onfault = true;
+ break;
+ }
+
+ error_report("parameter 'mem-lock' expects one of "
+ "'on', 'off', 'on-fault'");
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);
- break;
case QEMU_OPTION_compat:
{
CompatPolicy *opts_policy;
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] os: add an ability to lock memory on_fault
2024-12-11 0:04 ` [PATCH v2 1/2] os: add an ability to lock memory on_fault Daniil Tatianin
@ 2024-12-11 21:44 ` Peter Xu
0 siblings, 0 replies; 6+ messages in thread
From: Peter Xu @ 2024-12-11 21:44 UTC (permalink / raw)
To: Daniil Tatianin; +Cc: Paolo Bonzini, Stefan Weil, Fabiano Rosas, qemu-devel
On Wed, Dec 11, 2024 at 03:04:46AM +0300, Daniil Tatianin wrote:
> 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>
Reviewed-by: Peter Xu <peterx@redhat.com>
--
Peter Xu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] overcommit: introduce mem-lock=on-fault
2024-12-11 0:04 ` [PATCH v2 2/2] overcommit: introduce mem-lock=on-fault Daniil Tatianin
@ 2024-12-11 21:52 ` Peter Xu
2024-12-11 22:00 ` Daniil Tatianin
0 siblings, 1 reply; 6+ messages in thread
From: Peter Xu @ 2024-12-11 21:52 UTC (permalink / raw)
To: Daniil Tatianin; +Cc: Paolo Bonzini, Stefan Weil, Fabiano Rosas, qemu-devel
On Wed, Dec 11, 2024 at 03:04:47AM +0300, Daniil Tatianin wrote:
> 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.
Looks good but some nitpicks below..
>
> Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
> ---
> include/sysemu/sysemu.h | 1 +
> migration/postcopy-ram.c | 4 ++--
> qemu-options.hx | 14 +++++++-----
> system/globals.c | 1 +
> system/vl.c | 46 ++++++++++++++++++++++++++++++++--------
> 5 files changed, 50 insertions(+), 16 deletions(-)
>
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 7ec419ce13..b6519c3c1e 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -44,6 +44,7 @@ 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 36ec6a3d75..8ff8c73a27 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -651,8 +651,8 @@ 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 dacc9790a4..6c8360e62e 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4566,21 +4566,25 @@ 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 84ce943ac9..43501fe690 100644
> --- a/system/globals.c
> +++ b/system/globals.c
> @@ -35,6 +35,7 @@ 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 03819a80ef..4e2efd3ad4 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -347,7 +347,7 @@ static QemuOptsList qemu_overcommit_opts = {
> .desc = {
> {
> .name = "mem-lock",
> - .type = QEMU_OPT_BOOL,
> + .type = QEMU_OPT_STRING,
> },
> {
> .name = "cpu-pm",
> @@ -792,8 +792,8 @@ 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) {
IIUC it's still cleaner to make enable_mlock into an enum to be a
tri-state. IOW, we could have two flags set now with the current patch
when with things like:
-overcommit mem-lock=on -overcommit mem-lock=on-fault
> + if (os_mlock(enable_mlock_onfault) < 0) {
> error_report("locking memory failed");
> exit(1);
> }
> @@ -3532,14 +3532,42 @@ 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) {
> + {
> + const char *mem_lock_opt;
> +
> + opts = qemu_opts_parse_noisily(qemu_find_opts("overcommit"),
> + optarg, false);
> + if (!opts) {
> + exit(1);
> + }
> +
> + 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) {
> + break;
> + }
> +
> + if (strcmp(mem_lock_opt, "on") == 0) {
> + enable_mlock = true;
> + break;
> + }
> +
> + if (strcmp(mem_lock_opt, "off") == 0) {
> + enable_mlock = false;
> + enable_mlock_onfault = false;
> + break;
> + }
> +
> + if (strcmp(mem_lock_opt, "on-fault") == 0) {
> + enable_mlock_onfault = true;
> + break;
> + }
> +
> + error_report("parameter 'mem-lock' expects one of "
> + "'on', 'off', 'on-fault'");
Not the only one that open code this.. but still there're better references
like net_client_parse() or monitor_parse() that we could follow. So would
be good to put it into a function.
Thanks,
> 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);
> - break;
> case QEMU_OPTION_compat:
> {
> CompatPolicy *opts_policy;
> --
> 2.34.1
>
--
Peter Xu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] overcommit: introduce mem-lock=on-fault
2024-12-11 21:52 ` Peter Xu
@ 2024-12-11 22:00 ` Daniil Tatianin
0 siblings, 0 replies; 6+ messages in thread
From: Daniil Tatianin @ 2024-12-11 22:00 UTC (permalink / raw)
To: Peter Xu; +Cc: Paolo Bonzini, Stefan Weil, Fabiano Rosas, qemu-devel
On 12/12/24 12:52 AM, Peter Xu wrote:
> On Wed, Dec 11, 2024 at 03:04:47AM +0300, Daniil Tatianin wrote:
>> 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.
> Looks good but some nitpicks below..
>
>> Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
>> ---
>> include/sysemu/sysemu.h | 1 +
>> migration/postcopy-ram.c | 4 ++--
>> qemu-options.hx | 14 +++++++-----
>> system/globals.c | 1 +
>> system/vl.c | 46 ++++++++++++++++++++++++++++++++--------
>> 5 files changed, 50 insertions(+), 16 deletions(-)
>>
>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>> index 7ec419ce13..b6519c3c1e 100644
>> --- a/include/sysemu/sysemu.h
>> +++ b/include/sysemu/sysemu.h
>> @@ -44,6 +44,7 @@ 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 36ec6a3d75..8ff8c73a27 100644
>> --- a/migration/postcopy-ram.c
>> +++ b/migration/postcopy-ram.c
>> @@ -651,8 +651,8 @@ 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 dacc9790a4..6c8360e62e 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -4566,21 +4566,25 @@ 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 84ce943ac9..43501fe690 100644
>> --- a/system/globals.c
>> +++ b/system/globals.c
>> @@ -35,6 +35,7 @@ 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 03819a80ef..4e2efd3ad4 100644
>> --- a/system/vl.c
>> +++ b/system/vl.c
>> @@ -347,7 +347,7 @@ static QemuOptsList qemu_overcommit_opts = {
>> .desc = {
>> {
>> .name = "mem-lock",
>> - .type = QEMU_OPT_BOOL,
>> + .type = QEMU_OPT_STRING,
>> },
>> {
>> .name = "cpu-pm",
>> @@ -792,8 +792,8 @@ 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) {
> IIUC it's still cleaner to make enable_mlock into an enum to be a
> tri-state. IOW, we could have two flags set now with the current patch
> when with things like:
>
> -overcommit mem-lock=on -overcommit mem-lock=on-fault
Oh yeah good catch, that's a problem.
>> + if (os_mlock(enable_mlock_onfault) < 0) {
>> error_report("locking memory failed");
>> exit(1);
>> }
>> @@ -3532,14 +3532,42 @@ 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) {
>> + {
>> + const char *mem_lock_opt;
>> +
>> + opts = qemu_opts_parse_noisily(qemu_find_opts("overcommit"),
>> + optarg, false);
>> + if (!opts) {
>> + exit(1);
>> + }
>> +
>> + 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) {
>> + break;
>> + }
>> +
>> + if (strcmp(mem_lock_opt, "on") == 0) {
>> + enable_mlock = true;
>> + break;
>> + }
>> +
>> + if (strcmp(mem_lock_opt, "off") == 0) {
>> + enable_mlock = false;
>> + enable_mlock_onfault = false;
>> + break;
>> + }
>> +
>> + if (strcmp(mem_lock_opt, "on-fault") == 0) {
>> + enable_mlock_onfault = true;
>> + break;
>> + }
>> +
>> + error_report("parameter 'mem-lock' expects one of "
>> + "'on', 'off', 'on-fault'");
> Not the only one that open code this.. but still there're better references
> like net_client_parse() or monitor_parse() that we could follow. So would
> be good to put it into a function.
>
> Thanks,
Yeah I'm not a fan of this solution to parsing, but I wasn't able to
find better examples straight away so went with this. Will take a look
at the places you've mentioned.
Thanks!
>
>> 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);
>> - break;
>> case QEMU_OPTION_compat:
>> {
>> CompatPolicy *opts_policy;
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-12-11 22:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-11 0:04 [PATCH v2 0/2] overcommit: introduce mem-lock-onfault Daniil Tatianin
2024-12-11 0:04 ` [PATCH v2 1/2] os: add an ability to lock memory on_fault Daniil Tatianin
2024-12-11 21:44 ` Peter Xu
2024-12-11 0:04 ` [PATCH v2 2/2] overcommit: introduce mem-lock=on-fault Daniil Tatianin
2024-12-11 21:52 ` Peter Xu
2024-12-11 22:00 ` Daniil Tatianin
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.