LinuxLists.cc - [PATCH v2 00/12] conform tests to TAP format output

2024-02-02 11:31:39

Subject: [PATCH v2 00/12] conform tests to TAP format output

Changes since v1:
- Rebased the series on top of next-20240202

Muhammad Usama Anjum (12):
selftests/mm: map_fixed_noreplace: conform test to TAP format output
selftests/mm: map_hugetlb: conform test to TAP format output
selftests/mm: map_populate: conform test to TAP format output
selftests/mm: mlock-random-test: conform test to TAP format output
selftests/mm: mlock2-tests: conform test to TAP format output
selftests/mm: mrelease_test: conform test to TAP format output
selftests/mm: mremap_dontunmap: conform test to TAP format output
selftests/mm: split_huge_page_test: conform test to TAP format output
selftests/mm: thp_settings: conform to TAP format output
selftests/mm: thuge-gen: conform to TAP format output
selftests/mm: transhuge-stress: conform to TAP format output
selftests/mm: virtual_address_range: conform to TAP format output

tools/testing/selftests/mm/khugepaged.c | 3 +-
.../selftests/mm/map_fixed_noreplace.c | 96 ++----
tools/testing/selftests/mm/map_hugetlb.c | 42 ++-
tools/testing/selftests/mm/map_populate.c | 37 ++-
.../testing/selftests/mm/mlock-random-test.c | 136 ++++-----
tools/testing/selftests/mm/mlock2-tests.c | 282 +++++++-----------
tools/testing/selftests/mm/mlock2.h | 11 +-
tools/testing/selftests/mm/mrelease_test.c | 80 ++---
tools/testing/selftests/mm/mremap_dontunmap.c | 32 +-
.../selftests/mm/split_huge_page_test.c | 161 +++++-----
tools/testing/selftests/mm/thp_settings.c | 123 +++-----
tools/testing/selftests/mm/thp_settings.h | 4 +-
tools/testing/selftests/mm/thuge-gen.c | 147 ++++-----
tools/testing/selftests/mm/transhuge-stress.c | 36 ++-
.../selftests/mm/virtual_address_range.c | 44 +--
tools/testing/selftests/mm/vm_util.c | 6 +-
16 files changed, 537 insertions(+), 703 deletions(-)

--
2.42.0

2024-02-02 11:31:51

by Muhammad Usama Anjum

[permalink] [raw]

Subject: [PATCH v2 01/12] selftests/mm: map_fixed_noreplace: conform test to TAP format output

Conform the layout, informational and status messages to TAP. No
functional change is intended other than the layout of output messages.
While at it, convert commenting style from // to /**/.

Signed-off-by: Muhammad Usama Anjum <[email protected]>
---
.../selftests/mm/map_fixed_noreplace.c | 96 ++++++-------------
1 file changed, 31 insertions(+), 65 deletions(-)

diff --git a/tools/testing/selftests/mm/map_fixed_noreplace.c b/tools/testing/selftests/mm/map_fixed_noreplace.c
index 598159f3df1f2..b74813fdc9514 100644
--- a/tools/testing/selftests/mm/map_fixed_noreplace.c
+++ b/tools/testing/selftests/mm/map_fixed_noreplace.c
@@ -12,6 +12,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
+#include "../kselftest.h"

static void dump_maps(void)
{
@@ -28,15 +29,12 @@ static unsigned long find_base_addr(unsigned long size)

flags = MAP_PRIVATE | MAP_ANONYMOUS;
addr = mmap(NULL, size, PROT_NONE, flags, -1, 0);
- if (addr == MAP_FAILED) {
- printf("Error: couldn't map the space we need for the test\n");
- return 0;
- }
+ if (addr == MAP_FAILED)
+ ksft_exit_fail_msg("Error: couldn't map the space we need for the test\n");
+
+ if (munmap(addr, size) != 0)
+ ksft_exit_fail_msg("Error: munmap failed\n");

- if (munmap(addr, size) != 0) {
- printf("Error: couldn't map the space we need for the test\n");
- return 0;
- }
return (unsigned long)addr;
}

@@ -46,51 +44,39 @@ int main(void)
unsigned long flags, addr, size, page_size;
char *p;

+ ksft_print_header();
+ ksft_set_plan(9);
+
page_size = sysconf(_SC_PAGE_SIZE);

- //let's find a base addr that is free before we start the tests
+ /* let's find a base addr that is free before we start the tests */
size = 5 * page_size;
base_addr = find_base_addr(size);
- if (!base_addr) {
- printf("Error: couldn't map the space we need for the test\n");
- return 1;
- }

flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED_NOREPLACE;

- // Check we can map all the areas we need below
- errno = 0;
+ /* Check we can map all the areas we need below */
addr = base_addr;
size = 5 * page_size;
p = mmap((void *)addr, size, PROT_NONE, flags, -1, 0);
-
- printf("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
-
if (p == MAP_FAILED) {
dump_maps();
- printf("Error: couldn't map the space we need for the test\n");
- return 1;
+ ksft_exit_fail_msg("Error: couldn't map the space we need for the test\n");
}
-
- errno = 0;
if (munmap((void *)addr, 5 * page_size) != 0) {
dump_maps();
- printf("Error: munmap failed!?\n");
- return 1;
+ ksft_exit_fail_msg("Error: munmap failed!?\n");
}
- printf("unmap() successful\n");
+ ksft_test_result_pass("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);

- errno = 0;
addr = base_addr + page_size;
size = 3 * page_size;
p = mmap((void *)addr, size, PROT_NONE, flags, -1, 0);
- printf("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
-
if (p == MAP_FAILED) {
dump_maps();
- printf("Error: first mmap() failed unexpectedly\n");
- return 1;
+ ksft_exit_fail_msg("Error: first mmap() failed unexpectedly\n");
}
+ ksft_test_result_pass("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);

/*
* Exact same mapping again:
@@ -100,17 +86,14 @@ int main(void)
* +3 | mapped | new
* +4 | free | new
*/
- errno = 0;
addr = base_addr;
size = 5 * page_size;
p = mmap((void *)addr, size, PROT_NONE, flags, -1, 0);
- printf("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
-
if (p != MAP_FAILED) {
dump_maps();
- printf("Error:1: mmap() succeeded when it shouldn't have\n");
- return 1;
+ ksft_exit_fail_msg("Error:1: mmap() succeeded when it shouldn't have\n");
}
+ ksft_test_result_pass("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);

/*
* Second mapping contained within first:
@@ -121,17 +104,14 @@ int main(void)
* +3 | mapped |
* +4 | free |
*/
- errno = 0;
addr = base_addr + (2 * page_size);
size = page_size;
p = mmap((void *)addr, size, PROT_NONE, flags, -1, 0);
- printf("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
-
if (p != MAP_FAILED) {
dump_maps();
- printf("Error:2: mmap() succeeded when it shouldn't have\n");
- return 1;
+ ksft_exit_fail_msg("Error:2: mmap() succeeded when it shouldn't have\n");
}
+ ksft_test_result_pass("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);

/*
* Overlap end of existing mapping:
@@ -141,17 +121,14 @@ int main(void)
* +3 | mapped | new
* +4 | free | new
*/
- errno = 0;
addr = base_addr + (3 * page_size);
size = 2 * page_size;
p = mmap((void *)addr, size, PROT_NONE, flags, -1, 0);
- printf("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
-
if (p != MAP_FAILED) {
dump_maps();
- printf("Error:3: mmap() succeeded when it shouldn't have\n");
- return 1;
+ ksft_exit_fail_msg("Error:3: mmap() succeeded when it shouldn't have\n");
}
+ ksft_test_result_pass("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);

/*
* Overlap start of existing mapping:
@@ -161,17 +138,14 @@ int main(void)
* +3 | mapped |
* +4 | free |
*/
- errno = 0;
addr = base_addr;
size = 2 * page_size;
p = mmap((void *)addr, size, PROT_NONE, flags, -1, 0);
- printf("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
-
if (p != MAP_FAILED) {
dump_maps();
- printf("Error:4: mmap() succeeded when it shouldn't have\n");
- return 1;
+ ksft_exit_fail_msg("Error:4: mmap() succeeded when it shouldn't have\n");
}
+ ksft_test_result_pass("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);

/*
* Adjacent to start of existing mapping:
@@ -181,17 +155,14 @@ int main(void)
* +3 | mapped |
* +4 | free |
*/
- errno = 0;
addr = base_addr;
size = page_size;
p = mmap((void *)addr, size, PROT_NONE, flags, -1, 0);
- printf("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
-
if (p == MAP_FAILED) {
dump_maps();
- printf("Error:5: mmap() failed when it shouldn't have\n");
- return 1;
+ ksft_exit_fail_msg("Error:5: mmap() failed when it shouldn't have\n");
}
+ ksft_test_result_pass("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);

/*
* Adjacent to end of existing mapping:
@@ -201,27 +172,22 @@ int main(void)
* +3 | mapped |
* +4 | free | new
*/
- errno = 0;
addr = base_addr + (4 * page_size);
size = page_size;
p = mmap((void *)addr, size, PROT_NONE, flags, -1, 0);
- printf("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
-
if (p == MAP_FAILED) {
dump_maps();
- printf("Error:6: mmap() failed when it shouldn't have\n");
- return 1;
+ ksft_exit_fail_msg("Error:6: mmap() failed when it shouldn't have\n");
}
+ ksft_test_result_pass("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);

addr = base_addr;
size = 5 * page_size;
if (munmap((void *)addr, size) != 0) {
dump_maps();
- printf("Error: munmap failed!?\n");
- return 1;
+ ksft_exit_fail_msg("Error: munmap failed!?\n");
}
- printf("unmap() successful\n");
+ ksft_test_result_pass("Base Address unmap() successful\n");

- printf("OK\n");
- return 0;
+ ksft_finished();
}
--
2.42.0

2024-02-02 11:32:02

by Muhammad Usama Anjum

[permalink] [raw]

Subject: [PATCH v2 02/12] selftests/mm: map_hugetlb: conform test to TAP format output

2024-02-02 11:32:29

by Muhammad Usama Anjum

[permalink] [raw]

Subject: [PATCH v2 03/12] selftests/mm: map_populate: conform test to TAP format output

Conform the layout, informational and status messages to TAP. No
functional change is intended other than the layout of output messages.
Minor cleanups have also been included.

Signed-off-by: Muhammad Usama Anjum <[email protected]>
---
tools/testing/selftests/mm/map_populate.c | 37 ++++++++++++++---------
1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/mm/map_populate.c b/tools/testing/selftests/mm/map_populate.c
index 7945d07548751..5c8a53869b1bd 100644
--- a/tools/testing/selftests/mm/map_populate.c
+++ b/tools/testing/selftests/mm/map_populate.c
@@ -16,19 +16,21 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include "../kselftest.h"

#define MMAP_SZ 4096

-#define BUG_ON(condition, description) \
- do { \
- if (condition) { \
- fprintf(stderr, "[FAIL]\t%s:%d\t%s:%s\n", __func__, \
- __LINE__, (description), strerror(errno)); \
- exit(1); \
- } \
+#define BUG_ON(condition, description) \
+ do { \
+ if (condition) \
+ ksft_exit_fail_msg("[FAIL]\t%s:%d\t%s:%s\n", \
+ __func__, __LINE__, (description), \
+ strerror(errno)); \
} while (0)

-static int parent_f(int sock, unsigned long *smap, int child)
+#define TESTS_IN_CHILD 2
+
+static void parent_f(int sock, unsigned long *smap, int child)
{
int status, ret;

@@ -43,9 +45,10 @@ static int parent_f(int sock, unsigned long *smap, int child)
BUG_ON(ret <= 0, "write(sock)");

waitpid(child, &status, 0);
- BUG_ON(!WIFEXITED(status), "child in unexpected state");

- return WEXITSTATUS(status);
+ /* The ksft macros don't keep counters between processes */
+ ksft_cnt.ksft_pass = WEXITSTATUS(status);
+ ksft_cnt.ksft_fail = TESTS_IN_CHILD - WEXITSTATUS(status);
}

static int child_f(int sock, unsigned long *smap, int fd)
@@ -64,10 +67,11 @@ static int child_f(int sock, unsigned long *smap, int fd)
ret = read(sock, &buf, sizeof(int));
BUG_ON(ret <= 0, "read(sock)");

- BUG_ON(*smap == 0x22222BAD, "MAP_POPULATE didn't COW private page");
- BUG_ON(*smap != 0xdeadbabe, "mapping was corrupted");
+ ksft_test_result(*smap != 0x22222BAD, "MAP_POPULATE COW private page\n");
+ ksft_test_result(*smap == 0xdeadbabe, "The mapping state\n");

- return 0;
+ /* The ksft macros don't keep counters between processes */
+ return ksft_cnt.ksft_pass;
}

int main(int argc, char **argv)
@@ -76,6 +80,9 @@ int main(int argc, char **argv)
FILE *ftmp;
unsigned long *smap;

+ ksft_print_header();
+ ksft_set_plan(TESTS_IN_CHILD);
+
ftmp = tmpfile();
BUG_ON(!ftmp, "tmpfile()");

@@ -101,7 +108,9 @@ int main(int argc, char **argv)
ret = close(sock[0]);
BUG_ON(ret, "close()");

- return parent_f(sock[1], smap, child);
+ parent_f(sock[1], smap, child);
+
+ ksft_finished();
}

ret = close(sock[1]);
--
2.42.0

2024-02-02 11:33:19

by Muhammad Usama Anjum

[permalink] [raw]

Subject: [PATCH v2 07/12] selftests/mm: mremap_dontunmap: conform test to TAP format output

2024-02-02 11:33:58

by Muhammad Usama Anjum

[permalink] [raw]

Subject: [PATCH v2 09/12] selftests/mm: thp_settings: conform to TAP format output

Conform the layout, informational and status messages to TAP. No
functional change is intended other than the layout of output messages.

Signed-off-by: Muhammad Usama Anjum <[email protected]>
---
tools/testing/selftests/mm/khugepaged.c | 3 +-
tools/testing/selftests/mm/thp_settings.c | 123 ++++++++--------------
tools/testing/selftests/mm/thp_settings.h | 4 +-
3 files changed, 47 insertions(+), 83 deletions(-)

diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
index d51fdaee7dc6a..3f202da0867c5 100644
--- a/tools/testing/selftests/mm/khugepaged.c
+++ b/tools/testing/selftests/mm/khugepaged.c
@@ -152,8 +152,7 @@ static void get_finfo(const char *dir)
major(path_stat.st_dev), minor(path_stat.st_dev)) >= sizeof(path))
ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);

- if (read_file(path, buf, sizeof(buf)) < 0)
- ksft_exit_fail_msg("read_file(read_num): %s\n", strerror(errno));
+ read_file(path, buf, sizeof(buf));

if (strstr(buf, "DEVTYPE=disk")) {
/* Found it */
diff --git a/tools/testing/selftests/mm/thp_settings.c b/tools/testing/selftests/mm/thp_settings.c
index a4163438108ec..273a95d025285 100644
--- a/tools/testing/selftests/mm/thp_settings.c
+++ b/tools/testing/selftests/mm/thp_settings.c
@@ -5,7 +5,9 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include <errno.h>

+#include "../kselftest.h"
#include "thp_settings.h"

#define THP_SYSFS "/sys/kernel/mm/transparent_hugepage/"
@@ -42,58 +44,45 @@ static const char * const shmem_enabled_strings[] = {
NULL
};

-int read_file(const char *path, char *buf, size_t buflen)
+void read_file(const char *path, char *buf, size_t buflen)
{
int fd;
ssize_t numread;

fd = open(path, O_RDONLY);
if (fd == -1)
- return 0;
+ ksft_exit_fail_msg("%s open failed: %s\n", path, strerror(errno));

numread = read(fd, buf, buflen - 1);
if (numread < 1) {
close(fd);
- return 0;
+ ksft_exit_fail_msg("No data read\n");
}

buf[numread] = '\0';
close(fd);
-
- return (unsigned int) numread;
}

-int write_file(const char *path, const char *buf, size_t buflen)
+void write_file(const char *path, const char *buf, size_t buflen)
{
int fd;
ssize_t numwritten;

fd = open(path, O_WRONLY);
- if (fd == -1) {
- printf("open(%s)\n", path);
- exit(EXIT_FAILURE);
- return 0;
- }
+ if (fd == -1)
+ ksft_exit_fail_msg("%s open failed\n", path);

numwritten = write(fd, buf, buflen - 1);
close(fd);
- if (numwritten < 1) {
- printf("write(%s)\n", buf);
- exit(EXIT_FAILURE);
- return 0;
- }
-
- return (unsigned int) numwritten;
+ if (numwritten < 1)
+ ksft_exit_fail_msg("write failed (%s)\n", buf);
}

const unsigned long read_num(const char *path)
{
char buf[21];

- if (read_file(path, buf, sizeof(buf)) < 0) {
- perror("read_file()");
- exit(EXIT_FAILURE);
- }
+ read_file(path, buf, sizeof(buf));

return strtoul(buf, NULL, 10);
}
@@ -103,10 +92,7 @@ void write_num(const char *path, unsigned long num)
char buf[21];

sprintf(buf, "%ld", num);
- if (!write_file(path, buf, strlen(buf) + 1)) {
- perror(path);
- exit(EXIT_FAILURE);
- }
+ write_file(path, buf, strlen(buf) + 1);
}

int thp_read_string(const char *name, const char * const strings[])
@@ -117,30 +103,22 @@ int thp_read_string(const char *name, const char * const strings[])
int ret;

ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name);
- if (ret >= PATH_MAX) {
- printf("%s: Pathname is too long\n", __func__);
- exit(EXIT_FAILURE);
- }
+ if (ret >= PATH_MAX)
+ ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);

- if (!read_file(path, buf, sizeof(buf))) {
- perror(path);
- exit(EXIT_FAILURE);
- }
+ read_file(path, buf, sizeof(buf));

c = strchr(buf, '[');
- if (!c) {
- printf("%s: Parse failure\n", __func__);
- exit(EXIT_FAILURE);
- }
+ if (!c)
+ ksft_exit_fail_msg("%s: Parse failure\n", __func__);

c++;
memmove(buf, c, sizeof(buf) - (c - buf));

c = strchr(buf, ']');
- if (!c) {
- printf("%s: Parse failure\n", __func__);
- exit(EXIT_FAILURE);
- }
+ if (!c)
+ ksft_exit_fail_msg("%s: Parse failure\n", __func__);
+
*c = '\0';

ret = 0;
@@ -150,8 +128,8 @@ int thp_read_string(const char *name, const char * const strings[])
ret++;
}

- printf("Failed to parse %s\n", name);
- exit(EXIT_FAILURE);
+ ksft_exit_fail_msg("Failed to parse %s\n", name);
+ return -1;
}

void thp_write_string(const char *name, const char *val)
@@ -160,15 +138,10 @@ void thp_write_string(const char *name, const char *val)
int ret;

ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name);
- if (ret >= PATH_MAX) {
- printf("%s: Pathname is too long\n", __func__);
- exit(EXIT_FAILURE);
- }
+ if (ret >= PATH_MAX)
+ ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);

- if (!write_file(path, val, strlen(val) + 1)) {
- perror(path);
- exit(EXIT_FAILURE);
- }
+ write_file(path, val, strlen(val) + 1);
}

const unsigned long thp_read_num(const char *name)
@@ -177,10 +150,9 @@ const unsigned long thp_read_num(const char *name)
int ret;

ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name);
- if (ret >= PATH_MAX) {
- printf("%s: Pathname is too long\n", __func__);
- exit(EXIT_FAILURE);
- }
+ if (ret >= PATH_MAX)
+ ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
+
return read_num(path);
}

@@ -190,10 +162,9 @@ void thp_write_num(const char *name, unsigned long num)
int ret;

ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name);
- if (ret >= PATH_MAX) {
- printf("%s: Pathname is too long\n", __func__);
- exit(EXIT_FAILURE);
- }
+ if (ret >= PATH_MAX)
+ ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
+
write_num(path, num);
}

@@ -275,29 +246,26 @@ void thp_write_settings(struct thp_settings *settings)

struct thp_settings *thp_current_settings(void)
{
- if (!settings_index) {
- printf("Fail: No settings set");
- exit(EXIT_FAILURE);
- }
+ if (!settings_index)
+ ksft_exit_fail_msg("Fail: No settings set\n");
+
return settings_stack + settings_index - 1;
}

void thp_push_settings(struct thp_settings *settings)
{
- if (settings_index >= MAX_SETTINGS_DEPTH) {
- printf("Fail: Settings stack exceeded");
- exit(EXIT_FAILURE);
- }
+ if (settings_index >= MAX_SETTINGS_DEPTH)
+ ksft_exit_fail_msg("Fail: Settings stack exceeded\n");
+
settings_stack[settings_index++] = *settings;
thp_write_settings(thp_current_settings());
}

void thp_pop_settings(void)
{
- if (settings_index <= 0) {
- printf("Fail: Settings stack empty");
- exit(EXIT_FAILURE);
- }
+ if (settings_index <= 0)
+ ksft_exit_fail_msg("Fail: Settings stack empty\n");
+
--settings_index;
thp_write_settings(thp_current_settings());
}
@@ -335,14 +303,11 @@ unsigned long thp_supported_orders(void)
for (i = 0; i < NR_ORDERS; i++) {
ret = snprintf(path, PATH_MAX, THP_SYSFS "hugepages-%ukB/enabled",
(getpagesize() >> 10) << i);
- if (ret >= PATH_MAX) {
- printf("%s: Pathname is too long\n", __func__);
- exit(EXIT_FAILURE);
- }
+ if (ret >= PATH_MAX)
+ ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);

- ret = read_file(path, buf, sizeof(buf));
- if (ret)
- orders |= 1UL << i;
+ read_file(path, buf, sizeof(buf));
+ orders |= 1UL << i;
}

return orders;
diff --git a/tools/testing/selftests/mm/thp_settings.h b/tools/testing/selftests/mm/thp_settings.h
index 71cbff05f4c7f..04a6a7bbd08f8 100644
--- a/tools/testing/selftests/mm/thp_settings.h
+++ b/tools/testing/selftests/mm/thp_settings.h
@@ -56,8 +56,8 @@ struct thp_settings {
struct hugepages_settings hugepages[NR_ORDERS];
};

-int read_file(const char *path, char *buf, size_t buflen);
-int write_file(const char *path, const char *buf, size_t buflen);
+void read_file(const char *path, char *buf, size_t buflen);
+void write_file(const char *path, const char *buf, size_t buflen);
const unsigned long read_num(const char *path);
void write_num(const char *path, unsigned long num);

--
2.42.0

2024-02-02 11:34:15

by Muhammad Usama Anjum

[permalink] [raw]

Subject: [PATCH v2 10/12] selftests/mm: thuge-gen: conform to TAP format output

Conform the layout, informational and status messages to TAP. No
functional change is intended other than the layout of output messages.

Also remove un-needed logging which isn't enabled. Skip a hugepage size
if it has less free pages to avoid unnecessary failures. For examples,
some systems may not have 1GB hugepage free. So skip 1GB for testing in
this test instead of failing the entire test.

Signed-off-by: Muhammad Usama Anjum <[email protected]>
---
tools/testing/selftests/mm/thuge-gen.c | 147 +++++++++++++------------
1 file changed, 75 insertions(+), 72 deletions(-)

diff --git a/tools/testing/selftests/mm/thuge-gen.c b/tools/testing/selftests/mm/thuge-gen.c
index 622987f12c89a..ea7fd8fe28763 100644
--- a/tools/testing/selftests/mm/thuge-gen.c
+++ b/tools/testing/selftests/mm/thuge-gen.c
@@ -4,7 +4,7 @@
Before running this huge pages for each huge page size must have been
reserved.
For large pages beyond MAX_PAGE_ORDER (like 1GB on x86) boot options must
- be used.
+ be used. 1GB wouldn't be tested if it isn't available.
Also shmmax must be increased.
And you need to run as root to work around some weird permissions in shm.
And nothing using huge pages should run in parallel.
@@ -26,8 +26,7 @@
#include <stdarg.h>
#include <string.h>
#include "vm_util.h"
-
-#define err(x) perror(x), exit(1)
+#include "../kselftest.h"

#define MAP_HUGE_2MB (21 << MAP_HUGE_SHIFT)
#define MAP_HUGE_1GB (30 << MAP_HUGE_SHIFT)
@@ -44,11 +43,8 @@
#define SHM_HUGE_1GB (30 << SHM_HUGE_SHIFT)

#define NUM_PAGESIZES 5
-
#define NUM_PAGES 4

-#define Dprintf(fmt...) // printf(fmt)
-
unsigned long page_sizes[NUM_PAGESIZES];
int num_page_sizes;

@@ -60,28 +56,15 @@ int ilog2(unsigned long v)
return l;
}

-void find_pagesizes(void)
-{
- glob_t g;
- int i;
- glob("/sys/kernel/mm/hugepages/hugepages-*kB", 0, NULL, &g);
- assert(g.gl_pathc <= NUM_PAGESIZES);
- for (i = 0; i < g.gl_pathc; i++) {
- sscanf(g.gl_pathv[i], "/sys/kernel/mm/hugepages/hugepages-%lukB",
- &page_sizes[i]);
- page_sizes[i] <<= 10;
- printf("Found %luMB\n", page_sizes[i] >> 20);
- }
- num_page_sizes = g.gl_pathc;
- globfree(&g);
-}
-
void show(unsigned long ps)
{
char buf[100];
+
if (ps == getpagesize())
return;
- printf("%luMB: ", ps >> 20);
+
+ ksft_print_msg("%luMB: ", ps >> 20);
+
fflush(stdout);
snprintf(buf, sizeof buf,
"cat /sys/kernel/mm/hugepages/hugepages-%lukB/free_hugepages",
@@ -105,7 +88,7 @@ unsigned long read_sysfs(int warn, char *fmt, ...)
f = fopen(buf, "r");
if (!f) {
if (warn)
- printf("missing %s\n", buf);
+ ksft_print_msg("missing %s\n", buf);
return 0;
}
if (getline(&line, &linelen, f) > 0) {
@@ -119,123 +102,143 @@ unsigned long read_sysfs(int warn, char *fmt, ...)
unsigned long read_free(unsigned long ps)
{
return read_sysfs(ps != getpagesize(),
- "/sys/kernel/mm/hugepages/hugepages-%lukB/free_hugepages",
- ps >> 10);
+ "/sys/kernel/mm/hugepages/hugepages-%lukB/free_hugepages",
+ ps >> 10);
}

void test_mmap(unsigned long size, unsigned flags)
{
char *map;
unsigned long before, after;
- int err;

before = read_free(size);
map = mmap(NULL, size*NUM_PAGES, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_ANONYMOUS|MAP_HUGETLB|flags, -1, 0);
+ if (map == MAP_FAILED)
+ ksft_exit_fail_msg("mmap: %s\n", strerror(errno));

- if (map == (char *)-1) err("mmap");
memset(map, 0xff, size*NUM_PAGES);
after = read_free(size);
- Dprintf("before %lu after %lu diff %ld size %lu\n",
- before, after, before - after, size);
- assert(size == getpagesize() || (before - after) == NUM_PAGES);
+
show(size);
- err = munmap(map, size * NUM_PAGES);
- assert(!err);
+ ksft_test_result(size == getpagesize() || (before - after) == NUM_PAGES,
+ "%s mmap\n", __func__);
+
+ if (munmap(map, size * NUM_PAGES))
+ ksft_exit_fail_msg("%s: unmap %s\n", __func__, strerror(errno));
}

void test_shmget(unsigned long size, unsigned flags)
{
int id;
unsigned long before, after;
- int err;
+ struct shm_info i;
+ char *map;

before = read_free(size);
id = shmget(IPC_PRIVATE, size * NUM_PAGES, IPC_CREAT|0600|flags);
- if (id < 0) err("shmget");
-
- struct shm_info i;
- if (shmctl(id, SHM_INFO, (void *)&i) < 0) err("shmctl");
- Dprintf("alloc %lu res %lu\n", i.shm_tot, i.shm_rss);
+ if (id < 0) {
+ if (errno == EPERM) {
+ ksft_test_result_skip("shmget requires root privileges: %s\n",
+ strerror(errno));
+ return;
+ }
+ ksft_exit_fail_msg("shmget: %s\n", strerror(errno));
+ }

+ if (shmctl(id, SHM_INFO, (void *)&i) < 0)
+ ksft_exit_fail_msg("shmctl: %s\n", strerror(errno));

- Dprintf("id %d\n", id);
- char *map = shmat(id, NULL, 0600);
- if (map == (char*)-1) err("shmat");
+ map = shmat(id, NULL, 0600);
+ if (map == MAP_FAILED)
+ ksft_exit_fail_msg("shmat: %s\n", strerror(errno));

shmctl(id, IPC_RMID, NULL);

memset(map, 0xff, size*NUM_PAGES);
after = read_free(size);

- Dprintf("before %lu after %lu diff %ld size %lu\n",
- before, after, before - after, size);
- assert(size == getpagesize() || (before - after) == NUM_PAGES);
show(size);
- err = shmdt(map);
- assert(!err);
+ ksft_test_result(size == getpagesize() || (before - after) == NUM_PAGES,
+ "%s: mmap\n", __func__);
+ if (shmdt(map))
+ ksft_exit_fail_msg("%s: shmdt: %s\n", __func__, strerror(errno));
}

-void sanity_checks(void)
+void find_pagesizes(void)
{
- int i;
unsigned long largest = getpagesize();
+ int i;
+ glob_t g;

- for (i = 0; i < num_page_sizes; i++) {
- if (page_sizes[i] > largest)
+ glob("/sys/kernel/mm/hugepages/hugepages-*kB", 0, NULL, &g);
+ assert(g.gl_pathc <= NUM_PAGESIZES);
+ for (i = 0; (i < g.gl_pathc) && (num_page_sizes < NUM_PAGESIZES); i++) {
+ sscanf(g.gl_pathv[i], "/sys/kernel/mm/hugepages/hugepages-%lukB",
+ &page_sizes[num_page_sizes]);
+ page_sizes[num_page_sizes] <<= 10;
+ ksft_print_msg("Found %luMB\n", page_sizes[i] >> 20);
+
+ if (page_sizes[num_page_sizes] > largest)
largest = page_sizes[i];

- if (read_free(page_sizes[i]) < NUM_PAGES) {
- printf("Not enough huge pages for page size %lu MB, need %u\n",
- page_sizes[i] >> 20,
- NUM_PAGES);
- exit(0);
- }
+ if (read_free(page_sizes[num_page_sizes]) >= NUM_PAGES)
+ num_page_sizes++;
+ else
+ ksft_print_msg("SKIP for size %lu MB as not enough huge pages, need %u\n",
+ page_sizes[num_page_sizes] >> 20, NUM_PAGES);
}
+ globfree(&g);

- if (read_sysfs(0, "/proc/sys/kernel/shmmax") < NUM_PAGES * largest) {
- printf("Please do echo %lu > /proc/sys/kernel/shmmax", largest * NUM_PAGES);
- exit(0);
- }
+ if (read_sysfs(0, "/proc/sys/kernel/shmmax") < NUM_PAGES * largest)
+ ksft_exit_fail_msg("Please do echo %lu > /proc/sys/kernel/shmmax",
+ largest * NUM_PAGES);

#if defined(__x86_64__)
if (largest != 1U<<30) {
- printf("No GB pages available on x86-64\n"
- "Please boot with hugepagesz=1G hugepages=%d\n", NUM_PAGES);
- exit(0);
+ ksft_exit_fail_msg("No GB pages available on x86-64\n"
+ "Please boot with hugepagesz=1G hugepages=%d\n", NUM_PAGES);
}
#endif
}

int main(void)
{
- int i;
unsigned default_hps = default_huge_page_size();
+ int i;
+
+ ksft_print_header();

find_pagesizes();

- sanity_checks();
+ if (!num_page_sizes)
+ ksft_finished();
+
+ ksft_set_plan(2 * num_page_sizes + 3);

for (i = 0; i < num_page_sizes; i++) {
unsigned long ps = page_sizes[i];
int arg = ilog2(ps) << MAP_HUGE_SHIFT;
- printf("Testing %luMB mmap with shift %x\n", ps >> 20, arg);
+
+ ksft_print_msg("Testing %luMB mmap with shift %x\n", ps >> 20, arg);
test_mmap(ps, MAP_HUGETLB | arg);
}
- printf("Testing default huge mmap\n");
+
+ ksft_print_msg("Testing default huge mmap\n");
test_mmap(default_hps, MAP_HUGETLB);

- puts("Testing non-huge shmget");
+ ksft_print_msg("Testing non-huge shmget\n");
test_shmget(getpagesize(), 0);

for (i = 0; i < num_page_sizes; i++) {
unsigned long ps = page_sizes[i];
int arg = ilog2(ps) << SHM_HUGE_SHIFT;
- printf("Testing %luMB shmget with shift %x\n", ps >> 20, arg);
+ ksft_print_msg("Testing %luMB shmget with shift %x\n", ps >> 20, arg);
test_shmget(ps, SHM_HUGETLB | arg);
}
- puts("default huge shmget");
+
+ ksft_print_msg("default huge shmget\n");
test_shmget(default_hps, SHM_HUGETLB);

- return 0;
+ ksft_finished();
}
--
2.42.0

2024-02-02 11:34:30

by Muhammad Usama Anjum

[permalink] [raw]

Subject: [PATCH v2 11/12] selftests/mm: transhuge-stress: conform to TAP format output

Conform the layout, informational and status messages to TAP. No
functional change is intended other than the layout of output messages.

Signed-off-by: Muhammad Usama Anjum <[email protected]>
---
tools/testing/selftests/mm/transhuge-stress.c | 36 +++++++++++--------
tools/testing/selftests/mm/vm_util.c | 6 ++--
2 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/mm/transhuge-stress.c b/tools/testing/selftests/mm/transhuge-stress.c
index c61fb9350b8c2..68201192e37c8 100644
--- a/tools/testing/selftests/mm/transhuge-stress.c
+++ b/tools/testing/selftests/mm/transhuge-stress.c
@@ -16,6 +16,7 @@
#include <string.h>
#include <sys/mman.h>
#include "vm_util.h"
+#include "../kselftest.h"

int backing_fd = -1;
int mmap_flags = MAP_ANONYMOUS | MAP_NORESERVE | MAP_PRIVATE;
@@ -34,6 +35,8 @@ int main(int argc, char **argv)
int pagemap_fd;
int duration = 0;

+ ksft_print_header();
+
ram = sysconf(_SC_PHYS_PAGES);
if (ram > SIZE_MAX / psize() / 4)
ram = SIZE_MAX / 4;
@@ -43,7 +46,8 @@ int main(int argc, char **argv)

while (++i < argc) {
if (!strcmp(argv[i], "-h"))
- errx(1, "usage: %s [-f <filename>] [-d <duration>] [size in MiB]", argv[0]);
+ ksft_exit_fail_msg("usage: %s [-f <filename>] [-d <duration>] [size in MiB]\n",
+ argv[0]);
else if (!strcmp(argv[i], "-f"))
name = argv[++i];
else if (!strcmp(argv[i], "-d"))
@@ -52,10 +56,12 @@ int main(int argc, char **argv)
len = atoll(argv[i]) << 20;
}

+ ksft_set_plan(1);
+
if (name) {
backing_fd = open(name, O_RDWR);
if (backing_fd == -1)
- errx(2, "open %s", name);
+ ksft_exit_fail_msg("open %s\n", name);
mmap_flags = MAP_SHARED;
}

@@ -65,21 +71,21 @@ int main(int argc, char **argv)

pagemap_fd = open("/proc/self/pagemap", O_RDONLY);
if (pagemap_fd < 0)
- err(2, "open pagemap");
+ ksft_exit_fail_msg("open pagemap\n");

len -= len % HPAGE_SIZE;
ptr = mmap(NULL, len + HPAGE_SIZE, PROT_RW, mmap_flags, backing_fd, 0);
if (ptr == MAP_FAILED)
- err(2, "initial mmap");
+ ksft_exit_fail_msg("initial mmap");
ptr += HPAGE_SIZE - (uintptr_t)ptr % HPAGE_SIZE;

if (madvise(ptr, len, MADV_HUGEPAGE))
- err(2, "MADV_HUGEPAGE");
+ ksft_exit_fail_msg("MADV_HUGEPAGE");

map_len = ram >> (HPAGE_SHIFT - 1);
map = malloc(map_len);
if (!map)
- errx(2, "map malloc");
+ ksft_exit_fail_msg("map malloc\n");

clock_gettime(CLOCK_MONOTONIC, &start);

@@ -103,7 +109,7 @@ int main(int argc, char **argv)
if (idx >= map_len) {
map = realloc(map, idx + 1);
if (!map)
- errx(2, "map realloc");
+ ksft_exit_fail_msg("map realloc\n");
memset(map + map_len, 0, idx + 1 - map_len);
map_len = idx + 1;
}
@@ -114,17 +120,19 @@ int main(int argc, char **argv)

/* split transhuge page, keep last page */
if (madvise(p, HPAGE_SIZE - psize(), MADV_DONTNEED))
- err(2, "MADV_DONTNEED");
+ ksft_exit_fail_msg("MADV_DONTNEED");
}
clock_gettime(CLOCK_MONOTONIC, &b);
s = b.tv_sec - a.tv_sec + (b.tv_nsec - a.tv_nsec) / 1000000000.;

- warnx("%.3f s/loop, %.3f ms/page, %10.3f MiB/s\t"
- "%4d succeed, %4d failed, %4d different pages",
- s, s * 1000 / (len >> HPAGE_SHIFT), len / s / (1 << 20),
- nr_succeed, nr_failed, nr_pages);
+ ksft_print_msg("%.3f s/loop, %.3f ms/page, %10.3f MiB/s\t"
+ "%4d succeed, %4d failed, %4d different pages\n",
+ s, s * 1000 / (len >> HPAGE_SHIFT), len / s / (1 << 20),
+ nr_succeed, nr_failed, nr_pages);

- if (duration > 0 && b.tv_sec - start.tv_sec >= duration)
- return 0;
+ if (duration > 0 && b.tv_sec - start.tv_sec >= duration) {
+ ksft_test_result_pass("Completed\n");
+ ksft_finished();
+ }
}
}
diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
index 05736c615734f..5a62530da3b56 100644
--- a/tools/testing/selftests/mm/vm_util.c
+++ b/tools/testing/selftests/mm/vm_util.c
@@ -232,17 +232,17 @@ int64_t allocate_transhuge(void *ptr, int pagemap_fd)
if (mmap(ptr, HPAGE_SIZE, PROT_READ | PROT_WRITE,
MAP_FIXED | MAP_ANONYMOUS |
MAP_NORESERVE | MAP_PRIVATE, -1, 0) != ptr)
- errx(2, "mmap transhuge");
+ ksft_exit_fail_msg("mmap transhuge\n");

if (madvise(ptr, HPAGE_SIZE, MADV_HUGEPAGE))
- err(2, "MADV_HUGEPAGE");
+ ksft_exit_fail_msg("MADV_HUGEPAGE\n");

/* allocate transparent huge page */
*(volatile void **)ptr = ptr;

if (pread(pagemap_fd, ent, sizeof(ent),
(uintptr_t)ptr >> (pshift() - 3)) != sizeof(ent))
- err(2, "read pagemap");
+ ksft_exit_fail_msg("read pagemap\n");

if (PAGEMAP_PRESENT(ent[0]) && PAGEMAP_PRESENT(ent[1]) &&
PAGEMAP_PFN(ent[0]) + 1 == PAGEMAP_PFN(ent[1]) &&
--
2.42.0

2024-02-02 11:34:53

by Muhammad Usama Anjum

[permalink] [raw]

Subject: [PATCH v2 12/12] selftests/mm: virtual_address_range: conform to TAP format output

2024-02-02 11:35:32

by Muhammad Usama Anjum

[permalink] [raw]

Subject: [PATCH v2 08/12] selftests/mm: split_huge_page_test: conform test to TAP format output

Conform the layout, informational and status messages to TAP. No
functional change is intended other than the layout of output messages.

Signed-off-by: Muhammad Usama Anjum <[email protected]>
---
.../selftests/mm/split_huge_page_test.c | 161 ++++++++----------
1 file changed, 69 insertions(+), 92 deletions(-)

diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c
index 0e74635c8c3d9..7b698a848babf 100644
--- a/tools/testing/selftests/mm/split_huge_page_test.c
+++ b/tools/testing/selftests/mm/split_huge_page_test.c
@@ -17,6 +17,7 @@
#include <malloc.h>
#include <stdbool.h>
#include "vm_util.h"
+#include "../kselftest.h"

uint64_t pagesize;
unsigned int pageshift;
@@ -50,21 +51,19 @@ int is_backed_by_thp(char *vaddr, int pagemap_file, int kpageflags_file)
return 0;
}

-static int write_file(const char *path, const char *buf, size_t buflen)
+static void write_file(const char *path, const char *buf, size_t buflen)
{
int fd;
ssize_t numwritten;

fd = open(path, O_WRONLY);
if (fd == -1)
- return 0;
+ ksft_exit_fail_msg("%s open failed: %s\n", path, strerror(errno));

numwritten = write(fd, buf, buflen - 1);
close(fd);
if (numwritten < 1)
- return 0;
-
- return (unsigned int) numwritten;
+ ksft_exit_fail_msg("Write failed\n");
}

static void write_debugfs(const char *fmt, ...)
@@ -77,15 +76,10 @@ static void write_debugfs(const char *fmt, ...)
ret = vsnprintf(input, INPUT_MAX, fmt, argp);
va_end(argp);

- if (ret >= INPUT_MAX) {
- printf("%s: Debugfs input is too long\n", __func__);
- exit(EXIT_FAILURE);
- }
+ if (ret >= INPUT_MAX)
+ ksft_exit_fail_msg("%s: Debugfs input is too long\n", __func__);

- if (!write_file(SPLIT_DEBUGFS, input, ret + 1)) {
- perror(SPLIT_DEBUGFS);
- exit(EXIT_FAILURE);
- }
+ write_file(SPLIT_DEBUGFS, input, ret + 1);
}

void split_pmd_thp(void)
@@ -95,39 +89,30 @@ void split_pmd_thp(void)
size_t i;

one_page = memalign(pmd_pagesize, len);
-
- if (!one_page) {
- printf("Fail to allocate memory\n");
- exit(EXIT_FAILURE);
- }
+ if (!one_page)
+ ksft_exit_fail_msg("Fail to allocate memory: %s\n", strerror(errno));

madvise(one_page, len, MADV_HUGEPAGE);

for (i = 0; i < len; i++)
one_page[i] = (char)i;

- if (!check_huge_anon(one_page, 4, pmd_pagesize)) {
- printf("No THP is allocated\n");
- exit(EXIT_FAILURE);
- }
+ if (!check_huge_anon(one_page, 4, pmd_pagesize))
+ ksft_exit_fail_msg("No THP is allocated\n");

/* split all THPs */
write_debugfs(PID_FMT, getpid(), (uint64_t)one_page,
(uint64_t)one_page + len);

for (i = 0; i < len; i++)
- if (one_page[i] != (char)i) {
- printf("%ld byte corrupted\n", i);
- exit(EXIT_FAILURE);
- }
+ if (one_page[i] != (char)i)
+ ksft_exit_fail_msg("%ld byte corrupted\n", i);

- if (!check_huge_anon(one_page, 0, pmd_pagesize)) {
- printf("Still AnonHugePages not split\n");
- exit(EXIT_FAILURE);
- }
+ if (!check_huge_anon(one_page, 0, pmd_pagesize))
+ ksft_exit_fail_msg("Still AnonHugePages not split\n");

- printf("Split huge pages successful\n");
+ ksft_test_result_pass("Split huge pages successful\n");
free(one_page);
}

@@ -143,36 +128,29 @@ void split_pte_mapped_thp(void)
int pagemap_fd;
int kpageflags_fd;

- if (snprintf(pagemap_proc, 255, pagemap_template, getpid()) < 0) {
- perror("get pagemap proc error");
- exit(EXIT_FAILURE);
- }
- pagemap_fd = open(pagemap_proc, O_RDONLY);
+ if (snprintf(pagemap_proc, 255, pagemap_template, getpid()) < 0)
+ ksft_exit_fail_msg("get pagemap proc error: %s\n", strerror(errno));

- if (pagemap_fd == -1) {
- perror("read pagemap:");
- exit(EXIT_FAILURE);
- }
+ pagemap_fd = open(pagemap_proc, O_RDONLY);
+ if (pagemap_fd == -1)
+ ksft_exit_fail_msg("read pagemap: %s\n", strerror(errno));

kpageflags_fd = open(kpageflags_proc, O_RDONLY);
-
- if (kpageflags_fd == -1) {
- perror("read kpageflags:");
- exit(EXIT_FAILURE);
- }
+ if (kpageflags_fd == -1)
+ ksft_exit_fail_msg("read kpageflags: %s\n", strerror(errno));

one_page = mmap((void *)(1UL << 30), len, PROT_READ | PROT_WRITE,
MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+ if (one_page == MAP_FAILED)
+ ksft_exit_fail_msg("Fail to allocate memory: %s\n", strerror(errno));

madvise(one_page, len, MADV_HUGEPAGE);

for (i = 0; i < len; i++)
one_page[i] = (char)i;

- if (!check_huge_anon(one_page, 4, pmd_pagesize)) {
- printf("No THP is allocated\n");
- exit(EXIT_FAILURE);
- }
+ if (!check_huge_anon(one_page, 4, pmd_pagesize))
+ ksft_exit_fail_msg("No THP is allocated\n");

/* remap the first pagesize of first THP */
pte_mapped = mremap(one_page, pagesize, pagesize, MREMAP_MAYMOVE);
@@ -183,10 +161,8 @@ void split_pte_mapped_thp(void)
pagesize, pagesize,
MREMAP_MAYMOVE|MREMAP_FIXED,
pte_mapped + pagesize * i);
- if (pte_mapped2 == (char *)-1) {
- perror("mremap failed");
- exit(EXIT_FAILURE);
- }
+ if (pte_mapped2 == MAP_FAILED)
+ ksft_exit_fail_msg("mremap failed: %s\n", strerror(errno));
}

/* smap does not show THPs after mremap, use kpageflags instead */
@@ -196,10 +172,8 @@ void split_pte_mapped_thp(void)
is_backed_by_thp(&pte_mapped[i], pagemap_fd, kpageflags_fd))
thp_size++;

- if (thp_size != 4) {
- printf("Some THPs are missing during mremap\n");
- exit(EXIT_FAILURE);
- }
+ if (thp_size != 4)
+ ksft_exit_fail_msg("Some THPs are missing during mremap\n");

/* split all remapped THPs */
write_debugfs(PID_FMT, getpid(), (uint64_t)pte_mapped,
@@ -208,21 +182,18 @@ void split_pte_mapped_thp(void)
/* smap does not show THPs after mremap, use kpageflags instead */
thp_size = 0;
for (i = 0; i < pagesize * 4; i++) {
- if (pte_mapped[i] != (char)i) {
- printf("%ld byte corrupted\n", i);
- exit(EXIT_FAILURE);
- }
+ if (pte_mapped[i] != (char)i)
+ ksft_exit_fail_msg("%ld byte corrupted\n", i);
+
if (i % pagesize == 0 &&
is_backed_by_thp(&pte_mapped[i], pagemap_fd, kpageflags_fd))
thp_size++;
}

- if (thp_size) {
- printf("Still %ld THPs not split\n", thp_size);
- exit(EXIT_FAILURE);
- }
+ if (thp_size)
+ ksft_exit_fail_msg("Still %ld THPs not split\n", thp_size);

- printf("Split PTE-mapped huge pages successful\n");
+ ksft_test_result_pass("Split PTE-mapped huge pages successful\n");
munmap(one_page, len);
close(pagemap_fd);
close(kpageflags_fd);
@@ -238,24 +209,21 @@ void split_file_backed_thp(void)
char testfile[INPUT_MAX];
uint64_t pgoff_start = 0, pgoff_end = 1024;

- printf("Please enable pr_debug in split_huge_pages_in_file() if you need more info.\n");
+ ksft_print_msg("Please enable pr_debug in split_huge_pages_in_file() for more info.\n");

status = mount("tmpfs", tmpfs_loc, "tmpfs", 0, "huge=always,size=4m");

- if (status) {
- printf("Unable to create a tmpfs for testing\n");
- exit(EXIT_FAILURE);
- }
+ if (status)
+ ksft_exit_fail_msg("Unable to create a tmpfs for testing\n");

status = snprintf(testfile, INPUT_MAX, "%s/thp_file", tmpfs_loc);
if (status >= INPUT_MAX) {
- printf("Fail to create file-backed THP split testing file\n");
- goto cleanup;
+ ksft_exit_fail_msg("Fail to create file-backed THP split testing file\n");
}

fd = open(testfile, O_CREAT|O_WRONLY);
if (fd == -1) {
- perror("Cannot open testing file\n");
+ ksft_perror("Cannot open testing file");
goto cleanup;
}

@@ -264,7 +232,7 @@ void split_file_backed_thp(void)
close(fd);

if (num_written < 1) {
- printf("Fail to write data to testing file\n");
+ ksft_perror("Fail to write data to testing file");
goto cleanup;
}

@@ -272,42 +240,51 @@ void split_file_backed_thp(void)
write_debugfs(PATH_FMT, testfile, pgoff_start, pgoff_end);

status = unlink(testfile);
- if (status)
- perror("Cannot remove testing file\n");
+ if (status) {
+ ksft_perror("Cannot remove testing file");
+ goto cleanup;
+ }

-cleanup:
status = umount(tmpfs_loc);
if (status) {
- printf("Unable to umount %s\n", tmpfs_loc);
- exit(EXIT_FAILURE);
+ rmdir(tmpfs_loc);
+ ksft_exit_fail_msg("Unable to umount %s\n", tmpfs_loc);
}
+
status = rmdir(tmpfs_loc);
- if (status) {
- perror("cannot remove tmp dir");
- exit(EXIT_FAILURE);
- }
+ if (status)
+ ksft_exit_fail_msg("cannot remove tmp dir: %s\n", strerror(errno));

- printf("file-backed THP split test done, please check dmesg for more information\n");
+ ksft_print_msg("Please check dmesg for more information\n");
+ ksft_test_result_pass("File-backed THP split test done\n");
+ return;
+
+cleanup:
+ umount(tmpfs_loc);
+ rmdir(tmpfs_loc);
+ ksft_exit_fail_msg("Error occurred\n");
}

int main(int argc, char **argv)
{
+ ksft_print_header();
+
if (geteuid() != 0) {
- printf("Please run the benchmark as root\n");
- exit(EXIT_FAILURE);
+ ksft_print_msg("Please run the benchmark as root\n");
+ ksft_finished();
}

+ ksft_set_plan(3);
+
pagesize = getpagesize();
pageshift = ffs(pagesize) - 1;
pmd_pagesize = read_pmd_pagesize();
- if (!pmd_pagesize) {
- printf("Reading PMD pagesize failed\n");
- exit(EXIT_FAILURE);
- }
+ if (!pmd_pagesize)
+ ksft_exit_fail_msg("Reading PMD pagesize failed\n");

split_pmd_thp();
split_pte_mapped_thp();
split_file_backed_thp();

- return 0;
+ ksft_finished();
}
--
2.42.0

2024-02-02 11:55:19

by Muhammad Usama Anjum

[permalink] [raw]

Subject: [PATCH v2 04/12] selftests/mm: mlock-random-test: conform test to TAP format output

Conform the layout, informational and status messages to TAP. No
functional change is intended other than the layout of output messages.

Signed-off-by: Muhammad Usama Anjum <[email protected]>
---
.../testing/selftests/mm/mlock-random-test.c | 136 +++++++-----------
1 file changed, 54 insertions(+), 82 deletions(-)

diff --git a/tools/testing/selftests/mm/mlock-random-test.c b/tools/testing/selftests/mm/mlock-random-test.c
index 1fba77df7f628..1cd80b0f76c33 100644
--- a/tools/testing/selftests/mm/mlock-random-test.c
+++ b/tools/testing/selftests/mm/mlock-random-test.c
@@ -13,6 +13,7 @@
#include <sys/ipc.h>
#include <sys/shm.h>
#include <time.h>
+#include "../kselftest.h"
#include "mlock2.h"

#define CHUNK_UNIT (128 * 1024)
@@ -31,14 +32,14 @@ int set_cap_limits(rlim_t max)
new.rlim_cur = max;
new.rlim_max = max;
if (setrlimit(RLIMIT_MEMLOCK, &new)) {
- perror("setrlimit() returns error\n");
+ ksft_perror("setrlimit() returns error\n");
return -1;
}

/* drop capabilities including CAP_IPC_LOCK */
if (cap_set_proc(cap)) {
- perror("cap_set_proc() returns error\n");
- return -2;
+ ksft_perror("cap_set_proc() returns error\n");
+ return -1;
}

return 0;
@@ -52,27 +53,24 @@ int get_proc_locked_vm_size(void)
unsigned long lock_size = 0;

f = fopen("/proc/self/status", "r");
- if (!f) {
- perror("fopen");
- return -1;
- }
+ if (!f)
+ ksft_exit_fail_msg("fopen: %s\n", strerror(errno));

while (fgets(line, 1024, f)) {
if (strstr(line, "VmLck")) {
ret = sscanf(line, "VmLck:\t%8lu kB", &lock_size);
if (ret <= 0) {
- printf("sscanf() on VmLck error: %s: %d\n",
- line, ret);
fclose(f);
- return -1;
+ ksft_exit_fail_msg("sscanf() on VmLck error: %s: %d\n",
+ line, ret);
}
fclose(f);
return (int)(lock_size << 10);
}
}

- perror("cannot parse VmLck in /proc/self/status\n");
fclose(f);
+ ksft_exit_fail_msg("cannot parse VmLck in /proc/self/status: %s\n", strerror(errno));
return -1;
}

@@ -91,10 +89,8 @@ int get_proc_page_size(unsigned long addr)
size_t size;

smaps = seek_to_smaps_entry(addr);
- if (!smaps) {
- printf("Unable to parse /proc/self/smaps\n");
- return 0;
- }
+ if (!smaps)
+ ksft_exit_fail_msg("Unable to parse /proc/self/smaps\n");

while (getline(&line, &size, smaps) > 0) {
if (!strstr(line, "MMUPageSize")) {
@@ -105,12 +101,9 @@ int get_proc_page_size(unsigned long addr)
}

/* found the MMUPageSize of this section */
- if (sscanf(line, "MMUPageSize: %8lu kB",
- &mmupage_size) < 1) {
- printf("Unable to parse smaps entry for Size:%s\n",
- line);
- break;
- }
+ if (sscanf(line, "MMUPageSize: %8lu kB", &mmupage_size) < 1)
+ ksft_exit_fail_msg("Unable to parse smaps entry for Size:%s\n",
+ line);

}
free(line);
@@ -136,7 +129,7 @@ int get_proc_page_size(unsigned long addr)
* return value: 0 - success
* else: failure
*/
-int test_mlock_within_limit(char *p, int alloc_size)
+static void test_mlock_within_limit(char *p, int alloc_size)
{
int i;
int ret = 0;
@@ -145,11 +138,9 @@ int test_mlock_within_limit(char *p, int alloc_size)
int page_size = 0;

getrlimit(RLIMIT_MEMLOCK, &cur);
- if (cur.rlim_cur < alloc_size) {
- printf("alloc_size[%d] < %u rlimit,lead to mlock failure\n",
- alloc_size, (unsigned int)cur.rlim_cur);
- return -1;
- }
+ if (cur.rlim_cur < alloc_size)
+ ksft_exit_fail_msg("alloc_size[%d] < %u rlimit,lead to mlock failure\n",
+ alloc_size, (unsigned int)cur.rlim_cur);

srand(time(NULL));
for (i = 0; i < TEST_LOOP; i++) {
@@ -169,13 +160,11 @@ int test_mlock_within_limit(char *p, int alloc_size)
ret = mlock2_(p + start_offset, lock_size,
MLOCK_ONFAULT);

- if (ret) {
- printf("%s() failure at |%p(%d)| mlock:|%p(%d)|\n",
- is_mlock ? "mlock" : "mlock2",
- p, alloc_size,
- p + start_offset, lock_size);
- return ret;
- }
+ if (ret)
+ ksft_exit_fail_msg("%s() failure at |%p(%d)| mlock:|%p(%d)|\n",
+ is_mlock ? "mlock" : "mlock2",
+ p, alloc_size,
+ p + start_offset, lock_size);
}

/*
@@ -183,18 +172,12 @@ int test_mlock_within_limit(char *p, int alloc_size)
*/
locked_vm_size = get_proc_locked_vm_size();
page_size = get_proc_page_size((unsigned long)p);
- if (page_size == 0) {
- printf("cannot get proc MMUPageSize\n");
- return -1;
- }

- if (locked_vm_size > PAGE_ALIGN(alloc_size, page_size) + page_size) {
- printf("test_mlock_within_limit() left VmLck:%d on %d chunk\n",
- locked_vm_size, alloc_size);
- return -1;
- }
+ if (locked_vm_size > PAGE_ALIGN(alloc_size, page_size) + page_size)
+ ksft_exit_fail_msg("%s left VmLck:%d on %d chunk\n",
+ __func__, locked_vm_size, alloc_size);

- return 0;
+ ksft_test_result_pass("%s\n", __func__);
}

@@ -213,7 +196,7 @@ int test_mlock_within_limit(char *p, int alloc_size)
* return value: 0 - success
* else: failure
*/
-int test_mlock_outof_limit(char *p, int alloc_size)
+static void test_mlock_outof_limit(char *p, int alloc_size)
{
int i;
int ret = 0;
@@ -221,11 +204,9 @@ int test_mlock_outof_limit(char *p, int alloc_size)
struct rlimit cur;

getrlimit(RLIMIT_MEMLOCK, &cur);
- if (cur.rlim_cur >= alloc_size) {
- printf("alloc_size[%d] >%u rlimit, violates test condition\n",
- alloc_size, (unsigned int)cur.rlim_cur);
- return -1;
- }
+ if (cur.rlim_cur >= alloc_size)
+ ksft_exit_fail_msg("alloc_size[%d] >%u rlimit, violates test condition\n",
+ alloc_size, (unsigned int)cur.rlim_cur);

old_locked_vm_size = get_proc_locked_vm_size();
srand(time(NULL));
@@ -240,56 +221,47 @@ int test_mlock_outof_limit(char *p, int alloc_size)
else
ret = mlock2_(p + start_offset, lock_size,
MLOCK_ONFAULT);
- if (ret == 0) {
- printf("%s() succeeds? on %p(%d) mlock%p(%d)\n",
- is_mlock ? "mlock" : "mlock2",
- p, alloc_size,
- p + start_offset, lock_size);
- return -1;
- }
+ if (ret == 0)
+ ksft_exit_fail_msg("%s() succeeds? on %p(%d) mlock%p(%d)\n",
+ is_mlock ? "mlock" : "mlock2",
+ p, alloc_size, p + start_offset, lock_size);
}

locked_vm_size = get_proc_locked_vm_size();
- if (locked_vm_size != old_locked_vm_size) {
- printf("tests leads to new mlocked page: old[%d], new[%d]\n",
- old_locked_vm_size,
- locked_vm_size);
- return -1;
- }
+ if (locked_vm_size != old_locked_vm_size)
+ ksft_exit_fail_msg("tests leads to new mlocked page: old[%d], new[%d]\n",
+ old_locked_vm_size,
+ locked_vm_size);

- return 0;
+ ksft_test_result_pass("%s\n", __func__);
}

int main(int argc, char **argv)
{
char *p = NULL;
- int ret = 0;
+
+ ksft_print_header();

if (set_cap_limits(MLOCK_RLIMIT_SIZE))
- return -1;
+ ksft_finished();
+
+ ksft_set_plan(2);

p = malloc(MLOCK_WITHIN_LIMIT_SIZE);
- if (p == NULL) {
- perror("malloc() failure\n");
- return -1;
- }
- ret = test_mlock_within_limit(p, MLOCK_WITHIN_LIMIT_SIZE);
- if (ret)
- return ret;
+ if (p == NULL)
+ ksft_exit_fail_msg("malloc() failure: %s\n", strerror(errno));
+
+ test_mlock_within_limit(p, MLOCK_WITHIN_LIMIT_SIZE);
munlock(p, MLOCK_WITHIN_LIMIT_SIZE);
free(p);

-
p = malloc(MLOCK_OUTOF_LIMIT_SIZE);
- if (p == NULL) {
- perror("malloc() failure\n");
- return -1;
- }
- ret = test_mlock_outof_limit(p, MLOCK_OUTOF_LIMIT_SIZE);
- if (ret)
- return ret;
+ if (p == NULL)
+ ksft_exit_fail_msg("malloc() failure: %s\n", strerror(errno));
+
+ test_mlock_outof_limit(p, MLOCK_OUTOF_LIMIT_SIZE);
munlock(p, MLOCK_OUTOF_LIMIT_SIZE);
free(p);

- return 0;
+ ksft_finished();
}
--
2.42.0

2024-02-02 11:55:40

by Muhammad Usama Anjum

[permalink] [raw]

Subject: [PATCH v2 05/12] selftests/mm: mlock2-tests: conform test to TAP format output

Conform the layout, informational and status messages to TAP. No
functional change is intended other than the layout of output messages.
I've done some cleanups as well.

Signed-off-by: Muhammad Usama Anjum <[email protected]>
---
tools/testing/selftests/mm/mlock2-tests.c | 282 +++++++++-------------
tools/testing/selftests/mm/mlock2.h | 11 +-
2 files changed, 118 insertions(+), 175 deletions(-)

diff --git a/tools/testing/selftests/mm/mlock2-tests.c b/tools/testing/selftests/mm/mlock2-tests.c
index 80cddc0de2061..26f744188ad0c 100644
--- a/tools/testing/selftests/mm/mlock2-tests.c
+++ b/tools/testing/selftests/mm/mlock2-tests.c
@@ -7,9 +7,8 @@
#include <sys/time.h>
#include <sys/resource.h>
#include <stdbool.h>
-#include "mlock2.h"
-
#include "../kselftest.h"
+#include "mlock2.h"

struct vm_boundaries {
unsigned long start;
@@ -40,14 +39,14 @@ static int get_vm_area(unsigned long addr, struct vm_boundaries *area)
while(fgets(line, 1024, file)) {
end_addr = strchr(line, '-');
if (!end_addr) {
- printf("cannot parse /proc/self/maps\n");
+ ksft_print_msg("cannot parse /proc/self/maps\n");
goto out;
}
*end_addr = '\0';
end_addr++;
stop = strchr(end_addr, ' ');
if (!stop) {
- printf("cannot parse /proc/self/maps\n");
+ ksft_print_msg("cannot parse /proc/self/maps\n");
goto out;
}

@@ -78,7 +77,7 @@ static bool is_vmflag_set(unsigned long addr, const char *vmflag)

smaps = seek_to_smaps_entry(addr);
if (!smaps) {
- printf("Unable to parse /proc/self/smaps\n");
+ ksft_print_msg("Unable to parse /proc/self/smaps\n");
goto out;
}

@@ -115,7 +114,7 @@ static unsigned long get_value_for_name(unsigned long addr, const char *name)

smaps = seek_to_smaps_entry(addr);
if (!smaps) {
- printf("Unable to parse /proc/self/smaps\n");
+ ksft_print_msg("Unable to parse /proc/self/smaps\n");
goto out;
}

@@ -129,7 +128,7 @@ static unsigned long get_value_for_name(unsigned long addr, const char *name)

value_ptr = line + strlen(name);
if (sscanf(value_ptr, "%lu kB", &value) < 1) {
- printf("Unable to parse smaps entry for Size\n");
+ ksft_print_msg("Unable to parse smaps entry for Size\n");
goto out;
}
break;
@@ -180,57 +179,45 @@ static int lock_check(unsigned long addr)
static int unlock_lock_check(char *map)
{
if (is_vmflag_set((unsigned long)map, LOCKED)) {
- printf("VMA flag %s is present on page 1 after unlock\n", LOCKED);
+ ksft_print_msg("VMA flag %s is present on page 1 after unlock\n", LOCKED);
return 1;
}

return 0;
}

-static int test_mlock_lock()
+static void test_mlock_lock(void)
{
char *map;
- int ret = 1;
unsigned long page_size = getpagesize();

map = mmap(NULL, 2 * page_size, PROT_READ | PROT_WRITE,
MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
- if (map == MAP_FAILED) {
- perror("test_mlock_locked mmap");
- goto out;
- }
+ if (map == MAP_FAILED)
+ ksft_exit_fail_msg("mmap error: %s", strerror(errno));

if (mlock2_(map, 2 * page_size, 0)) {
- if (errno == ENOSYS) {
- printf("Cannot call new mlock family, skipping test\n");
- _exit(KSFT_SKIP);
- }
- perror("mlock2(0)");
- goto unmap;
+ munmap(map, 2 * page_size);
+ ksft_exit_fail_msg("mlock2(0): %s\n", strerror(errno));
}

- if (!lock_check((unsigned long)map))
- goto unmap;
+ ksft_test_result(lock_check((unsigned long)map), "%s: Locked\n", __func__);

/* Now unlock and recheck attributes */
if (munlock(map, 2 * page_size)) {
- perror("munlock()");
- goto unmap;
+ munmap(map, 2 * page_size);
+ ksft_exit_fail_msg("munlock(): %s\n", strerror(errno));
}

- ret = unlock_lock_check(map);
-
-unmap:
+ ksft_test_result(!unlock_lock_check(map), "%s: Locked\n", __func__);
munmap(map, 2 * page_size);
-out:
- return ret;
}

static int onfault_check(char *map)
{
*map = 'a';
if (!is_vma_lock_on_fault((unsigned long)map)) {
- printf("VMA is not marked for lock on fault\n");
+ ksft_print_msg("VMA is not marked for lock on fault\n");
return 1;
}

@@ -243,172 +230,131 @@ static int unlock_onfault_check(char *map)

if (is_vma_lock_on_fault((unsigned long)map) ||
is_vma_lock_on_fault((unsigned long)map + page_size)) {
- printf("VMA is still lock on fault after unlock\n");
+ ksft_print_msg("VMA is still lock on fault after unlock\n");
return 1;
}

return 0;
}

-static int test_mlock_onfault()
+static void test_mlock_onfault(void)
{
char *map;
- int ret = 1;
unsigned long page_size = getpagesize();

map = mmap(NULL, 2 * page_size, PROT_READ | PROT_WRITE,
MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
- if (map == MAP_FAILED) {
- perror("test_mlock_locked mmap");
- goto out;
- }
+ if (map == MAP_FAILED)
+ ksft_exit_fail_msg("mmap error: %s", strerror(errno));

if (mlock2_(map, 2 * page_size, MLOCK_ONFAULT)) {
- if (errno == ENOSYS) {
- printf("Cannot call new mlock family, skipping test\n");
- _exit(KSFT_SKIP);
- }
- perror("mlock2(MLOCK_ONFAULT)");
- goto unmap;
+ munmap(map, 2 * page_size);
+ ksft_exit_fail_msg("mlock2(MLOCK_ONFAULT): %s\n", strerror(errno));
}

- if (onfault_check(map))
- goto unmap;
+ ksft_test_result(!onfault_check(map), "%s: VMA marked for lock on fault\n", __func__);

/* Now unlock and recheck attributes */
if (munlock(map, 2 * page_size)) {
- if (errno == ENOSYS) {
- printf("Cannot call new mlock family, skipping test\n");
- _exit(KSFT_SKIP);
- }
- perror("munlock()");
- goto unmap;
+ munmap(map, 2 * page_size);
+ ksft_exit_fail_msg("munlock(): %s\n", strerror(errno));
}

- ret = unlock_onfault_check(map);
-unmap:
+ ksft_test_result(!unlock_onfault_check(map), "VMA open lock after fault\n");
munmap(map, 2 * page_size);
-out:
- return ret;
}

-static int test_lock_onfault_of_present()
+static void test_lock_onfault_of_present(void)
{
char *map;
- int ret = 1;
unsigned long page_size = getpagesize();

map = mmap(NULL, 2 * page_size, PROT_READ | PROT_WRITE,
MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
- if (map == MAP_FAILED) {
- perror("test_mlock_locked mmap");
- goto out;
- }
+ if (map == MAP_FAILED)
+ ksft_exit_fail_msg("mmap error: %s", strerror(errno));

*map = 'a';

if (mlock2_(map, 2 * page_size, MLOCK_ONFAULT)) {
- if (errno == ENOSYS) {
- printf("Cannot call new mlock family, skipping test\n");
- _exit(KSFT_SKIP);
- }
- perror("mlock2(MLOCK_ONFAULT)");
- goto unmap;
+ munmap(map, 2 * page_size);
+ ksft_test_result_fail("mlock2(MLOCK_ONFAULT) error: %s", strerror(errno));
}

- if (!is_vma_lock_on_fault((unsigned long)map) ||
- !is_vma_lock_on_fault((unsigned long)map + page_size)) {
- printf("VMA with present pages is not marked lock on fault\n");
- goto unmap;
- }
- ret = 0;
-unmap:
+ ksft_test_result(is_vma_lock_on_fault((unsigned long)map) ||
+ is_vma_lock_on_fault((unsigned long)map + page_size),
+ "VMA with present pages is not marked lock on fault\n");
munmap(map, 2 * page_size);
-out:
- return ret;
}

-static int test_munlockall()
+static void test_munlockall0(void)
{
char *map;
- int ret = 1;
unsigned long page_size = getpagesize();

map = mmap(NULL, 2 * page_size, PROT_READ | PROT_WRITE,
MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
-
- if (map == MAP_FAILED) {
- perror("test_munlockall mmap");
- goto out;
- }
+ if (map == MAP_FAILED)
+ ksft_exit_fail_msg("mmap error: %s\n", strerror(errno));

if (mlockall(MCL_CURRENT)) {
- perror("mlockall(MCL_CURRENT)");
- goto out;
+ munmap(map, 2 * page_size);
+ ksft_exit_fail_msg("mlockall(MCL_CURRENT): %s\n", strerror(errno));
}

- if (!lock_check((unsigned long)map))
- goto unmap;
+ ksft_test_result(lock_check((unsigned long)map), "%s: Locked memory area\n", __func__);

if (munlockall()) {
- perror("munlockall()");
- goto unmap;
+ munmap(map, 2 * page_size);
+ ksft_exit_fail_msg("munlockall(): %s\n", strerror(errno));
}

- if (unlock_lock_check(map))
- goto unmap;
-
+ ksft_test_result(!unlock_lock_check(map), "%s: No locked memory\n", __func__);
munmap(map, 2 * page_size);
+}
+
+static void test_munlockall1(void)
+{
+ char *map;
+ unsigned long page_size = getpagesize();

map = mmap(NULL, 2 * page_size, PROT_READ | PROT_WRITE,
MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
-
- if (map == MAP_FAILED) {
- perror("test_munlockall second mmap");
- goto out;
- }
+ if (map == MAP_FAILED)
+ ksft_exit_fail_msg("mmap error: %s", strerror(errno));

if (mlockall(MCL_CURRENT | MCL_ONFAULT)) {
- perror("mlockall(MCL_CURRENT | MCL_ONFAULT)");
- goto unmap;
+ munmap(map, 2 * page_size);
+ ksft_exit_fail_msg("mlockall(MCL_CURRENT | MCL_ONFAULT): %s\n", strerror(errno));
}

- if (onfault_check(map))
- goto unmap;
+ ksft_test_result(!onfault_check(map), "%s: VMA marked for lock on fault\n", __func__);

if (munlockall()) {
- perror("munlockall()");
- goto unmap;
+ munmap(map, 2 * page_size);
+ ksft_exit_fail_msg("munlockall(): %s\n", strerror(errno));
}

- if (unlock_onfault_check(map))
- goto unmap;
+ ksft_test_result(!unlock_onfault_check(map), "%s: Unlocked\n", __func__);

if (mlockall(MCL_CURRENT | MCL_FUTURE)) {
- perror("mlockall(MCL_CURRENT | MCL_FUTURE)");
- goto out;
+ munmap(map, 2 * page_size);
+ ksft_exit_fail_msg("mlockall(MCL_CURRENT | MCL_FUTURE): %s\n", strerror(errno));
}

- if (!lock_check((unsigned long)map))
- goto unmap;
+ ksft_test_result(lock_check((unsigned long)map), "%s: Locked\n", __func__);

if (munlockall()) {
- perror("munlockall()");
- goto unmap;
+ munmap(map, 2 * page_size);
+ ksft_exit_fail_msg("munlockall() %s\n", strerror(errno));
}

- ret = unlock_lock_check(map);
-
-unmap:
+ ksft_test_result(!unlock_lock_check(map), "%s: No locked memory\n", __func__);
munmap(map, 2 * page_size);
-out:
- munlockall();
- return ret;
}

-static int test_vma_management(bool call_mlock)
+static void test_vma_management(bool call_mlock)
{
- int ret = 1;
void *map;
unsigned long page_size = getpagesize();
struct vm_boundaries page1;
@@ -417,25 +363,19 @@ static int test_vma_management(bool call_mlock)

map = mmap(NULL, 3 * page_size, PROT_READ | PROT_WRITE,
MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
- if (map == MAP_FAILED) {
- perror("mmap()");
- return ret;
- }
+ if (map == MAP_FAILED)
+ ksft_exit_fail_msg("mmap error: %s", strerror(errno));

if (call_mlock && mlock2_(map, 3 * page_size, MLOCK_ONFAULT)) {
- if (errno == ENOSYS) {
- printf("Cannot call new mlock family, skipping test\n");
- _exit(KSFT_SKIP);
- }
- perror("mlock(ONFAULT)\n");
- goto out;
+ munmap(map, 3 * page_size);
+ ksft_test_result_fail("mlock error: %s", strerror(errno));
}

if (get_vm_area((unsigned long)map, &page1) ||
get_vm_area((unsigned long)map + page_size, &page2) ||
get_vm_area((unsigned long)map + page_size * 2, &page3)) {
- printf("couldn't find mapping in /proc/self/maps\n");
- goto out;
+ munmap(map, 3 * page_size);
+ ksft_test_result_fail("couldn't find mapping in /proc/self/maps");
}

/*
@@ -444,76 +384,86 @@ static int test_vma_management(bool call_mlock)
* not a failure)
*/
if (page1.start != page2.start || page2.start != page3.start) {
- printf("VMAs are not merged to start, aborting test\n");
- ret = 0;
- goto out;
+ munmap(map, 3 * page_size);
+ ksft_test_result_fail("VMAs are not merged to start, aborting test");
}

if (munlock(map + page_size, page_size)) {
- perror("munlock()");
- goto out;
+ munmap(map, 3 * page_size);
+ ksft_test_result_fail("munlock(): %s", strerror(errno));
}

if (get_vm_area((unsigned long)map, &page1) ||
get_vm_area((unsigned long)map + page_size, &page2) ||
get_vm_area((unsigned long)map + page_size * 2, &page3)) {
- printf("couldn't find mapping in /proc/self/maps\n");
- goto out;
+ munmap(map, 3 * page_size);
+ ksft_test_result_fail("couldn't find mapping in /proc/self/maps");
}

/* All three VMAs should be different */
if (page1.start == page2.start || page2.start == page3.start) {
- printf("failed to split VMA for munlock\n");
- goto out;
+ munmap(map, 3 * page_size);
+ ksft_test_result_fail("failed to split VMA for munlock");
}

/* Now unlock the first and third page and check the VMAs again */
if (munlock(map, page_size * 3)) {
- perror("munlock()");
- goto out;
+ munmap(map, 3 * page_size);
+ ksft_test_result_fail("munlock(): %s", strerror(errno));
}

if (get_vm_area((unsigned long)map, &page1) ||
get_vm_area((unsigned long)map + page_size, &page2) ||
get_vm_area((unsigned long)map + page_size * 2, &page3)) {
- printf("couldn't find mapping in /proc/self/maps\n");
- goto out;
+ munmap(map, 3 * page_size);
+ ksft_test_result_fail("couldn't find mapping in /proc/self/maps");
}

/* Now all three VMAs should be the same */
if (page1.start != page2.start || page2.start != page3.start) {
- printf("failed to merge VMAs after munlock\n");
- goto out;
+ munmap(map, 3 * page_size);
+ ksft_test_result_fail("failed to merge VMAs after munlock");
}

- ret = 0;
-out:
+ ksft_test_result_pass("%s call_mlock %d\n", __func__, call_mlock);
munmap(map, 3 * page_size);
- return ret;
}

-static int test_mlockall(int (test_function)(bool call_mlock))
+static void test_mlockall(void)
{
- int ret = 1;
+ if (mlockall(MCL_CURRENT | MCL_ONFAULT | MCL_FUTURE))
+ ksft_exit_fail_msg("mlockall failed: %s\n", strerror(errno));

- if (mlockall(MCL_CURRENT | MCL_ONFAULT | MCL_FUTURE)) {
- perror("mlockall");
- return ret;
- }
-
- ret = test_function(false);
+ test_vma_management(false);
munlockall();
- return ret;
}

int main(int argc, char **argv)
{
- int ret = 0;
- ret += test_mlock_lock();
- ret += test_mlock_onfault();
- ret += test_munlockall();
- ret += test_lock_onfault_of_present();
- ret += test_vma_management(true);
- ret += test_mlockall(test_vma_management);
- return ret;
+ int ret, size = 3 * getpagesize();
+ void *map;
+
+ ksft_print_header();
+
+ map = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+ if (map == MAP_FAILED)
+ ksft_exit_fail_msg("mmap error: %s", strerror(errno));
+
+ ret = mlock2_(map, size, MLOCK_ONFAULT);
+ if (ret && errno == ENOSYS)
+ ksft_finished();
+
+ munmap(map, size);
+
+ ksft_set_plan(13);
+
+ test_mlock_lock();
+ test_mlock_onfault();
+ test_munlockall0();
+ test_munlockall1();
+ test_lock_onfault_of_present();
+ test_vma_management(true);
+ test_mlockall();
+
+ ksft_finished();
}
diff --git a/tools/testing/selftests/mm/mlock2.h b/tools/testing/selftests/mm/mlock2.h
index 8e02991b313c8..4417eaa5cfb78 100644
--- a/tools/testing/selftests/mm/mlock2.h
+++ b/tools/testing/selftests/mm/mlock2.h
@@ -6,12 +6,7 @@

static int mlock2_(void *start, size_t len, int flags)
{
-#ifdef __NR_mlock2
return syscall(__NR_mlock2, start, len, flags);
-#else
- errno = ENOSYS;
- return -1;
-#endif
}

static FILE *seek_to_smaps_entry(unsigned long addr)
@@ -27,10 +22,8 @@ static FILE *seek_to_smaps_entry(unsigned long addr)
char path[BUFSIZ];

file = fopen("/proc/self/smaps", "r");
- if (!file) {
- perror("fopen smaps");
- _exit(1);
- }
+ if (!file)
+ ksft_exit_fail_msg("fopen smaps: %s\n", strerror(errno));

while (getline(&line, &size, file) > 0) {
if (sscanf(line, "%lx-%lx %s %lx %s %lu %s\n",
--
2.42.0

2024-02-02 11:55:49

by Muhammad Usama Anjum

[permalink] [raw]

Subject: [PATCH v2 06/12] selftests/mm: mrelease_test: conform test to TAP format output

2024-02-14 17:24:11

by Ryan Roberts

[permalink] [raw]

Subject: Re: [PATCH v2 09/12] selftests/mm: thp_settings: conform to TAP format output

On 02/02/2024 11:31, Muhammad Usama Anjum wrote:
> Conform the layout, informational and status messages to TAP. No
> functional change is intended other than the layout of output messages.
>
> Signed-off-by: Muhammad Usama Anjum <[email protected]>
> ---
> tools/testing/selftests/mm/khugepaged.c | 3 +-
> tools/testing/selftests/mm/thp_settings.c | 123 ++++++++--------------
> tools/testing/selftests/mm/thp_settings.h | 4 +-
> 3 files changed, 47 insertions(+), 83 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
> index d51fdaee7dc6a..3f202da0867c5 100644
> --- a/tools/testing/selftests/mm/khugepaged.c
> +++ b/tools/testing/selftests/mm/khugepaged.c
> @@ -152,8 +152,7 @@ static void get_finfo(const char *dir)
> major(path_stat.st_dev), minor(path_stat.st_dev)) >= sizeof(path))
> ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
>
> - if (read_file(path, buf, sizeof(buf)) < 0)
> - ksft_exit_fail_msg("read_file(read_num): %s\n", strerror(errno));
> + read_file(path, buf, sizeof(buf));
>
> if (strstr(buf, "DEVTYPE=disk")) {
> /* Found it */
> diff --git a/tools/testing/selftests/mm/thp_settings.c b/tools/testing/selftests/mm/thp_settings.c
> index a4163438108ec..273a95d025285 100644
> --- a/tools/testing/selftests/mm/thp_settings.c
> +++ b/tools/testing/selftests/mm/thp_settings.c
> @@ -5,7 +5,9 @@
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> +#include <errno.h>
>
> +#include "../kselftest.h"
> #include "thp_settings.h"
>
> #define THP_SYSFS "/sys/kernel/mm/transparent_hugepage/"
> @@ -42,58 +44,45 @@ static const char * const shmem_enabled_strings[] = {
> NULL
> };
>
> -int read_file(const char *path, char *buf, size_t buflen)
> +void read_file(const char *path, char *buf, size_t buflen)
> {
> int fd;
> ssize_t numread;
>
> fd = open(path, O_RDONLY);
> if (fd == -1)
> - return 0;
> + ksft_exit_fail_msg("%s open failed: %s\n", path, strerror(errno));

Hi,

This change has broken back compat. It's no longer possible to run cow and
khugepaged tests against older kernels.

This function, as well as others in this file are intended to return 0 to
indicate the file could not be accessed (e.g. doesn't exist or don't have
permission, etc). Then higher level code can decide how to handle that. For
example, thp_supported_orders() determines which THP orders are supported by the
system based on the existence of certain files in sysfs. Then cow decides which
test variants to run based on the supported orders. With your change, it all
goes bang on the first probe and the whole test suite gets failed without
running any tests.

I've no problem with improving the TAP output from tests, but this must only be
done at the test level, where it makes sense to do so. You can just call
ksft_exit_fail_msg() from deep within a utility function.

Please can we remove this from mm-unstable.

Thanks,
Ryan

>
> numread = read(fd, buf, buflen - 1);
> if (numread < 1) {
> close(fd);
> - return 0;
> + ksft_exit_fail_msg("No data read\n");
> }
>
> buf[numread] = '\0';
> close(fd);
> -
> - return (unsigned int) numread;
> }
>
> -int write_file(const char *path, const char *buf, size_t buflen)
> +void write_file(const char *path, const char *buf, size_t buflen)
> {
> int fd;
> ssize_t numwritten;
>
> fd = open(path, O_WRONLY);
> - if (fd == -1) {
> - printf("open(%s)\n", path);
> - exit(EXIT_FAILURE);
> - return 0;
> - }
> + if (fd == -1)
> + ksft_exit_fail_msg("%s open failed\n", path);
>
> numwritten = write(fd, buf, buflen - 1);
> close(fd);
> - if (numwritten < 1) {
> - printf("write(%s)\n", buf);
> - exit(EXIT_FAILURE);
> - return 0;
> - }
> -
> - return (unsigned int) numwritten;
> + if (numwritten < 1)
> + ksft_exit_fail_msg("write failed (%s)\n", buf);
> }
>
> const unsigned long read_num(const char *path)
> {
> char buf[21];
>
> - if (read_file(path, buf, sizeof(buf)) < 0) {
> - perror("read_file()");
> - exit(EXIT_FAILURE);
> - }
> + read_file(path, buf, sizeof(buf));
>
> return strtoul(buf, NULL, 10);
> }
> @@ -103,10 +92,7 @@ void write_num(const char *path, unsigned long num)
> char buf[21];
>
> sprintf(buf, "%ld", num);
> - if (!write_file(path, buf, strlen(buf) + 1)) {
> - perror(path);
> - exit(EXIT_FAILURE);
> - }
> + write_file(path, buf, strlen(buf) + 1);
> }
>
> int thp_read_string(const char *name, const char * const strings[])
> @@ -117,30 +103,22 @@ int thp_read_string(const char *name, const char * const strings[])
> int ret;
>
> ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name);
> - if (ret >= PATH_MAX) {
> - printf("%s: Pathname is too long\n", __func__);
> - exit(EXIT_FAILURE);
> - }
> + if (ret >= PATH_MAX)
> + ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
>
> - if (!read_file(path, buf, sizeof(buf))) {
> - perror(path);
> - exit(EXIT_FAILURE);
> - }
> + read_file(path, buf, sizeof(buf));
>
> c = strchr(buf, '[');
> - if (!c) {
> - printf("%s: Parse failure\n", __func__);
> - exit(EXIT_FAILURE);
> - }
> + if (!c)
> + ksft_exit_fail_msg("%s: Parse failure\n", __func__);
>
> c++;
> memmove(buf, c, sizeof(buf) - (c - buf));
>
> c = strchr(buf, ']');
> - if (!c) {
> - printf("%s: Parse failure\n", __func__);
> - exit(EXIT_FAILURE);
> - }
> + if (!c)
> + ksft_exit_fail_msg("%s: Parse failure\n", __func__);
> +
> *c = '\0';
>
> ret = 0;
> @@ -150,8 +128,8 @@ int thp_read_string(const char *name, const char * const strings[])
> ret++;
> }
>
> - printf("Failed to parse %s\n", name);
> - exit(EXIT_FAILURE);
> + ksft_exit_fail_msg("Failed to parse %s\n", name);
> + return -1;
> }
>
> void thp_write_string(const char *name, const char *val)
> @@ -160,15 +138,10 @@ void thp_write_string(const char *name, const char *val)
> int ret;
>
> ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name);
> - if (ret >= PATH_MAX) {
> - printf("%s: Pathname is too long\n", __func__);
> - exit(EXIT_FAILURE);
> - }
> + if (ret >= PATH_MAX)
> + ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
>
> - if (!write_file(path, val, strlen(val) + 1)) {
> - perror(path);
> - exit(EXIT_FAILURE);
> - }
> + write_file(path, val, strlen(val) + 1);
> }
>
> const unsigned long thp_read_num(const char *name)
> @@ -177,10 +150,9 @@ const unsigned long thp_read_num(const char *name)
> int ret;
>
> ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name);
> - if (ret >= PATH_MAX) {
> - printf("%s: Pathname is too long\n", __func__);
> - exit(EXIT_FAILURE);
> - }
> + if (ret >= PATH_MAX)
> + ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
> +
> return read_num(path);
> }
>
> @@ -190,10 +162,9 @@ void thp_write_num(const char *name, unsigned long num)
> int ret;
>
> ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name);
> - if (ret >= PATH_MAX) {
> - printf("%s: Pathname is too long\n", __func__);
> - exit(EXIT_FAILURE);
> - }
> + if (ret >= PATH_MAX)
> + ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
> +
> write_num(path, num);
> }
>
> @@ -275,29 +246,26 @@ void thp_write_settings(struct thp_settings *settings)
>
> struct thp_settings *thp_current_settings(void)
> {
> - if (!settings_index) {
> - printf("Fail: No settings set");
> - exit(EXIT_FAILURE);
> - }
> + if (!settings_index)
> + ksft_exit_fail_msg("Fail: No settings set\n");
> +
> return settings_stack + settings_index - 1;
> }
>
> void thp_push_settings(struct thp_settings *settings)
> {
> - if (settings_index >= MAX_SETTINGS_DEPTH) {
> - printf("Fail: Settings stack exceeded");
> - exit(EXIT_FAILURE);
> - }
> + if (settings_index >= MAX_SETTINGS_DEPTH)
> + ksft_exit_fail_msg("Fail: Settings stack exceeded\n");
> +
> settings_stack[settings_index++] = *settings;
> thp_write_settings(thp_current_settings());
> }
>
> void thp_pop_settings(void)
> {
> - if (settings_index <= 0) {
> - printf("Fail: Settings stack empty");
> - exit(EXIT_FAILURE);
> - }
> + if (settings_index <= 0)
> + ksft_exit_fail_msg("Fail: Settings stack empty\n");
> +
> --settings_index;
> thp_write_settings(thp_current_settings());
> }
> @@ -335,14 +303,11 @@ unsigned long thp_supported_orders(void)
> for (i = 0; i < NR_ORDERS; i++) {
> ret = snprintf(path, PATH_MAX, THP_SYSFS "hugepages-%ukB/enabled",
> (getpagesize() >> 10) << i);
> - if (ret >= PATH_MAX) {
> - printf("%s: Pathname is too long\n", __func__);
> - exit(EXIT_FAILURE);
> - }
> + if (ret >= PATH_MAX)
> + ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
>
> - ret = read_file(path, buf, sizeof(buf));
> - if (ret)
> - orders |= 1UL << i;
> + read_file(path, buf, sizeof(buf));
> + orders |= 1UL << i;
> }
>
> return orders;
> diff --git a/tools/testing/selftests/mm/thp_settings.h b/tools/testing/selftests/mm/thp_settings.h
> index 71cbff05f4c7f..04a6a7bbd08f8 100644
> --- a/tools/testing/selftests/mm/thp_settings.h
> +++ b/tools/testing/selftests/mm/thp_settings.h
> @@ -56,8 +56,8 @@ struct thp_settings {
> struct hugepages_settings hugepages[NR_ORDERS];
> };
>
> -int read_file(const char *path, char *buf, size_t buflen);
> -int write_file(const char *path, const char *buf, size_t buflen);
> +void read_file(const char *path, char *buf, size_t buflen);
> +void write_file(const char *path, const char *buf, size_t buflen);
> const unsigned long read_num(const char *path);
> void write_num(const char *path, unsigned long num);
>

2024-02-14 18:16:16

by Ryan Roberts

[permalink] [raw]

Subject: Re: [PATCH v2 09/12] selftests/mm: thp_settings: conform to TAP format output

On 14/02/2024 17:19, Ryan Roberts wrote:
> On 02/02/2024 11:31, Muhammad Usama Anjum wrote:
>> Conform the layout, informational and status messages to TAP. No
>> functional change is intended other than the layout of output messages.
>>
>> Signed-off-by: Muhammad Usama Anjum <[email protected]>
>> ---
>> tools/testing/selftests/mm/khugepaged.c | 3 +-
>> tools/testing/selftests/mm/thp_settings.c | 123 ++++++++--------------
>> tools/testing/selftests/mm/thp_settings.h | 4 +-
>> 3 files changed, 47 insertions(+), 83 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
>> index d51fdaee7dc6a..3f202da0867c5 100644
>> --- a/tools/testing/selftests/mm/khugepaged.c
>> +++ b/tools/testing/selftests/mm/khugepaged.c
>> @@ -152,8 +152,7 @@ static void get_finfo(const char *dir)
>> major(path_stat.st_dev), minor(path_stat.st_dev)) >= sizeof(path))
>> ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
>>
>> - if (read_file(path, buf, sizeof(buf)) < 0)
>> - ksft_exit_fail_msg("read_file(read_num): %s\n", strerror(errno));
>> + read_file(path, buf, sizeof(buf));
>>
>> if (strstr(buf, "DEVTYPE=disk")) {
>> /* Found it */
>> diff --git a/tools/testing/selftests/mm/thp_settings.c b/tools/testing/selftests/mm/thp_settings.c
>> index a4163438108ec..273a95d025285 100644
>> --- a/tools/testing/selftests/mm/thp_settings.c
>> +++ b/tools/testing/selftests/mm/thp_settings.c
>> @@ -5,7 +5,9 @@
>> #include <stdlib.h>
>> #include <string.h>
>> #include <unistd.h>
>> +#include <errno.h>
>>
>> +#include "../kselftest.h"
>> #include "thp_settings.h"
>>
>> #define THP_SYSFS "/sys/kernel/mm/transparent_hugepage/"
>> @@ -42,58 +44,45 @@ static const char * const shmem_enabled_strings[] = {
>> NULL
>> };
>>
>> -int read_file(const char *path, char *buf, size_t buflen)
>> +void read_file(const char *path, char *buf, size_t buflen)
>> {
>> int fd;
>> ssize_t numread;
>>
>> fd = open(path, O_RDONLY);
>> if (fd == -1)
>> - return 0;
>> + ksft_exit_fail_msg("%s open failed: %s\n", path, strerror(errno));
>
> Hi,
>
> This change has broken back compat. It's no longer possible to run cow and
> khugepaged tests against older kernels.
>
> This function, as well as others in this file are intended to return 0 to
> indicate the file could not be accessed (e.g. doesn't exist or don't have
> permission, etc). Then higher level code can decide how to handle that. For
> example, thp_supported_orders() determines which THP orders are supported by the
> system based on the existence of certain files in sysfs. Then cow decides which
> test variants to run based on the supported orders. With your change, it all
> goes bang on the first probe and the whole test suite gets failed without
> running any tests.
>
> I've no problem with improving the TAP output from tests, but this must only be
> done at the test level, where it makes sense to do so. You can just call
> ksft_exit_fail_msg() from deep within a utility function.
>
> Please can we remove this from mm-unstable.

Actually it is even worse than I first thought. With this change, the tests fail
even when running against a matched kernel.

thp_supported_orders() tries to probe every possible order and relies on the
probe succeeding or failing to determine which orders are actually supported.
The first order it tries is 0 (4K). That is not THP, so there is no
hugepages-4K/enabled file for it. So it blows up immediately.

So I don't think you even ran the tests locally after making the change?

>
> Thanks,
> Ryan
>
>
>>
>> numread = read(fd, buf, buflen - 1);
>> if (numread < 1) {
>> close(fd);
>> - return 0;
>> + ksft_exit_fail_msg("No data read\n");
>> }
>>
>> buf[numread] = '\0';
>> close(fd);
>> -
>> - return (unsigned int) numread;
>> }
>>
>> -int write_file(const char *path, const char *buf, size_t buflen)
>> +void write_file(const char *path, const char *buf, size_t buflen)
>> {
>> int fd;
>> ssize_t numwritten;
>>
>> fd = open(path, O_WRONLY);
>> - if (fd == -1) {
>> - printf("open(%s)\n", path);
>> - exit(EXIT_FAILURE);
>> - return 0;
>> - }
>> + if (fd == -1)
>> + ksft_exit_fail_msg("%s open failed\n", path);
>>
>> numwritten = write(fd, buf, buflen - 1);
>> close(fd);
>> - if (numwritten < 1) {
>> - printf("write(%s)\n", buf);
>> - exit(EXIT_FAILURE);
>> - return 0;
>> - }
>> -
>> - return (unsigned int) numwritten;
>> + if (numwritten < 1)
>> + ksft_exit_fail_msg("write failed (%s)\n", buf);
>> }
>>
>> const unsigned long read_num(const char *path)
>> {
>> char buf[21];
>>
>> - if (read_file(path, buf, sizeof(buf)) < 0) {
>> - perror("read_file()");
>> - exit(EXIT_FAILURE);
>> - }
>> + read_file(path, buf, sizeof(buf));
>>
>> return strtoul(buf, NULL, 10);
>> }
>> @@ -103,10 +92,7 @@ void write_num(const char *path, unsigned long num)
>> char buf[21];
>>
>> sprintf(buf, "%ld", num);
>> - if (!write_file(path, buf, strlen(buf) + 1)) {
>> - perror(path);
>> - exit(EXIT_FAILURE);
>> - }
>> + write_file(path, buf, strlen(buf) + 1);
>> }
>>
>> int thp_read_string(const char *name, const char * const strings[])
>> @@ -117,30 +103,22 @@ int thp_read_string(const char *name, const char * const strings[])
>> int ret;
>>
>> ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name);
>> - if (ret >= PATH_MAX) {
>> - printf("%s: Pathname is too long\n", __func__);
>> - exit(EXIT_FAILURE);
>> - }
>> + if (ret >= PATH_MAX)
>> + ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
>>
>> - if (!read_file(path, buf, sizeof(buf))) {
>> - perror(path);
>> - exit(EXIT_FAILURE);
>> - }
>> + read_file(path, buf, sizeof(buf));
>>
>> c = strchr(buf, '[');
>> - if (!c) {
>> - printf("%s: Parse failure\n", __func__);
>> - exit(EXIT_FAILURE);
>> - }
>> + if (!c)
>> + ksft_exit_fail_msg("%s: Parse failure\n", __func__);
>>
>> c++;
>> memmove(buf, c, sizeof(buf) - (c - buf));
>>
>> c = strchr(buf, ']');
>> - if (!c) {
>> - printf("%s: Parse failure\n", __func__);
>> - exit(EXIT_FAILURE);
>> - }
>> + if (!c)
>> + ksft_exit_fail_msg("%s: Parse failure\n", __func__);
>> +
>> *c = '\0';
>>
>> ret = 0;
>> @@ -150,8 +128,8 @@ int thp_read_string(const char *name, const char * const strings[])
>> ret++;
>> }
>>
>> - printf("Failed to parse %s\n", name);
>> - exit(EXIT_FAILURE);
>> + ksft_exit_fail_msg("Failed to parse %s\n", name);
>> + return -1;
>> }
>>
>> void thp_write_string(const char *name, const char *val)
>> @@ -160,15 +138,10 @@ void thp_write_string(const char *name, const char *val)
>> int ret;
>>
>> ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name);
>> - if (ret >= PATH_MAX) {
>> - printf("%s: Pathname is too long\n", __func__);
>> - exit(EXIT_FAILURE);
>> - }
>> + if (ret >= PATH_MAX)
>> + ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
>>
>> - if (!write_file(path, val, strlen(val) + 1)) {
>> - perror(path);
>> - exit(EXIT_FAILURE);
>> - }
>> + write_file(path, val, strlen(val) + 1);
>> }
>>
>> const unsigned long thp_read_num(const char *name)
>> @@ -177,10 +150,9 @@ const unsigned long thp_read_num(const char *name)
>> int ret;
>>
>> ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name);
>> - if (ret >= PATH_MAX) {
>> - printf("%s: Pathname is too long\n", __func__);
>> - exit(EXIT_FAILURE);
>> - }
>> + if (ret >= PATH_MAX)
>> + ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
>> +
>> return read_num(path);
>> }
>>
>> @@ -190,10 +162,9 @@ void thp_write_num(const char *name, unsigned long num)
>> int ret;
>>
>> ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name);
>> - if (ret >= PATH_MAX) {
>> - printf("%s: Pathname is too long\n", __func__);
>> - exit(EXIT_FAILURE);
>> - }
>> + if (ret >= PATH_MAX)
>> + ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
>> +
>> write_num(path, num);
>> }
>>
>> @@ -275,29 +246,26 @@ void thp_write_settings(struct thp_settings *settings)
>>
>> struct thp_settings *thp_current_settings(void)
>> {
>> - if (!settings_index) {
>> - printf("Fail: No settings set");
>> - exit(EXIT_FAILURE);
>> - }
>> + if (!settings_index)
>> + ksft_exit_fail_msg("Fail: No settings set\n");
>> +
>> return settings_stack + settings_index - 1;
>> }
>>
>> void thp_push_settings(struct thp_settings *settings)
>> {
>> - if (settings_index >= MAX_SETTINGS_DEPTH) {
>> - printf("Fail: Settings stack exceeded");
>> - exit(EXIT_FAILURE);
>> - }
>> + if (settings_index >= MAX_SETTINGS_DEPTH)
>> + ksft_exit_fail_msg("Fail: Settings stack exceeded\n");
>> +
>> settings_stack[settings_index++] = *settings;
>> thp_write_settings(thp_current_settings());
>> }
>>
>> void thp_pop_settings(void)
>> {
>> - if (settings_index <= 0) {
>> - printf("Fail: Settings stack empty");
>> - exit(EXIT_FAILURE);
>> - }
>> + if (settings_index <= 0)
>> + ksft_exit_fail_msg("Fail: Settings stack empty\n");
>> +
>> --settings_index;
>> thp_write_settings(thp_current_settings());
>> }
>> @@ -335,14 +303,11 @@ unsigned long thp_supported_orders(void)
>> for (i = 0; i < NR_ORDERS; i++) {
>> ret = snprintf(path, PATH_MAX, THP_SYSFS "hugepages-%ukB/enabled",
>> (getpagesize() >> 10) << i);
>> - if (ret >= PATH_MAX) {
>> - printf("%s: Pathname is too long\n", __func__);
>> - exit(EXIT_FAILURE);
>> - }
>> + if (ret >= PATH_MAX)
>> + ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
>>
>> - ret = read_file(path, buf, sizeof(buf));
>> - if (ret)
>> - orders |= 1UL << i;
>> + read_file(path, buf, sizeof(buf));
>> + orders |= 1UL << i;
>> }
>>
>> return orders;
>> diff --git a/tools/testing/selftests/mm/thp_settings.h b/tools/testing/selftests/mm/thp_settings.h
>> index 71cbff05f4c7f..04a6a7bbd08f8 100644
>> --- a/tools/testing/selftests/mm/thp_settings.h
>> +++ b/tools/testing/selftests/mm/thp_settings.h
>> @@ -56,8 +56,8 @@ struct thp_settings {
>> struct hugepages_settings hugepages[NR_ORDERS];
>> };
>>
>> -int read_file(const char *path, char *buf, size_t buflen);
>> -int write_file(const char *path, const char *buf, size_t buflen);
>> +void read_file(const char *path, char *buf, size_t buflen);
>> +void write_file(const char *path, const char *buf, size_t buflen);
>> const unsigned long read_num(const char *path);
>> void write_num(const char *path, unsigned long num);
>>
>

2024-02-15 04:59:51

by Muhammad Usama Anjum

[permalink] [raw]

Subject: Re: [PATCH v2 09/12] selftests/mm: thp_settings: conform to TAP format output

On 2/14/24 10:19 PM, Ryan Roberts wrote:
> On 02/02/2024 11:31, Muhammad Usama Anjum wrote:
>> Conform the layout, informational and status messages to TAP. No
>> functional change is intended other than the layout of output messages.
>>
>> Signed-off-by: Muhammad Usama Anjum <[email protected]>
>> ---
>> tools/testing/selftests/mm/khugepaged.c | 3 +-
>> tools/testing/selftests/mm/thp_settings.c | 123 ++++++++--------------
>> tools/testing/selftests/mm/thp_settings.h | 4 +-
>> 3 files changed, 47 insertions(+), 83 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
>> index d51fdaee7dc6a..3f202da0867c5 100644
>> --- a/tools/testing/selftests/mm/khugepaged.c
>> +++ b/tools/testing/selftests/mm/khugepaged.c
>> @@ -152,8 +152,7 @@ static void get_finfo(const char *dir)
>> major(path_stat.st_dev), minor(path_stat.st_dev)) >= sizeof(path))
>> ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
>>
>> - if (read_file(path, buf, sizeof(buf)) < 0)
>> - ksft_exit_fail_msg("read_file(read_num): %s\n", strerror(errno));
>> + read_file(path, buf, sizeof(buf));
>>
>> if (strstr(buf, "DEVTYPE=disk")) {
>> /* Found it */
>> diff --git a/tools/testing/selftests/mm/thp_settings.c b/tools/testing/selftests/mm/thp_settings.c
>> index a4163438108ec..273a95d025285 100644
>> --- a/tools/testing/selftests/mm/thp_settings.c
>> +++ b/tools/testing/selftests/mm/thp_settings.c
>> @@ -5,7 +5,9 @@
>> #include <stdlib.h>
>> #include <string.h>
>> #include <unistd.h>
>> +#include <errno.h>
>>
>> +#include "../kselftest.h"
>> #include "thp_settings.h"
>>
>> #define THP_SYSFS "/sys/kernel/mm/transparent_hugepage/"
>> @@ -42,58 +44,45 @@ static const char * const shmem_enabled_strings[] = {
>> NULL
>> };
>>
>> -int read_file(const char *path, char *buf, size_t buflen)
>> +void read_file(const char *path, char *buf, size_t buflen)
>> {
>> int fd;
>> ssize_t numread;
>>
>> fd = open(path, O_RDONLY);
>> if (fd == -1)
>> - return 0;
>> + ksft_exit_fail_msg("%s open failed: %s\n", path, strerror(errno));
>
> Hi,
>
> This change has broken back compat. It's no longer possible to run cow and
> khugepaged tests against older kernels.
>
> This function, as well as others in this file are intended to return 0 to
> indicate the file could not be accessed (e.g. doesn't exist or don't have
> permission, etc). Then higher level code can decide how to handle that. For
> example, thp_supported_orders() determines which THP orders are supported by the
> system based on the existence of certain files in sysfs. Then cow decides which
> test variants to run based on the supported orders. With your change, it all
> goes bang on the first probe and the whole test suite gets failed without
> running any tests.
>
> I've no problem with improving the TAP output from tests, but this must only be
> done at the test level, where it makes sense to do so. You can just call
> ksft_exit_fail_msg() from deep within a utility function.
>
> Please can we remove this from mm-unstable.
Sorry, not sure how I missed this. Let's drop this patch entirely.

>
> Thanks,
> Ryan
>
>
>>
>> numread = read(fd, buf, buflen - 1);
>> if (numread < 1) {
>> close(fd);
>> - return 0;
>> + ksft_exit_fail_msg("No data read\n");
>> }
>>
>> buf[numread] = '\0';
>> close(fd);
>> -
>> - return (unsigned int) numread;
>> }
>>
>> -int write_file(const char *path, const char *buf, size_t buflen)
>> +void write_file(const char *path, const char *buf, size_t buflen)
>> {
>> int fd;
>> ssize_t numwritten;
>>
>> fd = open(path, O_WRONLY);
>> - if (fd == -1) {
>> - printf("open(%s)\n", path);
>> - exit(EXIT_FAILURE);
>> - return 0;
>> - }
>> + if (fd == -1)
>> + ksft_exit_fail_msg("%s open failed\n", path);
>>
>> numwritten = write(fd, buf, buflen - 1);
>> close(fd);
>> - if (numwritten < 1) {
>> - printf("write(%s)\n", buf);
>> - exit(EXIT_FAILURE);
>> - return 0;
>> - }
>> -
>> - return (unsigned int) numwritten;
>> + if (numwritten < 1)
>> + ksft_exit_fail_msg("write failed (%s)\n", buf);
>> }
>>
>> const unsigned long read_num(const char *path)
>> {
>> char buf[21];
>>
>> - if (read_file(path, buf, sizeof(buf)) < 0) {
>> - perror("read_file()");
>> - exit(EXIT_FAILURE);
>> - }
>> + read_file(path, buf, sizeof(buf));
>>
>> return strtoul(buf, NULL, 10);
>> }
>> @@ -103,10 +92,7 @@ void write_num(const char *path, unsigned long num)
>> char buf[21];
>>
>> sprintf(buf, "%ld", num);
>> - if (!write_file(path, buf, strlen(buf) + 1)) {
>> - perror(path);
>> - exit(EXIT_FAILURE);
>> - }
>> + write_file(path, buf, strlen(buf) + 1);
>> }
>>
>> int thp_read_string(const char *name, const char * const strings[])
>> @@ -117,30 +103,22 @@ int thp_read_string(const char *name, const char * const strings[])
>> int ret;
>>
>> ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name);
>> - if (ret >= PATH_MAX) {
>> - printf("%s: Pathname is too long\n", __func__);
>> - exit(EXIT_FAILURE);
>> - }
>> + if (ret >= PATH_MAX)
>> + ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
>>
>> - if (!read_file(path, buf, sizeof(buf))) {
>> - perror(path);
>> - exit(EXIT_FAILURE);
>> - }
>> + read_file(path, buf, sizeof(buf));
>>
>> c = strchr(buf, '[');
>> - if (!c) {
>> - printf("%s: Parse failure\n", __func__);
>> - exit(EXIT_FAILURE);
>> - }
>> + if (!c)
>> + ksft_exit_fail_msg("%s: Parse failure\n", __func__);
>>
>> c++;
>> memmove(buf, c, sizeof(buf) - (c - buf));
>>
>> c = strchr(buf, ']');
>> - if (!c) {
>> - printf("%s: Parse failure\n", __func__);
>> - exit(EXIT_FAILURE);
>> - }
>> + if (!c)
>> + ksft_exit_fail_msg("%s: Parse failure\n", __func__);
>> +
>> *c = '\0';
>>
>> ret = 0;
>> @@ -150,8 +128,8 @@ int thp_read_string(const char *name, const char * const strings[])
>> ret++;
>> }
>>
>> - printf("Failed to parse %s\n", name);
>> - exit(EXIT_FAILURE);
>> + ksft_exit_fail_msg("Failed to parse %s\n", name);
>> + return -1;
>> }
>>
>> void thp_write_string(const char *name, const char *val)
>> @@ -160,15 +138,10 @@ void thp_write_string(const char *name, const char *val)
>> int ret;
>>
>> ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name);
>> - if (ret >= PATH_MAX) {
>> - printf("%s: Pathname is too long\n", __func__);
>> - exit(EXIT_FAILURE);
>> - }
>> + if (ret >= PATH_MAX)
>> + ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
>>
>> - if (!write_file(path, val, strlen(val) + 1)) {
>> - perror(path);
>> - exit(EXIT_FAILURE);
>> - }
>> + write_file(path, val, strlen(val) + 1);
>> }
>>
>> const unsigned long thp_read_num(const char *name)
>> @@ -177,10 +150,9 @@ const unsigned long thp_read_num(const char *name)
>> int ret;
>>
>> ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name);
>> - if (ret >= PATH_MAX) {
>> - printf("%s: Pathname is too long\n", __func__);
>> - exit(EXIT_FAILURE);
>> - }
>> + if (ret >= PATH_MAX)
>> + ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
>> +
>> return read_num(path);
>> }
>>
>> @@ -190,10 +162,9 @@ void thp_write_num(const char *name, unsigned long num)
>> int ret;
>>
>> ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name);
>> - if (ret >= PATH_MAX) {
>> - printf("%s: Pathname is too long\n", __func__);
>> - exit(EXIT_FAILURE);
>> - }
>> + if (ret >= PATH_MAX)
>> + ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
>> +
>> write_num(path, num);
>> }
>>
>> @@ -275,29 +246,26 @@ void thp_write_settings(struct thp_settings *settings)
>>
>> struct thp_settings *thp_current_settings(void)
>> {
>> - if (!settings_index) {
>> - printf("Fail: No settings set");
>> - exit(EXIT_FAILURE);
>> - }
>> + if (!settings_index)
>> + ksft_exit_fail_msg("Fail: No settings set\n");
>> +
>> return settings_stack + settings_index - 1;
>> }
>>
>> void thp_push_settings(struct thp_settings *settings)
>> {
>> - if (settings_index >= MAX_SETTINGS_DEPTH) {
>> - printf("Fail: Settings stack exceeded");
>> - exit(EXIT_FAILURE);
>> - }
>> + if (settings_index >= MAX_SETTINGS_DEPTH)
>> + ksft_exit_fail_msg("Fail: Settings stack exceeded\n");
>> +
>> settings_stack[settings_index++] = *settings;
>> thp_write_settings(thp_current_settings());
>> }
>>
>> void thp_pop_settings(void)
>> {
>> - if (settings_index <= 0) {
>> - printf("Fail: Settings stack empty");
>> - exit(EXIT_FAILURE);
>> - }
>> + if (settings_index <= 0)
>> + ksft_exit_fail_msg("Fail: Settings stack empty\n");
>> +
>> --settings_index;
>> thp_write_settings(thp_current_settings());
>> }
>> @@ -335,14 +303,11 @@ unsigned long thp_supported_orders(void)
>> for (i = 0; i < NR_ORDERS; i++) {
>> ret = snprintf(path, PATH_MAX, THP_SYSFS "hugepages-%ukB/enabled",
>> (getpagesize() >> 10) << i);
>> - if (ret >= PATH_MAX) {
>> - printf("%s: Pathname is too long\n", __func__);
>> - exit(EXIT_FAILURE);
>> - }
>> + if (ret >= PATH_MAX)
>> + ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
>>
>> - ret = read_file(path, buf, sizeof(buf));
>> - if (ret)
>> - orders |= 1UL << i;
>> + read_file(path, buf, sizeof(buf));
>> + orders |= 1UL << i;
>> }
>>
>> return orders;
>> diff --git a/tools/testing/selftests/mm/thp_settings.h b/tools/testing/selftests/mm/thp_settings.h
>> index 71cbff05f4c7f..04a6a7bbd08f8 100644
>> --- a/tools/testing/selftests/mm/thp_settings.h
>> +++ b/tools/testing/selftests/mm/thp_settings.h
>> @@ -56,8 +56,8 @@ struct thp_settings {
>> struct hugepages_settings hugepages[NR_ORDERS];
>> };
>>
>> -int read_file(const char *path, char *buf, size_t buflen);
>> -int write_file(const char *path, const char *buf, size_t buflen);
>> +void read_file(const char *path, char *buf, size_t buflen);
>> +void write_file(const char *path, const char *buf, size_t buflen);
>> const unsigned long read_num(const char *path);
>> void write_num(const char *path, unsigned long num);
>>
>
>

--
BR,
Muhammad Usama Anjum

2024-03-14 05:01:00

by Dev Jain

[permalink] [raw]

Subject: Re: [PATCH v2 12/12] selftests/mm: virtual_address_range: conform to TAP format output

On 2/2/24 17:01, Muhammad Usama Anjum wrote:
> Conform the layout, informational and status messages to TAP. No
> functional change is intended other than the layout of output messages.
>
> Signed-off-by: Muhammad Usama Anjum <[email protected]>
> ---
> .../selftests/mm/virtual_address_range.c | 44 +++++++++----------
> 1 file changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/virtual_address_range.c b/tools/testing/selftests/mm/virtual_address_range.c
> index bae0ceaf95b13..7bcf8d48256a6 100644
> --- a/tools/testing/selftests/mm/virtual_address_range.c
> +++ b/tools/testing/selftests/mm/virtual_address_range.c
> @@ -12,6 +12,7 @@
> #include <errno.h>
> #include <sys/mman.h>
> #include <sys/time.h>
> +#include "../kselftest.h"
>
> /*
> * Maximum address range mapped with a single mmap()
> @@ -68,23 +69,15 @@ static char *hind_addr(void)
> return (char *) (1UL << bits);
> }
>
> -static int validate_addr(char *ptr, int high_addr)
> +static void validate_addr(char *ptr, int high_addr)
> {
> unsigned long addr = (unsigned long) ptr;
>
> - if (high_addr) {
> - if (addr < HIGH_ADDR_MARK) {
> - printf("Bad address %lx\n", addr);
> - return 1;
> - }
> - return 0;
> - }
> + if (high_addr && addr < HIGH_ADDR_MARK)
> + ksft_exit_fail_msg("Bad address %lx\n", addr);
>
> - if (addr > HIGH_ADDR_MARK) {
> - printf("Bad address %lx\n", addr);
> - return 1;
> - }
> - return 0;
> + if (addr > HIGH_ADDR_MARK)
> + ksft_exit_fail_msg("Bad address %lx\n", addr);
> }
>
> static int validate_lower_address_hint(void)
> @@ -107,23 +100,29 @@ int main(int argc, char *argv[])
> char *hint;
> unsigned long i, lchunks, hchunks;
>
> + ksft_print_header();
> + ksft_set_plan(1);
> +
> for (i = 0; i < NR_CHUNKS_LOW; i++) {
> ptr[i] = mmap(NULL, MAP_CHUNK_SIZE, PROT_READ | PROT_WRITE,
> MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>
> if (ptr[i] == MAP_FAILED) {
> - if (validate_lower_address_hint())
> - return 1;
> + if (validate_lower_address_hint()) {
> + ksft_test_result_skip("Memory constraint not fulfilled\n");
> + ksft_finished();
> + }

Hi,

When validate_lower_address_hint() returns 1, it implies that despite
filling the lower

range, mmap succeeded. IMHO, ksft_exit_fail_msg() should be used
instead, with a

more descriptive message indicating that the memory was unexpectedly
allocated.

Regards

Dev

> break;
> }
>
> - if (validate_addr(ptr[i], 0))
> - return 1;
> + validate_addr(ptr[i], 0);
> }
> lchunks = i;
> hptr = (char **) calloc(NR_CHUNKS_HIGH, sizeof(char *));
> - if (hptr == NULL)
> - return 1;
> + if (hptr == NULL) {
> + ksft_test_result_skip("Memory constraint not fulfilled\n");
> + ksft_finished();
> + }
>
> for (i = 0; i < NR_CHUNKS_HIGH; i++) {
> hint = hind_addr();
> @@ -133,8 +132,7 @@ int main(int argc, char *argv[])
> if (hptr[i] == MAP_FAILED)
> break;
>
> - if (validate_addr(hptr[i], 1))
> - return 1;
> + validate_addr(hptr[i], 1);
> }
> hchunks = i;
>
> @@ -145,5 +143,7 @@ int main(int argc, char *argv[])
> munmap(hptr[i], MAP_CHUNK_SIZE);
>
> free(hptr);
> - return 0;
> +
> + ksft_test_result_pass("Test\n");
> + ksft_finished();
> }

2024-03-14 08:50:26

by Muhammad Usama Anjum

[permalink] [raw]

Subject: Re: [PATCH v2 12/12] selftests/mm: virtual_address_range: conform to TAP format output

On 3/14/24 10:00 AM, Dev Jain wrote:
>
> On 2/2/24 17:01, Muhammad Usama Anjum wrote:
>> Conform the layout, informational and status messages to TAP. No
>> functional change is intended other than the layout of output messages.
>>
>> Signed-off-by: Muhammad Usama Anjum <[email protected]>
>> ---
>> .../selftests/mm/virtual_address_range.c      | 44 +++++++++----------
>> 1 file changed, 22 insertions(+), 22 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mm/virtual_address_range.c
>> b/tools/testing/selftests/mm/virtual_address_range.c
>> index bae0ceaf95b13..7bcf8d48256a6 100644
>> --- a/tools/testing/selftests/mm/virtual_address_range.c
>> +++ b/tools/testing/selftests/mm/virtual_address_range.c
>> @@ -12,6 +12,7 @@
>> #include <errno.h>
>> #include <sys/mman.h>
>> #include <sys/time.h>
>> +#include "../kselftest.h"
>> /*
>>    * Maximum address range mapped with a single mmap()
>> @@ -68,23 +69,15 @@ static char *hind_addr(void)
>>       return (char *) (1UL << bits);
>> }
>> -static int validate_addr(char *ptr, int high_addr)
>> +static void validate_addr(char *ptr, int high_addr)
>> {
>>       unsigned long addr = (unsigned long) ptr;
>> -    if (high_addr) {
>> -        if (addr < HIGH_ADDR_MARK) {
>> -            printf("Bad address %lx\n", addr);
>> -            return 1;
>> -        }
>> -        return 0;
>> -    }
>> +    if (high_addr && addr < HIGH_ADDR_MARK)
>> +        ksft_exit_fail_msg("Bad address %lx\n", addr);
>> -    if (addr > HIGH_ADDR_MARK) {
>> -        printf("Bad address %lx\n", addr);
>> -        return 1;
>> -    }
>> -    return 0;
>> +    if (addr > HIGH_ADDR_MARK)
>> +        ksft_exit_fail_msg("Bad address %lx\n", addr);
>> }
>> static int validate_lower_address_hint(void)
>> @@ -107,23 +100,29 @@ int main(int argc, char *argv[])
>>       char *hint;
>>       unsigned long i, lchunks, hchunks;
>> +    ksft_print_header();
>> +    ksft_set_plan(1);
>> +
>>       for (i = 0; i < NR_CHUNKS_LOW; i++) {
>>           ptr[i] = mmap(NULL, MAP_CHUNK_SIZE, PROT_READ | PROT_WRITE,
>>                       MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>>           if (ptr[i] == MAP_FAILED) {
>> -            if (validate_lower_address_hint())
>> -                return 1;
>> +            if (validate_lower_address_hint()) {
>> +                ksft_test_result_skip("Memory constraint not fulfilled\n");
>> +                ksft_finished();
>> +            }
>
> Hi,
>
> When validate_lower_address_hint() returns 1, it implies that despite
> filling the lower
>
> range, mmap succeeded. IMHO, ksft_exit_fail_msg() should be used instead,
> with a
>
> more descriptive message indicating that the memory was unexpectedly
> allocated.
Can you fire up a patch for this? Otherwise I'll get back to it next week.

>
>
> Regards
>
> Dev
>
>>               break;
>>           }
>> -        if (validate_addr(ptr[i], 0))
>> -            return 1;
>> +        validate_addr(ptr[i], 0);
>>       }
>>       lchunks = i;
>>       hptr = (char **) calloc(NR_CHUNKS_HIGH, sizeof(char *));
>> -    if (hptr == NULL)
>> -        return 1;
>> +    if (hptr == NULL) {
>> +        ksft_test_result_skip("Memory constraint not fulfilled\n");
>> +        ksft_finished();
>> +    }
>>       for (i = 0; i < NR_CHUNKS_HIGH; i++) {
>>           hint = hind_addr();
>> @@ -133,8 +132,7 @@ int main(int argc, char *argv[])
>>           if (hptr[i] == MAP_FAILED)
>>               break;
>> -        if (validate_addr(hptr[i], 1))
>> -            return 1;
>> +        validate_addr(hptr[i], 1);
>>       }
>>       hchunks = i;
>> @@ -145,5 +143,7 @@ int main(int argc, char *argv[])
>>           munmap(hptr[i], MAP_CHUNK_SIZE);
>>       free(hptr);
>> -    return 0;
>> +
>> +    ksft_test_result_pass("Test\n");
>> +    ksft_finished();
>> }

--
BR,
Muhammad Usama Anjum

2024-03-14 12:23:49

by Dev Jain

[permalink] [raw]

Subject: [PATCH] selftests/mm: virtual_address_range: Switch to ksft_exit_fail_msg

mmap() must not succeed in validate_lower_address_hint(), for if it does, it
is a bug in mmap() itself. Reflect this behaviour with ksft_exit_fail_msg().
While at it, do some formatting changes.

Signed-off-by: Dev Jain <[email protected]>
---
tools/testing/selftests/mm/virtual_address_range.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/mm/virtual_address_range.c b/tools/testing/selftests/mm/virtual_address_range.c
index 7bcf8d48256a..426ddfc345fb 100644
--- a/tools/testing/selftests/mm/virtual_address_range.c
+++ b/tools/testing/selftests/mm/virtual_address_range.c
@@ -85,7 +85,7 @@ static int validate_lower_address_hint(void)
char *ptr;

ptr = mmap((void *) (1UL << 45), MAP_CHUNK_SIZE, PROT_READ |
- PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);

if (ptr == MAP_FAILED)
return 0;
@@ -105,13 +105,11 @@ int main(int argc, char *argv[])

for (i = 0; i < NR_CHUNKS_LOW; i++) {
ptr[i] = mmap(NULL, MAP_CHUNK_SIZE, PROT_READ | PROT_WRITE,
- MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);

if (ptr[i] == MAP_FAILED) {
- if (validate_lower_address_hint()) {
- ksft_test_result_skip("Memory constraint not fulfilled\n");
- ksft_finished();
- }
+ if (validate_lower_address_hint())
+ ksft_exit_fail_msg("mmap unexpectedly succeeded with hint\n");
break;
}

@@ -127,7 +125,7 @@ int main(int argc, char *argv[])
for (i = 0; i < NR_CHUNKS_HIGH; i++) {
hint = hind_addr();
hptr[i] = mmap(hint, MAP_CHUNK_SIZE, PROT_READ | PROT_WRITE,
- MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);

if (hptr[i] == MAP_FAILED)
break;
--
2.34.1

2024-03-14 12:33:04

by Muhammad Usama Anjum

[permalink] [raw]

Subject: Re: [PATCH] selftests/mm: virtual_address_range: Switch to ksft_exit_fail_msg

Thanks for the patch. The patch should have been sent to a separate new thread.

On 3/14/24 5:22 PM, Dev Jain wrote:
> mmap() must not succeed in validate_lower_address_hint(), for if it does, it
> is a bug in mmap() itself. Reflect this behaviour with ksft_exit_fail_msg().
> While at it, do some formatting changes.
>
> Signed-off-by: Dev Jain <[email protected]>
Reviewed-by: Muhammad Usama Anjum <[email protected]>

> ---
> tools/testing/selftests/mm/virtual_address_range.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/virtual_address_range.c b/tools/testing/selftests/mm/virtual_address_range.c
> index 7bcf8d48256a..426ddfc345fb 100644
> --- a/tools/testing/selftests/mm/virtual_address_range.c
> +++ b/tools/testing/selftests/mm/virtual_address_range.c
> @@ -85,7 +85,7 @@ static int validate_lower_address_hint(void)
> char *ptr;
>
> ptr = mmap((void *) (1UL << 45), MAP_CHUNK_SIZE, PROT_READ |
> - PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> + PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>
> if (ptr == MAP_FAILED)
> return 0;
> @@ -105,13 +105,11 @@ int main(int argc, char *argv[])
>
> for (i = 0; i < NR_CHUNKS_LOW; i++) {
> ptr[i] = mmap(NULL, MAP_CHUNK_SIZE, PROT_READ | PROT_WRITE,
> - MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>
> if (ptr[i] == MAP_FAILED) {
> - if (validate_lower_address_hint()) {
> - ksft_test_result_skip("Memory constraint not fulfilled\n");
> - ksft_finished();
> - }
> + if (validate_lower_address_hint())
> + ksft_exit_fail_msg("mmap unexpectedly succeeded with hint\n");
> break;
> }
>
> @@ -127,7 +125,7 @@ int main(int argc, char *argv[])
> for (i = 0; i < NR_CHUNKS_HIGH; i++) {
> hint = hind_addr();
> hptr[i] = mmap(hint, MAP_CHUNK_SIZE, PROT_READ | PROT_WRITE,
> - MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>
> if (hptr[i] == MAP_FAILED)
> break;

--
BR,
Muhammad Usama Anjum