| Message ID | 20240509133712.3383293-1-fstornio@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v4] syscalls/mlock05: add mlock test for locking and pre-faulting of memory | expand |
Hi! > +static unsigned long get_proc_smaps_field(unsigned long desired_mapping_address, char *desired_field) > +{ > + bool mapping_found = false; > + char buffer[LINELEN] = ""; > + FILE *file = NULL; > + int ret = 0; There is no point in initializing buffer, file and ret these are written to before we use the value. > + file = fopen("/proc/self/smaps", "r"); > + if (file == NULL) { > + tst_brk(TBROK | TERRNO, "cannot open file proc/self/smaps"); > + return 0; > + } Use SAFE_FOPEN() please. > + // find desired mapping > + while (fgets(buffer, LINELEN, file) != NULL) { > + unsigned long mapping_address; > + > + // check the starting address I do not find this comment to be useful, we do have a rule in LTP not to comment obvious and I would argue that this comment does so. > + ret = sscanf(buffer, "%lx[^-]", &mapping_address); > + > + if ((ret == 1) && (mapping_address == desired_mapping_address)) { > + mapping_found = true; > + break; > + } > + } > + > + if (!mapping_found) { > + fclose(file); SAFE_FCLOSE() please and in the rest of the cases below. > + tst_brk(TBROK, "cannot find mapping %lx in proc/self/smaps", desired_mapping_address); > + return 0; > + } > + > + // find desired field > + while (fgets(buffer, LINELEN, file) != NULL) { > + if (strstr(buffer, desired_field) != NULL) { Can we use strncmp() to match the prefix rather a substring? At the moment "Locked" and "Rss" are not substrings of other keys in that file but that may change at some point. > + unsigned long desired_value; > + > + // extract the value for the requested field Here as well, commenting the obvious. > + ret = sscanf(buffer, "%*[^0-9]%lu%*[^0-9]", &desired_value); The key value is divided by : and thge entries we are interested in are in kB so "%*[^:]%lu kB" should be a bit safer. > + fclose(file); > + > + if (ret != 1) { > + tst_brk(TBROK, "failure occured while reading field %s", desired_field); > + return 0; > + } > + > + return desired_value; > + } > + } > + > + fclose(file); > + tst_brk(TBROK, "cannot find %s field", desired_field); > + > + return 0; > +} > + > +static void verify_mlock(void) > +{ > + unsigned long Locked; > + unsigned long Rss; > + char *buf; > + > + buf = SAFE_MMAP(NULL, MMAPLEN, PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); > + SAFE_MLOCK(buf, MMAPLEN); > + > + Rss = get_proc_smaps_field((unsigned long)buf, "Rss"); > + Locked = get_proc_smaps_field((unsigned long)buf, "Locked"); Hmm, we do parse the whole file twice here, just the get to a nearly same line. Why can't we just point two pointrs to unsigned long to the function and collect both while we read the file? > + // Convertion from KiB to B > + Rss *= 1024; > + Locked *= 1024; > + > + TST_EXP_EQ_LU(Rss, MMAPLEN); > + TST_EXP_EQ_LU(Locked, MMAPLEN); > + > + SAFE_MUNLOCK(buf, MMAPLEN); > + SAFE_MUNMAP(buf, MMAPLEN); > +} > + > +static struct tst_test test = { > + .test_all = verify_mlock, > +};
diff --git a/runtest/syscalls b/runtest/syscalls index 252123d8b..05a52fc8f 100644 --- a/runtest/syscalls +++ b/runtest/syscalls @@ -781,6 +781,7 @@ mlock01 mlock01 mlock02 mlock02 mlock03 mlock03 -i 20 mlock04 mlock04 +mlock05 mlock05 mlock201 mlock201 mlock202 mlock202 diff --git a/testcases/kernel/syscalls/mlock/.gitignore b/testcases/kernel/syscalls/mlock/.gitignore index 306574bbc..1872229b8 100644 --- a/testcases/kernel/syscalls/mlock/.gitignore +++ b/testcases/kernel/syscalls/mlock/.gitignore @@ -2,3 +2,4 @@ /mlock02 /mlock03 /mlock04 +/mlock05 diff --git a/testcases/kernel/syscalls/mlock/mlock05.c b/testcases/kernel/syscalls/mlock/mlock05.c new file mode 100644 index 000000000..4b131d0dd --- /dev/null +++ b/testcases/kernel/syscalls/mlock/mlock05.c @@ -0,0 +1,103 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright Red Hat + * Author: Filippo Storniolo <fstornio@redhat.com> + */ + +/*\ + * [Description] + * + * Verify mlock() causes pre-faulting of PTEs and prevent memory to be swapped out. + * + * Find the new mapping in /proc/$pid/smaps and check Rss and Locked fields after + * mlock syscall: + * Rss and Locked size should be equal to the size of the memory allocation + */ + +#include "tst_test.h" + +#define MMAPLEN (1UL<<20) +#define LINELEN 256 + +static unsigned long get_proc_smaps_field(unsigned long desired_mapping_address, char *desired_field) +{ + bool mapping_found = false; + char buffer[LINELEN] = ""; + FILE *file = NULL; + int ret = 0; + + file = fopen("/proc/self/smaps", "r"); + if (file == NULL) { + tst_brk(TBROK | TERRNO, "cannot open file proc/self/smaps"); + return 0; + } + + // find desired mapping + while (fgets(buffer, LINELEN, file) != NULL) { + unsigned long mapping_address; + + // check the starting address + ret = sscanf(buffer, "%lx[^-]", &mapping_address); + + if ((ret == 1) && (mapping_address == desired_mapping_address)) { + mapping_found = true; + break; + } + } + + if (!mapping_found) { + fclose(file); + tst_brk(TBROK, "cannot find mapping %lx in proc/self/smaps", desired_mapping_address); + return 0; + } + + // find desired field + while (fgets(buffer, LINELEN, file) != NULL) { + if (strstr(buffer, desired_field) != NULL) { + unsigned long desired_value; + + // extract the value for the requested field + ret = sscanf(buffer, "%*[^0-9]%lu%*[^0-9]", &desired_value); + fclose(file); + + if (ret != 1) { + tst_brk(TBROK, "failure occured while reading field %s", desired_field); + return 0; + } + + return desired_value; + } + } + + fclose(file); + tst_brk(TBROK, "cannot find %s field", desired_field); + + return 0; +} + +static void verify_mlock(void) +{ + unsigned long Locked; + unsigned long Rss; + char *buf; + + buf = SAFE_MMAP(NULL, MMAPLEN, PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + SAFE_MLOCK(buf, MMAPLEN); + + Rss = get_proc_smaps_field((unsigned long)buf, "Rss"); + Locked = get_proc_smaps_field((unsigned long)buf, "Locked"); + + // Convertion from KiB to B + Rss *= 1024; + Locked *= 1024; + + TST_EXP_EQ_LU(Rss, MMAPLEN); + TST_EXP_EQ_LU(Locked, MMAPLEN); + + SAFE_MUNLOCK(buf, MMAPLEN); + SAFE_MUNMAP(buf, MMAPLEN); +} + +static struct tst_test test = { + .test_all = verify_mlock, +};
check Rss and Locked variables from /proc/$pid/smaps of the the new memory mapping. Rss and Locked size should be equal to the size of the memory allocation. Co-developed-by: Dennis Brendel <dbrendel@redhat.com> Signed-off-by: Filippo Storniolo <fstornio@redhat.com> --- runtest/syscalls | 1 + testcases/kernel/syscalls/mlock/.gitignore | 1 + testcases/kernel/syscalls/mlock/mlock05.c | 103 +++++++++++++++++++++ 3 files changed, 105 insertions(+) create mode 100644 testcases/kernel/syscalls/mlock/mlock05.c