qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 0/2] tests/functional: add memlock tests
@ 2025-05-21 13:55 Alexandr Moshkov
  2025-05-21 13:55 ` [PATCH v7 1/2] tests/functional: add skipLockedMemoryTest decorator Alexandr Moshkov
  2025-05-21 13:55 ` [PATCH v7 2/2] tests/functional: add memlock tests Alexandr Moshkov
  0 siblings, 2 replies; 12+ messages in thread
From: Alexandr Moshkov @ 2025-05-21 13:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Cleber Rosa, yc-core @ yandex-team . ru, Paolo Bonzini,
	Daniel P . Berrangé, Thomas Huth,
	Philippe Mathieu-Daudé, Alexandr Moshkov

Add new tests to check the correctness of the `-overcommit memlock`
option (possible values: off, on, on-fault) by using
`/proc/{qemu_pid}/smaps` file to check in Size, Rss and Locked fields of
anonymous segments:

* if `memlock=off`, then Locked = 0 on every anonymous smaps;
* if `memlock=on`, then Size, Rss and Locked values must be equal for
every anon smaps where Rss is not 0;
* if `memlock=on-fault`, then Rss and Locked must be equal on every anon
smaps and anonymous segment with Rss < Size must exists.

---
v6 -> v7:
* add skipTest if can't open smaps file of qemu process

v5 -> v6:
* add python3 shebang to tests/functional/test_memlock.py

v4 -> v5:
* refactor skipLockedMemoryTest decorator: using resource.getrlimit()
  function instead of spawning a process

v3 -> v4:
* add skipLockedMemoryTest decorator to skip test if system's locked
  memory limit is below the required threashold;
* add to MemlockTest skipLockedMemoryTest decorator with 2 GB limit.

v2 -> v3:
Move tests to tests/functional dir, as the tests/avocado dir is being phased out.
v2 was [PATCH v2] tests/avocado: add memlock tests.
Supersedes: <20250414075702.9248-1-dtalexundeer@yandex-team.ru>

v1 -> v2:
In the previous send, i forgot to specify new patch version (v2)
So i resend previous patch with version specified.


Alexandr Moshkov (2):
  tests/functional: add skipLockedMemoryTest decorator
  tests/functional: add memlock tests

 tests/functional/meson.build             |   1 +
 tests/functional/qemu_test/__init__.py   |   2 +-
 tests/functional/qemu_test/decorators.py |  18 ++++
 tests/functional/test_memlock.py         | 107 +++++++++++++++++++++++
 4 files changed, 127 insertions(+), 1 deletion(-)
 create mode 100755 tests/functional/test_memlock.py

-- 
2.34.1



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v7 1/2] tests/functional: add skipLockedMemoryTest decorator
  2025-05-21 13:55 [PATCH v7 0/2] tests/functional: add memlock tests Alexandr Moshkov
@ 2025-05-21 13:55 ` Alexandr Moshkov
  2025-05-21 13:55 ` [PATCH v7 2/2] tests/functional: add memlock tests Alexandr Moshkov
  1 sibling, 0 replies; 12+ messages in thread
From: Alexandr Moshkov @ 2025-05-21 13:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Cleber Rosa, yc-core @ yandex-team . ru, Paolo Bonzini,
	Daniel P . Berrangé, Thomas Huth,
	Philippe Mathieu-Daudé, Alexandr Moshkov

Used in future commit to skipping execution of a tests if the system's
locked memory limit is below the required threshold.

Signed-off-by: Alexandr Moshkov <dtalexundeer@yandex-team.ru>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 tests/functional/qemu_test/__init__.py   |  2 +-
 tests/functional/qemu_test/decorators.py | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/tests/functional/qemu_test/__init__.py b/tests/functional/qemu_test/__init__.py
index af41c2c6a2..6e666a059f 100644
--- a/tests/functional/qemu_test/__init__.py
+++ b/tests/functional/qemu_test/__init__.py
@@ -15,6 +15,6 @@
 from .linuxkernel import LinuxKernelTest
 from .decorators import skipIfMissingCommands, skipIfNotMachine, \
     skipFlakyTest, skipUntrustedTest, skipBigDataTest, skipSlowTest, \
-    skipIfMissingImports, skipIfOperatingSystem
+    skipIfMissingImports, skipIfOperatingSystem, skipLockedMemoryTest
 from .archive import archive_extract
 from .uncompress import uncompress
diff --git a/tests/functional/qemu_test/decorators.py b/tests/functional/qemu_test/decorators.py
index 50d29de533..c0d1567b14 100644
--- a/tests/functional/qemu_test/decorators.py
+++ b/tests/functional/qemu_test/decorators.py
@@ -5,6 +5,7 @@
 import importlib
 import os
 import platform
+import resource
 from unittest import skipIf, skipUnless
 
 from .cmd import which
@@ -131,3 +132,20 @@ def skipIfMissingImports(*args):
 
     return skipUnless(has_imports, 'required import(s) "%s" not installed' %
                                    ", ".join(args))
+
+'''
+Decorator to skip execution of a test if the system's
+locked memory limit is below the required threshold.
+Takes required locked memory threshold in kB.
+Example:
+
+  @skipLockedMemoryTest(2_097_152)
+'''
+def skipLockedMemoryTest(locked_memory):
+    # get memlock hard limit in bytes
+    _, ulimit_memory = resource.getrlimit(resource.RLIMIT_MEMLOCK)
+
+    return skipUnless(
+        ulimit_memory == resource.RLIM_INFINITY or ulimit_memory >= locked_memory * 1024,
+        f'Test required {locked_memory} kB of available locked memory',
+    )
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v7 2/2] tests/functional: add memlock tests
  2025-05-21 13:55 [PATCH v7 0/2] tests/functional: add memlock tests Alexandr Moshkov
  2025-05-21 13:55 ` [PATCH v7 1/2] tests/functional: add skipLockedMemoryTest decorator Alexandr Moshkov
@ 2025-05-21 13:55 ` Alexandr Moshkov
  2025-05-22  5:19   ` Thomas Huth
  2025-05-22  7:49   ` Thomas Huth
  1 sibling, 2 replies; 12+ messages in thread
From: Alexandr Moshkov @ 2025-05-21 13:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Cleber Rosa, yc-core @ yandex-team . ru, Paolo Bonzini,
	Daniel P . Berrangé, Thomas Huth,
	Philippe Mathieu-Daudé, Alexandr Moshkov

Add new tests to check the correctness of the `-overcommit memlock`
option (possible values: off, on, on-fault) by using
`/proc/{qemu_pid}/smaps` file to check in Size, Rss and Locked fields of
anonymous segments:

* if `memlock=off`, then Locked = 0 on every anonymous smaps;
* if `memlock=on`, then Size, Rss and Locked values must be equal for
every anon smaps where Rss is not 0;
* if `memlock=on-fault`, then Rss and Locked must be equal on every anon
smaps and anonymous segment with Rss < Size must exists.

Signed-off-by: Alexandr Moshkov <dtalexundeer@yandex-team.ru>
---
 tests/functional/meson.build     |   1 +
 tests/functional/test_memlock.py | 107 +++++++++++++++++++++++++++++++
 2 files changed, 108 insertions(+)
 create mode 100755 tests/functional/test_memlock.py

diff --git a/tests/functional/meson.build b/tests/functional/meson.build
index 0f8be30fe2..339af7835f 100644
--- a/tests/functional/meson.build
+++ b/tests/functional/meson.build
@@ -61,6 +61,7 @@ tests_generic_system = [
   'empty_cpu_model',
   'info_usernet',
   'version',
+  'memlock',
 ]
 
 tests_generic_linuxuser = [
diff --git a/tests/functional/test_memlock.py b/tests/functional/test_memlock.py
new file mode 100755
index 0000000000..a090e7f9ad
--- /dev/null
+++ b/tests/functional/test_memlock.py
@@ -0,0 +1,107 @@
+#!/usr/bin/env python3
+#
+# Functional test that check overcommit memlock options
+#
+# Copyright (c) Yandex Technologies LLC, 2025
+#
+# Author:
+#  Alexandr Moshkov <dtalexundeer@yandex-team.ru>
+#
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+import re
+
+from typing import List, Dict
+
+from qemu_test import QemuSystemTest
+from qemu_test import skipLockedMemoryTest
+
+
+SMAPS_HEADER_PATTERN = re.compile(r'^\w+-\w+', re.MULTILINE)
+SMAPS_VALUE_PATTERN = re.compile(r'^(\w+):\s+(\d+) kB', re.MULTILINE)
+
+
+@skipLockedMemoryTest(2_097_152)  # 2GB
+class MemlockTest(QemuSystemTest):
+    """
+    Boots a Linux system with memlock options.
+    Then verify, that this options is working correctly
+    by checking the smaps of the QEMU proccess.
+    """
+
+    def common_vm_setup_with_memlock(self, memlock):
+        self.vm.add_args('-overcommit', f'mem-lock={memlock}')
+        self.vm.launch()
+
+    def get_anon_smaps_by_pid(self, pid):
+        smaps_raw = self._get_raw_smaps_by_pid(pid)
+        return self._parse_anonymous_smaps(smaps_raw)
+
+    def test_memlock_off(self):
+        self.common_vm_setup_with_memlock('off')
+
+        anon_smaps = self.get_anon_smaps_by_pid(self.vm.get_pid())
+
+        # locked = 0 on every smap
+        for smap in anon_smaps:
+            self.assertEqual(smap['Locked'], 0)
+
+    def test_memlock_on(self):
+        self.common_vm_setup_with_memlock('on')
+
+        anon_smaps = self.get_anon_smaps_by_pid(self.vm.get_pid())
+
+        # size = rss = locked on every smap where rss not 0
+        for smap in anon_smaps:
+            if smap['Rss'] == 0:
+                continue
+            self.assertTrue(smap['Size'] == smap['Rss'] == smap['Locked'])
+
+    def test_memlock_onfault(self):
+        self.common_vm_setup_with_memlock('on-fault')
+
+        anon_smaps = self.get_anon_smaps_by_pid(self.vm.get_pid())
+
+        # rss = locked on every smap and segment with rss < size exists
+        exists = False
+        for smap in anon_smaps:
+            self.assertTrue(smap['Rss'] == smap['Locked'])
+            if smap['Rss'] < smap['Size']:
+                exists = True
+        self.assertTrue(exists)
+
+    def _parse_anonymous_smaps(self, smaps_raw: str) -> List[Dict[str, int]]:
+        result_segments = []
+        current_segment = {}
+        is_anonymous = False
+
+        for line in smaps_raw.split('\n'):
+            if SMAPS_HEADER_PATTERN.match(line):
+                if current_segment and is_anonymous:
+                    result_segments.append(current_segment)
+                current_segment = {}
+                # anonymous segment header looks like this:
+                # 7f3b8d3f0000-7f3b8d3f3000 rw-s 00000000 00:0f 1052
+                # and non anonymous header looks like this:
+                # 7f3b8d3f0000-7f3b8d3f3000 rw-s 00000000 00:0f 1052   [stack]
+                is_anonymous = len(line.split()) == 5
+            elif m := SMAPS_VALUE_PATTERN.match(line):
+                current_segment[m.group(1)] = int(m.group(2))
+
+        if current_segment and is_anonymous:
+            result_segments.append(current_segment)
+
+        return result_segments
+
+    def _get_raw_smaps_by_pid(self, pid: int) -> str:
+        try:
+            with open(f'/proc/{pid}/smaps', 'r') as f:
+                return f.read()
+        except IOError:
+            self.skipTest("Can't open smaps file of the process")
+
+
+if __name__ == '__main__':
+    MemlockTest.main()
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v7 2/2] tests/functional: add memlock tests
  2025-05-21 13:55 ` [PATCH v7 2/2] tests/functional: add memlock tests Alexandr Moshkov
@ 2025-05-22  5:19   ` Thomas Huth
  2025-05-22  7:49   ` Thomas Huth
  1 sibling, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2025-05-22  5:19 UTC (permalink / raw)
  To: Alexandr Moshkov, qemu-devel
  Cc: Cleber Rosa, yc-core @ yandex-team . ru, Paolo Bonzini,
	Daniel P . Berrangé, Philippe Mathieu-Daudé

On 21/05/2025 15.55, Alexandr Moshkov wrote:
> Add new tests to check the correctness of the `-overcommit memlock`
> option (possible values: off, on, on-fault) by using
> `/proc/{qemu_pid}/smaps` file to check in Size, Rss and Locked fields of
> anonymous segments:
> 
> * if `memlock=off`, then Locked = 0 on every anonymous smaps;
> * if `memlock=on`, then Size, Rss and Locked values must be equal for
> every anon smaps where Rss is not 0;
> * if `memlock=on-fault`, then Rss and Locked must be equal on every anon
> smaps and anonymous segment with Rss < Size must exists.
> 
> Signed-off-by: Alexandr Moshkov <dtalexundeer@yandex-team.ru>
> ---
>   tests/functional/meson.build     |   1 +
>   tests/functional/test_memlock.py | 107 +++++++++++++++++++++++++++++++
>   2 files changed, 108 insertions(+)
>   create mode 100755 tests/functional/test_memlock.py
> 
> diff --git a/tests/functional/meson.build b/tests/functional/meson.build
> index 0f8be30fe2..339af7835f 100644
> --- a/tests/functional/meson.build
> +++ b/tests/functional/meson.build
> @@ -61,6 +61,7 @@ tests_generic_system = [
>     'empty_cpu_model',
>     'info_usernet',
>     'version',
> +  'memlock',
>   ]
>   
>   tests_generic_linuxuser = [
> diff --git a/tests/functional/test_memlock.py b/tests/functional/test_memlock.py
> new file mode 100755
> index 0000000000..a090e7f9ad
> --- /dev/null
> +++ b/tests/functional/test_memlock.py
> @@ -0,0 +1,107 @@
> +#!/usr/bin/env python3
> +#
> +# Functional test that check overcommit memlock options
> +#
> +# Copyright (c) Yandex Technologies LLC, 2025
> +#
> +# Author:
> +#  Alexandr Moshkov <dtalexundeer@yandex-team.ru>
> +#
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or
> +# later.  See the COPYING file in the top-level directory.
> +
> +import re
> +
> +from typing import List, Dict
> +
> +from qemu_test import QemuSystemTest
> +from qemu_test import skipLockedMemoryTest
> +
> +
> +SMAPS_HEADER_PATTERN = re.compile(r'^\w+-\w+', re.MULTILINE)
> +SMAPS_VALUE_PATTERN = re.compile(r'^(\w+):\s+(\d+) kB', re.MULTILINE)
> +
> +
> +@skipLockedMemoryTest(2_097_152)  # 2GB
> +class MemlockTest(QemuSystemTest):
> +    """
> +    Boots a Linux system with memlock options.
> +    Then verify, that this options is working correctly
> +    by checking the smaps of the QEMU proccess.
> +    """
> +
> +    def common_vm_setup_with_memlock(self, memlock):
> +        self.vm.add_args('-overcommit', f'mem-lock={memlock}')
> +        self.vm.launch()
> +
> +    def get_anon_smaps_by_pid(self, pid):
> +        smaps_raw = self._get_raw_smaps_by_pid(pid)
> +        return self._parse_anonymous_smaps(smaps_raw)
> +
> +    def test_memlock_off(self):
> +        self.common_vm_setup_with_memlock('off')
> +
> +        anon_smaps = self.get_anon_smaps_by_pid(self.vm.get_pid())
> +
> +        # locked = 0 on every smap
> +        for smap in anon_smaps:
> +            self.assertEqual(smap['Locked'], 0)
> +
> +    def test_memlock_on(self):
> +        self.common_vm_setup_with_memlock('on')
> +
> +        anon_smaps = self.get_anon_smaps_by_pid(self.vm.get_pid())
> +
> +        # size = rss = locked on every smap where rss not 0
> +        for smap in anon_smaps:
> +            if smap['Rss'] == 0:
> +                continue
> +            self.assertTrue(smap['Size'] == smap['Rss'] == smap['Locked'])
> +
> +    def test_memlock_onfault(self):
> +        self.common_vm_setup_with_memlock('on-fault')
> +
> +        anon_smaps = self.get_anon_smaps_by_pid(self.vm.get_pid())
> +
> +        # rss = locked on every smap and segment with rss < size exists
> +        exists = False
> +        for smap in anon_smaps:
> +            self.assertTrue(smap['Rss'] == smap['Locked'])
> +            if smap['Rss'] < smap['Size']:
> +                exists = True
> +        self.assertTrue(exists)
> +
> +    def _parse_anonymous_smaps(self, smaps_raw: str) -> List[Dict[str, int]]:
> +        result_segments = []
> +        current_segment = {}
> +        is_anonymous = False
> +
> +        for line in smaps_raw.split('\n'):
> +            if SMAPS_HEADER_PATTERN.match(line):
> +                if current_segment and is_anonymous:
> +                    result_segments.append(current_segment)
> +                current_segment = {}
> +                # anonymous segment header looks like this:
> +                # 7f3b8d3f0000-7f3b8d3f3000 rw-s 00000000 00:0f 1052
> +                # and non anonymous header looks like this:
> +                # 7f3b8d3f0000-7f3b8d3f3000 rw-s 00000000 00:0f 1052   [stack]
> +                is_anonymous = len(line.split()) == 5
> +            elif m := SMAPS_VALUE_PATTERN.match(line):
> +                current_segment[m.group(1)] = int(m.group(2))
> +
> +        if current_segment and is_anonymous:
> +            result_segments.append(current_segment)
> +
> +        return result_segments
> +
> +    def _get_raw_smaps_by_pid(self, pid: int) -> str:
> +        try:
> +            with open(f'/proc/{pid}/smaps', 'r') as f:
> +                return f.read()
> +        except IOError:

FWIW, I think it should rather be FileNotFoundError in modern Python, but 
IOError still seems to work, too.

> +            self.skipTest("Can't open smaps file of the process")
> +
> +
> +if __name__ == '__main__':
> +    MemlockTest.main()

Thanks, I'll queue the patch for my next pull request with functional patches.

  Thomas



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v7 2/2] tests/functional: add memlock tests
  2025-05-21 13:55 ` [PATCH v7 2/2] tests/functional: add memlock tests Alexandr Moshkov
  2025-05-22  5:19   ` Thomas Huth
@ 2025-05-22  7:49   ` Thomas Huth
  2025-05-22  8:51     ` Alexandr Moshkov
  1 sibling, 1 reply; 12+ messages in thread
From: Thomas Huth @ 2025-05-22  7:49 UTC (permalink / raw)
  To: Alexandr Moshkov, qemu-devel
  Cc: Cleber Rosa, yc-core @ yandex-team . ru, Paolo Bonzini,
	Daniel P . Berrangé, Philippe Mathieu-Daudé

On 21/05/2025 15.55, Alexandr Moshkov wrote:
> Add new tests to check the correctness of the `-overcommit memlock`
> option (possible values: off, on, on-fault) by using
> `/proc/{qemu_pid}/smaps` file to check in Size, Rss and Locked fields of
> anonymous segments:
> 
> * if `memlock=off`, then Locked = 0 on every anonymous smaps;
> * if `memlock=on`, then Size, Rss and Locked values must be equal for
> every anon smaps where Rss is not 0;
> * if `memlock=on-fault`, then Rss and Locked must be equal on every anon
> smaps and anonymous segment with Rss < Size must exists.
> 
> Signed-off-by: Alexandr Moshkov <dtalexundeer@yandex-team.ru>
> ---
...
> diff --git a/tests/functional/test_memlock.py b/tests/functional/test_memlock.py
> new file mode 100755
> index 0000000000..a090e7f9ad
> --- /dev/null
> +++ b/tests/functional/test_memlock.py
> @@ -0,0 +1,107 @@
> +#!/usr/bin/env python3
> +#
> +# Functional test that check overcommit memlock options
> +#
> +# Copyright (c) Yandex Technologies LLC, 2025
> +#
> +# Author:
> +#  Alexandr Moshkov <dtalexundeer@yandex-team.ru>
> +#
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or
> +# later.  See the COPYING file in the top-level directory.

I just noticed: New file need a SPDX identifier nowadays to keep 
scripts/check_patch.pl happy.

Anyway, I now also tested the patch, but for me, it's not working: After 
setting ulimit -l to 2G and running the test, I get:

$ ~/devel/qemu/tests/functional/test_memlock.py
TAP version 13
ok 1 test_memlock.MemlockTest.test_memlock_off
Traceback (most recent call last):
   File "~/devel/qemu/tests/functional/test_memlock.py", line 60, in 
test_memlock_on
     self.assertTrue(smap['Size'] == smap['Rss'] == smap['Locked'])
     ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: False is not true

not ok 2 test_memlock.MemlockTest.test_memlock_on
Traceback (most recent call last):
   File "~/devel/qemu/tests/functional/test_memlock.py", line 70, in 
test_memlock_onfault
     self.assertTrue(smap['Rss'] == smap['Locked'])
     ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: False is not true

not ok 3 test_memlock.MemlockTest.test_memlock_onfault

I added two print statements right in front of the asserts to see the 
values, and for the first one it shows (after many successfully comparisons):

line 60: 4 == 4 == 0

And similar for the second one:

line 70: 4 == 0

FWIW, this is on Fedora 41.

Looking at the smaps file, it seems like this comes from a shared library:

7ff16c7c9000-7ff16c7ca000 r--p 00000000 00:2a 29149631 
/usr/lib64/spa-0.2/support/libspa-support.so
Size:                  4 kB
KernelPageSize:        4 kB
MMUPageSize:           4 kB
Rss:                   4 kB
Pss:                   0 kB
Pss_Dirty:             0 kB
Shared_Clean:          4 kB
Shared_Dirty:          0 kB
Private_Clean:         0 kB
Private_Dirty:         0 kB
Referenced:            4 kB
Anonymous:             0 kB
KSM:                   0 kB
LazyFree:              0 kB
AnonHugePages:         0 kB
ShmemPmdMapped:        0 kB
FilePmdMapped:         0 kB
Shared_Hugetlb:        0 kB
Private_Hugetlb:       0 kB
Swap:                  0 kB
SwapPss:               0 kB
Locked:                0 kB
THPeligible:           0
ProtectionKey:         0

So maybe you've got to ignore the mappings of .so files in your test?

  Thomas



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v7 2/2] tests/functional: add memlock tests
  2025-05-22  7:49   ` Thomas Huth
@ 2025-05-22  8:51     ` Alexandr Moshkov
  2025-05-22  9:16       ` Thomas Huth
  2025-05-22 11:13       ` Daniel P. Berrangé
  0 siblings, 2 replies; 12+ messages in thread
From: Alexandr Moshkov @ 2025-05-22  8:51 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel
  Cc: Cleber Rosa, yc-core @ yandex-team . ru, Paolo Bonzini,
	Daniel P . Berrangé, Philippe Mathieu-Daudé,
	Alexandr Moshkov


On 5/22/25 12:49, Thomas Huth wrote:
> On 21/05/2025 15.55, Alexandr Moshkov wrote:
>> Add new tests to check the correctness of the `-overcommit memlock`
>> option (possible values: off, on, on-fault) by using
>> `/proc/{qemu_pid}/smaps` file to check in Size, Rss and Locked fields of
>> anonymous segments:
>>
>> * if `memlock=off`, then Locked = 0 on every anonymous smaps;
>> * if `memlock=on`, then Size, Rss and Locked values must be equal for
>> every anon smaps where Rss is not 0;
>> * if `memlock=on-fault`, then Rss and Locked must be equal on every anon
>> smaps and anonymous segment with Rss < Size must exists.
>>
>> Signed-off-by: Alexandr Moshkov <dtalexundeer@yandex-team.ru>
>> ---
> ...
>> diff --git a/tests/functional/test_memlock.py 
>> b/tests/functional/test_memlock.py
>> new file mode 100755
>> index 0000000000..a090e7f9ad
>> --- /dev/null
>> +++ b/tests/functional/test_memlock.py
>> @@ -0,0 +1,107 @@
>> +#!/usr/bin/env python3
>> +#
>> +# Functional test that check overcommit memlock options
>> +#
>> +# Copyright (c) Yandex Technologies LLC, 2025
>> +#
>> +# Author:
>> +#  Alexandr Moshkov <dtalexundeer@yandex-team.ru>
>> +#
>> +#
>> +# This work is licensed under the terms of the GNU GPL, version 2 or
>> +# later.  See the COPYING file in the top-level directory.
>
> I just noticed: New file need a SPDX identifier nowadays to keep 
> scripts/check_patch.pl happy.

Hello, thanks for reply, i fix that in a moment!

>
> Anyway, I now also tested the patch, but for me, it's not working: 
> After setting ulimit -l to 2G and running the test, I get:
>
> $ ~/devel/qemu/tests/functional/test_memlock.py
> TAP version 13
> ok 1 test_memlock.MemlockTest.test_memlock_off
> Traceback (most recent call last):
>   File "~/devel/qemu/tests/functional/test_memlock.py", line 60, in 
> test_memlock_on
>     self.assertTrue(smap['Size'] == smap['Rss'] == smap['Locked'])
>     ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> AssertionError: False is not true
>
> not ok 2 test_memlock.MemlockTest.test_memlock_on
> Traceback (most recent call last):
>   File "~/devel/qemu/tests/functional/test_memlock.py", line 70, in 
> test_memlock_onfault
>     self.assertTrue(smap['Rss'] == smap['Locked'])
>     ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> AssertionError: False is not true
>
> not ok 3 test_memlock.MemlockTest.test_memlock_onfault
>
> I added two print statements right in front of the asserts to see the 
> values, and for the first one it shows (after many successfully 
> comparisons):
>
> line 60: 4 == 4 == 0
>
> And similar for the second one:
>
> line 70: 4 == 0
>
> FWIW, this is on Fedora 41.
>
> Looking at the smaps file, it seems like this comes from a shared 
> library:
>
> 7ff16c7c9000-7ff16c7ca000 r--p 00000000 00:2a 29149631 
> /usr/lib64/spa-0.2/support/libspa-support.so
> Size:                  4 kB
> KernelPageSize:        4 kB
> MMUPageSize:           4 kB
> Rss:                   4 kB
> Pss:                   0 kB
> Pss_Dirty:             0 kB
> Shared_Clean:          4 kB
> Shared_Dirty:          0 kB
> Private_Clean:         0 kB
> Private_Dirty:         0 kB
> Referenced:            4 kB
> Anonymous:             0 kB
> KSM:                   0 kB
> LazyFree:              0 kB
> AnonHugePages:         0 kB
> ShmemPmdMapped:        0 kB
> FilePmdMapped:         0 kB
> Shared_Hugetlb:        0 kB
> Private_Hugetlb:       0 kB
> Swap:                  0 kB
> SwapPss:               0 kB
> Locked:                0 kB
> THPeligible:           0
> ProtectionKey:         0
>
> So maybe you've got to ignore the mappings of .so files in your test?

Oh, this segments are already ignored in _parse_anonymous_smaps(), so 
this segment should not have returned from it.

Maybe it comes from another segment? Or i have bug in anon segments 
parsing.. I'll take a closer look, thanks.


Best regards,

Alexandr



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v7 2/2] tests/functional: add memlock tests
  2025-05-22  8:51     ` Alexandr Moshkov
@ 2025-05-22  9:16       ` Thomas Huth
  2025-05-22 10:36         ` Alexandr Moshkov
  2025-05-22 11:13       ` Daniel P. Berrangé
  1 sibling, 1 reply; 12+ messages in thread
From: Thomas Huth @ 2025-05-22  9:16 UTC (permalink / raw)
  To: Alexandr Moshkov, qemu-devel
  Cc: Cleber Rosa, yc-core @ yandex-team . ru, Paolo Bonzini,
	Daniel P . Berrangé, Philippe Mathieu-Daudé

On 22/05/2025 10.51, Alexandr Moshkov wrote:
> 
> On 5/22/25 12:49, Thomas Huth wrote:
>> On 21/05/2025 15.55, Alexandr Moshkov wrote:
>>> Add new tests to check the correctness of the `-overcommit memlock`
>>> option (possible values: off, on, on-fault) by using
>>> `/proc/{qemu_pid}/smaps` file to check in Size, Rss and Locked fields of
>>> anonymous segments:
>>>
>>> * if `memlock=off`, then Locked = 0 on every anonymous smaps;
>>> * if `memlock=on`, then Size, Rss and Locked values must be equal for
>>> every anon smaps where Rss is not 0;
>>> * if `memlock=on-fault`, then Rss and Locked must be equal on every anon
>>> smaps and anonymous segment with Rss < Size must exists.
>>>
>>> Signed-off-by: Alexandr Moshkov <dtalexundeer@yandex-team.ru>
>>> ---
>> ...
>>> diff --git a/tests/functional/test_memlock.py b/tests/functional/ 
>>> test_memlock.py
>>> new file mode 100755
>>> index 0000000000..a090e7f9ad
>>> --- /dev/null
>>> +++ b/tests/functional/test_memlock.py
>>> @@ -0,0 +1,107 @@
>>> +#!/usr/bin/env python3
>>> +#
>>> +# Functional test that check overcommit memlock options
>>> +#
>>> +# Copyright (c) Yandex Technologies LLC, 2025
>>> +#
>>> +# Author:
>>> +#  Alexandr Moshkov <dtalexundeer@yandex-team.ru>
>>> +#
>>> +#
>>> +# This work is licensed under the terms of the GNU GPL, version 2 or
>>> +# later.  See the COPYING file in the top-level directory.
>>
>> I just noticed: New file need a SPDX identifier nowadays to keep scripts/ 
>> check_patch.pl happy.
> 
> Hello, thanks for reply, i fix that in a moment!
> 
>>
>> Anyway, I now also tested the patch, but for me, it's not working: After 
>> setting ulimit -l to 2G and running the test, I get:
>>
>> $ ~/devel/qemu/tests/functional/test_memlock.py
>> TAP version 13
>> ok 1 test_memlock.MemlockTest.test_memlock_off
>> Traceback (most recent call last):
>>   File "~/devel/qemu/tests/functional/test_memlock.py", line 60, in 
>> test_memlock_on
>>     self.assertTrue(smap['Size'] == smap['Rss'] == smap['Locked'])
>>     ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> AssertionError: False is not true
>>
>> not ok 2 test_memlock.MemlockTest.test_memlock_on
>> Traceback (most recent call last):
>>   File "~/devel/qemu/tests/functional/test_memlock.py", line 70, in 
>> test_memlock_onfault
>>     self.assertTrue(smap['Rss'] == smap['Locked'])
>>     ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> AssertionError: False is not true
>>
>> not ok 3 test_memlock.MemlockTest.test_memlock_onfault
>>
>> I added two print statements right in front of the asserts to see the 
>> values, and for the first one it shows (after many successfully comparisons):
>>
>> line 60: 4 == 4 == 0
>>
>> And similar for the second one:
>>
>> line 70: 4 == 0
>>
>> FWIW, this is on Fedora 41.
>>
>> Looking at the smaps file, it seems like this comes from a shared library:
>>
>> 7ff16c7c9000-7ff16c7ca000 r--p 00000000 00:2a 29149631 /usr/lib64/spa-0.2/ 
>> support/libspa-support.so
>> Size:                  4 kB
>> KernelPageSize:        4 kB
>> MMUPageSize:           4 kB
>> Rss:                   4 kB
>> Pss:                   0 kB
>> Pss_Dirty:             0 kB
>> Shared_Clean:          4 kB
>> Shared_Dirty:          0 kB
>> Private_Clean:         0 kB
>> Private_Dirty:         0 kB
>> Referenced:            4 kB
>> Anonymous:             0 kB
>> KSM:                   0 kB
>> LazyFree:              0 kB
>> AnonHugePages:         0 kB
>> ShmemPmdMapped:        0 kB
>> FilePmdMapped:         0 kB
>> Shared_Hugetlb:        0 kB
>> Private_Hugetlb:       0 kB
>> Swap:                  0 kB
>> SwapPss:               0 kB
>> Locked:                0 kB
>> THPeligible:           0
>> ProtectionKey:         0
>>
>> So maybe you've got to ignore the mappings of .so files in your test?
> 
> Oh, this segments are already ignored in _parse_anonymous_smaps(), so this 
> segment should not have returned from it.
> 
> Maybe it comes from another segment? Or i have bug in anon segments 
> parsing.. I'll take a closer look, thanks.

Yes, it seems to be another segment. After looking through the smaps a 
little bit longer, I also spotted this one here:

7f8fcc6a1000-7f8fcc6a2000 rw-p 00000000 00:00 0
Size:                  4 kB
KernelPageSize:        4 kB
MMUPageSize:           4 kB
Rss:                   4 kB
Pss:                   4 kB
Pss_Dirty:             4 kB
Shared_Clean:          0 kB
Shared_Dirty:          0 kB
Private_Clean:         0 kB
Private_Dirty:         4 kB
Referenced:            4 kB
Anonymous:             4 kB
KSM:                   0 kB
LazyFree:              0 kB
AnonHugePages:         0 kB
ShmemPmdMapped:        0 kB
FilePmdMapped:         0 kB
Shared_Hugetlb:        0 kB
Private_Hugetlb:       0 kB
Swap:                  0 kB
SwapPss:               0 kB
Locked:                0 kB
THPeligible:           0
ProtectionKey:         0

  HTH,
   Thomas



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v7 2/2] tests/functional: add memlock tests
  2025-05-22  9:16       ` Thomas Huth
@ 2025-05-22 10:36         ` Alexandr Moshkov
  2025-05-22 13:09           ` Thomas Huth
  0 siblings, 1 reply; 12+ messages in thread
From: Alexandr Moshkov @ 2025-05-22 10:36 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel
  Cc: Cleber Rosa, yc-core @ yandex-team . ru, Paolo Bonzini,
	Daniel P . Berrangé, Philippe Mathieu-Daudé


On 5/22/25 14:16, Thomas Huth wrote:
> On 22/05/2025 10.51, Alexandr Moshkov wrote:
>>
>> On 5/22/25 12:49, Thomas Huth wrote:
>>> On 21/05/2025 15.55, Alexandr Moshkov wrote:
>>>> Add new tests to check the correctness of the `-overcommit memlock`
>>>> option (possible values: off, on, on-fault) by using
>>>> `/proc/{qemu_pid}/smaps` file to check in Size, Rss and Locked 
>>>> fields of
>>>> anonymous segments:
>>>>
>>>> * if `memlock=off`, then Locked = 0 on every anonymous smaps;
>>>> * if `memlock=on`, then Size, Rss and Locked values must be equal for
>>>> every anon smaps where Rss is not 0;
>>>> * if `memlock=on-fault`, then Rss and Locked must be equal on every 
>>>> anon
>>>> smaps and anonymous segment with Rss < Size must exists.
>>>>
>>>> Signed-off-by: Alexandr Moshkov <dtalexundeer@yandex-team.ru>
>>>> ---
>>> ...
>>>> diff --git a/tests/functional/test_memlock.py b/tests/functional/ 
>>>> test_memlock.py
>>>> new file mode 100755
>>>> index 0000000000..a090e7f9ad
>>>> --- /dev/null
>>>> +++ b/tests/functional/test_memlock.py
>>>> @@ -0,0 +1,107 @@
>>>> +#!/usr/bin/env python3
>>>> +#
>>>> +# Functional test that check overcommit memlock options
>>>> +#
>>>> +# Copyright (c) Yandex Technologies LLC, 2025
>>>> +#
>>>> +# Author:
>>>> +#  Alexandr Moshkov <dtalexundeer@yandex-team.ru>
>>>> +#
>>>> +#
>>>> +# This work is licensed under the terms of the GNU GPL, version 2 or
>>>> +# later.  See the COPYING file in the top-level directory.
>>>
>>> I just noticed: New file need a SPDX identifier nowadays to keep 
>>> scripts/ check_patch.pl happy.
>>
>> Hello, thanks for reply, i fix that in a moment!
>>
>>>
>>> Anyway, I now also tested the patch, but for me, it's not working: 
>>> After setting ulimit -l to 2G and running the test, I get:
>>>
>>> $ ~/devel/qemu/tests/functional/test_memlock.py
>>> TAP version 13
>>> ok 1 test_memlock.MemlockTest.test_memlock_off
>>> Traceback (most recent call last):
>>>   File "~/devel/qemu/tests/functional/test_memlock.py", line 60, in 
>>> test_memlock_on
>>>     self.assertTrue(smap['Size'] == smap['Rss'] == smap['Locked'])
>>> ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>> AssertionError: False is not true
>>>
>>> not ok 2 test_memlock.MemlockTest.test_memlock_on
>>> Traceback (most recent call last):
>>>   File "~/devel/qemu/tests/functional/test_memlock.py", line 70, in 
>>> test_memlock_onfault
>>>     self.assertTrue(smap['Rss'] == smap['Locked'])
>>>     ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>> AssertionError: False is not true
>>>
>>> not ok 3 test_memlock.MemlockTest.test_memlock_onfault
>>>
>>> I added two print statements right in front of the asserts to see 
>>> the values, and for the first one it shows (after many successfully 
>>> comparisons):
>>>
>>> line 60: 4 == 4 == 0
>>>
>>> And similar for the second one:
>>>
>>> line 70: 4 == 0
>>>
>>> FWIW, this is on Fedora 41.
>>>
>>> Looking at the smaps file, it seems like this comes from a shared 
>>> library:
>>>
>>> 7ff16c7c9000-7ff16c7ca000 r--p 00000000 00:2a 29149631 
>>> /usr/lib64/spa-0.2/ support/libspa-support.so
>>> Size:                  4 kB
>>> KernelPageSize:        4 kB
>>> MMUPageSize:           4 kB
>>> Rss:                   4 kB
>>> Pss:                   0 kB
>>> Pss_Dirty:             0 kB
>>> Shared_Clean:          4 kB
>>> Shared_Dirty:          0 kB
>>> Private_Clean:         0 kB
>>> Private_Dirty:         0 kB
>>> Referenced:            4 kB
>>> Anonymous:             0 kB
>>> KSM:                   0 kB
>>> LazyFree:              0 kB
>>> AnonHugePages:         0 kB
>>> ShmemPmdMapped:        0 kB
>>> FilePmdMapped:         0 kB
>>> Shared_Hugetlb:        0 kB
>>> Private_Hugetlb:       0 kB
>>> Swap:                  0 kB
>>> SwapPss:               0 kB
>>> Locked:                0 kB
>>> THPeligible:           0
>>> ProtectionKey:         0
>>>
>>> So maybe you've got to ignore the mappings of .so files in your test?
>>
>> Oh, this segments are already ignored in _parse_anonymous_smaps(), so 
>> this segment should not have returned from it.
>>
>> Maybe it comes from another segment? Or i have bug in anon segments 
>> parsing.. I'll take a closer look, thanks.
>
> Yes, it seems to be another segment. After looking through the smaps a 
> little bit longer, I also spotted this one here:
>
> 7f8fcc6a1000-7f8fcc6a2000 rw-p 00000000 00:00 0
> Size:                  4 kB
> KernelPageSize:        4 kB
> MMUPageSize:           4 kB
> Rss:                   4 kB
> Pss:                   4 kB
> Pss_Dirty:             4 kB
> Shared_Clean:          0 kB
> Shared_Dirty:          0 kB
> Private_Clean:         0 kB
> Private_Dirty:         4 kB
> Referenced:            4 kB
> Anonymous:             4 kB
> KSM:                   0 kB
> LazyFree:              0 kB
> AnonHugePages:         0 kB
> ShmemPmdMapped:        0 kB
> FilePmdMapped:         0 kB
> Shared_Hugetlb:        0 kB
> Private_Hugetlb:       0 kB
> Swap:                  0 kB
> SwapPss:               0 kB
> Locked:                0 kB
> THPeligible:           0
> ProtectionKey:         0
>
>  HTH,
>   Thomas


Can you, please, also provide QEMU package version or how you configure 
and build it?

Or maybe attach full smaps file, I think it would help me a lot. Thanks!


Best regards,

Alexandr



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v7 2/2] tests/functional: add memlock tests
  2025-05-22  8:51     ` Alexandr Moshkov
  2025-05-22  9:16       ` Thomas Huth
@ 2025-05-22 11:13       ` Daniel P. Berrangé
  2025-05-22 12:40         ` Alexandr Moshkov
  1 sibling, 1 reply; 12+ messages in thread
From: Daniel P. Berrangé @ 2025-05-22 11:13 UTC (permalink / raw)
  To: Alexandr Moshkov
  Cc: Thomas Huth, qemu-devel, Cleber Rosa, yc-core @ yandex-team . ru,
	Paolo Bonzini, Philippe Mathieu-Daudé

On Thu, May 22, 2025 at 01:51:44PM +0500, Alexandr Moshkov wrote:
> 
> On 5/22/25 12:49, Thomas Huth wrote:
> > On 21/05/2025 15.55, Alexandr Moshkov wrote:
> > > Add new tests to check the correctness of the `-overcommit memlock`
> > > option (possible values: off, on, on-fault) by using
> > > `/proc/{qemu_pid}/smaps` file to check in Size, Rss and Locked fields of
> > > anonymous segments:
> > > 
> > > * if `memlock=off`, then Locked = 0 on every anonymous smaps;
> > > * if `memlock=on`, then Size, Rss and Locked values must be equal for
> > > every anon smaps where Rss is not 0;
> > > * if `memlock=on-fault`, then Rss and Locked must be equal on every anon
> > > smaps and anonymous segment with Rss < Size must exists.
> > > 
> > > Signed-off-by: Alexandr Moshkov <dtalexundeer@yandex-team.ru>
> > > ---
> > ...
> > > diff --git a/tests/functional/test_memlock.py
> > > b/tests/functional/test_memlock.py
> > > new file mode 100755
> > > index 0000000000..a090e7f9ad
> > > --- /dev/null
> > > +++ b/tests/functional/test_memlock.py
> > > @@ -0,0 +1,107 @@
> > > +#!/usr/bin/env python3
> > > +#
> > > +# Functional test that check overcommit memlock options
> > > +#
> > > +# Copyright (c) Yandex Technologies LLC, 2025
> > > +#
> > > +# Author:
> > > +#  Alexandr Moshkov <dtalexundeer@yandex-team.ru>
> > > +#
> > > +#
> > > +# This work is licensed under the terms of the GNU GPL, version 2 or
> > > +# later.  See the COPYING file in the top-level directory.
> > 
> > I just noticed: New file need a SPDX identifier nowadays to keep
> > scripts/check_patch.pl happy.
> 
> Hello, thanks for reply, i fix that in a moment!
> 
> > 
> > Anyway, I now also tested the patch, but for me, it's not working: After
> > setting ulimit -l to 2G and running the test, I get:
> > 
> > $ ~/devel/qemu/tests/functional/test_memlock.py
> > TAP version 13
> > ok 1 test_memlock.MemlockTest.test_memlock_off
> > Traceback (most recent call last):
> >   File "~/devel/qemu/tests/functional/test_memlock.py", line 60, in
> > test_memlock_on
> >     self.assertTrue(smap['Size'] == smap['Rss'] == smap['Locked'])
> >     ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > AssertionError: False is not true
> > 
> > not ok 2 test_memlock.MemlockTest.test_memlock_on
> > Traceback (most recent call last):
> >   File "~/devel/qemu/tests/functional/test_memlock.py", line 70, in
> > test_memlock_onfault
> >     self.assertTrue(smap['Rss'] == smap['Locked'])
> >     ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > AssertionError: False is not true
> > 
> > not ok 3 test_memlock.MemlockTest.test_memlock_onfault
> > 
> > I added two print statements right in front of the asserts to see the
> > values, and for the first one it shows (after many successfully
> > comparisons):
> > 
> > line 60: 4 == 4 == 0
> > 
> > And similar for the second one:
> > 
> > line 70: 4 == 0
> > 
> > FWIW, this is on Fedora 41.
> > 
> > Looking at the smaps file, it seems like this comes from a shared
> > library:
> > 
> > 7ff16c7c9000-7ff16c7ca000 r--p 00000000 00:2a 29149631
> > /usr/lib64/spa-0.2/support/libspa-support.so
> > Size:                  4 kB
> > KernelPageSize:        4 kB
> > MMUPageSize:           4 kB
> > Rss:                   4 kB
> > Pss:                   0 kB
> > Pss_Dirty:             0 kB
> > Shared_Clean:          4 kB
> > Shared_Dirty:          0 kB
> > Private_Clean:         0 kB
> > Private_Dirty:         0 kB
> > Referenced:            4 kB
> > Anonymous:             0 kB
> > KSM:                   0 kB
> > LazyFree:              0 kB
> > AnonHugePages:         0 kB
> > ShmemPmdMapped:        0 kB
> > FilePmdMapped:         0 kB
> > Shared_Hugetlb:        0 kB
> > Private_Hugetlb:       0 kB
> > Swap:                  0 kB
> > SwapPss:               0 kB
> > Locked:                0 kB
> > THPeligible:           0
> > ProtectionKey:         0
> > 
> > So maybe you've got to ignore the mappings of .so files in your test?
> 
> Oh, this segments are already ignored in _parse_anonymous_smaps(), so this
> segment should not have returned from it.
> 
> Maybe it comes from another segment? Or i have bug in anon segments
> parsing.. I'll take a closer look, thanks.

It is strange that smaps reports regioons as not locked, despite
being resident, as mlockall() claims that it locks *everything*.

None the less, I wonder if using smaps is overkill & more trouble
than it is worth ?

The /proc/$PID/status file is simpler giving a process level overview:

Normal usage:

  VmSize:   378964 kB
  VmLck:         0 kB

mem-lock=on usage:

  VmSize:   378964 kB
  VmLck:    378964 kB

and mem-lock=on-fault will be some value in between the two extremes.

Parsing this seems likely to be more reliable and easier than smaps.

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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v7 2/2] tests/functional: add memlock tests
  2025-05-22 11:13       ` Daniel P. Berrangé
@ 2025-05-22 12:40         ` Alexandr Moshkov
  2025-05-22 13:04           ` Thomas Huth
  0 siblings, 1 reply; 12+ messages in thread
From: Alexandr Moshkov @ 2025-05-22 12:40 UTC (permalink / raw)
  To: Daniel P. Berrangé, qemu-devel
  Cc: Thomas Huth, Cleber Rosa, yc-core @ yandex-team . ru,
	Paolo Bonzini, Philippe Mathieu-Daudé


On 5/22/25 16:13, Daniel P. Berrangé wrote:
> On Thu, May 22, 2025 at 01:51:44PM +0500, Alexandr Moshkov wrote:
>> On 5/22/25 12:49, Thomas Huth wrote:
>>> On 21/05/2025 15.55, Alexandr Moshkov wrote:
>>>> Add new tests to check the correctness of the `-overcommit memlock`
>>>> option (possible values: off, on, on-fault) by using
>>>> `/proc/{qemu_pid}/smaps` file to check in Size, Rss and Locked fields of
>>>> anonymous segments:
>>>>
>>>> * if `memlock=off`, then Locked = 0 on every anonymous smaps;
>>>> * if `memlock=on`, then Size, Rss and Locked values must be equal for
>>>> every anon smaps where Rss is not 0;
>>>> * if `memlock=on-fault`, then Rss and Locked must be equal on every anon
>>>> smaps and anonymous segment with Rss < Size must exists.
>>>>
>>>> Signed-off-by: Alexandr Moshkov <dtalexundeer@yandex-team.ru>
>>>> ---
>>> ...
>>>> diff --git a/tests/functional/test_memlock.py
>>>> b/tests/functional/test_memlock.py
>>>> new file mode 100755
>>>> index 0000000000..a090e7f9ad
>>>> --- /dev/null
>>>> +++ b/tests/functional/test_memlock.py
>>>> @@ -0,0 +1,107 @@
>>>> +#!/usr/bin/env python3
>>>> +#
>>>> +# Functional test that check overcommit memlock options
>>>> +#
>>>> +# Copyright (c) Yandex Technologies LLC, 2025
>>>> +#
>>>> +# Author:
>>>> +#  Alexandr Moshkov <dtalexundeer@yandex-team.ru>
>>>> +#
>>>> +#
>>>> +# This work is licensed under the terms of the GNU GPL, version 2 or
>>>> +# later.  See the COPYING file in the top-level directory.
>>> I just noticed: New file need a SPDX identifier nowadays to keep
>>> scripts/check_patch.pl happy.
>> Hello, thanks for reply, i fix that in a moment!
>>
>>> Anyway, I now also tested the patch, but for me, it's not working: After
>>> setting ulimit -l to 2G and running the test, I get:
>>>
>>> $ ~/devel/qemu/tests/functional/test_memlock.py
>>> TAP version 13
>>> ok 1 test_memlock.MemlockTest.test_memlock_off
>>> Traceback (most recent call last):
>>>    File "~/devel/qemu/tests/functional/test_memlock.py", line 60, in
>>> test_memlock_on
>>>      self.assertTrue(smap['Size'] == smap['Rss'] == smap['Locked'])
>>>      ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>> AssertionError: False is not true
>>>
>>> not ok 2 test_memlock.MemlockTest.test_memlock_on
>>> Traceback (most recent call last):
>>>    File "~/devel/qemu/tests/functional/test_memlock.py", line 70, in
>>> test_memlock_onfault
>>>      self.assertTrue(smap['Rss'] == smap['Locked'])
>>>      ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>> AssertionError: False is not true
>>>
>>> not ok 3 test_memlock.MemlockTest.test_memlock_onfault
>>>
>>> I added two print statements right in front of the asserts to see the
>>> values, and for the first one it shows (after many successfully
>>> comparisons):
>>>
>>> line 60: 4 == 4 == 0
>>>
>>> And similar for the second one:
>>>
>>> line 70: 4 == 0
>>>
>>> FWIW, this is on Fedora 41.
>>>
>>> Looking at the smaps file, it seems like this comes from a shared
>>> library:
>>>
>>> 7ff16c7c9000-7ff16c7ca000 r--p 00000000 00:2a 29149631
>>> /usr/lib64/spa-0.2/support/libspa-support.so
>>> Size:                  4 kB
>>> KernelPageSize:        4 kB
>>> MMUPageSize:           4 kB
>>> Rss:                   4 kB
>>> Pss:                   0 kB
>>> Pss_Dirty:             0 kB
>>> Shared_Clean:          4 kB
>>> Shared_Dirty:          0 kB
>>> Private_Clean:         0 kB
>>> Private_Dirty:         0 kB
>>> Referenced:            4 kB
>>> Anonymous:             0 kB
>>> KSM:                   0 kB
>>> LazyFree:              0 kB
>>> AnonHugePages:         0 kB
>>> ShmemPmdMapped:        0 kB
>>> FilePmdMapped:         0 kB
>>> Shared_Hugetlb:        0 kB
>>> Private_Hugetlb:       0 kB
>>> Swap:                  0 kB
>>> SwapPss:               0 kB
>>> Locked:                0 kB
>>> THPeligible:           0
>>> ProtectionKey:         0
>>>
>>> So maybe you've got to ignore the mappings of .so files in your test?
>> Oh, this segments are already ignored in _parse_anonymous_smaps(), so this
>> segment should not have returned from it.
>>
>> Maybe it comes from another segment? Or i have bug in anon segments
>> parsing.. I'll take a closer look, thanks.
> It is strange that smaps reports regioons as not locked, despite
> being resident, as mlockall() claims that it locks *everything*.

Yes it is. Maybe its just segment from mlock=off test case?

>
> None the less, I wonder if using smaps is overkill & more trouble
> than it is worth ?
>
> The /proc/$PID/status file is simpler giving a process level overview:
>
> Normal usage:
>
>    VmSize:   378964 kB
>    VmLck:         0 kB
>
> mem-lock=on usage:
>
>    VmSize:   378964 kB
>    VmLck:    378964 kB
>
> and mem-lock=on-fault will be some value in between the two extremes.
>
> Parsing this seems likely to be more reliable and easier than smaps.

Maybe you right. I guess I choose smaps over status for more detailed 
assertions in tests.


Best regards,

Alexandr



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v7 2/2] tests/functional: add memlock tests
  2025-05-22 12:40         ` Alexandr Moshkov
@ 2025-05-22 13:04           ` Thomas Huth
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2025-05-22 13:04 UTC (permalink / raw)
  To: Alexandr Moshkov, Daniel P. Berrangé, qemu-devel
  Cc: Cleber Rosa, yc-core @ yandex-team . ru, Paolo Bonzini,
	Philippe Mathieu-Daudé

On 22/05/2025 14.40, Alexandr Moshkov wrote:
> 
> On 5/22/25 16:13, Daniel P. Berrangé wrote:
>> On Thu, May 22, 2025 at 01:51:44PM +0500, Alexandr Moshkov wrote:
>>> On 5/22/25 12:49, Thomas Huth wrote:
>>>> On 21/05/2025 15.55, Alexandr Moshkov wrote:
>>>>> Add new tests to check the correctness of the `-overcommit memlock`
>>>>> option (possible values: off, on, on-fault) by using
>>>>> `/proc/{qemu_pid}/smaps` file to check in Size, Rss and Locked fields of
>>>>> anonymous segments:
>>>>>
>>>>> * if `memlock=off`, then Locked = 0 on every anonymous smaps;
>>>>> * if `memlock=on`, then Size, Rss and Locked values must be equal for
>>>>> every anon smaps where Rss is not 0;
>>>>> * if `memlock=on-fault`, then Rss and Locked must be equal on every anon
>>>>> smaps and anonymous segment with Rss < Size must exists.
>>>>>
>>>>> Signed-off-by: Alexandr Moshkov <dtalexundeer@yandex-team.ru>
>>>>> ---
>>>> ...
>>>>> diff --git a/tests/functional/test_memlock.py
>>>>> b/tests/functional/test_memlock.py
>>>>> new file mode 100755
>>>>> index 0000000000..a090e7f9ad
>>>>> --- /dev/null
>>>>> +++ b/tests/functional/test_memlock.py
>>>>> @@ -0,0 +1,107 @@
>>>>> +#!/usr/bin/env python3
>>>>> +#
>>>>> +# Functional test that check overcommit memlock options
>>>>> +#
>>>>> +# Copyright (c) Yandex Technologies LLC, 2025
>>>>> +#
>>>>> +# Author:
>>>>> +#  Alexandr Moshkov <dtalexundeer@yandex-team.ru>
>>>>> +#
>>>>> +#
>>>>> +# This work is licensed under the terms of the GNU GPL, version 2 or
>>>>> +# later.  See the COPYING file in the top-level directory.
>>>> I just noticed: New file need a SPDX identifier nowadays to keep
>>>> scripts/check_patch.pl happy.
>>> Hello, thanks for reply, i fix that in a moment!
>>>
>>>> Anyway, I now also tested the patch, but for me, it's not working: After
>>>> setting ulimit -l to 2G and running the test, I get:
>>>>
>>>> $ ~/devel/qemu/tests/functional/test_memlock.py
>>>> TAP version 13
>>>> ok 1 test_memlock.MemlockTest.test_memlock_off
>>>> Traceback (most recent call last):
>>>>    File "~/devel/qemu/tests/functional/test_memlock.py", line 60, in
>>>> test_memlock_on
>>>>      self.assertTrue(smap['Size'] == smap['Rss'] == smap['Locked'])
>>>>      ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>> AssertionError: False is not true
>>>>
>>>> not ok 2 test_memlock.MemlockTest.test_memlock_on
>>>> Traceback (most recent call last):
>>>>    File "~/devel/qemu/tests/functional/test_memlock.py", line 70, in
>>>> test_memlock_onfault
>>>>      self.assertTrue(smap['Rss'] == smap['Locked'])
>>>>      ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>> AssertionError: False is not true
>>>>
>>>> not ok 3 test_memlock.MemlockTest.test_memlock_onfault
>>>>
>>>> I added two print statements right in front of the asserts to see the
>>>> values, and for the first one it shows (after many successfully
>>>> comparisons):
>>>>
>>>> line 60: 4 == 4 == 0
>>>>
>>>> And similar for the second one:
>>>>
>>>> line 70: 4 == 0
>>>>
>>>> FWIW, this is on Fedora 41.
>>>>
>>>> Looking at the smaps file, it seems like this comes from a shared
>>>> library:
>>>>
>>>> 7ff16c7c9000-7ff16c7ca000 r--p 00000000 00:2a 29149631
>>>> /usr/lib64/spa-0.2/support/libspa-support.so
>>>> Size:                  4 kB
>>>> KernelPageSize:        4 kB
>>>> MMUPageSize:           4 kB
>>>> Rss:                   4 kB
>>>> Pss:                   0 kB
>>>> Pss_Dirty:             0 kB
>>>> Shared_Clean:          4 kB
>>>> Shared_Dirty:          0 kB
>>>> Private_Clean:         0 kB
>>>> Private_Dirty:         0 kB
>>>> Referenced:            4 kB
>>>> Anonymous:             0 kB
>>>> KSM:                   0 kB
>>>> LazyFree:              0 kB
>>>> AnonHugePages:         0 kB
>>>> ShmemPmdMapped:        0 kB
>>>> FilePmdMapped:         0 kB
>>>> Shared_Hugetlb:        0 kB
>>>> Private_Hugetlb:       0 kB
>>>> Swap:                  0 kB
>>>> SwapPss:               0 kB
>>>> Locked:                0 kB
>>>> THPeligible:           0
>>>> ProtectionKey:         0
>>>>
>>>> So maybe you've got to ignore the mappings of .so files in your test?
>>> Oh, this segments are already ignored in _parse_anonymous_smaps(), so this
>>> segment should not have returned from it.
>>>
>>> Maybe it comes from another segment? Or i have bug in anon segments
>>> parsing.. I'll take a closer look, thanks.
>> It is strange that smaps reports regioons as not locked, despite
>> being resident, as mlockall() claims that it locks *everything*.
> 
> Yes it is. Maybe its just segment from mlock=off test case?

I've taken it from a QEMU that I ran like this:

  ./qemu-system-x86_64 -overcommit mem-lock=on -display none

By the way, the information from the status file looks like this:

VmSize:	 1580432 kB
VmLck:	 1580404 kB

So looks like almost all got locked, except for some few kilobytes.
I guess your test likely just has to take into account some wiggle room...?

  Thomas



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v7 2/2] tests/functional: add memlock tests
  2025-05-22 10:36         ` Alexandr Moshkov
@ 2025-05-22 13:09           ` Thomas Huth
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2025-05-22 13:09 UTC (permalink / raw)
  To: Alexandr Moshkov, qemu-devel
  Cc: yc-core @ yandex-team . ru, Paolo Bonzini,
	Daniel P . Berrangé, Philippe Mathieu-Daudé

On 22/05/2025 12.36, Alexandr Moshkov wrote:
> 
> On 5/22/25 14:16, Thomas Huth wrote:
>> On 22/05/2025 10.51, Alexandr Moshkov wrote:
>>>
>>> On 5/22/25 12:49, Thomas Huth wrote:
>>>> On 21/05/2025 15.55, Alexandr Moshkov wrote:
>>>>> Add new tests to check the correctness of the `-overcommit memlock`
>>>>> option (possible values: off, on, on-fault) by using
>>>>> `/proc/{qemu_pid}/smaps` file to check in Size, Rss and Locked fields of
>>>>> anonymous segments:
>>>>>
>>>>> * if `memlock=off`, then Locked = 0 on every anonymous smaps;
>>>>> * if `memlock=on`, then Size, Rss and Locked values must be equal for
>>>>> every anon smaps where Rss is not 0;
>>>>> * if `memlock=on-fault`, then Rss and Locked must be equal on every anon
>>>>> smaps and anonymous segment with Rss < Size must exists.
>>>>>
>>>>> Signed-off-by: Alexandr Moshkov <dtalexundeer@yandex-team.ru>
>>>>> ---
>>>> ...
>>>>> diff --git a/tests/functional/test_memlock.py b/tests/functional/ 
>>>>> test_memlock.py
>>>>> new file mode 100755
>>>>> index 0000000000..a090e7f9ad
>>>>> --- /dev/null
>>>>> +++ b/tests/functional/test_memlock.py
>>>>> @@ -0,0 +1,107 @@
>>>>> +#!/usr/bin/env python3
>>>>> +#
>>>>> +# Functional test that check overcommit memlock options
>>>>> +#
>>>>> +# Copyright (c) Yandex Technologies LLC, 2025
>>>>> +#
>>>>> +# Author:
>>>>> +#  Alexandr Moshkov <dtalexundeer@yandex-team.ru>
>>>>> +#
>>>>> +#
>>>>> +# This work is licensed under the terms of the GNU GPL, version 2 or
>>>>> +# later.  See the COPYING file in the top-level directory.
>>>>
>>>> I just noticed: New file need a SPDX identifier nowadays to keep 
>>>> scripts/ check_patch.pl happy.
>>>
>>> Hello, thanks for reply, i fix that in a moment!
>>>
>>>>
>>>> Anyway, I now also tested the patch, but for me, it's not working: After 
>>>> setting ulimit -l to 2G and running the test, I get:
>>>>
>>>> $ ~/devel/qemu/tests/functional/test_memlock.py
>>>> TAP version 13
>>>> ok 1 test_memlock.MemlockTest.test_memlock_off
>>>> Traceback (most recent call last):
>>>>   File "~/devel/qemu/tests/functional/test_memlock.py", line 60, in 
>>>> test_memlock_on
>>>>     self.assertTrue(smap['Size'] == smap['Rss'] == smap['Locked'])
>>>> ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>> AssertionError: False is not true
>>>>
>>>> not ok 2 test_memlock.MemlockTest.test_memlock_on
>>>> Traceback (most recent call last):
>>>>   File "~/devel/qemu/tests/functional/test_memlock.py", line 70, in 
>>>> test_memlock_onfault
>>>>     self.assertTrue(smap['Rss'] == smap['Locked'])
>>>>     ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>> AssertionError: False is not true
>>>>
>>>> not ok 3 test_memlock.MemlockTest.test_memlock_onfault
>>>>
>>>> I added two print statements right in front of the asserts to see the 
>>>> values, and for the first one it shows (after many successfully 
>>>> comparisons):
>>>>
>>>> line 60: 4 == 4 == 0
>>>>
>>>> And similar for the second one:
>>>>
>>>> line 70: 4 == 0
>>>>
>>>> FWIW, this is on Fedora 41.
>>>>
>>>> Looking at the smaps file, it seems like this comes from a shared library:
>>>>
>>>> 7ff16c7c9000-7ff16c7ca000 r--p 00000000 00:2a 29149631 /usr/lib64/ 
>>>> spa-0.2/ support/libspa-support.so
>>>> Size:                  4 kB
>>>> KernelPageSize:        4 kB
>>>> MMUPageSize:           4 kB
>>>> Rss:                   4 kB
>>>> Pss:                   0 kB
>>>> Pss_Dirty:             0 kB
>>>> Shared_Clean:          4 kB
>>>> Shared_Dirty:          0 kB
>>>> Private_Clean:         0 kB
>>>> Private_Dirty:         0 kB
>>>> Referenced:            4 kB
>>>> Anonymous:             0 kB
>>>> KSM:                   0 kB
>>>> LazyFree:              0 kB
>>>> AnonHugePages:         0 kB
>>>> ShmemPmdMapped:        0 kB
>>>> FilePmdMapped:         0 kB
>>>> Shared_Hugetlb:        0 kB
>>>> Private_Hugetlb:       0 kB
>>>> Swap:                  0 kB
>>>> SwapPss:               0 kB
>>>> Locked:                0 kB
>>>> THPeligible:           0
>>>> ProtectionKey:         0
>>>>
>>>> So maybe you've got to ignore the mappings of .so files in your test?
>>>
>>> Oh, this segments are already ignored in _parse_anonymous_smaps(), so 
>>> this segment should not have returned from it.
>>>
>>> Maybe it comes from another segment? Or i have bug in anon segments 
>>> parsing.. I'll take a closer look, thanks.
>>
>> Yes, it seems to be another segment. After looking through the smaps a 
>> little bit longer, I also spotted this one here:
>>
>> 7f8fcc6a1000-7f8fcc6a2000 rw-p 00000000 00:00 0
>> Size:                  4 kB
>> KernelPageSize:        4 kB
>> MMUPageSize:           4 kB
>> Rss:                   4 kB
>> Pss:                   4 kB
>> Pss_Dirty:             4 kB
>> Shared_Clean:          0 kB
>> Shared_Dirty:          0 kB
>> Private_Clean:         0 kB
>> Private_Dirty:         4 kB
>> Referenced:            4 kB
>> Anonymous:             4 kB
>> KSM:                   0 kB
>> LazyFree:              0 kB
>> AnonHugePages:         0 kB
>> ShmemPmdMapped:        0 kB
>> FilePmdMapped:         0 kB
>> Shared_Hugetlb:        0 kB
>> Private_Hugetlb:       0 kB
>> Swap:                  0 kB
>> SwapPss:               0 kB
>> Locked:                0 kB
>> THPeligible:           0
>> ProtectionKey:         0
>>
>>  HTH,
>>   Thomas
> 
> 
> Can you, please, also provide QEMU package version or how you configure and 
> build it?

I originally built it from the master branch, but I get the same behavior 
with the binary from the Fedora 41 distribution when I run it like this:

/usr/bin/qemu-system-x86_64 -overcommit mem-lock=on -display none

That's from the qemu-system-x86-core-9.1.3-2.fc41.x86_64 package.

  HTH,
   Thomas



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-05-22 13:10 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-21 13:55 [PATCH v7 0/2] tests/functional: add memlock tests Alexandr Moshkov
2025-05-21 13:55 ` [PATCH v7 1/2] tests/functional: add skipLockedMemoryTest decorator Alexandr Moshkov
2025-05-21 13:55 ` [PATCH v7 2/2] tests/functional: add memlock tests Alexandr Moshkov
2025-05-22  5:19   ` Thomas Huth
2025-05-22  7:49   ` Thomas Huth
2025-05-22  8:51     ` Alexandr Moshkov
2025-05-22  9:16       ` Thomas Huth
2025-05-22 10:36         ` Alexandr Moshkov
2025-05-22 13:09           ` Thomas Huth
2025-05-22 11:13       ` Daniel P. Berrangé
2025-05-22 12:40         ` Alexandr Moshkov
2025-05-22 13:04           ` Thomas Huth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).