Re: [PULL v2 11/14] os: add an ability to lock memory on_fault

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Feb 12, 2025 at 12:38:23PM -0500, Peter Xu wrote:
> From: Daniil Tatianin <d-tatianin@yandex-team.ru>
> 
> 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>
> Link: https://lore.kernel.org/r/20250212143920.1269754-2-d-tatianin@yandex-team.ru
> [peterx: fail os_mlock(on_fault=1) when not supported]
> [peterx: use G_GNUC_UNUSED instead of "(void)on_fault", per Dan]
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  meson.build               |  6 ++++++
>  include/system/os-posix.h |  2 +-
>  include/system/os-win32.h |  2 +-
>  migration/postcopy-ram.c  |  2 +-
>  os-posix.c                | 15 +++++++++++++--
>  system/vl.c               |  2 +-
>  6 files changed, 23 insertions(+), 6 deletions(-)
> 
> 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/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..bc623061d8 100644
> --- a/include/system/os-win32.h
> +++ b/include/system/os-win32.h
> @@ -123,7 +123,7 @@ static inline bool is_daemonized(void)
>      return false;
>  }
>  
> -static inline int os_mlock(void)
> +static inline int os_mlock(bool on_fault G_GNUC_UNUSED)

So did this actually generate a warning ? We don' even need
G_GNUC_UNUSED unless we're actually seeing warnings about this.

>  {
>      return -ENOSYS;
>  }
> 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..52925c23d3 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -327,18 +327,29 @@ 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) {
> +#ifdef HAVE_MLOCK_ONFAULT
> +        flags |= MCL_ONFAULT;
> +#else
> +        error_report("mlockall: on_fault not supported");
> +        return -EINVAL;
> +#endif
> +    }
> +
> +    ret = mlockall(flags);
>      if (ret < 0) {
>          error_report("mlockall: %s", strerror(errno));
>      }
>  
>      return ret;
>  #else
> +    (void)on_fault;

Still has the pointless (void) cast I mentioned on the previous postign.

>      return -ENOSYS;
>  #endif
>  }

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 :|