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

[Daniil Tatianin]

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-tatia...@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