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/system/os-posix.h | 2 +-
include/system/os-win32.h | 3 ++-
meson.build | 6 ++++++
migration/postcopy-ram.c | 2 +-
os-posix.c | 14 ++++++++++++--
system/vl.c | 2 +-
6 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/include/system/os-posix.h b/include/system/os-posix.h
index b881ac6c6f..ce5b3bccf8 100644
--- a/include/system/os-posix.h
+++ b/include/system/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/system/os-win32.h b/include/system/os-win32.h
index b82a5d3ad9..cd61d69e10 100644
--- a/include/system/os-win32.h
+++ b/include/system/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/meson.build b/meson.build
index 18cf9e2913..59953cbe6b 100644
--- a/meson.build
+++ b/meson.build
@@ -2885,6 +2885,12 @@ config_host_data.set('HAVE_MLOCKALL', cc.links(gnu_source_prefix + '''
return mlockall(MCL_FUTURE);
}'''))
+config_host_data.set('HAVE_MLOCK_ONFAULT', cc.links(gnu_source_prefix + '''
+ #include <sys/mman.h>
+ int main(void) {
+ return mlockall(MCL_FUTURE | MCL_ONFAULT);
+ }'''))
+
have_l2tpv3 = false
if get_option('l2tpv3').allowed() and have_system
have_l2tpv3 = cc.has_type('struct mmsghdr',
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 6a6da6ba7f..fc4d8a10df 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 9cce55ff2f..17b144c0a2 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -327,18 +327,28 @@ 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);
+#ifdef HAVE_MLOCK_ONFAULT
+ if (on_fault) {
+ flags |= MCL_ONFAULT;
+ }
+#else
+ (void)on_fault;
+#endif
+
+ 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 9c6942c6cf..e94fc7ea35 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -797,7 +797,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.1On Wed, Feb 12, 2025 at 05:39:17PM +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>
> ---
> include/system/os-posix.h | 2 +-
> include/system/os-win32.h | 3 ++-
> meson.build | 6 ++++++
> migration/postcopy-ram.c | 2 +-
> os-posix.c | 14 ++++++++++++--
> system/vl.c | 2 +-
> 6 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/include/system/os-posix.h b/include/system/os-posix.h
> index b881ac6c6f..ce5b3bccf8 100644
> --- a/include/system/os-posix.h
> +++ b/include/system/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/system/os-win32.h b/include/system/os-win32.h
> index b82a5d3ad9..cd61d69e10 100644
> --- a/include/system/os-win32.h
> +++ b/include/system/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;
Is this really needed ? Our compiler flags don't enable warnings
about unused variables.
If they did, this would not be the way to hide them. Instead you
would use the "G_GNUC_UNUSED" annotation against the parameter.
eg
static inline int os_mlock(bool on_fault G_GNUC_UNUSED)
> return -ENOSYS;
> }
>
> diff --git a/os-posix.c b/os-posix.c
> index 9cce55ff2f..17b144c0a2 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -327,18 +327,28 @@ 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);
> +#ifdef HAVE_MLOCK_ONFAULT
> + if (on_fault) {
> + flags |= MCL_ONFAULT;
> + }
> +#else
> + (void)on_fault;
> +#endif
> +
> + ret = mlockall(flags);
> if (ret < 0) {
> error_report("mlockall: %s", strerror(errno));
> }
>
> return ret;
> #else
> + (void)on_fault;
> return -ENOSYS;
> #endif
Again casting to (void) should not be required AFAIK.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 2/12/25 6:30 PM, Daniel P. Berrangé wrote:
> On Wed, Feb 12, 2025 at 05:39:17PM +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>
>> ---
>> include/system/os-posix.h | 2 +-
>> include/system/os-win32.h | 3 ++-
>> meson.build | 6 ++++++
>> migration/postcopy-ram.c | 2 +-
>> os-posix.c | 14 ++++++++++++--
>> system/vl.c | 2 +-
>> 6 files changed, 23 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/system/os-posix.h b/include/system/os-posix.h
>> index b881ac6c6f..ce5b3bccf8 100644
>> --- a/include/system/os-posix.h
>> +++ b/include/system/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/system/os-win32.h b/include/system/os-win32.h
>> index b82a5d3ad9..cd61d69e10 100644
>> --- a/include/system/os-win32.h
>> +++ b/include/system/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;
> Is this really needed ? Our compiler flags don't enable warnings
> about unused variables.
Hmm, I was not aware of that, thank you.
Peter, do you want me to resend, or can you squash remove this as well?
>
> If they did, this would not be the way to hide them. Instead you
> would use the "G_GNUC_UNUSED" annotation against the parameter.
> eg
>
> static inline int os_mlock(bool on_fault G_GNUC_UNUSED)
>
>> return -ENOSYS;
>> }
>>
>> diff --git a/os-posix.c b/os-posix.c
>> index 9cce55ff2f..17b144c0a2 100644
>> --- a/os-posix.c
>> +++ b/os-posix.c
>> @@ -327,18 +327,28 @@ 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);
>> +#ifdef HAVE_MLOCK_ONFAULT
>> + if (on_fault) {
>> + flags |= MCL_ONFAULT;
>> + }
>> +#else
>> + (void)on_fault;
>> +#endif
>> +
>> + ret = mlockall(flags);
>> if (ret < 0) {
>> error_report("mlockall: %s", strerror(errno));
>> }
>>
>> return ret;
>> #else
>> + (void)on_fault;
>> return -ENOSYS;
>> #endif
> Again casting to (void) should not be required AFAIK.
>
>
> With regards,
> Daniel
On Wed, Feb 12, 2025 at 07:17:45PM +0300, Daniil Tatianin wrote:
> > > -static inline int os_mlock(void)
> > > +static inline int os_mlock(bool on_fault)
> > > {
> > > + (void)on_fault;
> > Is this really needed ? Our compiler flags don't enable warnings
> > about unused variables.
>
> Hmm, I was not aware of that, thank you.
>
> Peter, do you want me to resend, or can you squash remove this as well?
I'll do, no worry.
--
Peter Xu
On 2/12/25 7:42 PM, Peter Xu wrote:
> On Wed, Feb 12, 2025 at 07:17:45PM +0300, Daniil Tatianin wrote:
>>>> -static inline int os_mlock(void)
>>>> +static inline int os_mlock(bool on_fault)
>>>> {
>>>> + (void)on_fault;
>>> Is this really needed ? Our compiler flags don't enable warnings
>>> about unused variables.
>> Hmm, I was not aware of that, thank you.
>>
>> Peter, do you want me to resend, or can you squash remove this as well?
> I'll do, no worry.
Much appreciated!
>
On Wed, Feb 12, 2025 at 03:30:21PM +0000, Daniel P. Berrangé wrote:
> > diff --git a/include/system/os-win32.h b/include/system/os-win32.h
> > index b82a5d3ad9..cd61d69e10 100644
> > --- a/include/system/os-win32.h
> > +++ b/include/system/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;
>
> Is this really needed ? Our compiler flags don't enable warnings
> about unused variables.
>
> If they did, this would not be the way to hide them. Instead you
> would use the "G_GNUC_UNUSED" annotation against the parameter.
> eg
>
> static inline int os_mlock(bool on_fault G_GNUC_UNUSED)
To be on the safe side.. I'll try to keep the marker if no one disagrees.
So that's:
diff --git a/include/system/os-win32.h b/include/system/os-win32.h
index cd61d69e10..bc623061d8 100644
--- a/include/system/os-win32.h
+++ b/include/system/os-win32.h
@@ -123,9 +123,8 @@ static inline bool is_daemonized(void)
return false;
}
-static inline int os_mlock(bool on_fault)
+static inline int os_mlock(bool on_fault G_GNUC_UNUSED)
{
- (void)on_fault;
return -ENOSYS;
}
>
> > return -ENOSYS;
> > }
--
Peter Xu
On Wed, Feb 12, 2025 at 05:39:17PM +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>
> ---
> include/system/os-posix.h | 2 +-
> include/system/os-win32.h | 3 ++-
> meson.build | 6 ++++++
> migration/postcopy-ram.c | 2 +-
> os-posix.c | 14 ++++++++++++--
> system/vl.c | 2 +-
> 6 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/include/system/os-posix.h b/include/system/os-posix.h
> index b881ac6c6f..ce5b3bccf8 100644
> --- a/include/system/os-posix.h
> +++ b/include/system/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/system/os-win32.h b/include/system/os-win32.h
> index b82a5d3ad9..cd61d69e10 100644
> --- a/include/system/os-win32.h
> +++ b/include/system/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/meson.build b/meson.build
> index 18cf9e2913..59953cbe6b 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2885,6 +2885,12 @@ config_host_data.set('HAVE_MLOCKALL', cc.links(gnu_source_prefix + '''
> return mlockall(MCL_FUTURE);
> }'''))
>
> +config_host_data.set('HAVE_MLOCK_ONFAULT', cc.links(gnu_source_prefix + '''
> + #include <sys/mman.h>
> + int main(void) {
> + return mlockall(MCL_FUTURE | MCL_ONFAULT);
> + }'''))
> +
> have_l2tpv3 = false
> if get_option('l2tpv3').allowed() and have_system
> have_l2tpv3 = cc.has_type('struct mmsghdr',
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 6a6da6ba7f..fc4d8a10df 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 9cce55ff2f..17b144c0a2 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -327,18 +327,28 @@ 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);
> +#ifdef HAVE_MLOCK_ONFAULT
> + if (on_fault) {
> + flags |= MCL_ONFAULT;
> + }
> +#else
> + (void)on_fault;
> +#endif
IIUC we should fail this when not supported.
Would you mind I squash this in?
diff --git a/os-posix.c b/os-posix.c
index 17b144c0a2..52925c23d3 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -333,13 +333,14 @@ int os_mlock(bool on_fault)
int ret = 0;
int flags = MCL_CURRENT | MCL_FUTURE;
-#ifdef HAVE_MLOCK_ONFAULT
if (on_fault) {
+#ifdef HAVE_MLOCK_ONFAULT
flags |= MCL_ONFAULT;
- }
#else
- (void)on_fault;
+ error_report("mlockall: on_fault not supported");
+ return -EINVAL;
#endif
+ }
ret = mlockall(flags);
if (ret < 0) {
> +
> + 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 9c6942c6cf..e94fc7ea35 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -797,7 +797,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
>
--
Peter Xu
On 12/2/25 16:23, Peter Xu wrote:
> On Wed, Feb 12, 2025 at 05:39:17PM +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>
>> ---
>> include/system/os-posix.h | 2 +-
>> include/system/os-win32.h | 3 ++-
>> meson.build | 6 ++++++
>> migration/postcopy-ram.c | 2 +-
>> os-posix.c | 14 ++++++++++++--
>> system/vl.c | 2 +-
>> 6 files changed, 23 insertions(+), 6 deletions(-)
>> diff --git a/os-posix.c b/os-posix.c
>> index 9cce55ff2f..17b144c0a2 100644
>> --- a/os-posix.c
>> +++ b/os-posix.c
>> @@ -327,18 +327,28 @@ 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);
>> +#ifdef HAVE_MLOCK_ONFAULT
>> + if (on_fault) {
>> + flags |= MCL_ONFAULT;
>> + }
>> +#else
>> + (void)on_fault;
>> +#endif
>
> IIUC we should fail this when not supported.
>
> Would you mind I squash this in?
>
> diff --git a/os-posix.c b/os-posix.c
> index 17b144c0a2..52925c23d3 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -333,13 +333,14 @@ int os_mlock(bool on_fault)
> int ret = 0;
> int flags = MCL_CURRENT | MCL_FUTURE;
>
> -#ifdef HAVE_MLOCK_ONFAULT
> if (on_fault) {
> +#ifdef HAVE_MLOCK_ONFAULT
> flags |= MCL_ONFAULT;
> - }
> #else
> - (void)on_fault;
> + error_report("mlockall: on_fault not supported");
> + return -EINVAL;
Good point!
> #endif
> + }
>
> ret = mlockall(flags);
> if (ret < 0) {
On 2/12/25 6:23 PM, Peter Xu wrote:
> On Wed, Feb 12, 2025 at 05:39:17PM +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>
>> ---
>> include/system/os-posix.h | 2 +-
>> include/system/os-win32.h | 3 ++-
>> meson.build | 6 ++++++
>> migration/postcopy-ram.c | 2 +-
>> os-posix.c | 14 ++++++++++++--
>> system/vl.c | 2 +-
>> 6 files changed, 23 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/system/os-posix.h b/include/system/os-posix.h
>> index b881ac6c6f..ce5b3bccf8 100644
>> --- a/include/system/os-posix.h
>> +++ b/include/system/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/system/os-win32.h b/include/system/os-win32.h
>> index b82a5d3ad9..cd61d69e10 100644
>> --- a/include/system/os-win32.h
>> +++ b/include/system/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/meson.build b/meson.build
>> index 18cf9e2913..59953cbe6b 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -2885,6 +2885,12 @@ config_host_data.set('HAVE_MLOCKALL', cc.links(gnu_source_prefix + '''
>> return mlockall(MCL_FUTURE);
>> }'''))
>>
>> +config_host_data.set('HAVE_MLOCK_ONFAULT', cc.links(gnu_source_prefix + '''
>> + #include <sys/mman.h>
>> + int main(void) {
>> + return mlockall(MCL_FUTURE | MCL_ONFAULT);
>> + }'''))
>> +
>> have_l2tpv3 = false
>> if get_option('l2tpv3').allowed() and have_system
>> have_l2tpv3 = cc.has_type('struct mmsghdr',
>> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
>> index 6a6da6ba7f..fc4d8a10df 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 9cce55ff2f..17b144c0a2 100644
>> --- a/os-posix.c
>> +++ b/os-posix.c
>> @@ -327,18 +327,28 @@ 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);
>> +#ifdef HAVE_MLOCK_ONFAULT
>> + if (on_fault) {
>> + flags |= MCL_ONFAULT;
>> + }
>> +#else
>> + (void)on_fault;
>> +#endif
> IIUC we should fail this when not supported.
>
> Would you mind I squash this in?
Sure, go ahead. Thanks!
>
> diff --git a/os-posix.c b/os-posix.c
> index 17b144c0a2..52925c23d3 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -333,13 +333,14 @@ int os_mlock(bool on_fault)
> int ret = 0;
> int flags = MCL_CURRENT | MCL_FUTURE;
>
> -#ifdef HAVE_MLOCK_ONFAULT
> if (on_fault) {
> +#ifdef HAVE_MLOCK_ONFAULT
> flags |= MCL_ONFAULT;
> - }
> #else
> - (void)on_fault;
> + error_report("mlockall: on_fault not supported");
> + return -EINVAL;
> #endif
> + }
>
> ret = mlockall(flags);
> if (ret < 0) {
>
>
>> +
>> + 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 9c6942c6cf..e94fc7ea35 100644
>> --- a/system/vl.c
>> +++ b/system/vl.c
>> @@ -797,7 +797,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
>>
Hi Daniil, On 12/2/25 15:39, 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> > --- > include/system/os-posix.h | 2 +- > include/system/os-win32.h | 3 ++- > meson.build | 6 ++++++ > migration/postcopy-ram.c | 2 +- > os-posix.c | 14 ++++++++++++-- > system/vl.c | 2 +- > 6 files changed, 23 insertions(+), 6 deletions(-) > > diff --git a/include/system/os-posix.h b/include/system/os-posix.h > index b881ac6c6f..ce5b3bccf8 100644 > --- a/include/system/os-posix.h > +++ b/include/system/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); If we need to support more flag, is your plan to add more arguments? Otherwise, why not use a 'int flags' argument, and have the callers pass MCL_ONFAULT?
On 2/12/25 5:48 PM, Philippe Mathieu-Daudé wrote: > Hi Daniil, > > On 12/2/25 15:39, 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> >> --- >> include/system/os-posix.h | 2 +- >> include/system/os-win32.h | 3 ++- >> meson.build | 6 ++++++ >> migration/postcopy-ram.c | 2 +- >> os-posix.c | 14 ++++++++++++-- >> system/vl.c | 2 +- >> 6 files changed, 23 insertions(+), 6 deletions(-) >> >> diff --git a/include/system/os-posix.h b/include/system/os-posix.h >> index b881ac6c6f..ce5b3bccf8 100644 >> --- a/include/system/os-posix.h >> +++ b/include/system/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); > > If we need to support more flag, is your plan to add more arguments? > Otherwise, why not use a 'int flags' argument, and have the callers > pass MCL_ONFAULT? Hi! I chose this approach because MCL_ONFAULT is a POSIX/linux-specific flag, and this function is called in places that are platform-agnostic, thus a bool flag seemed more fitting.
On 12/2/25 15:51, Daniil Tatianin wrote: > On 2/12/25 5:48 PM, Philippe Mathieu-Daudé wrote: > >> Hi Daniil, >> >> On 12/2/25 15:39, 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> >>> --- >>> include/system/os-posix.h | 2 +- >>> include/system/os-win32.h | 3 ++- >>> meson.build | 6 ++++++ >>> migration/postcopy-ram.c | 2 +- >>> os-posix.c | 14 ++++++++++++-- >>> system/vl.c | 2 +- >>> 6 files changed, 23 insertions(+), 6 deletions(-) >>> >>> diff --git a/include/system/os-posix.h b/include/system/os-posix.h >>> index b881ac6c6f..ce5b3bccf8 100644 >>> --- a/include/system/os-posix.h >>> +++ b/include/system/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); >> >> If we need to support more flag, is your plan to add more arguments? >> Otherwise, why not use a 'int flags' argument, and have the callers >> pass MCL_ONFAULT? > > Hi! > > I chose this approach because MCL_ONFAULT is a POSIX/linux-specific > flag, and this function is called in places that are platform-agnostic, > thus a bool flag seemed more fitting. OK then.
On 12.02.25 17:39, 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: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> -- Best regards, Vladimir
© 2016 - 2026 Red Hat, Inc.