Re: [PATCH v9 0/5] selftests/mm: refactor pkey helpers and fix mmap error handling

Re: [PATCH v9 0/5] selftests/mm: refactor pkey helpers and fix mmap error handling

[Andrew Morton]

On Thu,  2 Jul 2026 14:23:27 +0800 Hongfu Li <lihongfu@kylinos.cn> wrote:

> The main changes in this series are to refactor shared tracing and assertion
> helpers into a common file, unify both pkey selftests on pkey_assert() and
> per-test tracing for consistent diagnostics, and add missing mmap() return
> checks with MAP_FAILED used throughout for readability and consistency.

Updated, thanks.

AI review might have found a pre-existing thing:
	https://sashiko.dev/#/patchset/20260702062332.911786-1-lihongfu@kylinos.cn

> v9:
> - In patch 1/5, define test_nr, iteration_nr and dprint_in_signal in
>   pkey_util.c and remove them from protection_keys.c and
>   pkey_sighandler_tests.c.
> - In patch 1/5, cat_into_file() used the wrong argument in the open()
>   failure message; print file instead of str. Update the commit message
>   accordingly.
> - In patch 2/5, align clone_raw() continuation lines to the opening parenthesis.
> - In patch 5/5, set errno from pthread_create() return value before
>   pkey_assert(0) on thread creation failure.
> - Add Acked-by, Reviewed-by and Tested-by tags to each patch.

Here's how v9 altered mm.git:

 tools/testing/selftests/mm/pkey_sighandler_tests.c |   38 +++++------
 tools/testing/selftests/mm/pkey_util.c             |    6 +
 tools/testing/selftests/mm/protection_keys.c       |    4 -
 3 files changed, 25 insertions(+), 23 deletions(-)

--- a/tools/testing/selftests/mm/pkey_sighandler_tests.c~b
+++ a/tools/testing/selftests/mm/pkey_sighandler_tests.c
@@ -35,10 +35,6 @@ static pthread_mutex_t mutex = PTHREAD_M
 static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
 static siginfo_t siginfo = {0};
 
-int iteration_nr = 1;
-int test_nr;
-int dprint_in_signal;
-
 /*
  * We need to use inline assembly instead of glibc's syscall because glibc's
  * syscall will attempt to access the PLT in order to call a library function
@@ -225,7 +221,10 @@ static void test_sigsegv_handler_with_pk
 	pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
 
 	ret = pthread_create(&thr, &attr, thread_segv_with_pkey0_disabled, NULL);
-	pkey_assert(ret == 0);
+	if (ret) {
+		errno = ret;
+		pkey_assert(0);
+	}
 
 	pthread_mutex_lock(&mutex);
 	while (siginfo.si_signo == 0)
@@ -265,7 +264,10 @@ static void test_sigsegv_handler_cannot_
 	pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
 
 	ret = pthread_create(&thr, &attr, thread_segv_pkuerr_stack, NULL);
-	pkey_assert(ret == 0);
+	if (ret) {
+		errno = ret;
+		pkey_assert(0);
+	}
 
 	pthread_mutex_lock(&mutex);
 	while (siginfo.si_signo == 0)
@@ -327,12 +329,12 @@ static void test_sigsegv_handler_with_di
 
 	/* Use clone to avoid newer glibcs using rseq on new threads */
 	ret = clone_raw(CLONE_VM | CLONE_FS | CLONE_FILES |
-			     CLONE_SIGHAND | CLONE_THREAD | CLONE_SYSVSEM |
-			     CLONE_PARENT_SETTID | CLONE_CHILD_CLEARTID |
-			     CLONE_DETACHED,
-			     stack + STACK_SIZE,
-			     &parent_pid,
-			     &child_pid);
+			CLONE_SIGHAND | CLONE_THREAD | CLONE_SYSVSEM |
+			CLONE_PARENT_SETTID | CLONE_CHILD_CLEARTID |
+			CLONE_DETACHED,
+			stack + STACK_SIZE,
+			&parent_pid,
+			&child_pid);
 
 	if (ret < 0) {
 		errno = -ret;
@@ -501,12 +503,12 @@ static void test_pkru_sigreturn(void)
 
 	/* Use clone to avoid newer glibcs using rseq on new threads */
 	ret = clone_raw(CLONE_VM | CLONE_FS | CLONE_FILES |
-			     CLONE_SIGHAND | CLONE_THREAD | CLONE_SYSVSEM |
-			     CLONE_PARENT_SETTID | CLONE_CHILD_CLEARTID |
-			     CLONE_DETACHED,
-			     stack + STACK_SIZE,
-			     &parent_pid,
-			     &child_pid);
+			CLONE_SIGHAND | CLONE_THREAD | CLONE_SYSVSEM |
+			CLONE_PARENT_SETTID | CLONE_CHILD_CLEARTID |
+			CLONE_DETACHED,
+			stack + STACK_SIZE,
+			&parent_pid,
+			&child_pid);
 
 	if (ret < 0) {
 		errno = -ret;
--- a/tools/testing/selftests/mm/pkey_util.c~b
+++ a/tools/testing/selftests/mm/pkey_util.c
@@ -8,6 +8,10 @@
 
 #include "pkey-helpers.h"
 
+int iteration_nr = 1;
+int test_nr;
+int dprint_in_signal;
+
 #if CONTROL_TRACING > 0
 static void cat_into_file(char *str, char *file)
 {
@@ -20,7 +24,7 @@ static void cat_into_file(char *str, cha
 	 * pkey_assert()
 	 */
 	if (fd < 0) {
-		fprintf(stderr, "error opening '%s'\n", str);
+		fprintf(stderr, "error opening '%s'\n", file);
 		perror("error: ");
 		exit(__LINE__);
 	}
--- a/tools/testing/selftests/mm/protection_keys.c~b
+++ a/tools/testing/selftests/mm/protection_keys.c
@@ -49,11 +49,7 @@
 #include "hugepage_settings.h"
 #include "pkey-helpers.h"
 
-int iteration_nr = 1;
-int test_nr;
-
 u64 shadow_pkey_reg;
-int dprint_in_signal;
 
 noinline int read_ptr(int *ptr)
 {
_

Re: [PATCH v9 0/5] selftests/mm: refactor pkey helpers and fix mmap error handling

[Hongfu Li]

Hi,

> > The main changes in this series are to refactor shared tracing and assertion
> > helpers into a common file, unify both pkey selftests on pkey_assert() and
> > per-test tracing for consistent diagnostics, and add missing mmap() return
> > checks with MAP_FAILED used throughout for readability and consistency.
> 
> Updated, thanks.
> 
> AI review might have found a pre-existing thing:
> 	https://sashiko.dev/#/patchset/20260702062332.911786-1-lihongfu@kylinos.cn

Thanks for the review and the Sashiko link.
I will fix the pre-existing issue pointed out by AI review in the next revision.

Best regards,
Hongfu

Re: [PATCH v9 1/5] selftests/mm: move pkey selftest helpers to pkey_util.c

[David Hildenbrand (Arm)]

On 7/2/26 08:23, Hongfu Li wrote:
> Move pkey selftest debugging helpers into shared code so both pkey
> selftests can use the same tracing and abort-hook logic. Also fix
> cat_into_file() to print file, not str, in the open() failure message.
> 
> Signed-off-by: Hongfu Li <lihongfu@kylinos.cn>
> Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> Acked-by: Liam R. Howlett (Oracle) <liam@infradead.org>
> Reviewed-by: Kevin Brodsky <kevin.brodsky@arm.com>
> Tested-by: Kevin Brodsky <kevin.brodsky@arm.com>
> ---
>  tools/testing/selftests/mm/pkey-helpers.h    |  4 +-
>  tools/testing/selftests/mm/pkey_util.c       | 90 ++++++++++++++++++++
>  tools/testing/selftests/mm/protection_keys.c | 87 -------------------
>  3 files changed, 93 insertions(+), 88 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/pkey-helpers.h b/tools/testing/selftests/mm/pkey-helpers.h
> index 2c377f4e9df1..46a8a1878dc1 100644
> --- a/tools/testing/selftests/mm/pkey-helpers.h
> +++ b/tools/testing/selftests/mm/pkey-helpers.h
> @@ -68,7 +68,9 @@ static inline void sigsafe_printf(const char *format, ...)
>  #define dprintf3(args...) dprintf_level(3, args)
>  #define dprintf4(args...) dprintf_level(4, args)
>  
> -extern void abort_hooks(void);
> +void tracing_on(void);
> +void tracing_off(void);
> +void abort_hooks(void);
>  #define pkey_assert(condition) do {		\
>  	if (!(condition)) {			\
>  		dprintf0("# assert() at %s::%d test_nr: %d iteration: %d\n", \
> diff --git a/tools/testing/selftests/mm/pkey_util.c b/tools/testing/selftests/mm/pkey_util.c
> index 255b332f7a08..fbef3cd45447 100644
> --- a/tools/testing/selftests/mm/pkey_util.c
> +++ b/tools/testing/selftests/mm/pkey_util.c
> @@ -2,9 +2,99 @@
>  #define __SANE_USERSPACE_TYPES__
>  #include <sys/syscall.h>
>  #include <unistd.h>
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <string.h>
>  
>  #include "pkey-helpers.h"
>  
> +int iteration_nr = 1;
> +int test_nr;
> +int dprint_in_signal;
> +
> +#if CONTROL_TRACING > 0

Who sets that CONTROL_TRACING? And if it's nobody, why are we dragging this along?

-- 
Cheers,

David

Re: [PATCH v9 1/5] selftests/mm: move pkey selftest helpers to pkey_util.c

[Hongfu Li]

Hi,

> > diff --git a/tools/testing/selftests/mm/pkey_util.c b/tools/testing/selftests/mm/pkey_util.c
> > index 255b332f7a08..fbef3cd45447 100644
> > --- a/tools/testing/selftests/mm/pkey_util.c
> > +++ b/tools/testing/selftests/mm/pkey_util.c
> > @@ -2,9 +2,99 @@
> >  #define __SANE_USERSPACE_TYPES__
> >  #include <sys/syscall.h>
> >  #include <unistd.h>
> > +#include <fcntl.h>
> > +#include <stdio.h>
> > +#include <string.h>
> >  
> >  #include "pkey-helpers.h"
> >  
> > +int iteration_nr = 1;
> > +int test_nr;
> > +int dprint_in_signal;
> > +
> > +#if CONTROL_TRACING > 0
> 
> Who sets that CONTROL_TRACING? And if it's nobody, why are we dragging this along?

Thanks a lot for your comment.

CONTROL_TRACING is disabled by default. It can be enabled at build time
with -DCONTROL_TRACING=1 to capture extra debug info via ftrace for pkey
failure debugging. This patch just moves the tracing and abort-hook code
to the shared pkey_util.c, fully retaining the original opt-in debug logic.
This is based on my understanding of the existing code design, are
there any corner cases I overlooked?

Best regards,
Hongfu

Re: [PATCH v9 1/5] selftests/mm: move pkey selftest helpers to pkey_util.c

[David Hildenbrand (Arm)]

On 7/6/26 12:31, Hongfu Li wrote:
> Hi,
> 
>>> diff --git a/tools/testing/selftests/mm/pkey_util.c b/tools/testing/selftests/mm/pkey_util.c
>>> index 255b332f7a08..fbef3cd45447 100644
>>> --- a/tools/testing/selftests/mm/pkey_util.c
>>> +++ b/tools/testing/selftests/mm/pkey_util.c
>>> @@ -2,9 +2,99 @@
>>>  #define __SANE_USERSPACE_TYPES__
>>>  #include <sys/syscall.h>
>>>  #include <unistd.h>
>>> +#include <fcntl.h>
>>> +#include <stdio.h>
>>> +#include <string.h>
>>>  
>>>  #include "pkey-helpers.h"
>>>  
>>> +int iteration_nr = 1;
>>> +int test_nr;
>>> +int dprint_in_signal;
>>> +
>>> +#if CONTROL_TRACING > 0
>>
>> Who sets that CONTROL_TRACING? And if it's nobody, why are we dragging this along?
> 
> Thanks a lot for your comment.
> 
> CONTROL_TRACING is disabled by default. It can be enabled at build time
> with -DCONTROL_TRACING=1 to capture extra debug info via ftrace for pkey
> failure debugging. This patch just moves the tracing and abort-hook code
> to the shared pkey_util.c, fully retaining the original opt-in debug logic.
> This is based on my understanding of the existing code design, are
> there any corner cases I overlooked?

This is not documented anywhere, so it's rather odd. I wonder if there is a
single person on earth that uses this :)

-- 
Cheers,

David