| Message ID | 20250212143920.1269754-2-d-tatianin@yandex-team.ru (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | overcommit: introduce mem-lock-onfault | expand |
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>
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 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 >
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 >>
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. 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 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; > > }
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: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.
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! >
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); }
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(-)