Re: [PATCH v5 1/4] os: add an ability to lock memory on_fault

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

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