Re: [PATCH] selftests/mm: Add process_madvise() tests

2025-06-30 Thread David Hildenbrand

On 30.06.25 15:51, Lorenzo Stoakes wrote:

On Sat, Jun 28, 2025 at 07:39:45PM +0800, wang lian wrote:

This patch adds tests for the process_madvise(), focusing on
verifying behavior under various conditions including valid
usage and error cases.


Hi sorry to be a pain, but I'm not sure this will get picked up properly like 
this.

Please resend this _not in reply to anything_ and with the subject:

[PATCH v2] selftests/mm: add process_madvise() tests




Hehe, there is a

[PATCH v2] selftests/vm: Add process_madvise() tests

in my inbox, but looks like only you and me were on CC.

--
Cheers,

David / dhildenb




Re: [PATCH] selftests/mm: Add process_madvise() tests

2025-06-30 Thread Lorenzo Stoakes
On Sat, Jun 28, 2025 at 07:39:45PM +0800, wang lian wrote:
> This patch adds tests for the process_madvise(), focusing on
> verifying behavior under various conditions including valid
> usage and error cases.

Hi sorry to be a pain, but I'm not sure this will get picked up properly like 
this.

Please resend this _not in reply to anything_ and with the subject:

[PATCH v2] selftests/mm: add process_madvise() tests

Note we do not capitalise the first letter in the subject line.

>
> Changelog v2:
> - Drop MADV_DONTNEED tests based on feedback
> - Focus solely on process_madvise() syscall
> - Improve error handling and structure
> - Add future-proof flag test
> - Style and comment cleanups
>

^ This is good but please put changelogs under the --- below, otherwise they end
up as part of the commit msg which isn't appropriate.

Once you do that we can review in reply to the new email.

Thanks!

> Signed-off-by: wang lian
> Suggested-by: Lorenzo Stoakes 
> Suggested-by: David Hildenbrand 
> ---
>  tools/testing/selftests/mm/.gitignore |   1 +
>  tools/testing/selftests/mm/Makefile   |   1 +
>  tools/testing/selftests/mm/process_madv.c | 414 ++
>  tools/testing/selftests/mm/run_vmtests.sh |   5 +
>  4 files changed, 421 insertions(+)
>  create mode 100644 tools/testing/selftests/mm/process_madv.c
>
> diff --git a/tools/testing/selftests/mm/.gitignore 
> b/tools/testing/selftests/mm/.gitignore
> index 911f39d634be..a8c3be02188c 100644
> --- a/tools/testing/selftests/mm/.gitignore
> +++ b/tools/testing/selftests/mm/.gitignore
> @@ -42,6 +42,7 @@ memfd_secret
>  hugetlb_dio
>  pkey_sighandler_tests_32
>  pkey_sighandler_tests_64
> +process_madv
>  soft-dirty
>  split_huge_page_test
>  ksm_tests
> diff --git a/tools/testing/selftests/mm/Makefile 
> b/tools/testing/selftests/mm/Makefile
> index 2352252f3914..725612e09582 100644
> --- a/tools/testing/selftests/mm/Makefile
> +++ b/tools/testing/selftests/mm/Makefile
> @@ -86,6 +86,7 @@ TEST_GEN_FILES += mseal_test
>  TEST_GEN_FILES += on-fault-limit
>  TEST_GEN_FILES += pagemap_ioctl
>  TEST_GEN_FILES += pfnmap
> +TEST_GEN_FILES += process_madv
>  TEST_GEN_FILES += thuge-gen
>  TEST_GEN_FILES += transhuge-stress
>  TEST_GEN_FILES += uffd-stress
> diff --git a/tools/testing/selftests/mm/process_madv.c 
> b/tools/testing/selftests/mm/process_madv.c
> new file mode 100644
> index ..73999c8e3570
> --- /dev/null
> +++ b/tools/testing/selftests/mm/process_madv.c
> @@ -0,0 +1,414 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#define _GNU_SOURCE
> +#include "../kselftest_harness.h"
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "vm_util.h"
> +
> +#include "../pidfd/pidfd.h"
> +
> +/*
> + * Ignore the checkpatch warning, as per the C99 standard, section 7.14.1.1:
> + *
> + * "If the signal occurs other than as the result of calling the abort or 
> raise
> + *  function, the behavior is undefined if the signal handler refers to any
> + *  object with static storage duration other than by assigning a value to an
> + *  object declared as volatile sig_atomic_t"
> + */
> +static volatile sig_atomic_t signal_jump_set;
> +static sigjmp_buf signal_jmp_buf;
> +
> +/*
> + * Ignore the checkpatch warning, we must read from x but don't want to do
> + * anything with it in order to trigger a read page fault. We therefore must 
> use
> + * volatile to stop the compiler from optimising this away.
> + */
> +#define FORCE_READ(x) (*(volatile typeof(x) *)x)
> +
> +static void handle_fatal(int c)
> +{
> + if (!signal_jump_set)
> + return;
> +
> + siglongjmp(signal_jmp_buf, c);
> +}
> +
> +FIXTURE(process_madvise)
> +{
> + int pidfd;
> + int flag;
> +};
> +
> +static void setup_sighandler(void)
> +{
> + struct sigaction act = {
> + .sa_handler = &handle_fatal,
> + .sa_flags = SA_NODEFER,
> + };
> +
> + sigemptyset(&act.sa_mask);
> + if (sigaction(SIGSEGV, &act, NULL))
> + ksft_exit_fail_perror("sigaction");
> +}
> +
> +static void teardown_sighandler(void)
> +{
> + struct sigaction act = {
> + .sa_handler = SIG_DFL,
> + .sa_flags = SA_NODEFER,
> + };
> +
> + sigemptyset(&act.sa_mask);
> + sigaction(SIGSEGV, &act, NULL);
> +}
> +
> +FIXTURE_SETUP(process_madvise)
> +{
> + self->pidfd = PIDFD_SELF;
> + self->flag = 0;
> + setup_sighandler();
> +};
> +
> +FIXTURE_TEARDOWN_PARENT(process_madvise)
> +{
> + teardown_sighandler();
> +}
> +
> +static ssize_t sys_process_madvise(int pidfd, const struct iovec *iovec,
> +size_t vlen, int advice, unsigned int flags)
> +{
> + return syscall(__NR_process_madvise, pidfd, iovec, vlen, advice, flags);
> +}
> +
> +/*
> + * Enable our signal catcher and try to read/write the specified buffer. The
> + * return value indicates 

[PATCH] selftests/mm: Add process_madvise() tests

2025-06-28 Thread wang lian
This patch adds tests for the process_madvise(), focusing on
verifying behavior under various conditions including valid
usage and error cases.

Changelog v2:
- Drop MADV_DONTNEED tests based on feedback
- Focus solely on process_madvise() syscall
- Improve error handling and structure
- Add future-proof flag test
- Style and comment cleanups

Signed-off-by: wang lian
Suggested-by: Lorenzo Stoakes 
Suggested-by: David Hildenbrand 
---
 tools/testing/selftests/mm/.gitignore |   1 +
 tools/testing/selftests/mm/Makefile   |   1 +
 tools/testing/selftests/mm/process_madv.c | 414 ++
 tools/testing/selftests/mm/run_vmtests.sh |   5 +
 4 files changed, 421 insertions(+)
 create mode 100644 tools/testing/selftests/mm/process_madv.c

diff --git a/tools/testing/selftests/mm/.gitignore 
b/tools/testing/selftests/mm/.gitignore
index 911f39d634be..a8c3be02188c 100644
--- a/tools/testing/selftests/mm/.gitignore
+++ b/tools/testing/selftests/mm/.gitignore
@@ -42,6 +42,7 @@ memfd_secret
 hugetlb_dio
 pkey_sighandler_tests_32
 pkey_sighandler_tests_64
+process_madv
 soft-dirty
 split_huge_page_test
 ksm_tests
diff --git a/tools/testing/selftests/mm/Makefile 
b/tools/testing/selftests/mm/Makefile
index 2352252f3914..725612e09582 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -86,6 +86,7 @@ TEST_GEN_FILES += mseal_test
 TEST_GEN_FILES += on-fault-limit
 TEST_GEN_FILES += pagemap_ioctl
 TEST_GEN_FILES += pfnmap
+TEST_GEN_FILES += process_madv
 TEST_GEN_FILES += thuge-gen
 TEST_GEN_FILES += transhuge-stress
 TEST_GEN_FILES += uffd-stress
diff --git a/tools/testing/selftests/mm/process_madv.c 
b/tools/testing/selftests/mm/process_madv.c
new file mode 100644
index ..73999c8e3570
--- /dev/null
+++ b/tools/testing/selftests/mm/process_madv.c
@@ -0,0 +1,414 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#define _GNU_SOURCE
+#include "../kselftest_harness.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "vm_util.h"
+
+#include "../pidfd/pidfd.h"
+
+/*
+ * Ignore the checkpatch warning, as per the C99 standard, section 7.14.1.1:
+ *
+ * "If the signal occurs other than as the result of calling the abort or raise
+ *  function, the behavior is undefined if the signal handler refers to any
+ *  object with static storage duration other than by assigning a value to an
+ *  object declared as volatile sig_atomic_t"
+ */
+static volatile sig_atomic_t signal_jump_set;
+static sigjmp_buf signal_jmp_buf;
+
+/*
+ * Ignore the checkpatch warning, we must read from x but don't want to do
+ * anything with it in order to trigger a read page fault. We therefore must 
use
+ * volatile to stop the compiler from optimising this away.
+ */
+#define FORCE_READ(x) (*(volatile typeof(x) *)x)
+
+static void handle_fatal(int c)
+{
+   if (!signal_jump_set)
+   return;
+
+   siglongjmp(signal_jmp_buf, c);
+}
+
+FIXTURE(process_madvise)
+{
+   int pidfd;
+   int flag;
+};
+
+static void setup_sighandler(void)
+{
+   struct sigaction act = {
+   .sa_handler = &handle_fatal,
+   .sa_flags = SA_NODEFER,
+   };
+
+   sigemptyset(&act.sa_mask);
+   if (sigaction(SIGSEGV, &act, NULL))
+   ksft_exit_fail_perror("sigaction");
+}
+
+static void teardown_sighandler(void)
+{
+   struct sigaction act = {
+   .sa_handler = SIG_DFL,
+   .sa_flags = SA_NODEFER,
+   };
+
+   sigemptyset(&act.sa_mask);
+   sigaction(SIGSEGV, &act, NULL);
+}
+
+FIXTURE_SETUP(process_madvise)
+{
+   self->pidfd = PIDFD_SELF;
+   self->flag = 0;
+   setup_sighandler();
+};
+
+FIXTURE_TEARDOWN_PARENT(process_madvise)
+{
+   teardown_sighandler();
+}
+
+static ssize_t sys_process_madvise(int pidfd, const struct iovec *iovec,
+  size_t vlen, int advice, unsigned int flags)
+{
+   return syscall(__NR_process_madvise, pidfd, iovec, vlen, advice, flags);
+}
+
+/*
+ * Enable our signal catcher and try to read/write the specified buffer. The
+ * return value indicates whether the read/write succeeds without a fatal
+ * signal.
+ */
+static bool try_access_buf(char *ptr, bool write)
+{
+   bool failed;
+
+   /* Tell signal handler to jump back here on fatal signal. */
+   signal_jump_set = true;
+   /* If a fatal signal arose, we will jump back here and failed is set. */
+   failed = sigsetjmp(signal_jmp_buf, 0) != 0;
+
+   if (!failed) {
+   if (write)
+   *ptr = 'x';
+   else
+   FORCE_READ(ptr);
+   }
+
+   signal_jump_set = false;
+   return !failed;
+}
+
+/* Try and read from a buffer, return true if no fatal signal. */
+static bool try_read_buf(char *ptr)
+{
+   return try_access_buf(ptr, false);
+}
+
+TEST_F(process_madvise, basic)
+{
+