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

[LTP] [PATCH v2 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>
---
Changes in v2:
- process_mrelease01: change the algorithm to ensure we released the
  memory after process_mrelease() is called
- process_mrelease02: verify ESRCH error
- Link to v1: https://lore.kernel.org/r/20240522-process_mrelease-v1-0-41fe2fa44194@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 | 168 +++++++++++++++++++++
 .../syscalls/process_mrelease/process_mrelease02.c |  84 +++++++++++
 22 files changed, 281 insertions(+)
---
base-commit: e7ebc637d0d99295490adf57660a3b3a177d65d3
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 v2 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 61d4450bf..219f53b86 100644
--- a/include/lapi/syscalls/aarch64.in
+++ b/include/lapi/syscalls/aarch64.in
@@ -299,6 +299,7 @@ quotactl_fd 443
 landlock_create_ruleset 444
 landlock_add_rule 445
 landlock_restrict_self 446
+process_mrelease 448
 futex_waitv 449
 cachestat 451
 fchmodat2 452
diff --git a/include/lapi/syscalls/arc.in b/include/lapi/syscalls/arc.in
index 752cc54fd..6519acc6f 100644
--- a/include/lapi/syscalls/arc.in
+++ b/include/lapi/syscalls/arc.in
@@ -319,6 +319,7 @@ quotactl_fd 443
 landlock_create_ruleset 444
 landlock_add_rule 445
 landlock_restrict_self 446
+process_mrelease 448
 futex_waitv 449
 cachestat 451
 fchmodat2 452
diff --git a/include/lapi/syscalls/arm.in b/include/lapi/syscalls/arm.in
index 84203ca4d..655038994 100644
--- a/include/lapi/syscalls/arm.in
+++ b/include/lapi/syscalls/arm.in
@@ -398,6 +398,7 @@ landlock_create_ruleset (__NR_SYSCALL_BASE+444)
 landlock_add_rule (__NR_SYSCALL_BASE+445)
 landlock_restrict_self (__NR_SYSCALL_BASE+446)
 memfd_secret (__NR_SYSCALL_BASE+447)
+process_mrelease (__NR_SYSCALL_BASE+448)
 futex_waitv (__NR_SYSCALL_BASE+449)
 cachestat (__NR_SYSCALL_BASE+451)
 fchmodat2 (__NR_SYSCALL_BASE+452)
diff --git a/include/lapi/syscalls/hppa.in b/include/lapi/syscalls/hppa.in
index 8240c69ce..a7178213f 100644
--- a/include/lapi/syscalls/hppa.in
+++ b/include/lapi/syscalls/hppa.in
@@ -46,6 +46,7 @@ quotactl_fd 443
 landlock_create_ruleset 444
 landlock_add_rule 445
 landlock_restrict_self 446
+process_mrelease 448
 futex_waitv 449
 cachestat 451
 fchmodat2 452
diff --git a/include/lapi/syscalls/i386.in b/include/lapi/syscalls/i386.in
index f6e8c7258..1800f35cd 100644
--- a/include/lapi/syscalls/i386.in
+++ b/include/lapi/syscalls/i386.in
@@ -433,6 +433,7 @@ quotactl_fd 443
 landlock_create_ruleset 444
 landlock_add_rule 445
 landlock_restrict_self 446
+process_mrelease 448
 futex_waitv 449
 cachestat 451
 fchmodat2 452
diff --git a/include/lapi/syscalls/ia64.in b/include/lapi/syscalls/ia64.in
index 8f55029a9..45798e890 100644
--- a/include/lapi/syscalls/ia64.in
+++ b/include/lapi/syscalls/ia64.in
@@ -346,6 +346,7 @@ quotactl_fd 1467
 landlock_create_ruleset 1468
 landlock_add_rule 1469
 landlock_restrict_self 1470
+process_mrelease 1472
 futex_waitv 1473
 cachestat 1475
 fchmodat2 1476
diff --git a/include/lapi/syscalls/mips_n32.in b/include/lapi/syscalls/mips_n32.in
index d85c567c7..dc1d120cc 100644
--- a/include/lapi/syscalls/mips_n32.in
+++ b/include/lapi/syscalls/mips_n32.in
@@ -373,6 +373,7 @@ quotactl_fd 6443
 landlock_create_ruleset 6444
 landlock_add_rule 6445
 landlock_restrict_self 6446
+process_mrelease 6448
 futex_waitv 6449
 cachestat 6451
 fchmodat2 6452
diff --git a/include/lapi/syscalls/mips_n64.in b/include/lapi/syscalls/mips_n64.in
index c34a85bbe..b8e18ea70 100644
--- a/include/lapi/syscalls/mips_n64.in
+++ b/include/lapi/syscalls/mips_n64.in
@@ -349,6 +349,7 @@ quotactl_fd 5443
 landlock_create_ruleset 5444
 landlock_add_rule 5445
 landlock_restrict_self 5446
+process_mrelease 5448
 futex_waitv 5449
 cachestat 5451
 fchmodat2 5452
diff --git a/include/lapi/syscalls/mips_o32.in b/include/lapi/syscalls/mips_o32.in
index 10d77787b..630f3a4ef 100644
--- a/include/lapi/syscalls/mips_o32.in
+++ b/include/lapi/syscalls/mips_o32.in
@@ -419,6 +419,7 @@ quotactl_fd 4443
 landlock_create_ruleset 4444
 landlock_add_rule 4445
 landlock_restrict_self 4446
+process_mrelease 4448
 futex_waitv 4449
 cachestat 4451
 fchmodat2 4452
diff --git a/include/lapi/syscalls/powerpc.in b/include/lapi/syscalls/powerpc.in
index af3ae5c90..3c9b9a2f2 100644
--- a/include/lapi/syscalls/powerpc.in
+++ b/include/lapi/syscalls/powerpc.in
@@ -426,6 +426,7 @@ quotactl_fd 443
 landlock_create_ruleset 444
 landlock_add_rule 445
 landlock_restrict_self 446
+process_mrelease 448
 futex_waitv 449
 cachestat 451
 fchmodat2 452
diff --git a/include/lapi/syscalls/powerpc64.in b/include/lapi/syscalls/powerpc64.in
index af3ae5c90..3c9b9a2f2 100644
--- a/include/lapi/syscalls/powerpc64.in
+++ b/include/lapi/syscalls/powerpc64.in
@@ -426,6 +426,7 @@ quotactl_fd 443
 landlock_create_ruleset 444
 landlock_add_rule 445
 landlock_restrict_self 446
+process_mrelease 448
 futex_waitv 449
 cachestat 451
 fchmodat2 452
diff --git a/include/lapi/syscalls/s390.in b/include/lapi/syscalls/s390.in
index e8e7fff0b..28f913bef 100644
--- a/include/lapi/syscalls/s390.in
+++ b/include/lapi/syscalls/s390.in
@@ -413,6 +413,7 @@ quotactl_fd 443
 landlock_create_ruleset 444
 landlock_add_rule 445
 landlock_restrict_self 446
+process_mrelease 448
 futex_waitv 449
 cachestat 451
 fchmodat2 452
diff --git a/include/lapi/syscalls/s390x.in b/include/lapi/syscalls/s390x.in
index 0ee3bd897..5b74fedd4 100644
--- a/include/lapi/syscalls/s390x.in
+++ b/include/lapi/syscalls/s390x.in
@@ -361,6 +361,7 @@ quotactl_fd 443
 landlock_create_ruleset 444
 landlock_add_rule 445
 landlock_restrict_self 446
+process_mrelease 448
 futex_waitv 449
 cachestat 451
 fchmodat2 452
diff --git a/include/lapi/syscalls/sh.in b/include/lapi/syscalls/sh.in
index 5701f2285..4dd344903 100644
--- a/include/lapi/syscalls/sh.in
+++ b/include/lapi/syscalls/sh.in
@@ -407,6 +407,7 @@ quotactl_fd 443
 landlock_create_ruleset 444
 landlock_add_rule 445
 landlock_restrict_self 446
+process_mrelease 448
 futex_waitv 449
 cachestat 451
 fchmodat2 452
diff --git a/include/lapi/syscalls/sparc.in b/include/lapi/syscalls/sparc.in
index 172969f60..4507c0aff 100644
--- a/include/lapi/syscalls/sparc.in
+++ b/include/lapi/syscalls/sparc.in
@@ -412,6 +412,7 @@ quotactl_fd 443
 landlock_create_ruleset 444
 landlock_add_rule 445
 landlock_restrict_self 446
+process_mrelease 448
 futex_waitv 449
 cachestat 451
 fchmodat2 452
diff --git a/include/lapi/syscalls/sparc64.in b/include/lapi/syscalls/sparc64.in
index 5b667f10f..95e7e6baa 100644
--- a/include/lapi/syscalls/sparc64.in
+++ b/include/lapi/syscalls/sparc64.in
@@ -377,6 +377,7 @@ quotactl_fd 443
 landlock_create_ruleset 444
 landlock_add_rule 445
 landlock_restrict_self 446
+process_mrelease 448
 futex_waitv 449
 cachestat 451
 fchmodat2 452
diff --git a/include/lapi/syscalls/x86_64.in b/include/lapi/syscalls/x86_64.in
index 1993f343a..8e5ea998c 100644
--- a/include/lapi/syscalls/x86_64.in
+++ b/include/lapi/syscalls/x86_64.in
@@ -354,6 +354,7 @@ quotactl_fd 443
 landlock_create_ruleset 444
 landlock_add_rule 445
 landlock_restrict_self 446
+process_mrelease 448
 futex_waitv 449
 cachestat 451
 fchmodat2 452

-- 
2.43.0


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

[LTP] [PATCH v2 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 | 168 +++++++++++++++++++++
 4 files changed, 178 insertions(+)

diff --git a/runtest/syscalls b/runtest/syscalls
index 706dd56dc..de90e9ba3 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1073,6 +1073,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..8a0a2c3b4
--- /dev/null
+++ b/testcases/kernel/syscalls/process_mrelease/process_mrelease01.c
@@ -0,0 +1,168 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 SUSE LLC Andrea Cervesaend_addr <andrea.cervesaend_addr@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * This test verifies that process_mrelease() syscall is releasing memory start_addr
+ * a killed process with memory allocation pending.
+ */
+
+#include "tst_test.h"
+#include "tst_safe_stdio.h"
+#include "lapi/syscalls.h"
+
+#define CHUNK (1 * TST_MB)
+#define MAX_SIZE_MB (128 * TST_MB)
+
+static unsigned long *mem_addr;
+static volatile int mem_size;
+
+static void do_child(int size)
+{
+	void *mem;
+
+	tst_res(TINFO, "Child: allocate %d bytes", size);
+
+	mem = SAFE_MMAP(NULL,
+		size,
+		PROT_READ | PROT_WRITE,
+		MAP_PRIVATE | MAP_ANON,
+		0, 0);
+
+	memset(mem, 0, size);
+
+	*mem_addr = (unsigned long)mem;
+
+	TST_CHECKPOINT_WAKE_AND_WAIT(0);
+
+	tst_res(TINFO, "Child: releasing memory");
+
+	SAFE_MUNMAP(mem, size);
+}
+
+static int memory_is_mapped(pid_t pid, unsigned long start, unsigned long end)
+{
+	FILE *fmaps;
+	int mapped = 0;
+	char buff[1024];
+	char pid_maps[128] = {0};
+	unsigned long start_addr, end_addr;
+
+	snprintf(pid_maps, sizeof(pid_maps), "/proc/%d/maps", pid);
+	fmaps = SAFE_FOPEN(pid_maps, "r");
+
+	while (!feof(fmaps)) {
+		memset(buff, 0, sizeof(buff));
+
+		if (!fgets(buff, sizeof(buff), fmaps))
+			break;
+
+		if (sscanf(buff, "%lx-%lx", &start_addr, &end_addr) != 2) {
+			tst_brk(TBROK | TERRNO, "Couldn't parse /proc/%ud/maps line.", pid);
+			break;
+		}
+
+		if (start == start_addr && end == end_addr) {
+			mapped = 1;
+			break;
+		}
+	}
+
+	SAFE_FCLOSE(fmaps);
+
+	return mapped;
+}
+
+static void run(void)
+{
+	int ret;
+	int pidfd;
+	int status;
+	pid_t pid;
+	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);
+
+		if (!memory_is_mapped(pid, *mem_addr, *mem_addr + mem_size)) {
+			tst_res(TFAIL, "Memory is not mapped");
+			break;
+		}
+
+		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 {
+			int timeout_ms = 1000;
+
+			tst_res(TPASS, "process_mrelease(%d,0) passed", pidfd);
+
+			while (memory_is_mapped(pid, *mem_addr, *mem_addr + mem_size) &&
+				timeout_ms--)
+				usleep(1000);
+
+			if (memory_is_mapped(pid, *mem_addr, *mem_addr + mem_size))
+				tst_res(TFAIL, "Memory is still mapped inside child memory");
+			else
+				tst_res(TPASS, "Memory has been released");
+		}
+
+		SAFE_WAITPID(-1, &status, 0);
+		SAFE_CLOSE(pidfd);
+
+		if (!restart)
+			break;
+	}
+}
+
+static void setup(void)
+{
+	mem_addr = SAFE_MMAP(NULL,
+		sizeof(unsigned long),
+		PROT_READ | PROT_WRITE,
+		MAP_SHARED | MAP_ANON,
+		0, 0);
+}
+
+static void cleanup(void)
+{
+	if (mem_addr)
+		SAFE_MUNMAP(mem_addr, sizeof(unsigned long));
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.forks_child = 1,
+	.min_kver = "5.15",
+	.needs_checkpoints = 1,
+};

-- 
2.43.0


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

[LTP] [PATCH v2 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 | 84 ++++++++++++++++++++++
 3 files changed, 86 insertions(+)

diff --git a/runtest/syscalls b/runtest/syscalls
index de90e9ba3..f3cb7d465 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1074,6 +1074,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..ced556243
--- /dev/null
+++ b/testcases/kernel/syscalls/process_mrelease/process_mrelease02.c
@@ -0,0 +1,84 @@
+// 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
+ * * ESRCH when child has been closed
+ */
+
+#include "tst_test.h"
+#include "lapi/syscalls.h"
+
+static int badfd = -1;
+static int pidfd;
+
+enum {
+	NO_CHILD,
+	EXIT_CHILD,
+	WAIT_CHILD,
+};
+
+static struct tcase {
+	int child_type;
+	int *fd;
+	int flags;
+	int exp_errno;
+	char *msg;
+} tcases[] = {
+	{NO_CHILD, &badfd, 0, EBADF, "bad file descriptor"},
+	{WAIT_CHILD, &pidfd, -1, EINVAL, "flags is not 0"},
+	{WAIT_CHILD, &pidfd, 0, EINVAL, "task memory cannot be released"},
+	{EXIT_CHILD, &pidfd, 0, ESRCH, "child is not running"},
+};
+
+static void run(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+	int status;
+
+	if (tc->child_type != NO_CHILD) {
+		pid_t pid;
+
+		pid = SAFE_FORK();
+		if (!pid) {
+			if (tc->child_type == WAIT_CHILD)
+				TST_CHECKPOINT_WAIT(0);
+
+			exit(0);
+		}
+
+		tst_res(TINFO, "Spawned waiting child with pid=%d", pid);
+
+		pidfd = SAFE_PIDFD_OPEN(pid, 0);
+
+		if (tc->child_type == EXIT_CHILD)
+			SAFE_WAITPID(pid, &status, 0);
+	}
+
+	TST_EXP_FAIL(tst_syscall(__NR_process_mrelease, *tc->fd, tc->flags),
+		tc->exp_errno,
+		"%s", tc->msg);
+
+	if (tc->child_type != NO_CHILD) {
+		if (tc->child_type == WAIT_CHILD)
+			TST_CHECKPOINT_WAKE(0);
+
+		SAFE_CLOSE(pidfd);
+	}
+}
+
+static struct tst_test test = {
+	.test = run,
+	.tcnt = ARRAY_SIZE(tcases),
+	.needs_root = 1,
+	.forks_child = 1,
+	.min_kver = "5.15",
+	.needs_checkpoints = 1,
+};

-- 
2.43.0


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

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

[Cyril Hrubis]

Hi!
> +static void run(void)
> +{
> +	int ret;
> +	int pidfd;
> +	int status;
> +	pid_t pid;
> +	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);
> +
> +		if (!memory_is_mapped(pid, *mem_addr, *mem_addr + mem_size)) {
> +			tst_res(TFAIL, "Memory is not mapped");
> +			break;
> +		}
> +
> +		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;

Does this even happen? I suppose that until the child has been waited
for you shouldn't get ESRCH at all. The memory may be freed
asynchronously but the pidfd is valid until we do waitpid, at least
that's what the description says:

https://lore.kernel.org/linux-mm/20210902220029.bfau3YxNP%25akpm@linux-foundation.org/

But selftest seems to do the same loop on ESRCH so either the test or
the documentation is wrong.

Michal any idea which is correct?

> +			} else {
> +				tst_res(TFAIL | TERRNO, "process_mrelease(%d,0) error", pidfd);
> +			}
> +		} else {
> +			int timeout_ms = 1000;
> +
> +			tst_res(TPASS, "process_mrelease(%d,0) passed", pidfd);
> +
> +			while (memory_is_mapped(pid, *mem_addr, *mem_addr + mem_size) &&
> +				timeout_ms--)
> +				usleep(1000);
> +
> +			if (memory_is_mapped(pid, *mem_addr, *mem_addr + mem_size))
> +				tst_res(TFAIL, "Memory is still mapped inside child memory");
> +			else
> +				tst_res(TPASS, "Memory has been released");

As far as I understand this this will likely pass even without the
process_mrelease() call since the process address space is being teared
down anyways. But I do not have an idea how to make things better. I
guess that if we wanted to know for sure we would have to run some
complex statistics with and without the syscall and compare the
timings...


> +		}
> +
> +		SAFE_WAITPID(-1, &status, 0);
> +		SAFE_CLOSE(pidfd);
> +
> +		if (!restart)
> +			break;
> +	}
> +}
> +
> +static void setup(void)
> +{
> +	mem_addr = SAFE_MMAP(NULL,
> +		sizeof(unsigned long),
> +		PROT_READ | PROT_WRITE,
> +		MAP_SHARED | MAP_ANON,
> +		0, 0);
> +}
> +
> +static void cleanup(void)
> +{
> +	if (mem_addr)
> +		SAFE_MUNMAP(mem_addr, sizeof(unsigned long));
> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.needs_root = 1,
> +	.forks_child = 1,
> +	.min_kver = "5.15",
> +	.needs_checkpoints = 1,
> +};
> 
> -- 
> 2.43.0
> 
> 
> -- 
> 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 v2 3/3] Add process_mrelease02 test

[Cyril Hrubis]

Hi!
> +/*\
> + * [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
> + * * ESRCH when child has been closed
> + */
> +
> +#include "tst_test.h"
> +#include "lapi/syscalls.h"
> +
> +static int badfd = -1;
> +static int pidfd;
> +
> +enum {
> +	NO_CHILD,
> +	EXIT_CHILD,
> +	WAIT_CHILD,
> +};
> +
> +static struct tcase {
> +	int child_type;
> +	int *fd;
> +	int flags;
> +	int exp_errno;
> +	char *msg;
> +} tcases[] = {
> +	{NO_CHILD, &badfd, 0, EBADF, "bad file descriptor"},
> +	{WAIT_CHILD, &pidfd, -1, EINVAL, "flags is not 0"},
> +	{WAIT_CHILD, &pidfd, 0, EINVAL, "task memory cannot be released"},
> +	{EXIT_CHILD, &pidfd, 0, ESRCH, "child is not running"},
> +};
> +
> +static void run(unsigned int n)
> +{
> +	struct tcase *tc = &tcases[n];
> +	int status;
> +
> +	if (tc->child_type != NO_CHILD) {
> +		pid_t pid;
> +
> +		pid = SAFE_FORK();
> +		if (!pid) {
> +			if (tc->child_type == WAIT_CHILD)
> +				TST_CHECKPOINT_WAIT(0);
> +
> +			exit(0);
> +		}
> +
> +		tst_res(TINFO, "Spawned waiting child with pid=%d", pid);
> +
> +		pidfd = SAFE_PIDFD_OPEN(pid, 0);
> +
> +		if (tc->child_type == EXIT_CHILD)
> +			SAFE_WAITPID(pid, &status, 0);
> +	}

Why don't we instead fork two children in the setup, one of the waits
and second exits and just set the pidfd once?

> +	TST_EXP_FAIL(tst_syscall(__NR_process_mrelease, *tc->fd, tc->flags),
> +		tc->exp_errno,
> +		"%s", tc->msg);
> +
> +	if (tc->child_type != NO_CHILD) {
> +		if (tc->child_type == WAIT_CHILD)
> +			TST_CHECKPOINT_WAKE(0);
> +
> +		SAFE_CLOSE(pidfd);
> +	}
> +}
> +
> +static struct tst_test test = {
> +	.test = run,
> +	.tcnt = ARRAY_SIZE(tcases),
> +	.needs_root = 1,
> +	.forks_child = 1,
> +	.min_kver = "5.15",
> +	.needs_checkpoints = 1,
> +};
> 
> -- 
> 2.43.0
> 
> 
> -- 
> 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 v2 3/3] Add process_mrelease02 test

[Andrea Cervesato via ltp]

Hi,

On 10/7/24 17:03, Cyril Hrubis wrote:
> Hi!
>> +/*\
>> + * [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
>> + * * ESRCH when child has been closed
>> + */
>> +
>> +#include "tst_test.h"
>> +#include "lapi/syscalls.h"
>> +
>> +static int badfd = -1;
>> +static int pidfd;
>> +
>> +enum {
>> +	NO_CHILD,
>> +	EXIT_CHILD,
>> +	WAIT_CHILD,
>> +};
>> +
>> +static struct tcase {
>> +	int child_type;
>> +	int *fd;
>> +	int flags;
>> +	int exp_errno;
>> +	char *msg;
>> +} tcases[] = {
>> +	{NO_CHILD, &badfd, 0, EBADF, "bad file descriptor"},
>> +	{WAIT_CHILD, &pidfd, -1, EINVAL, "flags is not 0"},
>> +	{WAIT_CHILD, &pidfd, 0, EINVAL, "task memory cannot be released"},
>> +	{EXIT_CHILD, &pidfd, 0, ESRCH, "child is not running"},
>> +};
>> +
>> +static void run(unsigned int n)
>> +{
>> +	struct tcase *tc = &tcases[n];
>> +	int status;
>> +
>> +	if (tc->child_type != NO_CHILD) {
>> +		pid_t pid;
>> +
>> +		pid = SAFE_FORK();
>> +		if (!pid) {
>> +			if (tc->child_type == WAIT_CHILD)
>> +				TST_CHECKPOINT_WAIT(0);
>> +
>> +			exit(0);
>> +		}
>> +
>> +		tst_res(TINFO, "Spawned waiting child with pid=%d", pid);
>> +
>> +		pidfd = SAFE_PIDFD_OPEN(pid, 0);
>> +
>> +		if (tc->child_type == EXIT_CHILD)
>> +			SAFE_WAITPID(pid, &status, 0);
>> +	}
> Why don't we instead fork two children in the setup, one of the waits
> and second exits and just set the pidfd once?
At the end of each test() the library calls tst_reap_children() and the 
waiting child will fail with ETIMEDOUT, so we need to spawn a child for 
each new test.
>> +	TST_EXP_FAIL(tst_syscall(__NR_process_mrelease, *tc->fd, tc->flags),
>> +		tc->exp_errno,
>> +		"%s", tc->msg);
>> +
>> +	if (tc->child_type != NO_CHILD) {
>> +		if (tc->child_type == WAIT_CHILD)
>> +			TST_CHECKPOINT_WAKE(0);
>> +
>> +		SAFE_CLOSE(pidfd);
>> +	}
>> +}
>> +
>> +static struct tst_test test = {
>> +	.test = run,
>> +	.tcnt = ARRAY_SIZE(tcases),
>> +	.needs_root = 1,
>> +	.forks_child = 1,
>> +	.min_kver = "5.15",
>> +	.needs_checkpoints = 1,
>> +};
>>
>> -- 
>> 2.43.0
>>
>>
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp

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

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

[Andrea Cervesato via ltp]

Hi!

On 9/12/24 18:28, Cyril Hrubis wrote:
> Hi!
>> +static void run(void)
>> +{
>> +	int ret;
>> +	int pidfd;
>> +	int status;
>> +	pid_t pid;
>> +	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);
>> +
>> +		if (!memory_is_mapped(pid, *mem_addr, *mem_addr + mem_size)) {
>> +			tst_res(TFAIL, "Memory is not mapped");
>> +			break;
>> +		}
>> +
>> +		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;
> Does this even happen? I suppose that until the child has been waited
> for you shouldn't get ESRCH at all. The memory may be freed
> asynchronously but the pidfd is valid until we do waitpid, at least
> that's what the description says:
>
> https://lore.kernel.org/linux-mm/20210902220029.bfau3YxNP%25akpm@linux-foundation.org/
>
> But selftest seems to do the same loop on ESRCH so either the test or
> the documentation is wrong.
>
> Michal any idea which is correct?
>
>> +			} else {
>> +				tst_res(TFAIL | TERRNO, "process_mrelease(%d,0) error", pidfd);
>> +			}
>> +		} else {
>> +			int timeout_ms = 1000;
>> +
>> +			tst_res(TPASS, "process_mrelease(%d,0) passed", pidfd);
>> +
>> +			while (memory_is_mapped(pid, *mem_addr, *mem_addr + mem_size) &&
>> +				timeout_ms--)
>> +				usleep(1000);
>> +
>> +			if (memory_is_mapped(pid, *mem_addr, *mem_addr + mem_size))
>> +				tst_res(TFAIL, "Memory is still mapped inside child memory");
>> +			else
>> +				tst_res(TPASS, "Memory has been released");
> As far as I understand this this will likely pass even without the
> process_mrelease() call since the process address space is being teared
> down anyways. But I do not have an idea how to make things better. I
> guess that if we wanted to know for sure we would have to run some
> complex statistics with and without the syscall and compare the
> timings...
I don't know, I tried to port the kselftest that seemed to be 
reasonable. Let me know if this is still good, otherwise we need to 
change the whole algorithm. But honestly I don't see many other options 
than the current one.
>
>> +		}
>> +
>> +		SAFE_WAITPID(-1, &status, 0);
>> +		SAFE_CLOSE(pidfd);
>> +
>> +		if (!restart)
>> +			break;
>> +	}
>> +}
>> +
>> +static void setup(void)
>> +{
>> +	mem_addr = SAFE_MMAP(NULL,
>> +		sizeof(unsigned long),
>> +		PROT_READ | PROT_WRITE,
>> +		MAP_SHARED | MAP_ANON,
>> +		0, 0);
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> +	if (mem_addr)
>> +		SAFE_MUNMAP(mem_addr, sizeof(unsigned long));
>> +}
>> +
>> +static struct tst_test test = {
>> +	.test_all = run,
>> +	.setup = setup,
>> +	.cleanup = cleanup,
>> +	.needs_root = 1,
>> +	.forks_child = 1,
>> +	.min_kver = "5.15",
>> +	.needs_checkpoints = 1,
>> +};
>>
>> -- 
>> 2.43.0
>>
>>
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp
Andrea

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

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

[Cyril Hrubis]

Hi!
> At the end of each test() the library calls tst_reap_children() and the 
> waiting child will fail with ETIMEDOUT, so we need to spawn a child for 
> each new test.

Ah, right, we do not support run-away children like this.

-- 
Cyril Hrubis
chrubis@suse.cz

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

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

[Andrea Cervesato via ltp]

Ping @Cyril

On 10/10/24 14:33, Andrea Cervesato wrote:
> Hi!
>
> On 9/12/24 18:28, Cyril Hrubis wrote:
>> Hi!
>>> +static void run(void)
>>> +{
>>> +    int ret;
>>> +    int pidfd;
>>> +    int status;
>>> +    pid_t pid;
>>> +    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);
>>> +
>>> +        if (!memory_is_mapped(pid, *mem_addr, *mem_addr + mem_size)) {
>>> +            tst_res(TFAIL, "Memory is not mapped");
>>> +            break;
>>> +        }
>>> +
>>> +        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;
>> Does this even happen? I suppose that until the child has been waited
>> for you shouldn't get ESRCH at all. The memory may be freed
>> asynchronously but the pidfd is valid until we do waitpid, at least
>> that's what the description says:
>>
>> https://lore.kernel.org/linux-mm/20210902220029.bfau3YxNP%25akpm@linux-foundation.org/ 
>>
>>
>> But selftest seems to do the same loop on ESRCH so either the test or
>> the documentation is wrong.
>>
>> Michal any idea which is correct?
>>
>>> +            } else {
>>> +                tst_res(TFAIL | TERRNO, "process_mrelease(%d,0) 
>>> error", pidfd);
>>> +            }
>>> +        } else {
>>> +            int timeout_ms = 1000;
>>> +
>>> +            tst_res(TPASS, "process_mrelease(%d,0) passed", pidfd);
>>> +
>>> +            while (memory_is_mapped(pid, *mem_addr, *mem_addr + 
>>> mem_size) &&
>>> +                timeout_ms--)
>>> +                usleep(1000);
>>> +
>>> +            if (memory_is_mapped(pid, *mem_addr, *mem_addr + 
>>> mem_size))
>>> +                tst_res(TFAIL, "Memory is still mapped inside child 
>>> memory");
>>> +            else
>>> +                tst_res(TPASS, "Memory has been released");
>> As far as I understand this this will likely pass even without the
>> process_mrelease() call since the process address space is being teared
>> down anyways. But I do not have an idea how to make things better. I
>> guess that if we wanted to know for sure we would have to run some
>> complex statistics with and without the syscall and compare the
>> timings...
> I don't know, I tried to port the kselftest that seemed to be 
> reasonable. Let me know if this is still good, otherwise we need to 
> change the whole algorithm. But honestly I don't see many other 
> options than the current one.
>>
>>> +        }
>>> +
>>> +        SAFE_WAITPID(-1, &status, 0);
>>> +        SAFE_CLOSE(pidfd);
>>> +
>>> +        if (!restart)
>>> +            break;
>>> +    }
>>> +}
>>> +
>>> +static void setup(void)
>>> +{
>>> +    mem_addr = SAFE_MMAP(NULL,
>>> +        sizeof(unsigned long),
>>> +        PROT_READ | PROT_WRITE,
>>> +        MAP_SHARED | MAP_ANON,
>>> +        0, 0);
>>> +}
>>> +
>>> +static void cleanup(void)
>>> +{
>>> +    if (mem_addr)
>>> +        SAFE_MUNMAP(mem_addr, sizeof(unsigned long));
>>> +}
>>> +
>>> +static struct tst_test test = {
>>> +    .test_all = run,
>>> +    .setup = setup,
>>> +    .cleanup = cleanup,
>>> +    .needs_root = 1,
>>> +    .forks_child = 1,
>>> +    .min_kver = "5.15",
>>> +    .needs_checkpoints = 1,
>>> +};
>>>
>>> -- 
>>> 2.43.0
>>>
>>>
>>> -- 
>>> Mailing list info: https://lists.linux.it/listinfo/ltp
> Andrea

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

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

[Wei Gao via ltp]

On Mon, Aug 12, 2024 at 01:46:30PM +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 | 84 ++++++++++++++++++++++
>  3 files changed, 86 insertions(+)
> 
> diff --git a/runtest/syscalls b/runtest/syscalls
> index de90e9ba3..f3cb7d465 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1074,6 +1074,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..ced556243
> --- /dev/null
> +++ b/testcases/kernel/syscalls/process_mrelease/process_mrelease02.c
> @@ -0,0 +1,84 @@
> +// 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
> + * * ESRCH when child has been closed
> + */
> +
> +#include "tst_test.h"
> +#include "lapi/syscalls.h"
> +
> +static int badfd = -1;
> +static int pidfd;
> +
> +enum {
> +	NO_CHILD,
> +	EXIT_CHILD,
> +	WAIT_CHILD,
> +};
> +
> +static struct tcase {
> +	int child_type;
> +	int *fd;
> +	int flags;
> +	int exp_errno;
> +	char *msg;
> +} tcases[] = {
> +	{NO_CHILD, &badfd, 0, EBADF, "bad file descriptor"},
> +	{WAIT_CHILD, &pidfd, -1, EINVAL, "flags is not 0"},
> +	{WAIT_CHILD, &pidfd, 0, EINVAL, "task memory cannot be released"},
> +	{EXIT_CHILD, &pidfd, 0, ESRCH, "child is not running"},
> +};
> +
> +static void run(unsigned int n)
> +{
> +	struct tcase *tc = &tcases[n];
> +	int status;
> +
> +	if (tc->child_type != NO_CHILD) {
> +		pid_t pid;
> +
> +		pid = SAFE_FORK();
> +		if (!pid) {
> +			if (tc->child_type == WAIT_CHILD)
> +				TST_CHECKPOINT_WAIT(0);
> +
> +			exit(0);
> +		}
> +
> +		tst_res(TINFO, "Spawned waiting child with pid=%d", pid);
> +
> +		pidfd = SAFE_PIDFD_OPEN(pid, 0);
> +
> +		if (tc->child_type == EXIT_CHILD)
> +			SAFE_WAITPID(pid, &status, 0);
> +	}
> +
> +	TST_EXP_FAIL(tst_syscall(__NR_process_mrelease, *tc->fd, tc->flags),
> +		tc->exp_errno,
> +		"%s", tc->msg);
> +
> +	if (tc->child_type != NO_CHILD) {
> +		if (tc->child_type == WAIT_CHILD)
> +			TST_CHECKPOINT_WAKE(0);
> +
> +		SAFE_CLOSE(pidfd);
> +	}
> +}
> +
> +static struct tst_test test = {
> +	.test = run,
> +	.tcnt = ARRAY_SIZE(tcases),
> +	.needs_root = 1,
> +	.forks_child = 1,
> +	.min_kver = "5.15",

I suggest also using syscall check process_mrelease support or not like
kernel selftest. Such as:
        if (!syscall(__NR_process_mrelease, -1, 0) || errno != EBADF) {
                if (errno == ENOSYS) {
                        ksft_test_result_skip("process_mrelease not implemented\n");
                        ksft_finished();

> +	.needs_checkpoints = 1,
> +};
> 
> -- 
> 2.43.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

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

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

[Wei Gao via ltp]

On Thu, Oct 10, 2024 at 02:33:09PM +0200, Andrea Cervesato via ltp wrote:
> Hi!
> 
> On 9/12/24 18:28, Cyril Hrubis wrote:
> > Hi!
> > > +static void run(void)
> > > +{
> > > +	int ret;
> > > +	int pidfd;
> > > +	int status;
> > > +	pid_t pid;
> > > +	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);
> > > +
> > > +		if (!memory_is_mapped(pid, *mem_addr, *mem_addr + mem_size)) {
> > > +			tst_res(TFAIL, "Memory is not mapped");
> > > +			break;
> > > +		}
> > > +
> > > +		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;
> > Does this even happen? I suppose that until the child has been waited
> > for you shouldn't get ESRCH at all. The memory may be freed
> > asynchronously but the pidfd is valid until we do waitpid, at least
> > that's what the description says:
> > 
> > https://lore.kernel.org/linux-mm/20210902220029.bfau3YxNP%25akpm@linux-foundation.org/
> > 
> > But selftest seems to do the same loop on ESRCH so either the test or
> > the documentation is wrong.
If you add extra sleep between SAFE_KILL(pid, SIGKILL) and tst_syscall
you will get ESRCH, so i guess child process has chance to be fully
reaped before process_mrelease syscall.
> > 
> > Michal any idea which is correct?
> > 
> > > +			} else {
> > > +				tst_res(TFAIL | TERRNO, "process_mrelease(%d,0) error", pidfd);
> > > +			}
> > > +		} else {
> > > +			int timeout_ms = 1000;
> > > +
> > > +			tst_res(TPASS, "process_mrelease(%d,0) passed", pidfd);
> > > +
> > > +			while (memory_is_mapped(pid, *mem_addr, *mem_addr + mem_size) &&
> > > +				timeout_ms--)
> > > +				usleep(1000);
> > > +
> > > +			if (memory_is_mapped(pid, *mem_addr, *mem_addr + mem_size))
> > > +				tst_res(TFAIL, "Memory is still mapped inside child memory");
> > > +			else
> > > +				tst_res(TPASS, "Memory has been released");
> > As far as I understand this this will likely pass even without the
> > process_mrelease() call since the process address space is being teared
> > down anyways. But I do not have an idea how to make things better. I
> > guess that if we wanted to know for sure we would have to run some
> > complex statistics with and without the syscall and compare the
> > timings...
> I don't know, I tried to port the kselftest that seemed to be reasonable.
> Let me know if this is still good, otherwise we need to change the whole
> algorithm. But honestly I don't see many other options than the current one.
kselftest does not has this memory check, i have done some test in my
env, this memory check can pass even without the process_mrelease, so i
think we do not need this check.
> > 
> > > +		}
> > > +
> > > +		SAFE_WAITPID(-1, &status, 0);
> > > +		SAFE_CLOSE(pidfd);
> > > +
> > > +		if (!restart)
> > > +			break;
> > > +	}
> > > +}
> > > +
> > > +static void setup(void)
> > > +{
> > > +	mem_addr = SAFE_MMAP(NULL,
> > > +		sizeof(unsigned long),
> > > +		PROT_READ | PROT_WRITE,
> > > +		MAP_SHARED | MAP_ANON,
> > > +		0, 0);
> > > +}
> > > +
> > > +static void cleanup(void)
> > > +{
> > > +	if (mem_addr)
> > > +		SAFE_MUNMAP(mem_addr, sizeof(unsigned long));
> > > +}
> > > +
> > > +static struct tst_test test = {
> > > +	.test_all = run,
> > > +	.setup = setup,
> > > +	.cleanup = cleanup,
> > > +	.needs_root = 1,
> > > +	.forks_child = 1,
> > > +	.min_kver = "5.15",
> > > +	.needs_checkpoints = 1,
> > > +};
> > > 
> > > -- 
> > > 2.43.0
> > > 
> > > 
> > > -- 
> > > Mailing list info: https://lists.linux.it/listinfo/ltp
> Andrea
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

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