| Message ID | 20221006110642.12410-5-andrea.cervesato@suse.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | Add process_madvise support | expand |
Hello, Andrea Cervesato via ltp <ltp@lists.linux.it> writes: > Test for checking MADV_PAGEOUT functionality over memory-mapped file > in process_madvise syscall. So this one doesn't need swap, but it has some other issues. > > Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> > --- > testcases/kernel/syscalls/cma/.gitignore | 1 + > .../kernel/syscalls/cma/process_madvise03.c | 139 ++++++++++++++++++ > 2 files changed, 140 insertions(+) > create mode 100644 testcases/kernel/syscalls/cma/process_madvise03.c > > diff --git a/testcases/kernel/syscalls/cma/.gitignore b/testcases/kernel/syscalls/cma/.gitignore > index 47ae3e445..147b03c48 100644 > --- a/testcases/kernel/syscalls/cma/.gitignore > +++ b/testcases/kernel/syscalls/cma/.gitignore > @@ -4,3 +4,4 @@ > /process_vm_writev02 > /process_madvise01 > /process_madvise02 > +/process_madvise03 > diff --git a/testcases/kernel/syscalls/cma/process_madvise03.c b/testcases/kernel/syscalls/cma/process_madvise03.c > new file mode 100644 > index 000000000..3f12ef530 > --- /dev/null > +++ b/testcases/kernel/syscalls/cma/process_madvise03.c > @@ -0,0 +1,139 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com> > + */ > + > +/*\ > + * [Description] > + * > + * Spawn child inside cgroup and set max memory. Allocate file-backed memory > + * pages inside child and reclaim it with MADV_PAGEOUT. Then check if memory > + * pages have been written back to the backing storage. > + * > + * The advice might be ignored for some pages in the range when it is > + * not applicable, so test passes if pages mapped in RAM decrease after > + * reclaiming memory with MADV_PAGEOUT and RAM doesn't contain > + * reclaimed memory anymore. > + */ > + > +#define _GNU_SOURCE > + > +#include <sys/mman.h> > +#include "tst_test.h" > +#include "lapi/mmap.h" > +#include "lapi/syscalls.h" > +#include "cma.h" > + > +#define MEM_CHILD (10 * 1024 * 1024) > + > +static char *filename = "file.bin"; > +static void **data_ptr; > + > +static void child_alloc(void) > +{ > + int fd; > + char *ptr; > + int freed = 1; > + struct addr_mapping map_before; > + struct addr_mapping map_after; > + > + tst_res(TINFO, "Allocate file-backed memory"); > + > + fd = SAFE_OPEN(filename, O_CREAT | O_RDWR); > + SAFE_FTRUNCATE(fd, MEM_CHILD); > + > + *data_ptr = SAFE_MMAP(NULL, MEM_CHILD, > + PROT_READ | PROT_WRITE, > + MAP_SHARED, fd, 0); > + > + tst_res(TINFO, "Dirty memory"); > + memset(*data_ptr, 'a', MEM_CHILD); > + > + read_address_mapping((unsigned long)*data_ptr, &map_before); > + > + TST_CHECKPOINT_WAKE_AND_WAIT(0); > + > + for (ptr = *data_ptr; *ptr != '\0'; ptr++) { > + if (*ptr == 'a') { > + freed = 0; > + break; > + } > + } This will loop once, or? > + > + if (freed) { > + tst_res(TFAIL, "Memory has been freed"); We'll probably get a segfault or sigbus if its unmapped somehow. I guess you could do a memcmp on the range to test that it didn't randomly change though. Otherwise this one looks good.
Richard Palethorpe <rpalethorpe@suse.de> writes: > Hello, > > Andrea Cervesato via ltp <ltp@lists.linux.it> writes: > >> Test for checking MADV_PAGEOUT functionality over memory-mapped file >> in process_madvise syscall. > > So this one doesn't need swap, but it has some other issues. > >> >> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> >> --- >> testcases/kernel/syscalls/cma/.gitignore | 1 + >> .../kernel/syscalls/cma/process_madvise03.c | 139 ++++++++++++++++++ >> 2 files changed, 140 insertions(+) >> create mode 100644 testcases/kernel/syscalls/cma/process_madvise03.c >> >> diff --git a/testcases/kernel/syscalls/cma/.gitignore b/testcases/kernel/syscalls/cma/.gitignore >> index 47ae3e445..147b03c48 100644 >> --- a/testcases/kernel/syscalls/cma/.gitignore >> +++ b/testcases/kernel/syscalls/cma/.gitignore >> @@ -4,3 +4,4 @@ >> /process_vm_writev02 >> /process_madvise01 >> /process_madvise02 >> +/process_madvise03 >> diff --git a/testcases/kernel/syscalls/cma/process_madvise03.c b/testcases/kernel/syscalls/cma/process_madvise03.c >> new file mode 100644 >> index 000000000..3f12ef530 >> --- /dev/null >> +++ b/testcases/kernel/syscalls/cma/process_madvise03.c >> @@ -0,0 +1,139 @@ >> +// SPDX-License-Identifier: GPL-2.0-or-later >> +/* >> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com> >> + */ >> + >> +/*\ >> + * [Description] >> + * >> + * Spawn child inside cgroup and set max memory. Allocate file-backed memory >> + * pages inside child and reclaim it with MADV_PAGEOUT. Then check if memory >> + * pages have been written back to the backing storage. Actually, one more thing. You don't check if it has been written to the backing store and it's quite hard to check for this. At best you could reopen the file in the parent and check the contents are correct. Otherwise it requires checking the page cache has been discarded using a side channel (e.g. timing loads). So I would just not bother for this test. Also process_madvise returns the number of bytes *advised* not what was actually reclaimed. Even that is not guaranteed to be the same as the amount requested.
diff --git a/testcases/kernel/syscalls/cma/.gitignore b/testcases/kernel/syscalls/cma/.gitignore index 47ae3e445..147b03c48 100644 --- a/testcases/kernel/syscalls/cma/.gitignore +++ b/testcases/kernel/syscalls/cma/.gitignore @@ -4,3 +4,4 @@ /process_vm_writev02 /process_madvise01 /process_madvise02 +/process_madvise03 diff --git a/testcases/kernel/syscalls/cma/process_madvise03.c b/testcases/kernel/syscalls/cma/process_madvise03.c new file mode 100644 index 000000000..3f12ef530 --- /dev/null +++ b/testcases/kernel/syscalls/cma/process_madvise03.c @@ -0,0 +1,139 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com> + */ + +/*\ + * [Description] + * + * Spawn child inside cgroup and set max memory. Allocate file-backed memory + * pages inside child and reclaim it with MADV_PAGEOUT. Then check if memory + * pages have been written back to the backing storage. + * + * The advice might be ignored for some pages in the range when it is + * not applicable, so test passes if pages mapped in RAM decrease after + * reclaiming memory with MADV_PAGEOUT and RAM doesn't contain + * reclaimed memory anymore. + */ + +#define _GNU_SOURCE + +#include <sys/mman.h> +#include "tst_test.h" +#include "lapi/mmap.h" +#include "lapi/syscalls.h" +#include "cma.h" + +#define MEM_CHILD (10 * 1024 * 1024) + +static char *filename = "file.bin"; +static void **data_ptr; + +static void child_alloc(void) +{ + int fd; + char *ptr; + int freed = 1; + struct addr_mapping map_before; + struct addr_mapping map_after; + + tst_res(TINFO, "Allocate file-backed memory"); + + fd = SAFE_OPEN(filename, O_CREAT | O_RDWR); + SAFE_FTRUNCATE(fd, MEM_CHILD); + + *data_ptr = SAFE_MMAP(NULL, MEM_CHILD, + PROT_READ | PROT_WRITE, + MAP_SHARED, fd, 0); + + tst_res(TINFO, "Dirty memory"); + memset(*data_ptr, 'a', MEM_CHILD); + + read_address_mapping((unsigned long)*data_ptr, &map_before); + + TST_CHECKPOINT_WAKE_AND_WAIT(0); + + for (ptr = *data_ptr; *ptr != '\0'; ptr++) { + if (*ptr == 'a') { + freed = 0; + break; + } + } + + if (freed) { + tst_res(TFAIL, "Memory has been freed"); + return; + } + + read_address_mapping((unsigned long)*data_ptr, &map_after); + + SAFE_MUNMAP(*data_ptr, MEM_CHILD); + *data_ptr = NULL; + + SAFE_CLOSE(fd); + + if (map_before.rss > map_after.rss) + tst_res(TPASS, "Memory has been reclaimed"); + else + tst_res(TFAIL, "RAM has increased"); +} + +static void setup(void) +{ + data_ptr = SAFE_MMAP(NULL, sizeof(void *), + PROT_READ | PROT_WRITE, + MAP_SHARED | MAP_ANONYMOUS, -1, 0); +} + +static void cleanup(void) +{ + if (*data_ptr) + SAFE_MUNMAP(*data_ptr, MEM_CHILD); + + if (data_ptr) + SAFE_MUNMAP(data_ptr, sizeof(void *)); +} + +static void run(void) +{ + int ret; + int pidfd; + pid_t pid_alloc; + struct iovec vec; + + pid_alloc = SAFE_FORK(); + if (!pid_alloc) { + child_alloc(); + return; + } + + TST_CHECKPOINT_WAIT(0); + + tst_res(TINFO, "Apply MADV_PAGEOUT advise rule"); + + pidfd = SAFE_PIDFD_OPEN(pid_alloc, 0); + + vec.iov_base = *data_ptr; + vec.iov_len = MEM_CHILD; + + ret = tst_syscall(__NR_process_madvise, pidfd, &vec, 1UL, + MADV_PAGEOUT, 0UL); + + if (ret == -1) + tst_brk(TBROK | TERRNO, "process_madvise failed"); + + if (ret != MEM_CHILD) + tst_brk(TBROK, "process_madvise reclaimed only %d bytes", ret); + + TST_CHECKPOINT_WAKE(0); +} + +static struct tst_test test = { + .setup = setup, + .cleanup = cleanup, + .test_all = run, + .forks_child = 1, + .min_kver = "5.10", + .needs_tmpdir = 1, + .needs_checkpoints = 1, +};
Test for checking MADV_PAGEOUT functionality over memory-mapped file in process_madvise syscall. Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> --- testcases/kernel/syscalls/cma/.gitignore | 1 + .../kernel/syscalls/cma/process_madvise03.c | 139 ++++++++++++++++++ 2 files changed, 140 insertions(+) create mode 100644 testcases/kernel/syscalls/cma/process_madvise03.c