diff mbox series

[v2,2/3] Add process_mrelease01 test

Message ID 20240812-process_mrelease-v2-2-e61249986a0a@suse.com
State Changes Requested
Headers show
Series Add process_mrelease testing suite | expand

Commit Message

Andrea Cervesato Aug. 12, 2024, 11:46 a.m. UTC
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(+)

Comments

Cyril Hrubis Sept. 12, 2024, 4:28 p.m. UTC | #1
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
diff mbox series

Patch

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,
+};