* [LTP] [PATCH v3 0/3] Introduce new field .hptype for reserving gigantic page
@ 2023-11-06 9:30 Li Wang
2023-11-06 9:30 ` [LTP] [PATCH v3 1/3] lib: add support for kinds of hpsize reservation Li Wang
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Li Wang @ 2023-11-06 9:30 UTC (permalink / raw)
To: ltp
In the beginning, I was just thinking of using specified .hpsize by users
to reserve gigantic or default huge pages. But after thinking over and
discussing with Richard, we both think that it might be easy to get typos and
limit the test case to run on more potential platforms.
Here change the field to .hptype with two enum TST_HUGE and TST_GIGANITC,
as the name suggests, if people want to reserve variant page size in their
case, this field could be defined.
But considering there are many different sizes and it could be configured
by linux users, we don't assume what the size of the gigantic page is, it
all depends on the system set, for instance, an aarch64 has four types:
hugepages-1048576kB, hugepages-2048kB,
hugepages-32768kB, hugepages-64kB
LTP library will pick up the largest one as gigantic page. For a system
without configuring the 1GB or similar large size, it also gives a chance
to test on default hpage size. Compared with just TCONF on no-gigantic page,
this is adding test coverage.
(To Richard: the reason why I didn't go to use the smallest and second
smallest size is because, platform like aarch64 typically configure many
medium sizes for user, but the default huge is 2MB and gigantic is 1GB,
that doesn't fit to most popular scenarios. But to x86_64, situations
becomes common and easy, they only has two: 2MB, 1GB, this method could
handle the case smoothly.)
Note: Patchset test passed on the RHEL8.9 and RHEL9.3 (x86_64/aarch64).
V2 --> V3
* introduce new field .hptype
* add enum tst_hp_type { TST_HUGE, TST_GIGANTIC }
* drop the is_hugetlb_gigantic function
* just choose the largest huge pagesize as gigantic
Li Wang (3):
lib: add support for kinds of hpsize reservation
hugemmap32: improvement test
hugemmap34: Test to detect bug with migrating gigantic pages
doc/C-Test-API.asciidoc | 42 +++++-
include/tst_hugepage.h | 11 ++
lib/tst_hugepage.c | 63 ++++++++-
runtest/hugetlb | 1 +
testcases/kernel/mem/.gitignore | 1 +
.../kernel/mem/hugetlb/hugemmap/hugemmap32.c | 59 +-------
.../kernel/mem/hugetlb/hugemmap/hugemmap34.c | 129 ++++++++++++++++++
7 files changed, 240 insertions(+), 66 deletions(-)
create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap34.c
--
2.40.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH v3 1/3] lib: add support for kinds of hpsize reservation
2023-11-06 9:30 [LTP] [PATCH v3 0/3] Introduce new field .hptype for reserving gigantic page Li Wang
@ 2023-11-06 9:30 ` Li Wang
2024-02-16 11:22 ` Petr Vorel
2023-11-06 9:30 ` [LTP] [PATCH v3 2/3] hugemmap32: improvement test Li Wang
2023-11-06 9:30 ` [LTP] [PATCH v3 3/3] hugemmap34: Test to detect bug with migrating gigantic pages Li Wang
2 siblings, 1 reply; 8+ messages in thread
From: Li Wang @ 2023-11-06 9:30 UTC (permalink / raw)
To: ltp
Typically when we make use of huge page via LTP library, .hugepages choose
the default hugepage size, but this can not satisfy all scenarios.
So this patch introduces applying a specified types of hugepage for user.
There is nothing that needs to change for the existing test cases which
already using .hugepages, it only needs to fill one more field in the
structure of .hugepages if a different type (GIGANTIC or HUGE) is required.
e.g.
static struct tst_test test = {
.needs_root = 1,
...
.hugepages = {2, TST_NEEDS, TST_GIGANTIC},
};
Signed-off-by: Li Wang <liwang@redhat.com>
---
doc/C-Test-API.asciidoc | 42 +++++++++++++++++++++++++--
include/tst_hugepage.h | 11 +++++++
lib/tst_hugepage.c | 63 ++++++++++++++++++++++++++++++++++++-----
3 files changed, 107 insertions(+), 9 deletions(-)
diff --git a/doc/C-Test-API.asciidoc b/doc/C-Test-API.asciidoc
index dab811564..82a1866d3 100644
--- a/doc/C-Test-API.asciidoc
+++ b/doc/C-Test-API.asciidoc
@@ -2034,9 +2034,13 @@ For full documentation see the comments in 'include/tst_fuzzy_sync.h'.
~~~~~~~~~~~~~~~~~~~~~~~~
Many of the LTP tests need to use hugepage in their testing, this allows the
-test can reserve hugepages from system via '.hugepages = {xx, TST_REQUEST}'.
+test can reserve specify size of hugepages from system via:
+ '.hugepages = {xx, TST_REQUEST, TST_HUGE}' or,
+ '.hugepages = {xx, TST_NEEDS, TST_GIGANTIC}'.
-We achieved two policies for reserving hugepages:
+xx: This is used to set how many pages we wanted.
+
+Two policies for reserving hugepage:
TST_REQUEST:
It will try the best to reserve available huge pages and return the number
@@ -2049,6 +2053,17 @@ TST_NEEDS:
use these specified numbers correctly. Otherwise, test exits with TCONF if
the attempt to reserve hugepages fails or reserves less than requested.
+Two types of the reserved hugepage (optional field):
+
+TST_HUGE:
+ It means target for reserve the default hugepage size (e.g. 2MB on x86_64).
+ And, if nothing fills in this field LTP also chooses the default hugepage
+ size to reserve. i.e.
+ '.hugepages = {xx, TST_REQUEST}' == '.hugepages = {xx, TST_REQUEST, TST_HUGE}'
+
+TST_GIGANTIC:
+ It means target for reserve the largest hugepage size (e.g. 1GB on x86_64)
+
With success test stores the reserved hugepage number in 'tst_hugepages'. For
system without hugetlb supporting, variable 'tst_hugepages' will be set to 0.
If the hugepage number needs to be set to 0 on supported hugetlb system, please
@@ -2103,6 +2118,29 @@ struct tst_test test = {
};
-------------------------------------------------------------------------------
+or,
+
+[source,c]
+-------------------------------------------------------------------------------
+#include "tst_test.h"
+
+static void run(void)
+{
+ ...
+}
+
+struct tst_test test = {
+ .test_all = run,
+ /*
+ * Specify gigantic page sizes reserved automatically in the library
+ * $ echo 2 > /sys/kernel/mm//hugepages/hugepages-1048576kB/nr_hugepages
+ * Do check if 2 hpages are reserved correctly in there.
+ */
+ .hugepages = {2, TST_NEEDS, TST_GIGANTIC},
+ ...
+};
+-------------------------------------------------------------------------------
+
1.35 Checking for required commands
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/include/tst_hugepage.h b/include/tst_hugepage.h
index 46327c79a..725b4ddaf 100644
--- a/include/tst_hugepage.h
+++ b/include/tst_hugepage.h
@@ -24,9 +24,15 @@ enum tst_hp_policy {
TST_NEEDS,
};
+enum tst_hp_type {
+ TST_HUGE,
+ TST_GIGANTIC,
+};
+
struct tst_hugepage {
const unsigned long number;
enum tst_hp_policy policy;
+ enum tst_hp_type hptype;
};
/*
@@ -34,6 +40,11 @@ struct tst_hugepage {
*/
size_t tst_get_hugepage_size(void);
+/*
+ * Get the largest hugepage (gigantic) size. Returns 0 if hugepages are not supported.
+ */
+size_t tst_get_gigantic_size(void);
+
/*
* Try the best to request a specified number of huge pages from system,
* it will store the reserved hpage number in tst_hugepages.
diff --git a/lib/tst_hugepage.c b/lib/tst_hugepage.c
index d2e70a955..3a7c412f3 100644
--- a/lib/tst_hugepage.c
+++ b/lib/tst_hugepage.c
@@ -5,6 +5,7 @@
#define TST_NO_DEFAULT_MAIN
+#include <stdio.h>
#include "tst_test.h"
#include "tst_hugepage.h"
@@ -20,11 +21,35 @@ size_t tst_get_hugepage_size(void)
return SAFE_READ_MEMINFO("Hugepagesize:") * 1024;
}
+size_t tst_get_gigantic_size(void)
+{
+ DIR *dir;
+ struct dirent *ent;
+ unsigned long max, g_pgsz;
+
+ max = tst_get_hugepage_size() / 1024;
+
+ /*
+ * Scanning the largest hugepage sisze, for example aarch64 configuration:
+ * hugepages-1048576kB hugepages-32768kB hugepages-2048kB hugepages-64kB
+ */
+ dir = SAFE_OPENDIR(PATH_HUGEPAGES);
+ while ((ent = SAFE_READDIR(dir)) != NULL) {
+ if (sscanf(ent->d_name, "hugepages-%lukB", &g_pgsz)
+ && (g_pgsz > max))
+ max = g_pgsz;
+ }
+
+ SAFE_CLOSEDIR(dir);
+ return max * 1024;
+}
+
unsigned long tst_reserve_hugepages(struct tst_hugepage *hp)
{
- unsigned long val, max_hpages;
+ unsigned long val, max_hpages, hpsize;
+ char hugepage_path[PATH_MAX];
struct tst_path_val pvl = {
- .path = PATH_NR_HPAGES,
+ .path = hugepage_path,
.val = NULL,
.flags = TST_SR_SKIP_MISSING | TST_SR_TCONF_RO
};
@@ -41,6 +66,19 @@ unsigned long tst_reserve_hugepages(struct tst_hugepage *hp)
else
tst_hugepages = hp->number;
+ if (hp->hptype == TST_GIGANTIC)
+ hpsize = tst_get_gigantic_size() / 1024;
+ else
+ hpsize = tst_get_hugepage_size() / 1024;
+
+ sprintf(hugepage_path, PATH_HUGEPAGES"/hugepages-%lukB/nr_hugepages", hpsize);
+ if (access(hugepage_path, F_OK)) {
+ if (hp->policy == TST_NEEDS)
+ tst_brk(TCONF, "Hugepage size %lu not supported", hpsize);
+ tst_hugepages = 0;
+ goto out;
+ }
+
if (hp->number == TST_NO_HUGEPAGES) {
tst_hugepages = 0;
goto set_hugepages;
@@ -49,11 +87,18 @@ unsigned long tst_reserve_hugepages(struct tst_hugepage *hp)
SAFE_FILE_PRINTF("/proc/sys/vm/drop_caches", "3");
SAFE_FILE_PRINTF("/proc/sys/vm/compact_memory", "1");
if (hp->policy == TST_NEEDS) {
+ /*
+ * In case of the gigantic page configured as the default hugepage size,
+ * we have to garantee the TST_NEEDS take effect.
+ */
+ if (tst_get_gigantic_size() != tst_get_hugepage_size())
+ goto set_hugepages;
+
tst_hugepages += SAFE_READ_MEMINFO("HugePages_Total:");
goto set_hugepages;
}
- max_hpages = SAFE_READ_MEMINFO("MemFree:") / SAFE_READ_MEMINFO("Hugepagesize:");
+ max_hpages = SAFE_READ_MEMINFO("MemFree:") / hpsize;
if (tst_hugepages > max_hpages) {
tst_res(TINFO, "Requested number(%lu) of hugepages is too large, "
"limiting to 80%% of the max hugepage count %lu",
@@ -66,22 +111,26 @@ unsigned long tst_reserve_hugepages(struct tst_hugepage *hp)
set_hugepages:
tst_sys_conf_save(&pvl);
- SAFE_FILE_PRINTF(PATH_NR_HPAGES, "%lu", tst_hugepages);
- SAFE_FILE_SCANF(PATH_NR_HPAGES, "%lu", &val);
+
+ SAFE_FILE_PRINTF(hugepage_path, "%lu", tst_hugepages);
+ SAFE_FILE_SCANF(hugepage_path, "%lu", &val);
+
if (val != tst_hugepages)
tst_brk(TCONF, "nr_hugepages = %lu, but expect %lu. "
"Not enough hugepages for testing.",
val, tst_hugepages);
if (hp->policy == TST_NEEDS) {
- unsigned long free_hpages = SAFE_READ_MEMINFO("HugePages_Free:");
+ unsigned long free_hpages;
+ sprintf(hugepage_path, PATH_HUGEPAGES"/hugepages-%lukB/free_hugepages", hpsize);
+ SAFE_FILE_SCANF(hugepage_path, "%lu", &free_hpages);
if (hp->number > free_hpages)
tst_brk(TCONF, "free_hpages = %lu, but expect %lu. "
"Not enough hugepages for testing.",
free_hpages, hp->number);
}
- tst_res(TINFO, "%lu hugepage(s) reserved", tst_hugepages);
+ tst_res(TINFO, "%lu (%luMB) hugepage(s) reserved", tst_hugepages, hpsize/1024);
out:
return tst_hugepages;
}
--
2.40.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [LTP] [PATCH v3 2/3] hugemmap32: improvement test
2023-11-06 9:30 [LTP] [PATCH v3 0/3] Introduce new field .hptype for reserving gigantic page Li Wang
2023-11-06 9:30 ` [LTP] [PATCH v3 1/3] lib: add support for kinds of hpsize reservation Li Wang
@ 2023-11-06 9:30 ` Li Wang
2024-02-16 12:09 ` Petr Vorel
2023-11-06 9:30 ` [LTP] [PATCH v3 3/3] hugemmap34: Test to detect bug with migrating gigantic pages Li Wang
2 siblings, 1 reply; 8+ messages in thread
From: Li Wang @ 2023-11-06 9:30 UTC (permalink / raw)
To: ltp
Signed-off-by: Li Wang <liwang@redhat.com>
---
.../kernel/mem/hugetlb/hugemmap/hugemmap32.c | 59 +------------------
1 file changed, 2 insertions(+), 57 deletions(-)
diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap32.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap32.c
index d27e5b8b2..8f57a79ac 100644
--- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap32.c
+++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap32.c
@@ -18,67 +18,13 @@
#define _GNU_SOURCE
#include <dirent.h>
-
#include <stdio.h>
#include "hugetlb.h"
-#define PATH_HUGEPAGE "/sys/kernel/mm/hugepages"
-#define GIGANTIC_MIN_ORDER 10
-
-static int org_g_hpages;
-static char g_hpage_path[4096];
-
static void run_test(void)
{
- if (FILE_PRINTF(g_hpage_path, "%d", 1))
- tst_brk(TCONF, "Can't update the gigantic hugepages.");
- SAFE_FILE_PRINTF(g_hpage_path, "%d", 0);
-
- if (tst_taint_check())
- tst_res(TFAIL, "Freeing Gigantic pages resulted in Bad Page State bug.");
- else
- tst_res(TPASS, "Successfully freed the gigantic hugepages");
-}
-
-static void setup(void)
-{
- DIR *dir;
- struct dirent *ent;
- unsigned long hpage_size;
-
- if (access(PATH_HUGEPAGE, F_OK))
- tst_brk(TCONF, "hugetlbfs is not supported");
-
- dir = SAFE_OPENDIR(PATH_HUGEPAGE);
- while ((ent = SAFE_READDIR(dir))) {
- if ((sscanf(ent->d_name, "hugepages-%lukB", &hpage_size) == 1) &&
- is_hugetlb_gigantic(hpage_size * 1024)) {
- sprintf(g_hpage_path, "%s/%s/%s", PATH_HUGEPAGE,
- ent->d_name, "nr_hugepages");
- break;
- }
- }
- if (!g_hpage_path[0])
- tst_brk(TCONF, "Gigantic hugepages not supported");
-
- SAFE_CLOSEDIR(dir);
-
- SAFE_FILE_PRINTF("/proc/sys/vm/drop_caches", "3");
- SAFE_FILE_PRINTF("/proc/sys/vm/compact_memory", "1");
-
- if (tst_available_mem() < (long long)hpage_size) {
- g_hpage_path[0] = '\0';
- tst_brk(TCONF, "No enough memory for gigantic hugepage reservation");
- }
-
- SAFE_FILE_LINES_SCANF(g_hpage_path, "%d", &org_g_hpages);
-}
-
-static void cleanup(void)
-{
- if (g_hpage_path[0])
- SAFE_FILE_PRINTF(g_hpage_path, "%d", org_g_hpages);
+ tst_res(TPASS, "If reserved & freed the gigantic page completely, then regard as pass");
}
static struct tst_test test = {
@@ -88,8 +34,7 @@ static struct tst_test test = {
{}
},
.needs_root = 1,
- .setup = setup,
- .cleanup = cleanup,
+ .hugepages = {1, TST_NEEDS, TST_GIGANTIC},
.test_all = run_test,
.taint_check = TST_TAINT_B,
};
--
2.40.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [LTP] [PATCH v3 3/3] hugemmap34: Test to detect bug with migrating gigantic pages
2023-11-06 9:30 [LTP] [PATCH v3 0/3] Introduce new field .hptype for reserving gigantic page Li Wang
2023-11-06 9:30 ` [LTP] [PATCH v3 1/3] lib: add support for kinds of hpsize reservation Li Wang
2023-11-06 9:30 ` [LTP] [PATCH v3 2/3] hugemmap32: improvement test Li Wang
@ 2023-11-06 9:30 ` Li Wang
2024-02-16 12:01 ` Petr Vorel
2 siblings, 1 reply; 8+ messages in thread
From: Li Wang @ 2023-11-06 9:30 UTC (permalink / raw)
To: ltp
Signed-off-by: Li Wang <liwang@redhat.com>
---
runtest/hugetlb | 1 +
testcases/kernel/mem/.gitignore | 1 +
.../kernel/mem/hugetlb/hugemmap/hugemmap34.c | 129 ++++++++++++++++++
3 files changed, 131 insertions(+)
create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap34.c
diff --git a/runtest/hugetlb b/runtest/hugetlb
index 299c07ac9..0c812c780 100644
--- a/runtest/hugetlb
+++ b/runtest/hugetlb
@@ -35,6 +35,7 @@ hugemmap29 hugemmap29
hugemmap30 hugemmap30
hugemmap31 hugemmap31
hugemmap32 hugemmap32
+hugemmap34 hugemmap34
hugemmap05_1 hugemmap05 -m
hugemmap05_2 hugemmap05 -s
hugemmap05_3 hugemmap05 -s -m
diff --git a/testcases/kernel/mem/.gitignore b/testcases/kernel/mem/.gitignore
index 7258489ed..41f547edf 100644
--- a/testcases/kernel/mem/.gitignore
+++ b/testcases/kernel/mem/.gitignore
@@ -34,6 +34,7 @@
/hugetlb/hugemmap/hugemmap30
/hugetlb/hugemmap/hugemmap31
/hugetlb/hugemmap/hugemmap32
+/hugetlb/hugemmap/hugemmap34
/hugetlb/hugeshmat/hugeshmat01
/hugetlb/hugeshmat/hugeshmat02
/hugetlb/hugeshmat/hugeshmat03
diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap34.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap34.c
new file mode 100644
index 000000000..2e55e5f15
--- /dev/null
+++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap34.c
@@ -0,0 +1,129 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) Linux Test Project, 2023
+ * Copyright (C) 2023, Red Hat, Inc.
+ *
+ * Author: David Hildenbrand <david@redhat.com>
+ * Port-to-LTP: Li Wang <liwang@redhat.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Migration code will first unmap the old page and replace the present PTE
+ * by a migration entry. Then migrate the page. Once that succeeded (and there
+ * are no unexpected page references), we replace the migration entries by
+ * proper present PTEs pointing at the new page.
+ *
+ * For ordinary pages we handle PTEs. For 2 MiB hugetlb/THP, it's PMDs.
+ * For 1 GiB hugetlb, it's PUDs.
+ *
+ * So without below commit, GUP-fast code was simply not aware that we could
+ * have migration entries stored in PUDs. Migration + GUP-fast code should be
+ * able to handle any such races.
+ *
+ * For example, GUP-fast will re-verify the PUD after pinning to make sure it
+ * didn't change. If it did change, it backs off.
+ *
+ * Migration code should detect the additional page references and back off
+ * as well.
+ *
+ * commit 15494520b776aa2eadc3e2fefae524764cab9cea
+ * Author: Qiujun Huang <hqjagain@gmail.com>
+ * Date: Thu Jan 30 22:12:10 2020 -0800
+ *
+ * mm: fix gup_pud_range
+ *
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <pthread.h>
+#include <unistd.h>
+#include <stdbool.h>
+#include <sys/mman.h>
+#include <sys/syscall.h>
+#include <sys/ioctl.h>
+#include <linux/mempolicy.h>
+#include <linux/mman.h>
+
+#include "lapi/syscalls.h"
+#include "tst_safe_pthread.h"
+#include "numa_helper.h"
+#include "hugetlb.h"
+
+static char *mem;
+static size_t pagesize;
+static size_t hugetlbsize;
+static volatile int looping = 1;
+
+static void *migration_thread_fn(void *arg LTP_ATTRIBUTE_UNUSED)
+{
+ while (looping) {
+ TST_EXP_PASS_SILENT(syscall(__NR_mbind, mem, hugetlbsize,
+ MPOL_LOCAL, NULL, 0x7fful, MPOL_MF_MOVE));
+ }
+
+ return NULL;
+}
+
+static void run_test(void)
+{
+ ssize_t transferred;
+ struct iovec iov;
+ int fds[2];
+
+ pthread_t migration_thread;
+
+ pagesize = getpagesize();
+ hugetlbsize = 1 * 1024 * 1024 * 1024u;
+
+ mem = mmap(NULL, hugetlbsize, PROT_READ|PROT_WRITE,
+ MAP_PRIVATE|MAP_ANON|MAP_HUGETLB|MAP_HUGE_1GB,
+ -1, 0);
+ if (mem == MAP_FAILED)
+ tst_brk(TBROK | TERRNO, "mmap() failed");
+
+ memset(mem, 1, hugetlbsize);
+
+ /* Keep migrating the page around ... */
+ SAFE_PTHREAD_CREATE(&migration_thread, NULL, migration_thread_fn, NULL);
+
+ while (looping) {
+ SAFE_PIPE(fds);
+
+ iov.iov_base = mem;
+ iov.iov_len = pagesize;
+ transferred = vmsplice(fds[1], &iov, 1, 0);
+ if (transferred <= 0)
+ tst_brk(TBROK | TERRNO, "vmsplice() failed");
+
+ SAFE_CLOSE(fds[0]);
+ SAFE_CLOSE(fds[1]);
+
+ if (!tst_remaining_runtime()) {
+ tst_res(TINFO, "Runtime exhausted, exiting");
+ looping = 0;
+ }
+ }
+
+ SAFE_PTHREAD_JOIN(migration_thread, NULL);
+
+ tst_res(TPASS, "Test completed successfully");
+}
+
+static struct tst_test test = {
+ .needs_root = 1,
+ .test_all = run_test,
+ .max_runtime = 60,
+ .taint_check = TST_TAINT_W | TST_TAINT_D,
+ .hugepages = {2, TST_NEEDS, TST_GIGANTIC},
+ .needs_kconfigs = (const char *const[]){
+ "CONFIG_NUMA=y",
+ NULL
+ },
+ .tags = (struct tst_tag[]) {
+ {"linux-git", "15494520b776"},
+ {}
+ },
+};
--
2.40.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [LTP] [PATCH v3 1/3] lib: add support for kinds of hpsize reservation
2023-11-06 9:30 ` [LTP] [PATCH v3 1/3] lib: add support for kinds of hpsize reservation Li Wang
@ 2024-02-16 11:22 ` Petr Vorel
0 siblings, 0 replies; 8+ messages in thread
From: Petr Vorel @ 2024-02-16 11:22 UTC (permalink / raw)
To: Li Wang; +Cc: ltp
Hi Li,
> Typically when we make use of huge page via LTP library, .hugepages choose
> the default hugepage size, but this can not satisfy all scenarios.
> So this patch introduces applying a specified types of hugepage for user.
> There is nothing that needs to change for the existing test cases which
> already using .hugepages, it only needs to fill one more field in the
> structure of .hugepages if a different type (GIGANTIC or HUGE) is required.
> e.g.
> static struct tst_test test = {
> .needs_root = 1,
> ...
> .hugepages = {2, TST_NEEDS, TST_GIGANTIC},
> };
+1
Algorithm looks good to me.
It would be nice to add a new test or to modify at least one of the library
tests to use some of the new functionality.
Below only very minor formatting notes.
Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Signed-off-by: Li Wang <liwang@redhat.com>
> ---
> doc/C-Test-API.asciidoc | 42 +++++++++++++++++++++++++--
> include/tst_hugepage.h | 11 +++++++
> lib/tst_hugepage.c | 63 ++++++++++++++++++++++++++++++++++++-----
> 3 files changed, 107 insertions(+), 9 deletions(-)
> diff --git a/doc/C-Test-API.asciidoc b/doc/C-Test-API.asciidoc
> index dab811564..82a1866d3 100644
> --- a/doc/C-Test-API.asciidoc
> +++ b/doc/C-Test-API.asciidoc
> @@ -2034,9 +2034,13 @@ For full documentation see the comments in 'include/tst_fuzzy_sync.h'.
> ~~~~~~~~~~~~~~~~~~~~~~~~
> Many of the LTP tests need to use hugepage in their testing, this allows the
> -test can reserve hugepages from system via '.hugepages = {xx, TST_REQUEST}'.
> +test can reserve specify size of hugepages from system via:
> + '.hugepages = {xx, TST_REQUEST, TST_HUGE}' or,
> + '.hugepages = {xx, TST_NEEDS, TST_GIGANTIC}'.
very nit: this formats in wiki as inline. If you meant to have the previous following
showing also as a list, it would have to be:
test can reserve specify size of hugepages from system via:
* '.hugepages = {xx, TST_REQUEST, TST_HUGE}' or,
* '.hugepages = {xx, TST_NEEDS, TST_GIGANTIC}'.
very nit: maybe N instead of XX?
> -We achieved two policies for reserving hugepages:
> +xx: This is used to set how many pages we wanted.
> +
> +Two policies for reserving hugepage:
> TST_REQUEST:
> It will try the best to reserve available huge pages and return the number
> @@ -2049,6 +2053,17 @@ TST_NEEDS:
> use these specified numbers correctly. Otherwise, test exits with TCONF if
> the attempt to reserve hugepages fails or reserves less than requested.
> +Two types of the reserved hugepage (optional field):
> +
> +TST_HUGE:
> + It means target for reserve the default hugepage size (e.g. 2MB on x86_64).
> + And, if nothing fills in this field LTP also chooses the default hugepage
> + size to reserve. i.e.
> + '.hugepages = {xx, TST_REQUEST}' == '.hugepages = {xx, TST_REQUEST, TST_HUGE}'
> +
> +TST_GIGANTIC:
> + It means target for reserve the largest hugepage size (e.g. 1GB on x86_64)
very nit: missing '.' at the end.
> +
...
> +or,
> +
> +[source,c]
> +-------------------------------------------------------------------------------
> +#include "tst_test.h"
> +
> +static void run(void)
> +{
> + ...
> +}
> +
> +struct tst_test test = {
> + .test_all = run,
> + /*
> + * Specify gigantic page sizes reserved automatically in the library
> + * $ echo 2 > /sys/kernel/mm//hugepages/hugepages-1048576kB/nr_hugepages
> + * Do check if 2 hpages are reserved correctly in there.
Maybe my bad English, but this looked to me at first as imperative, e.g.:
"Hey, test author, do check the value!" Maybe something like "Library checks
whether 2 hpages is actually set.".
> + */
> + .hugepages = {2, TST_NEEDS, TST_GIGANTIC},
> + ...
> +};
> +-------------------------------------------------------------------------------
> +
> 1.35 Checking for required commands
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> diff --git a/include/tst_hugepage.h b/include/tst_hugepage.h
> index 46327c79a..725b4ddaf 100644
> --- a/include/tst_hugepage.h
> +++ b/include/tst_hugepage.h
> @@ -24,9 +24,15 @@ enum tst_hp_policy {
> TST_NEEDS,
> };
> +enum tst_hp_type {
> + TST_HUGE,
> + TST_GIGANTIC,
> +};
> +
> struct tst_hugepage {
> const unsigned long number;
> enum tst_hp_policy policy;
> + enum tst_hp_type hptype;
> };
> /*
> @@ -34,6 +40,11 @@ struct tst_hugepage {
> */
> size_t tst_get_hugepage_size(void);
> +/*
> + * Get the largest hugepage (gigantic) size. Returns 0 if hugepages are not supported.
> + */
> +size_t tst_get_gigantic_size(void);
..
> +++ b/lib/tst_hugepage.c
> @@ -5,6 +5,7 @@
> #define TST_NO_DEFAULT_MAIN
> +#include <stdio.h>
> #include "tst_test.h"
> #include "tst_hugepage.h"
> @@ -20,11 +21,35 @@ size_t tst_get_hugepage_size(void)
> return SAFE_READ_MEMINFO("Hugepagesize:") * 1024;
> }
> +size_t tst_get_gigantic_size(void)
> +{
> + DIR *dir;
> + struct dirent *ent;
> + unsigned long max, g_pgsz;
> +
> + max = tst_get_hugepage_size() / 1024;
> +
> + /*
> + * Scanning the largest hugepage sisze, for example aarch64 configuration:
nit: s/sisze/size/
> + * hugepages-1048576kB hugepages-32768kB hugepages-2048kB hugepages-64kB
> + */
> + dir = SAFE_OPENDIR(PATH_HUGEPAGES);
> + while ((ent = SAFE_READDIR(dir)) != NULL) {
> + if (sscanf(ent->d_name, "hugepages-%lukB", &g_pgsz)
> + && (g_pgsz > max))
> + max = g_pgsz;
> + }
> +
> + SAFE_CLOSEDIR(dir);
> + return max * 1024;
> +}
...
The rest LGTM.
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [LTP] [PATCH v3 3/3] hugemmap34: Test to detect bug with migrating gigantic pages
2023-11-06 9:30 ` [LTP] [PATCH v3 3/3] hugemmap34: Test to detect bug with migrating gigantic pages Li Wang
@ 2024-02-16 12:01 ` Petr Vorel
2024-02-16 12:12 ` Petr Vorel
0 siblings, 1 reply; 8+ messages in thread
From: Petr Vorel @ 2024-02-16 12:01 UTC (permalink / raw)
To: Li Wang; +Cc: ltp
Hi Li,
FYI hugemmap34 fails on x86_64 and s390x various SLES kernels (tested: x86_64
from 4.4 to 6.4, s390x from 4.12. to 6.4):
tst_test.c:1627: TINFO: Timeout per run is 0h 01m 30s
hugemmap34.c:85: TBROK: mmap() failed: EINVAL (22)
Removing MAP_HUGE_1GB shows TBROK due ENOMEM.
I suppose low memory resource for this VM.
=> Maybe we need .min_mem_avail = 1024?
+ define the value and reuse also here (so that we don't forget to update on
other place if later changed):
hugetlbsize = 1 * 1024 * 1024 * 1024u;
(also remove '1 *')
Although you do #include <linux/mman.h>, for some reason I get some failures on
aarch64 and s390x on old gcc 4.8.5:
hugemmap34.c:82:39: error: ‘MAP_HUGE_1GB’ undeclared (first use in this function)
MAP_PRIVATE|MAP_ANON|MAP_HUGETLB|MAP_HUGE_1GB,
^
But works on the same toolchain with gcc 7.5.0, also x86_64 and ppc64le works
fine on them). It looks to me MAP_HUGE_1GB will need to be backported to lapi.
It works on x86_64 and ppc64le.
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [LTP] [PATCH v3 2/3] hugemmap32: improvement test
2023-11-06 9:30 ` [LTP] [PATCH v3 2/3] hugemmap32: improvement test Li Wang
@ 2024-02-16 12:09 ` Petr Vorel
0 siblings, 0 replies; 8+ messages in thread
From: Petr Vorel @ 2024-02-16 12:09 UTC (permalink / raw)
To: Li Wang; +Cc: ltp
Hi Li,
FYI previous version on Tumbleweed in our CI reported:
hugemmap32.c:41: TPASS: Successfully freed the gigantic hugepages
Now, after check, I get:
tst_hugepage.c:119: TCONF: nr_hugepages = 0, but expect 1. Not enough hugepages for testing.
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [LTP] [PATCH v3 3/3] hugemmap34: Test to detect bug with migrating gigantic pages
2024-02-16 12:01 ` Petr Vorel
@ 2024-02-16 12:12 ` Petr Vorel
0 siblings, 0 replies; 8+ messages in thread
From: Petr Vorel @ 2024-02-16 12:12 UTC (permalink / raw)
To: Li Wang, ltp
> Hi Li,
> FYI hugemmap34 fails on x86_64 and s390x various SLES kernels (tested: x86_64
> from 4.4 to 6.4, s390x from 4.12. to 6.4):
Also Tumbleweed with 6.8.0-rc4 fails
hugemmap34.c:85: TBROK: mmap() failed: EINVAL (22)
(no code modification - with MAP_HUGE_1GB).
But in CI I reproduced also tst_hugepage.c:119: TCONF: nr_hugepages = 0, but
expect 2. Not enough hugepages for testing.
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-02-16 12:13 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-06 9:30 [LTP] [PATCH v3 0/3] Introduce new field .hptype for reserving gigantic page Li Wang
2023-11-06 9:30 ` [LTP] [PATCH v3 1/3] lib: add support for kinds of hpsize reservation Li Wang
2024-02-16 11:22 ` Petr Vorel
2023-11-06 9:30 ` [LTP] [PATCH v3 2/3] hugemmap32: improvement test Li Wang
2024-02-16 12:09 ` Petr Vorel
2023-11-06 9:30 ` [LTP] [PATCH v3 3/3] hugemmap34: Test to detect bug with migrating gigantic pages Li Wang
2024-02-16 12:01 ` Petr Vorel
2024-02-16 12:12 ` Petr Vorel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox