| Message ID | 20240812-process_mrelease-v2-2-e61249986a0a@suse.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add process_mrelease testing suite | expand |
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
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
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, +};