[LTP] [PATCH 0/3] Add process_mrelease testing suite

[LTP] [PATCH 0/3] Add process_mrelease testing suite

[Andrea Cervesato]

This testing suite is meant to add coverage for process_mrelease()
syscall, introduced in the kernel 5.15.

The testing suite is providing following tests:
- process_mrelease01
- process_mrelease02

Since there's no man pages, please consider the following
documentation instead:

https://lwn.net/Articles/864184/
https://lwn.net/Articles/865341/

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
Andrea Cervesato (3):
      Add process_mrelease syscalls definition
      Add process_mrelease01 test
      Add process_mrelease02 test

 include/lapi/syscalls/aarch64.in                   |   1 +
 include/lapi/syscalls/arc.in                       |   1 +
 include/lapi/syscalls/arm.in                       |   1 +
 include/lapi/syscalls/hppa.in                      |   1 +
 include/lapi/syscalls/i386.in                      |   1 +
 include/lapi/syscalls/ia64.in                      |   1 +
 include/lapi/syscalls/mips_n32.in                  |   1 +
 include/lapi/syscalls/mips_n64.in                  |   1 +
 include/lapi/syscalls/mips_o32.in                  |   1 +
 include/lapi/syscalls/powerpc.in                   |   1 +
 include/lapi/syscalls/powerpc64.in                 |   1 +
 include/lapi/syscalls/s390.in                      |   1 +
 include/lapi/syscalls/s390x.in                     |   1 +
 include/lapi/syscalls/sh.in                        |   1 +
 include/lapi/syscalls/sparc.in                     |   1 +
 include/lapi/syscalls/sparc64.in                   |   1 +
 include/lapi/syscalls/x86_64.in                    |   1 +
 runtest/syscalls                                   |   3 +
 .../kernel/syscalls/process_mrelease/.gitignore    |   2 +
 .../kernel/syscalls/process_mrelease/Makefile      |   7 ++
 .../syscalls/process_mrelease/process_mrelease01.c | 118 +++++++++++++++++++++
 .../syscalls/process_mrelease/process_mrelease02.c |  75 +++++++++++++
 22 files changed, 222 insertions(+)
---
base-commit: e644691d30c3948a9788b735c51e09ca849ea47f
change-id: 20240522-process_mrelease-3f2632b432e6

Best regards,
-- 
Andrea Cervesato <andrea.cervesato@suse.com>


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 1/3] Add process_mrelease syscalls definition

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 include/lapi/syscalls/aarch64.in   | 1 +
 include/lapi/syscalls/arc.in       | 1 +
 include/lapi/syscalls/arm.in       | 1 +
 include/lapi/syscalls/hppa.in      | 1 +
 include/lapi/syscalls/i386.in      | 1 +
 include/lapi/syscalls/ia64.in      | 1 +
 include/lapi/syscalls/mips_n32.in  | 1 +
 include/lapi/syscalls/mips_n64.in  | 1 +
 include/lapi/syscalls/mips_o32.in  | 1 +
 include/lapi/syscalls/powerpc.in   | 1 +
 include/lapi/syscalls/powerpc64.in | 1 +
 include/lapi/syscalls/s390.in      | 1 +
 include/lapi/syscalls/s390x.in     | 1 +
 include/lapi/syscalls/sh.in        | 1 +
 include/lapi/syscalls/sparc.in     | 1 +
 include/lapi/syscalls/sparc64.in   | 1 +
 include/lapi/syscalls/x86_64.in    | 1 +
 17 files changed, 17 insertions(+)

diff --git a/include/lapi/syscalls/aarch64.in b/include/lapi/syscalls/aarch64.in
index 2cb6c2d87..98e6c69c5 100644
--- a/include/lapi/syscalls/aarch64.in
+++ b/include/lapi/syscalls/aarch64.in
@@ -296,5 +296,6 @@ pidfd_getfd 438
 faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
+process_mrelease 448
 futex_waitv 449
 _sysctl 1078
diff --git a/include/lapi/syscalls/arc.in b/include/lapi/syscalls/arc.in
index 3e2ee9061..8ed05ce68 100644
--- a/include/lapi/syscalls/arc.in
+++ b/include/lapi/syscalls/arc.in
@@ -316,4 +316,5 @@ pidfd_getfd 438
 faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
+process_mrelease 448
 futex_waitv 449
diff --git a/include/lapi/syscalls/arm.in b/include/lapi/syscalls/arm.in
index 7bdbca533..d133a44be 100644
--- a/include/lapi/syscalls/arm.in
+++ b/include/lapi/syscalls/arm.in
@@ -394,4 +394,5 @@ pidfd_getfd (__NR_SYSCALL_BASE+438)
 faccessat2 (__NR_SYSCALL_BASE+439)
 epoll_pwait2 (__NR_SYSCALL_BASE+441)
 quotactl_fd (__NR_SYSCALL_BASE+443)
+process_mrelease (__NR_SYSCALL_BASE+448)
 futex_waitv (__NR_SYSCALL_BASE+449)
diff --git a/include/lapi/syscalls/hppa.in b/include/lapi/syscalls/hppa.in
index 8ebdafafb..578751e2e 100644
--- a/include/lapi/syscalls/hppa.in
+++ b/include/lapi/syscalls/hppa.in
@@ -43,4 +43,5 @@ close_range 436
 faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
+process_mrelease 448
 futex_waitv 449
diff --git a/include/lapi/syscalls/i386.in b/include/lapi/syscalls/i386.in
index 1472631c4..59a1d030b 100644
--- a/include/lapi/syscalls/i386.in
+++ b/include/lapi/syscalls/i386.in
@@ -430,4 +430,5 @@ pidfd_getfd 438
 faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
+process_mrelease 448
 futex_waitv 449
diff --git a/include/lapi/syscalls/ia64.in b/include/lapi/syscalls/ia64.in
index 0ea6e9722..e410582d8 100644
--- a/include/lapi/syscalls/ia64.in
+++ b/include/lapi/syscalls/ia64.in
@@ -343,4 +343,5 @@ pidfd_getfd 1462
 faccessat2 1463
 epoll_pwait2 1465
 quotactl_fd 1467
+process_mrelease 1472
 futex_waitv 1473
diff --git a/include/lapi/syscalls/mips_n32.in b/include/lapi/syscalls/mips_n32.in
index e818c9d92..f2602a02f 100644
--- a/include/lapi/syscalls/mips_n32.in
+++ b/include/lapi/syscalls/mips_n32.in
@@ -370,4 +370,5 @@ process_madvise 6440
 epoll_pwait2 6441
 mount_setattr 6442
 quotactl_fd 6443
+process_mrelease 6448
 futex_waitv 6449
diff --git a/include/lapi/syscalls/mips_n64.in b/include/lapi/syscalls/mips_n64.in
index 6e15f43b3..f16762c69 100644
--- a/include/lapi/syscalls/mips_n64.in
+++ b/include/lapi/syscalls/mips_n64.in
@@ -346,4 +346,5 @@ process_madvise 5440
 epoll_pwait2 5441
 mount_setattr 5442
 quotactl_fd 5443
+process_mrelease 5448
 futex_waitv 5449
diff --git a/include/lapi/syscalls/mips_o32.in b/include/lapi/syscalls/mips_o32.in
index 921d5d331..88a0d7ca8 100644
--- a/include/lapi/syscalls/mips_o32.in
+++ b/include/lapi/syscalls/mips_o32.in
@@ -416,4 +416,5 @@ process_madvise 4440
 epoll_pwait2 4441
 mount_setattr 4442
 quotactl_fd 4443
+process_mrelease 4448
 futex_waitv 4449
diff --git a/include/lapi/syscalls/powerpc.in b/include/lapi/syscalls/powerpc.in
index 545d9d3d6..20233339d 100644
--- a/include/lapi/syscalls/powerpc.in
+++ b/include/lapi/syscalls/powerpc.in
@@ -423,4 +423,5 @@ pidfd_getfd 438
 faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
+process_mrelease 448
 futex_waitv 449
diff --git a/include/lapi/syscalls/powerpc64.in b/include/lapi/syscalls/powerpc64.in
index 545d9d3d6..20233339d 100644
--- a/include/lapi/syscalls/powerpc64.in
+++ b/include/lapi/syscalls/powerpc64.in
@@ -423,4 +423,5 @@ pidfd_getfd 438
 faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
+process_mrelease 448
 futex_waitv 449
diff --git a/include/lapi/syscalls/s390.in b/include/lapi/syscalls/s390.in
index 7213ac5f8..ae0c83636 100644
--- a/include/lapi/syscalls/s390.in
+++ b/include/lapi/syscalls/s390.in
@@ -410,4 +410,5 @@ pidfd_getfd 438
 faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
+process_mrelease 448
 futex_waitv 449
diff --git a/include/lapi/syscalls/s390x.in b/include/lapi/syscalls/s390x.in
index 879012e2b..d1476e98f 100644
--- a/include/lapi/syscalls/s390x.in
+++ b/include/lapi/syscalls/s390x.in
@@ -358,4 +358,5 @@ pidfd_getfd 438
 faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
+process_mrelease 448
 futex_waitv 449
diff --git a/include/lapi/syscalls/sh.in b/include/lapi/syscalls/sh.in
index 7d5192a27..845deff4d 100644
--- a/include/lapi/syscalls/sh.in
+++ b/include/lapi/syscalls/sh.in
@@ -404,4 +404,5 @@ pidfd_getfd 438
 faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
+process_mrelease 448
 futex_waitv 449
diff --git a/include/lapi/syscalls/sparc.in b/include/lapi/syscalls/sparc.in
index 91d2fb1c2..ec73eaa2b 100644
--- a/include/lapi/syscalls/sparc.in
+++ b/include/lapi/syscalls/sparc.in
@@ -409,4 +409,5 @@ pidfd_getfd 438
 faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
+process_mrelease 448
 futex_waitv 449
diff --git a/include/lapi/syscalls/sparc64.in b/include/lapi/syscalls/sparc64.in
index 1f2fc59b7..ecf8e1d22 100644
--- a/include/lapi/syscalls/sparc64.in
+++ b/include/lapi/syscalls/sparc64.in
@@ -374,4 +374,5 @@ pidfd_getfd 438
 faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
+process_mrelease 448
 futex_waitv 449
diff --git a/include/lapi/syscalls/x86_64.in b/include/lapi/syscalls/x86_64.in
index dc61aa56e..415f3011d 100644
--- a/include/lapi/syscalls/x86_64.in
+++ b/include/lapi/syscalls/x86_64.in
@@ -351,6 +351,7 @@ pidfd_getfd 438
 faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
+process_mrelease 448
 futex_waitv 449
 rt_sigaction 512
 rt_sigreturn 513

-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/3] Add process_mrelease01 test

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

This test verifies that process_mrelease() syscall is releasing memory
from a killed process with memory allocation pending.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 runtest/syscalls                                   |   2 +
 .../kernel/syscalls/process_mrelease/.gitignore    |   1 +
 .../kernel/syscalls/process_mrelease/Makefile      |   7 ++
 .../syscalls/process_mrelease/process_mrelease01.c | 118 +++++++++++++++++++++
 4 files changed, 128 insertions(+)

diff --git a/runtest/syscalls b/runtest/syscalls
index cf06ee563..46a85fd31 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1050,6 +1050,8 @@ preadv203_64 preadv203_64
 
 profil01 profil01
 
+process_mrelease01 process_mrelease01
+
 process_vm_readv01 process_vm01 -r
 process_vm_readv02 process_vm_readv02
 process_vm_readv03 process_vm_readv03
diff --git a/testcases/kernel/syscalls/process_mrelease/.gitignore b/testcases/kernel/syscalls/process_mrelease/.gitignore
new file mode 100644
index 000000000..673983858
--- /dev/null
+++ b/testcases/kernel/syscalls/process_mrelease/.gitignore
@@ -0,0 +1 @@
+/process_mrelease01
diff --git a/testcases/kernel/syscalls/process_mrelease/Makefile b/testcases/kernel/syscalls/process_mrelease/Makefile
new file mode 100644
index 000000000..8cf1b9024
--- /dev/null
+++ b/testcases/kernel/syscalls/process_mrelease/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/process_mrelease/process_mrelease01.c b/testcases/kernel/syscalls/process_mrelease/process_mrelease01.c
new file mode 100644
index 000000000..aa7c9960c
--- /dev/null
+++ b/testcases/kernel/syscalls/process_mrelease/process_mrelease01.c
@@ -0,0 +1,118 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * This test verifies that process_mrelease() syscall is releasing memory from
+ * a killed process with memory allocation pending.
+ */
+
+#include "tst_test.h"
+#include "lapi/syscalls.h"
+
+#define CHUNK (1 * TST_MB)
+#define MAX_SIZE_MB (128 * TST_MB)
+
+static void *mem;
+static volatile int *mem_size;
+
+static void do_child(int size)
+{
+	tst_res(TINFO, "Child: allocate %d bytes", size);
+
+	mem = SAFE_MMAP(NULL,
+		size,
+		PROT_READ | PROT_WRITE,
+		MAP_PRIVATE | MAP_ANON,
+		0, 0);
+
+	TST_CHECKPOINT_WAKE_AND_WAIT(0);
+
+	tst_res(TINFO, "Child: releasing memory");
+
+	SAFE_MUNMAP(mem, size);
+}
+
+static void run(void)
+{
+	int ret;
+	int pidfd;
+	int status;
+	pid_t pid;
+	volatile int restart;
+
+	for (*mem_size = CHUNK; *mem_size < MAX_SIZE_MB; *mem_size += CHUNK) {
+		restart = 0;
+
+		pid = SAFE_FORK();
+		if (!pid) {
+			do_child(*mem_size);
+			exit(0);
+		}
+
+		TST_CHECKPOINT_WAIT(0);
+
+		tst_disable_oom_protection(pid);
+
+		pidfd = SAFE_PIDFD_OPEN(pid, 0);
+
+		tst_res(TINFO, "Parent: killing child with PID=%d", pid);
+
+		SAFE_KILL(pid, SIGKILL);
+
+		ret = tst_syscall(__NR_process_mrelease, pidfd, 0);
+		if (ret == -1) {
+			if (errno == ESRCH) {
+				tst_res(TINFO, "Parent: child terminated before process_mrelease()."
+					" Increase memory size and restart test");
+
+				restart = 1;
+			} else {
+				tst_res(TFAIL | TERRNO, "process_mrelease(%d,0) error", pidfd);
+			}
+		} else {
+			tst_res(TPASS, "process_mrelease(%d,0) passed", pidfd);
+		}
+
+		SAFE_WAITPID(-1, &status, 0);
+		SAFE_CLOSE(pidfd);
+
+		if (!restart)
+			break;
+	}
+}
+
+static void setup(void)
+{
+	mem_size = SAFE_MMAP(
+		NULL,
+		sizeof(int),
+		PROT_READ | PROT_WRITE,
+		MAP_SHARED | MAP_ANONYMOUS,
+		-1, 0);
+}
+
+static void cleanup(void)
+{
+	if (mem)
+		SAFE_MUNMAP(mem, *mem_size);
+
+	if (mem_size)
+		SAFE_MUNMAP((void *)mem_size, sizeof(int));
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run,
+	.needs_root = 1,
+	.forks_child = 1,
+	.min_kver = "5.15",
+	.needs_checkpoints = 1,
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_MMU=y",
+	},
+};

-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 3/3] Add process_mrelease02 test

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

This test verifies that process_mrelease() syscall correctly raises
the errors.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 runtest/syscalls                                   |  1 +
 .../kernel/syscalls/process_mrelease/.gitignore    |  1 +
 .../syscalls/process_mrelease/process_mrelease02.c | 75 ++++++++++++++++++++++
 3 files changed, 77 insertions(+)

diff --git a/runtest/syscalls b/runtest/syscalls
index 46a85fd31..c2fe919f0 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1051,6 +1051,7 @@ preadv203_64 preadv203_64
 profil01 profil01
 
 process_mrelease01 process_mrelease01
+process_mrelease02 process_mrelease02
 
 process_vm_readv01 process_vm01 -r
 process_vm_readv02 process_vm_readv02
diff --git a/testcases/kernel/syscalls/process_mrelease/.gitignore b/testcases/kernel/syscalls/process_mrelease/.gitignore
index 673983858..f1e7a8fea 100644
--- a/testcases/kernel/syscalls/process_mrelease/.gitignore
+++ b/testcases/kernel/syscalls/process_mrelease/.gitignore
@@ -1 +1,2 @@
 /process_mrelease01
+/process_mrelease02
diff --git a/testcases/kernel/syscalls/process_mrelease/process_mrelease02.c b/testcases/kernel/syscalls/process_mrelease/process_mrelease02.c
new file mode 100644
index 000000000..ac13317ee
--- /dev/null
+++ b/testcases/kernel/syscalls/process_mrelease/process_mrelease02.c
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * This test verifies that process_mrelease() syscall is raising errors:
+ * * EBADF when a bad file descriptor is given
+ * * EINVAL when flags is not zero
+ * * EINVAL when memory of a task cannot be released because it's still running
+ */
+
+#include "tst_test.h"
+#include "lapi/syscalls.h"
+
+static int badfd = -1;
+static int pidfd;
+
+static struct tcase {
+	int needs_child;
+	int *fd;
+	int flags;
+	int exp_errno;
+	char *msg;
+} tcases[] = {
+	{0, &badfd,  0, EBADF,  "bad file descriptor"},
+	{1, &pidfd, -1, EINVAL, "flags is not 0"},
+	{1, &pidfd,  0, EINVAL,  "memory of running task cannot be released"},
+};
+
+static void run(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+
+	if (tc->needs_child) {
+		pid_t pid;
+
+		pid = SAFE_FORK();
+		if (!pid) {
+			tst_res(TINFO, "Keep child alive");
+
+			TST_CHECKPOINT_WAKE_AND_WAIT(0);
+			exit(0);
+		}
+
+		TST_CHECKPOINT_WAIT(0);
+
+		pidfd = SAFE_PIDFD_OPEN(pid, 0);
+	}
+
+	TST_EXP_FAIL(tst_syscall(__NR_process_mrelease, *tc->fd, tc->flags),
+		tc->exp_errno,
+		"%s", tc->msg);
+
+	if (tc->needs_child) {
+		SAFE_CLOSE(pidfd);
+
+		TST_CHECKPOINT_WAKE(0);
+	}
+}
+
+static struct tst_test test = {
+	.test = run,
+	.tcnt = ARRAY_SIZE(tcases),
+	.needs_root = 1,
+	.forks_child = 1,
+	.min_kver = "5.15",
+	.needs_checkpoints = 1,
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_MMU=y",
+		NULL,
+	},
+};

-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] Add process_mrelease01 test

[Cyril Hrubis]

Hi!
> +/*\
> + * [Description]
> + *
> + * This test verifies that process_mrelease() syscall is releasing memory from
> + * a killed process with memory allocation pending.
> + */
> +
> +#include "tst_test.h"
> +#include "lapi/syscalls.h"
> +
> +#define CHUNK (1 * TST_MB)
> +#define MAX_SIZE_MB (128 * TST_MB)
> +
> +static void *mem;
> +static volatile int *mem_size;
> +
> +static void do_child(int size)
> +{
> +	tst_res(TINFO, "Child: allocate %d bytes", size);
> +
> +	mem = SAFE_MMAP(NULL,
> +		size,
> +		PROT_READ | PROT_WRITE,
> +		MAP_PRIVATE | MAP_ANON,
> +		0, 0);

This does not actually alloate any memory, it set ups the vmas but the
actual memory is not allocated until you fault it by accessing it, so
you have to actually touch every page to get it faulted, e.g. memset()
it to something.


> +	TST_CHECKPOINT_WAKE_AND_WAIT(0);
> +
> +	tst_res(TINFO, "Child: releasing memory");
> +
> +	SAFE_MUNMAP(mem, size);
> +}
> +
> +static void run(void)
> +{
> +	int ret;
> +	int pidfd;
> +	int status;
> +	pid_t pid;
> +	volatile int restart;
> +
> +	for (*mem_size = CHUNK; *mem_size < MAX_SIZE_MB; *mem_size += CHUNK) {
> +		restart = 0;
> +
> +		pid = SAFE_FORK();
> +		if (!pid) {
> +			do_child(*mem_size);
> +			exit(0);
> +		}
> +
> +		TST_CHECKPOINT_WAIT(0);
> +		tst_disable_oom_protection(pid);

What is this needed for?

> +		pidfd = SAFE_PIDFD_OPEN(pid, 0);
> +
> +		tst_res(TINFO, "Parent: killing child with PID=%d", pid);
> +
> +		SAFE_KILL(pid, SIGKILL);
> +
> +		ret = tst_syscall(__NR_process_mrelease, pidfd, 0);
> +		if (ret == -1) {
> +			if (errno == ESRCH) {
> +				tst_res(TINFO, "Parent: child terminated before process_mrelease()."
> +					" Increase memory size and restart test");
> +
> +				restart = 1;

As far as I understand the documentation his condition should not happen
until you have called wait() on the process.

> +			} else {
> +				tst_res(TFAIL | TERRNO, "process_mrelease(%d,0) error", pidfd);
> +			}
> +		} else {
> +			tst_res(TPASS, "process_mrelease(%d,0) passed", pidfd);

Okay it passed, but did we free any memory?

Is the /proc/$pid/ still accessible before we wait on the killed
process? If it is we can parse the proc maps before and after
process_mrelease and check if the memory was really freed.

> +		}
> +
> +		SAFE_WAITPID(-1, &status, 0);
> +		SAFE_CLOSE(pidfd);
> +
> +		if (!restart)
> +			break;
> +	}
> +}
> +
> +static void setup(void)
> +{
> +	mem_size = SAFE_MMAP(
> +		NULL,
> +		sizeof(int),
> +		PROT_READ | PROT_WRITE,
> +		MAP_SHARED | MAP_ANONYMOUS,
> +		-1, 0);

Why do we keep the size in shared memory? The forked child has COW
access to the parent memory, we can use the variables set in the parent
just fine.

> +}
> +
> +static void cleanup(void)
> +{
> +	if (mem)
> +		SAFE_MUNMAP(mem, *mem_size);

This is allocated in chid and never non NULL in parent.

> +	if (mem_size)
> +		SAFE_MUNMAP((void *)mem_size, sizeof(int));
> +}
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_all = run,
> +	.needs_root = 1,
> +	.forks_child = 1,
> +	.min_kver = "5.15",
> +	.needs_checkpoints = 1,
> +	.needs_kconfigs = (const char *[]) {
> +		"CONFIG_MMU=y",
> +	},
> +};
> 
> -- 
> 2.35.3
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 3/3] Add process_mrelease02 test

[Cyril Hrubis]

On Wed, May 22, 2024 at 04:33:07PM +0200, Andrea Cervesato wrote:
> From: Andrea Cervesato <andrea.cervesato@suse.com>
> 
> This test verifies that process_mrelease() syscall correctly raises
> the errors.
> 
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>  runtest/syscalls                                   |  1 +
>  .../kernel/syscalls/process_mrelease/.gitignore    |  1 +
>  .../syscalls/process_mrelease/process_mrelease02.c | 75 ++++++++++++++++++++++
>  3 files changed, 77 insertions(+)
> 
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 46a85fd31..c2fe919f0 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1051,6 +1051,7 @@ preadv203_64 preadv203_64
>  profil01 profil01
>  
>  process_mrelease01 process_mrelease01
> +process_mrelease02 process_mrelease02
>  
>  process_vm_readv01 process_vm01 -r
>  process_vm_readv02 process_vm_readv02
> diff --git a/testcases/kernel/syscalls/process_mrelease/.gitignore b/testcases/kernel/syscalls/process_mrelease/.gitignore
> index 673983858..f1e7a8fea 100644
> --- a/testcases/kernel/syscalls/process_mrelease/.gitignore
> +++ b/testcases/kernel/syscalls/process_mrelease/.gitignore
> @@ -1 +1,2 @@
>  /process_mrelease01
> +/process_mrelease02
> diff --git a/testcases/kernel/syscalls/process_mrelease/process_mrelease02.c b/testcases/kernel/syscalls/process_mrelease/process_mrelease02.c
> new file mode 100644
> index 000000000..ac13317ee
> --- /dev/null
> +++ b/testcases/kernel/syscalls/process_mrelease/process_mrelease02.c
> @@ -0,0 +1,75 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * This test verifies that process_mrelease() syscall is raising errors:
> + * * EBADF when a bad file descriptor is given
> + * * EINVAL when flags is not zero
> + * * EINVAL when memory of a task cannot be released because it's still running
> + */
> +
> +#include "tst_test.h"
> +#include "lapi/syscalls.h"
> +
> +static int badfd = -1;
> +static int pidfd;
> +
> +static struct tcase {
> +	int needs_child;
> +	int *fd;
> +	int flags;
> +	int exp_errno;
> +	char *msg;
> +} tcases[] = {
> +	{0, &badfd,  0, EBADF,  "bad file descriptor"},
> +	{1, &pidfd, -1, EINVAL, "flags is not 0"},
> +	{1, &pidfd,  0, EINVAL,  "memory of running task cannot be released"},

We can easily trigger ESCHR as well, just fork a child that just exits,
get pidfd to that child and then wait the child.

> +};
> +
> +static void run(unsigned int n)
> +{
> +	struct tcase *tc = &tcases[n];
> +
> +	if (tc->needs_child) {
> +		pid_t pid;
> +
> +		pid = SAFE_FORK();
> +		if (!pid) {
> +			tst_res(TINFO, "Keep child alive");
> +
> +			TST_CHECKPOINT_WAKE_AND_WAIT(0);
> +			exit(0);
> +		}
> +
> +		TST_CHECKPOINT_WAIT(0);
> +
> +		pidfd = SAFE_PIDFD_OPEN(pid, 0);
> +	}

We can set up several types of a child processes in the test setup,
there is no need to do this on every iteration.

Similarily for the ESCHR case we can just do the same for EINVAL cases.
For the invalid flags we would need a process that did actually exit but
wasn't waited for. And for the second EINVAL case we would need a
running process, so perhaps just a child that sleeps in pause().

> +	TST_EXP_FAIL(tst_syscall(__NR_process_mrelease, *tc->fd, tc->flags),
> +		tc->exp_errno,
> +		"%s", tc->msg);
> +
> +	if (tc->needs_child) {
> +		SAFE_CLOSE(pidfd);
> +
> +		TST_CHECKPOINT_WAKE(0);
> +	}
> +}
> +
> +static struct tst_test test = {
> +	.test = run,
> +	.tcnt = ARRAY_SIZE(tcases),
> +	.needs_root = 1,
> +	.forks_child = 1,
> +	.min_kver = "5.15",
> +	.needs_checkpoints = 1,
> +	.needs_kconfigs = (const char *[]) {
> +		"CONFIG_MMU=y",

I do not think this is necessary, LTP does not run on non-MMU targets
anyways.

> +		NULL,
> +	},
> +};

Also I think that it would make sense to CC Michal Hocko on the v2 since
he did review the kernel patches for this syscall.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] Add process_mrelease01 test

[Andrea Cervesato via ltp]

Hi!

On 7/17/24 15:14, Cyril Hrubis wrote:
> Hi!
>> +/*\
>> + * [Description]
>> + *
>> + * This test verifies that process_mrelease() syscall is releasing memory from
>> + * a killed process with memory allocation pending.
>> + */
>> +
>> +#include "tst_test.h"
>> +#include "lapi/syscalls.h"
>> +
>> +#define CHUNK (1 * TST_MB)
>> +#define MAX_SIZE_MB (128 * TST_MB)
>> +
>> +static void *mem;
>> +static volatile int *mem_size;
>> +
>> +static void do_child(int size)
>> +{
>> +	tst_res(TINFO, "Child: allocate %d bytes", size);
>> +
>> +	mem = SAFE_MMAP(NULL,
>> +		size,
>> +		PROT_READ | PROT_WRITE,
>> +		MAP_PRIVATE | MAP_ANON,
>> +		0, 0);
> This does not actually alloate any memory, it set ups the vmas but the
> actual memory is not allocated until you fault it by accessing it, so
> you have to actually touch every page to get it faulted, e.g. memset()
> it to something.
>
Yes this is true. I will fix it.
>> +	TST_CHECKPOINT_WAKE_AND_WAIT(0);
>> +
>> +	tst_res(TINFO, "Child: releasing memory");
>> +
>> +	SAFE_MUNMAP(mem, size);
>> +}
>> +
>> +static void run(void)
>> +{
>> +	int ret;
>> +	int pidfd;
>> +	int status;
>> +	pid_t pid;
>> +	volatile int restart;
>> +
>> +	for (*mem_size = CHUNK; *mem_size < MAX_SIZE_MB; *mem_size += CHUNK) {
>> +		restart = 0;
>> +
>> +		pid = SAFE_FORK();
>> +		if (!pid) {
>> +			do_child(*mem_size);
>> +			exit(0);
>> +		}
>> +
>> +		TST_CHECKPOINT_WAIT(0);
>> +		tst_disable_oom_protection(pid);
> What is this needed for?
process_mrelease() overlaps with OOM, since it's used to free up thread 
memory instead of OOM. For this reason we need to disable it, so any OOM 
operation after kill() won't be taken by it.
>
>> +		pidfd = SAFE_PIDFD_OPEN(pid, 0);
>> +
>> +		tst_res(TINFO, "Parent: killing child with PID=%d", pid);
>> +
>> +		SAFE_KILL(pid, SIGKILL);
>> +
>> +		ret = tst_syscall(__NR_process_mrelease, pidfd, 0);
>> +		if (ret == -1) {
>> +			if (errno == ESRCH) {
>> +				tst_res(TINFO, "Parent: child terminated before process_mrelease()."
>> +					" Increase memory size and restart test");
>> +
>> +				restart = 1;
> As far as I understand the documentation his condition should not happen
> until you have called wait() on the process.

The minimum requirement is that child has been killed and it's 
accessible from process_mrelease, so if we call wait() on the child, we 
won't be able to reach it anymore, causing ESRCH in process_mrelease().

Please check mrelease_test.c kselftest.

>
>> +			} else {
>> +				tst_res(TFAIL | TERRNO, "process_mrelease(%d,0) error", pidfd);
>> +			}
>> +		} else {
>> +			tst_res(TPASS, "process_mrelease(%d,0) passed", pidfd);
> Okay it passed, but did we free any memory?
>
> Is the /proc/$pid/ still accessible before we wait on the killed
> process? If it is we can parse the proc maps before and after
> process_mrelease and check if the memory was really freed.
Good idea, I will do that.
>> +		}
>> +
>> +		SAFE_WAITPID(-1, &status, 0);
>> +		SAFE_CLOSE(pidfd);
>> +
>> +		if (!restart)
>> +			break;
>> +	}
>> +}
>> +
>> +static void setup(void)
>> +{
>> +	mem_size = SAFE_MMAP(
>> +		NULL,
>> +		sizeof(int),
>> +		PROT_READ | PROT_WRITE,
>> +		MAP_SHARED | MAP_ANONYMOUS,
>> +		-1, 0);
> Why do we keep the size in shared memory? The forked child has COW
> access to the parent memory, we can use the variables set in the parent
> just fine.
+1
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> +	if (mem)
>> +		SAFE_MUNMAP(mem, *mem_size);
> This is allocated in chid and never non NULL in parent.
>
>> +	if (mem_size)
>> +		SAFE_MUNMAP((void *)mem_size, sizeof(int));
>> +}
>> +
>> +static struct tst_test test = {
>> +	.setup = setup,
>> +	.cleanup = cleanup,
>> +	.test_all = run,
>> +	.needs_root = 1,
>> +	.forks_child = 1,
>> +	.min_kver = "5.15",
>> +	.needs_checkpoints = 1,
>> +	.needs_kconfigs = (const char *[]) {
>> +		"CONFIG_MMU=y",
>> +	},
>> +};
>>
>> -- 
>> 2.35.3
>>
>>
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp
Andrea

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 3/3] Add process_mrelease02 test

[Andrea Cervesato via ltp]

Hi Cyril,

On 7/17/24 15:24, Cyril Hrubis wrote:
> On Wed, May 22, 2024 at 04:33:07PM +0200, Andrea Cervesato wrote:
>> From: Andrea Cervesato <andrea.cervesato@suse.com>
>>
>> This test verifies that process_mrelease() syscall correctly raises
>> the errors.
>>
>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
>> ---
>>   runtest/syscalls                                   |  1 +
>>   .../kernel/syscalls/process_mrelease/.gitignore    |  1 +
>>   .../syscalls/process_mrelease/process_mrelease02.c | 75 ++++++++++++++++++++++
>>   3 files changed, 77 insertions(+)
>>
>> diff --git a/runtest/syscalls b/runtest/syscalls
>> index 46a85fd31..c2fe919f0 100644
>> --- a/runtest/syscalls
>> +++ b/runtest/syscalls
>> @@ -1051,6 +1051,7 @@ preadv203_64 preadv203_64
>>   profil01 profil01
>>   
>>   process_mrelease01 process_mrelease01
>> +process_mrelease02 process_mrelease02
>>   
>>   process_vm_readv01 process_vm01 -r
>>   process_vm_readv02 process_vm_readv02
>> diff --git a/testcases/kernel/syscalls/process_mrelease/.gitignore b/testcases/kernel/syscalls/process_mrelease/.gitignore
>> index 673983858..f1e7a8fea 100644
>> --- a/testcases/kernel/syscalls/process_mrelease/.gitignore
>> +++ b/testcases/kernel/syscalls/process_mrelease/.gitignore
>> @@ -1 +1,2 @@
>>   /process_mrelease01
>> +/process_mrelease02
>> diff --git a/testcases/kernel/syscalls/process_mrelease/process_mrelease02.c b/testcases/kernel/syscalls/process_mrelease/process_mrelease02.c
>> new file mode 100644
>> index 000000000..ac13317ee
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/process_mrelease/process_mrelease02.c
>> @@ -0,0 +1,75 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
>> + */
>> +
>> +/*\
>> + * [Description]
>> + *
>> + * This test verifies that process_mrelease() syscall is raising errors:
>> + * * EBADF when a bad file descriptor is given
>> + * * EINVAL when flags is not zero
>> + * * EINVAL when memory of a task cannot be released because it's still running
>> + */
>> +
>> +#include "tst_test.h"
>> +#include "lapi/syscalls.h"
>> +
>> +static int badfd = -1;
>> +static int pidfd;
>> +
>> +static struct tcase {
>> +	int needs_child;
>> +	int *fd;
>> +	int flags;
>> +	int exp_errno;
>> +	char *msg;
>> +} tcases[] = {
>> +	{0, &badfd,  0, EBADF,  "bad file descriptor"},
>> +	{1, &pidfd, -1, EINVAL, "flags is not 0"},
>> +	{1, &pidfd,  0, EINVAL,  "memory of running task cannot be released"},
> We can easily trigger ESCHR as well, just fork a child that just exits,
> get pidfd to that child and then wait the child.
>
>> +};
>> +
>> +static void run(unsigned int n)
>> +{
>> +	struct tcase *tc = &tcases[n];
>> +
>> +	if (tc->needs_child) {
>> +		pid_t pid;
>> +
>> +		pid = SAFE_FORK();
>> +		if (!pid) {
>> +			tst_res(TINFO, "Keep child alive");
>> +
>> +			TST_CHECKPOINT_WAKE_AND_WAIT(0);
>> +			exit(0);
>> +		}
>> +
>> +		TST_CHECKPOINT_WAIT(0);
>> +
>> +		pidfd = SAFE_PIDFD_OPEN(pid, 0);
>> +	}
> We can set up several types of a child processes in the test setup,
> there is no need to do this on every iteration.
After working a bit on this I can say that LTP doesn't allow it. Simply 
because tst_reap_children() is called before the end of the test, which 
means we have to end all children, even if they need to be alive. And 
the have to be alive for the next step. So the first approach is the 
right one.
>
> Similarily for the ESCHR case we can just do the same for EINVAL cases.
> For the invalid flags we would need a process that did actually exit but
> wasn't waited for. And for the second EINVAL case we would need a
> running process, so perhaps just a child that sleeps in pause().
>
>> +	TST_EXP_FAIL(tst_syscall(__NR_process_mrelease, *tc->fd, tc->flags),
>> +		tc->exp_errno,
>> +		"%s", tc->msg);
>> +
>> +	if (tc->needs_child) {
>> +		SAFE_CLOSE(pidfd);
>> +
>> +		TST_CHECKPOINT_WAKE(0);
>> +	}
>> +}
>> +
>> +static struct tst_test test = {
>> +	.test = run,
>> +	.tcnt = ARRAY_SIZE(tcases),
>> +	.needs_root = 1,
>> +	.forks_child = 1,
>> +	.min_kver = "5.15",
>> +	.needs_checkpoints = 1,
>> +	.needs_kconfigs = (const char *[]) {
>> +		"CONFIG_MMU=y",
> I do not think this is necessary, LTP does not run on non-MMU targets
> anyways.
>
>> +		NULL,
>> +	},
>> +};
> Also I think that it would make sense to CC Michal Hocko on the v2 since
> he did review the kernel patches for this syscall.
>
Andrea

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp