* [PATCH] mm/selftests: improve UFFD-WP feature detection in KSM test
@ 2025-06-22 8:10 Li Wang
2025-06-23 8:33 ` David Hildenbrand
2025-06-24 4:24 ` [PATCH v2] selftests/mm: Fix UFFDIO_API usage with proper two-step feature negotiation Li Wang
0 siblings, 2 replies; 5+ messages in thread
From: Li Wang @ 2025-06-22 8:10 UTC (permalink / raw)
To: akpm, linux-kselftest, linux-mm, linux-kernel
Cc: Aruna Ramakrishna, Bagas Sanjaya, Catalin Marinas, Dave Hansen,
David Hildenbrand, Joey Gouly, Johannes Weiner, Keith Lucas,
Ryan Roberts, Shuah Khan
The current implementation of test_unmerge_uffd_wp() explicitly sets
`uffdio_api.features = UFFD_FEATURE_PAGEFAULT_FLAG_WP` before calling
UFFDIO_API. This can cause the ioctl() call to fail with EINVAL on kernels
that do not support UFFD-WP, leading the test to fail unnecessarily:
# ------------------------------
# running ./ksm_functional_tests
# ------------------------------
# TAP version 13
# 1..9
# # [RUN] test_unmerge
# ok 1 Pages were unmerged
# # [RUN] test_unmerge_zero_pages
# ok 2 KSM zero pages were unmerged
# # [RUN] test_unmerge_discarded
# ok 3 Pages were unmerged
# # [RUN] test_unmerge_uffd_wp
# not ok 4 UFFDIO_API failed <-----
# # [RUN] test_prot_none
# ok 5 Pages were unmerged
# # [RUN] test_prctl
# ok 6 Setting/clearing PR_SET_MEMORY_MERGE works
# # [RUN] test_prctl_fork
# # No pages got merged
# # [RUN] test_prctl_fork_exec
# ok 7 PR_SET_MEMORY_MERGE value is inherited
# # [RUN] test_prctl_unmerge
# ok 8 Pages were unmerged
# Bail out! 1 out of 8 tests failed
# # Planned tests != run tests (9 != 8)
# # Totals: pass:7 fail:1 xfail:0 xpass:0 skip:0 error:0
# [FAIL]
This patch improves compatibility and error handling by:
1. Changes the feature check to first query supported features (features=0)
rather than specifically requesting WP support.
2. Gracefully skipping the test if:
- UFFDIO_API fails with EINVAL (feature not supported), or
- UFFD_FEATURE_PAGEFAULT_FLAG_WP is not advertised by the kernel.
3. Providing better diagnostics by distinguishing expected failures (e.g.,
EINVAL) from unexpected ones and reporting them using strerror().
The updated logic makes the test more robust across different kernel versions
and configurations, while preserving existing behavior on systems that do
support UFFD-WP.
Signed-off-by: Li Wang <liwang@redhat.com>
Cc: Aruna Ramakrishna <aruna.ramakrishna@oracle.com>
Cc: Bagas Sanjaya <bagasdotme@gmail.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Keith Lucas <keith.lucas@oracle.com>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Shuah Khan <shuah@kernel.org>
---
tools/testing/selftests/mm/ksm_functional_tests.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c
index b61803e36d1c..f3db257dc555 100644
--- a/tools/testing/selftests/mm/ksm_functional_tests.c
+++ b/tools/testing/selftests/mm/ksm_functional_tests.c
@@ -393,9 +393,13 @@ static void test_unmerge_uffd_wp(void)
/* See if UFFD-WP is around. */
uffdio_api.api = UFFD_API;
- uffdio_api.features = UFFD_FEATURE_PAGEFAULT_FLAG_WP;
+ uffdio_api.features = 0;
if (ioctl(uffd, UFFDIO_API, &uffdio_api) < 0) {
- ksft_test_result_fail("UFFDIO_API failed\n");
+ if (errno == EINVAL)
+ ksft_test_result_skip("UFFDIO_API not supported (EINVAL)\n");
+ else
+ ksft_test_result_fail("UFFDIO_API failed: %s\n", strerror(errno));
+
goto close_uffd;
}
if (!(uffdio_api.features & UFFD_FEATURE_PAGEFAULT_FLAG_WP)) {
--
2.49.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/selftests: improve UFFD-WP feature detection in KSM test
2025-06-22 8:10 [PATCH] mm/selftests: improve UFFD-WP feature detection in KSM test Li Wang
@ 2025-06-23 8:33 ` David Hildenbrand
2025-06-24 4:24 ` [PATCH v2] selftests/mm: Fix UFFDIO_API usage with proper two-step feature negotiation Li Wang
1 sibling, 0 replies; 5+ messages in thread
From: David Hildenbrand @ 2025-06-23 8:33 UTC (permalink / raw)
To: Li Wang, akpm, linux-kselftest, linux-mm, linux-kernel
Cc: Aruna Ramakrishna, Bagas Sanjaya, Catalin Marinas, Dave Hansen,
Joey Gouly, Johannes Weiner, Keith Lucas, Ryan Roberts,
Shuah Khan
On 22.06.25 10:10, Li Wang wrote:
> The current implementation of test_unmerge_uffd_wp() explicitly sets
> `uffdio_api.features = UFFD_FEATURE_PAGEFAULT_FLAG_WP` before calling
> UFFDIO_API. This can cause the ioctl() call to fail with EINVAL on kernels
> that do not support UFFD-WP, leading the test to fail unnecessarily:
>
> # ------------------------------
> # running ./ksm_functional_tests
> # ------------------------------
> # TAP version 13
> # 1..9
> # # [RUN] test_unmerge
> # ok 1 Pages were unmerged
> # # [RUN] test_unmerge_zero_pages
> # ok 2 KSM zero pages were unmerged
> # # [RUN] test_unmerge_discarded
> # ok 3 Pages were unmerged
> # # [RUN] test_unmerge_uffd_wp
> # not ok 4 UFFDIO_API failed <-----
> # # [RUN] test_prot_none
> # ok 5 Pages were unmerged
> # # [RUN] test_prctl
> # ok 6 Setting/clearing PR_SET_MEMORY_MERGE works
> # # [RUN] test_prctl_fork
> # # No pages got merged
> # # [RUN] test_prctl_fork_exec
> # ok 7 PR_SET_MEMORY_MERGE value is inherited
> # # [RUN] test_prctl_unmerge
> # ok 8 Pages were unmerged
> # Bail out! 1 out of 8 tests failed
> # # Planned tests != run tests (9 != 8)
> # # Totals: pass:7 fail:1 xfail:0 xpass:0 skip:0 error:0
> # [FAIL]
>
> This patch improves compatibility and error handling by:
>
> 1. Changes the feature check to first query supported features (features=0)
> rather than specifically requesting WP support.
>
> 2. Gracefully skipping the test if:
> - UFFDIO_API fails with EINVAL (feature not supported), or
> - UFFD_FEATURE_PAGEFAULT_FLAG_WP is not advertised by the kernel.
>
> 3. Providing better diagnostics by distinguishing expected failures (e.g.,
> EINVAL) from unexpected ones and reporting them using strerror().
>
> The updated logic makes the test more robust across different kernel versions
> and configurations, while preserving existing behavior on systems that do
> support UFFD-WP.
>
> Signed-off-by: Li Wang <liwang@redhat.com>
> Cc: Aruna Ramakrishna <aruna.ramakrishna@oracle.com>
> Cc: Bagas Sanjaya <bagasdotme@gmail.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Joey Gouly <joey.gouly@arm.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Keith Lucas <keith.lucas@oracle.com>
> Cc: Ryan Roberts <ryan.roberts@arm.com>
> Cc: Shuah Khan <shuah@kernel.org>
> ---
> tools/testing/selftests/mm/ksm_functional_tests.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c
> index b61803e36d1c..f3db257dc555 100644
> --- a/tools/testing/selftests/mm/ksm_functional_tests.c
> +++ b/tools/testing/selftests/mm/ksm_functional_tests.c
> @@ -393,9 +393,13 @@ static void test_unmerge_uffd_wp(void)
>
> /* See if UFFD-WP is around. */
> uffdio_api.api = UFFD_API;
> - uffdio_api.features = UFFD_FEATURE_PAGEFAULT_FLAG_WP;
> + uffdio_api.features = 0;
> if (ioctl(uffd, UFFDIO_API, &uffdio_api) < 0) {
> - ksft_test_result_fail("UFFDIO_API failed\n");
> + if (errno == EINVAL)
> + ksft_test_result_skip("UFFDIO_API not supported (EINVAL)\n");
> + else
> + ksft_test_result_fail("UFFDIO_API failed: %s\n", strerror(errno));
> +
> goto close_uffd;
> }
> if (!(uffdio_api.features & UFFD_FEATURE_PAGEFAULT_FLAG_WP)) {
The man page (man UFFDIO_API) documents:
Since Linux 4.11, applications should use the features field to perform a two-step handshake.
First, UFFDIO_API is called with the features field set to zero. The kernel responds by setting
all supported feature bits.
Applications which do not require any specific features can begin using the userfaultfd immedi‐
ately. Applications which do need specific features should call UFFDIO_API again with a subset
of the reported feature bits set to enable those features.
So likely, what you want in this patch here is something like:
diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c
index b61803e36d1cf..5cf819ac958d0 100644
--- a/tools/testing/selftests/mm/ksm_functional_tests.c
+++ b/tools/testing/selftests/mm/ksm_functional_tests.c
@@ -393,7 +393,7 @@ static void test_unmerge_uffd_wp(void)
/* See if UFFD-WP is around. */
uffdio_api.api = UFFD_API;
- uffdio_api.features = UFFD_FEATURE_PAGEFAULT_FLAG_WP;
+ uffdio_api.features = 0;
if (ioctl(uffd, UFFDIO_API, &uffdio_api) < 0) {
ksft_test_result_fail("UFFDIO_API failed\n");
goto close_uffd;
@@ -403,6 +403,14 @@ static void test_unmerge_uffd_wp(void)
goto close_uffd;
}
+ /* Now, enable it ("two-step handshake") */
+ uffdio_api.api = UFFD_API;
+ uffdio_api.features = UFFD_FEATURE_PAGEFAULT_FLAG_WP;
+ if (ioctl(uffd, UFFDIO_API, &uffdio_api) < 0) {
+ ksft_test_result_fail("UFFDIO_API failed\n");
+ goto close_uffd;
+ }
+
/* Register UFFD-WP, no need for an actual handler. */
if (uffd_register(uffd, map, size, false, true, false)) {
ksft_test_result_fail("UFFDIO_REGISTER_MODE_WP failed\n");
--
Cheers,
David / dhildenb
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2] selftests/mm: Fix UFFDIO_API usage with proper two-step feature negotiation
2025-06-22 8:10 [PATCH] mm/selftests: improve UFFD-WP feature detection in KSM test Li Wang
2025-06-23 8:33 ` David Hildenbrand
@ 2025-06-24 4:24 ` Li Wang
2025-06-24 8:07 ` David Hildenbrand
2025-06-24 15:17 ` David Hildenbrand
1 sibling, 2 replies; 5+ messages in thread
From: Li Wang @ 2025-06-24 4:24 UTC (permalink / raw)
To: akpm, david, linux-kselftest, linux-mm, linux-kernel
Cc: Aruna Ramakrishna, Bagas Sanjaya, Catalin Marinas, Dave Hansen,
Joey Gouly, Johannes Weiner, Keith Lucas, Ryan Roberts,
Shuah Khan
The current implementation of test_unmerge_uffd_wp() explicitly sets
`uffdio_api.features = UFFD_FEATURE_PAGEFAULT_FLAG_WP` before calling
UFFDIO_API. This can cause the ioctl() call to fail with EINVAL on kernels
that do not support UFFD-WP, leading the test to fail unnecessarily:
# ------------------------------
# running ./ksm_functional_tests
# ------------------------------
# TAP version 13
# 1..9
# # [RUN] test_unmerge
# ok 1 Pages were unmerged
# # [RUN] test_unmerge_zero_pages
# ok 2 KSM zero pages were unmerged
# # [RUN] test_unmerge_discarded
# ok 3 Pages were unmerged
# # [RUN] test_unmerge_uffd_wp
# not ok 4 UFFDIO_API failed <-----
# # [RUN] test_prot_none
# ok 5 Pages were unmerged
# # [RUN] test_prctl
# ok 6 Setting/clearing PR_SET_MEMORY_MERGE works
# # [RUN] test_prctl_fork
# # No pages got merged
# # [RUN] test_prctl_fork_exec
# ok 7 PR_SET_MEMORY_MERGE value is inherited
# # [RUN] test_prctl_unmerge
# ok 8 Pages were unmerged
# Bail out! 1 out of 8 tests failed
# # Planned tests != run tests (9 != 8)
# # Totals: pass:7 fail:1 xfail:0 xpass:0 skip:0 error:0
# [FAIL]
This patch improves compatibility and robustness of the UFFD-WP test
(test_unmerge_uffd_wp) by correctly implementing the UFFDIO_API
two-step handshake as recommended by the userfaultfd(2) man page.
Key changes:
1. Use features=0 in the initial UFFDIO_API call to query supported
feature bits, rather than immediately requesting WP support.
2. Skip the test gracefully if:
- UFFDIO_API fails with EINVAL (e.g. unsupported API version), or
- UFFD_FEATURE_PAGEFAULT_FLAG_WP is not advertised by the kernel.
3. Close the initial userfaultfd and create a new one before enabling
the required feature, since UFFDIO_API can only be called once per fd.
4. Improve diagnostics by distinguishing between expected and unexpected
failures, using strerror() to report errors.
This ensures the test behaves correctly across a wider range of kernel
versions and configurations, while preserving the intended behavior on
kernels that support UFFD-WP.
Suggestted-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Li Wang <liwang@redhat.com>
Cc: Aruna Ramakrishna <aruna.ramakrishna@oracle.com>
Cc: Bagas Sanjaya <bagasdotme@gmail.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Keith Lucas <keith.lucas@oracle.com>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Shuah Khan <shuah@kernel.org>
---
Notes:
v1 --> v2:
* Close the original userfaultfd and open a new one before enabling features
* Reworked UFFDIO_API negotiation to follow the official two-step handshake
.../selftests/mm/ksm_functional_tests.c | 28 +++++++++++++++++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c
index b61803e36d1c..19e5b741893a 100644
--- a/tools/testing/selftests/mm/ksm_functional_tests.c
+++ b/tools/testing/selftests/mm/ksm_functional_tests.c
@@ -393,9 +393,13 @@ static void test_unmerge_uffd_wp(void)
/* See if UFFD-WP is around. */
uffdio_api.api = UFFD_API;
- uffdio_api.features = UFFD_FEATURE_PAGEFAULT_FLAG_WP;
+ uffdio_api.features = 0;
if (ioctl(uffd, UFFDIO_API, &uffdio_api) < 0) {
- ksft_test_result_fail("UFFDIO_API failed\n");
+ if (errno == EINVAL)
+ ksft_test_result_skip("The API version requested is not supported\n");
+ else
+ ksft_test_result_fail("UFFDIO_API failed: %s\n", strerror(errno));
+
goto close_uffd;
}
if (!(uffdio_api.features & UFFD_FEATURE_PAGEFAULT_FLAG_WP)) {
@@ -403,6 +407,26 @@ static void test_unmerge_uffd_wp(void)
goto close_uffd;
}
+ /*
+ * UFFDIO_API must only be called once to enable features.
+ * So we close the old userfaultfd and create a new one to
+ * actually enable UFFD_FEATURE_PAGEFAULT_FLAG_WP.
+ */
+ close(uffd);
+ uffd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
+ if (uffd < 0) {
+ ksft_test_result_skip("__NR_userfaultfd failed\n");
+ goto unmap;
+ }
+
+ /* Now, enable it ("two-step handshake") */
+ uffdio_api.api = UFFD_API;
+ uffdio_api.features = UFFD_FEATURE_PAGEFAULT_FLAG_WP;
+ if (ioctl(uffd, UFFDIO_API, &uffdio_api) < 0) {
+ ksft_test_result_fail("UFFDIO_API failed: %s\n", strerror(errno));
+ goto close_uffd;
+ }
+
/* Register UFFD-WP, no need for an actual handler. */
if (uffd_register(uffd, map, size, false, true, false)) {
ksft_test_result_fail("UFFDIO_REGISTER_MODE_WP failed\n");
--
2.49.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] selftests/mm: Fix UFFDIO_API usage with proper two-step feature negotiation
2025-06-24 4:24 ` [PATCH v2] selftests/mm: Fix UFFDIO_API usage with proper two-step feature negotiation Li Wang
@ 2025-06-24 8:07 ` David Hildenbrand
2025-06-24 15:17 ` David Hildenbrand
1 sibling, 0 replies; 5+ messages in thread
From: David Hildenbrand @ 2025-06-24 8:07 UTC (permalink / raw)
To: Li Wang, akpm, linux-kselftest, linux-mm, linux-kernel
Cc: Aruna Ramakrishna, Bagas Sanjaya, Catalin Marinas, Dave Hansen,
Joey Gouly, Johannes Weiner, Keith Lucas, Ryan Roberts,
Shuah Khan
On 24.06.25 06:24, Li Wang wrote:
> The current implementation of test_unmerge_uffd_wp() explicitly sets
> `uffdio_api.features = UFFD_FEATURE_PAGEFAULT_FLAG_WP` before calling
> UFFDIO_API. This can cause the ioctl() call to fail with EINVAL on kernels
> that do not support UFFD-WP, leading the test to fail unnecessarily:
>
> # ------------------------------
> # running ./ksm_functional_tests
> # ------------------------------
> # TAP version 13
> # 1..9
> # # [RUN] test_unmerge
> # ok 1 Pages were unmerged
> # # [RUN] test_unmerge_zero_pages
> # ok 2 KSM zero pages were unmerged
> # # [RUN] test_unmerge_discarded
> # ok 3 Pages were unmerged
> # # [RUN] test_unmerge_uffd_wp
> # not ok 4 UFFDIO_API failed <-----
> # # [RUN] test_prot_none
> # ok 5 Pages were unmerged
> # # [RUN] test_prctl
> # ok 6 Setting/clearing PR_SET_MEMORY_MERGE works
> # # [RUN] test_prctl_fork
> # # No pages got merged
> # # [RUN] test_prctl_fork_exec
> # ok 7 PR_SET_MEMORY_MERGE value is inherited
> # # [RUN] test_prctl_unmerge
> # ok 8 Pages were unmerged
> # Bail out! 1 out of 8 tests failed
> # # Planned tests != run tests (9 != 8)
> # # Totals: pass:7 fail:1 xfail:0 xpass:0 skip:0 error:0
> # [FAIL]
>
> This patch improves compatibility and robustness of the UFFD-WP test
> (test_unmerge_uffd_wp) by correctly implementing the UFFDIO_API
> two-step handshake as recommended by the userfaultfd(2) man page.
>
> Key changes:
>
> 1. Use features=0 in the initial UFFDIO_API call to query supported
> feature bits, rather than immediately requesting WP support.
>
> 2. Skip the test gracefully if:
> - UFFDIO_API fails with EINVAL (e.g. unsupported API version), or
> - UFFD_FEATURE_PAGEFAULT_FLAG_WP is not advertised by the kernel.
>
> 3. Close the initial userfaultfd and create a new one before enabling
> the required feature, since UFFDIO_API can only be called once per fd.
>
> 4. Improve diagnostics by distinguishing between expected and unexpected
> failures, using strerror() to report errors.
>
> This ensures the test behaves correctly across a wider range of kernel
> versions and configurations, while preserving the intended behavior on
> kernels that support UFFD-WP.
>
> Suggestted-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Li Wang <liwang@redhat.com>
> Cc: Aruna Ramakrishna <aruna.ramakrishna@oracle.com>
> Cc: Bagas Sanjaya <bagasdotme@gmail.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Joey Gouly <joey.gouly@arm.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Keith Lucas <keith.lucas@oracle.com>
> Cc: Ryan Roberts <ryan.roberts@arm.com>
> Cc: Shuah Khan <shuah@kernel.org>
> ---
>
> Notes:
> v1 --> v2:
> * Close the original userfaultfd and open a new one before enabling features
> * Reworked UFFDIO_API negotiation to follow the official two-step handshake
>
> .../selftests/mm/ksm_functional_tests.c | 28 +++++++++++++++++--
> 1 file changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c
> index b61803e36d1c..19e5b741893a 100644
> --- a/tools/testing/selftests/mm/ksm_functional_tests.c
> +++ b/tools/testing/selftests/mm/ksm_functional_tests.c
> @@ -393,9 +393,13 @@ static void test_unmerge_uffd_wp(void)
>
> /* See if UFFD-WP is around. */
> uffdio_api.api = UFFD_API;
> - uffdio_api.features = UFFD_FEATURE_PAGEFAULT_FLAG_WP;
> + uffdio_api.features = 0;
> if (ioctl(uffd, UFFDIO_API, &uffdio_api) < 0) {
> - ksft_test_result_fail("UFFDIO_API failed\n");
> + if (errno == EINVAL)
> + ksft_test_result_skip("The API version requested is not supported\n");
> + else
> + ksft_test_result_fail("UFFDIO_API failed: %s\n", strerror(errno));
> +
Not sure if that is really required. If UFFDIO_API failed after
__NR_userfaultfd worked something unexpected is happening.
> goto close_uffd;
> }
> if (!(uffdio_api.features & UFFD_FEATURE_PAGEFAULT_FLAG_WP)) {
> @@ -403,6 +407,26 @@ static void test_unmerge_uffd_wp(void)
> goto close_uffd;
> }
>
> + /*
> + * UFFDIO_API must only be called once to enable features.
> + * So we close the old userfaultfd and create a new one to
> + * actually enable UFFD_FEATURE_PAGEFAULT_FLAG_WP.
> + */
> + close(uffd);
Is that actually required?
The man page explicitly documents:
" EINVAL A previous UFFDIO_API call already enabled one or more
features for this userfaultfd. Calling UFF‐
DIO_API twice, the first time with no features set, is
explicitly allowed as per the two-step feature
detection handshake.
"
So if that doesn't work, something might be broken.
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] selftests/mm: Fix UFFDIO_API usage with proper two-step feature negotiation
2025-06-24 4:24 ` [PATCH v2] selftests/mm: Fix UFFDIO_API usage with proper two-step feature negotiation Li Wang
2025-06-24 8:07 ` David Hildenbrand
@ 2025-06-24 15:17 ` David Hildenbrand
1 sibling, 0 replies; 5+ messages in thread
From: David Hildenbrand @ 2025-06-24 15:17 UTC (permalink / raw)
To: Li Wang, akpm, linux-kselftest, linux-mm, linux-kernel
Cc: Aruna Ramakrishna, Bagas Sanjaya, Catalin Marinas, Dave Hansen,
Joey Gouly, Johannes Weiner, Keith Lucas, Ryan Roberts,
Shuah Khan
On 24.06.25 06:24, Li Wang wrote:
> The current implementation of test_unmerge_uffd_wp() explicitly sets
> `uffdio_api.features = UFFD_FEATURE_PAGEFAULT_FLAG_WP` before calling
> UFFDIO_API. This can cause the ioctl() call to fail with EINVAL on kernels
> that do not support UFFD-WP, leading the test to fail unnecessarily:
>
> # ------------------------------
> # running ./ksm_functional_tests
> # ------------------------------
> # TAP version 13
> # 1..9
> # # [RUN] test_unmerge
> # ok 1 Pages were unmerged
> # # [RUN] test_unmerge_zero_pages
> # ok 2 KSM zero pages were unmerged
> # # [RUN] test_unmerge_discarded
> # ok 3 Pages were unmerged
> # # [RUN] test_unmerge_uffd_wp
> # not ok 4 UFFDIO_API failed <-----
> # # [RUN] test_prot_none
> # ok 5 Pages were unmerged
> # # [RUN] test_prctl
> # ok 6 Setting/clearing PR_SET_MEMORY_MERGE works
> # # [RUN] test_prctl_fork
> # # No pages got merged
> # # [RUN] test_prctl_fork_exec
> # ok 7 PR_SET_MEMORY_MERGE value is inherited
> # # [RUN] test_prctl_unmerge
> # ok 8 Pages were unmerged
> # Bail out! 1 out of 8 tests failed
> # # Planned tests != run tests (9 != 8)
> # # Totals: pass:7 fail:1 xfail:0 xpass:0 skip:0 error:0
> # [FAIL]
>
> This patch improves compatibility and robustness of the UFFD-WP test
> (test_unmerge_uffd_wp) by correctly implementing the UFFDIO_API
> two-step handshake as recommended by the userfaultfd(2) man page.
>
> Key changes:
>
> 1. Use features=0 in the initial UFFDIO_API call to query supported
> feature bits, rather than immediately requesting WP support.
>
> 2. Skip the test gracefully if:
> - UFFDIO_API fails with EINVAL (e.g. unsupported API version), or
> - UFFD_FEATURE_PAGEFAULT_FLAG_WP is not advertised by the kernel.
>
> 3. Close the initial userfaultfd and create a new one before enabling
> the required feature, since UFFDIO_API can only be called once per fd.
>
> 4. Improve diagnostics by distinguishing between expected and unexpected
> failures, using strerror() to report errors.
>
> This ensures the test behaves correctly across a wider range of kernel
> versions and configurations, while preserving the intended behavior on
> kernels that support UFFD-WP.
>
> Suggestted-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Li Wang <liwang@redhat.com>
> Cc: Aruna Ramakrishna <aruna.ramakrishna@oracle.com>
> Cc: Bagas Sanjaya <bagasdotme@gmail.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Joey Gouly <joey.gouly@arm.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Keith Lucas <keith.lucas@oracle.com>
> Cc: Ryan Roberts <ryan.roberts@arm.com>
> Cc: Shuah Khan <shuah@kernel.org>
> ---
>
> Notes:
> v1 --> v2:
> * Close the original userfaultfd and open a new one before enabling features
> * Reworked UFFDIO_API negotiation to follow the official two-step handshake
>
> .../selftests/mm/ksm_functional_tests.c | 28 +++++++++++++++++--
> 1 file changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c
> index b61803e36d1c..19e5b741893a 100644
> --- a/tools/testing/selftests/mm/ksm_functional_tests.c
> +++ b/tools/testing/selftests/mm/ksm_functional_tests.c
> @@ -393,9 +393,13 @@ static void test_unmerge_uffd_wp(void)
>
> /* See if UFFD-WP is around. */
> uffdio_api.api = UFFD_API;
> - uffdio_api.features = UFFD_FEATURE_PAGEFAULT_FLAG_WP;
> + uffdio_api.features = 0;
> if (ioctl(uffd, UFFDIO_API, &uffdio_api) < 0) {
> - ksft_test_result_fail("UFFDIO_API failed\n");
> + if (errno == EINVAL)
> + ksft_test_result_skip("The API version requested is not supported\n");
> + else
> + ksft_test_result_fail("UFFDIO_API failed: %s\n", strerror(errno));
> +
> goto close_uffd;
> }
> if (!(uffdio_api.features & UFFD_FEATURE_PAGEFAULT_FLAG_WP)) {
> @@ -403,6 +407,26 @@ static void test_unmerge_uffd_wp(void)
> goto close_uffd;
> }
>
> + /*
> + * UFFDIO_API must only be called once to enable features.
> + * So we close the old userfaultfd and create a new one to
> + * actually enable UFFD_FEATURE_PAGEFAULT_FLAG_WP.
> + */
> + close(uffd);
> + uffd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
> + if (uffd < 0) {
> + ksft_test_result_skip("__NR_userfaultfd failed\n");
If it now suddenly fails (after it working above), this sure is a fail,
right?
Apart from that
Acked-by: David Hildenbrand <david@redhat.com>
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-06-24 15:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-22 8:10 [PATCH] mm/selftests: improve UFFD-WP feature detection in KSM test Li Wang
2025-06-23 8:33 ` David Hildenbrand
2025-06-24 4:24 ` [PATCH v2] selftests/mm: Fix UFFDIO_API usage with proper two-step feature negotiation Li Wang
2025-06-24 8:07 ` David Hildenbrand
2025-06-24 15:17 ` David Hildenbrand
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).