On Tue, May 10, 2022 at 9:36 AM Shuah Khan skhan@linuxfoundation.org wrote:
On 5/10/22 10:29 AM, Suren Baghdasaryan wrote:
On Tue, May 10, 2022 at 8:43 AM Shuah Khan skhan@linuxfoundation.org wrote:
On 5/9/22 9:00 PM, Suren Baghdasaryan wrote:
Introduce process_mrelease syscall sanity tests. They include tests of invalid pidfd and flags inputs, attempting to call process_mrelease with a live process and a valid usage of process_mrelease. Because process_mrelease has to be used against a process with a pending SIGKILL, it's possible that the process exits before process_mrelease gets called. In such cases we retry the test with a victim that allocates twice more memory up to 1GB. This would require the victim process to spend more time during exit and process_mrelease has a better chance of catching the process before it exits.
+1 on Mike's comments on improving the change log. List what is getting tested as opposed to describing the test code.
I'll try to improve the description but IMHO it does describe what it's testing - the process_mrelease syscall with valid and invalid inputs. I could omit the implementation details if that helps.
Signed-off-by: Suren Baghdasaryan surenb@google.com
tools/testing/selftests/vm/Makefile | 1 + tools/testing/selftests/vm/mrelease_test.c | 176 +++++++++++++++++++++ tools/testing/selftests/vm/run_vmtests.sh | 16 ++ 3 files changed, 193 insertions(+) create mode 100644 tools/testing/selftests/vm/mrelease_test.c
Please update .gitignore with the new executable.
Ack.
diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile index 04a49e876a46..733fccbff0ef 100644 --- a/tools/testing/selftests/vm/Makefile +++ b/tools/testing/selftests/vm/Makefile @@ -43,6 +43,7 @@ TEST_GEN_FILES += map_populate TEST_GEN_FILES += memfd_secret TEST_GEN_FILES += mlock-random-test TEST_GEN_FILES += mlock2-tests +TEST_GEN_FILES += mrelease_test TEST_GEN_FILES += mremap_dontunmap TEST_GEN_FILES += mremap_test TEST_GEN_FILES += on-fault-limit diff --git a/tools/testing/selftests/vm/mrelease_test.c b/tools/testing/selftests/vm/mrelease_test.c new file mode 100644 index 000000000000..a61061bf8433 --- /dev/null +++ b/tools/testing/selftests/vm/mrelease_test.c @@ -0,0 +1,176 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright 2022 Google LLC
- */
+#define _GNU_SOURCE +#include <errno.h> +#include <stdio.h> +#include <stdlib.h> +#include <sys/wait.h> +#include <unistd.h>
+#include "util.h"
+static inline int pidfd_open(pid_t pid, unsigned int flags) +{ +#ifdef __NR_pidfd_open
return syscall(__NR_pidfd_open, pid, flags);+#else
errno = ENOSYS;This isn't an error - this would be skip because this syscall isn't supported.
Ack.
return -1;+#endif
Key off of syscall return instead of these ifdefs - same comment on all of the ifdefs
Ack. I was using some other test as an example but I guess that was not a good model.
+}
I am not seeing any reason for breaking this code up have a separate routine for pidfd_open().
I'm a bit unclear what you mean. Do you mean that userspace headers should already define pidfd_open() and I don't need to define it?
Do you need pidfd_open() or can this be part of main? Without the ifdefs, it is really a one line code.
Ah, I see. I think it's cleaner that way but I'll make them one-line inline functions.
thanks, -- Shuah