[LTP] [PATCH v2 1/3] lib: extend .request_hugepages to guarantee enough hpages
Cyril Hrubis
chrubis@suse.cz
Fri Jun 3 15:08:20 CEST 2022
Hi!
> This is to satisfy tests which need to reserve hugepage precisely for
> using, with eliminating other process side effects at the same time.
>
> For example, if there are 'N' (N > 1) hpages reserved but all occupying.
> New '.request_hugepage = 1' in another test will only save the N and do
> echo 1 into hugetlbfs. That obviously may cause problems during test run.
>
> Here, we introduce two policies to make hugepage reserve work fit for
> more requirements but no need to care about details. And simply rename
> .request_hugepages to .hugepages.
>
> Signed-off-by: Li Wang <liwang@redhat.com>
> ---
> doc/c-test-api.txt | 34 +++++++++++++++++----------
> include/tst_hugepage.h | 14 +++++++++--
> include/tst_test.h | 26 +++++++++++++-------
> lib/newlib_tests/test20.c | 14 ++++++-----
> lib/newlib_tests/test_zero_hugepage.c | 9 +++----
> lib/tst_hugepage.c | 22 +++++++++++++----
> lib/tst_test.c | 4 ++--
> 7 files changed, 85 insertions(+), 38 deletions(-)
>
> diff --git a/doc/c-test-api.txt b/doc/c-test-api.txt
> index 711b445d9..16409b97e 100644
> --- a/doc/c-test-api.txt
> +++ b/doc/c-test-api.txt
> @@ -1998,15 +1998,24 @@ 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 only via '.request_hugepages = xx'.
> +test can reserve hugepages from system via '.hugepages = {xx, TST_REQUEST}'.
>
> -If set non-zero number of 'request_hugepages', test will try to reserve the
> -expected number of hugepage for testing in setup phase. If system does not
> -have enough hpage for using, it will try the best to reserve 80% available
> -number of hpages. With success test stores the reserved hugepage number in
> -'tst_hugepages'. For the 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 use '.request_hugepages = TST_NO_HUGEPAGES'.
> +We achieved two policies for hugepage reserving:
^
reserving hugepages:
> +
> +TST_REQUEST:
> + If set non-zero number in .hugepages, ltp will try to reserve the expected
> + number of hugepage for testing in setup phase. If system doesn't have enough
> + memory for that, it will try the best to reserve 80% available huge pages.
Maybe just "it will try the best to reserver available huge pages and
return the number of available hugepages in .hugepages.number, which may
be 0 if hugepages are not supported at all."
> +TST_NEEDS:
> + This is an enforced requirement for hugepage reserve, LTP should strictly do
> + hpages applying and guarantee the 'HugePages_Free' no less than pages which
> + gives that test can use these specified numbers correctly.
We should stress here that test exits with TCONF if the attempt to
reserver hugepages fails or reserves less than requested.
> +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
> +use '.hugepages = {TST_NO_HUGEPAGES, TST_REQUEST}'.
I guess that we don't even set the second value here, we can as well
simplify to: .hugepages = {TST_NO_HUGEPAGES}
> Also, we do cleanup and restore work for the hpages resetting automatically.
>
> @@ -2018,7 +2027,7 @@ static void run(void)
> {
> ...
>
> - if (tst_hugepages == test.request_hugepages)
> + if (tst_hugepages == test.hugepages.number)
> TEST(do_hpage_test);
> else
> ...
> @@ -2027,7 +2036,7 @@ static void run(void)
>
> struct tst_test test = {
> .test_all = run,
> - .request_hugepages = 2,
> + .hugepages = {2, TST_REQUEST},
> ...
> };
> -------------------------------------------------------------------------------
> @@ -2045,13 +2054,14 @@ static void run(void)
>
> static void setup(void)
> {
> -?? ?? ?? ?? if (tst_hugepages != test.requested_hugepages)
> + /* TST_NEEDS achieved this automatically in the library */
> +?? ?? ?? ?? if (tst_hugepages != test.hugepages.number)
> ?? ?? ?? ?? ?? ?? ?? ?? tst_brk(TCONF, "...");
^
Non ascii whitespaces?
> }
>
> struct tst_test test = {
> .test_all = run,
> - .request_hugepages = 2,
> + .hugepages = {2, TST_NEEDS},
> ...
> };
> -------------------------------------------------------------------------------
> diff --git a/include/tst_hugepage.h b/include/tst_hugepage.h
> index e08a2daa2..7fba02c40 100644
> --- a/include/tst_hugepage.h
> +++ b/include/tst_hugepage.h
> @@ -12,6 +12,16 @@
> extern char *nr_opt; /* -s num Set the number of the been allocated hugepages */
> extern char *Hopt; /* -H /.. Location of hugetlbfs, i.e. -H /var/hugetlbfs */
>
> +enum tst_hp_policy {
> + TST_REQUEST,
> + TST_NEEDS,
> +};
> +
> +struct tst_hugepage {
> + const unsigned long number;
> + enum tst_hp_policy policy;
> +};
> +
> /*
> * Get the default hugepage size. Returns 0 if hugepages are not supported.
> */
> @@ -23,11 +33,11 @@ size_t tst_get_hugepage_size(void);
> *
> * Note: this depend on the status of system memory fragmentation.
> */
> -unsigned long tst_request_hugepages(unsigned long hpages);
> +unsigned long tst_reserve_hugepages(struct tst_hugepage *hp);
>
> /*
> * This variable is used for recording the number of hugepages which system can
> - * provides. It will be equal to 'hpages' if tst_request_hugepages on success,
> + * provides. It will be equal to 'hpages' if tst_reserve_hugepages on success,
> * otherwise set it to a number of hugepages that we were able to reserve.
> *
> * If system does not support hugetlb, then it will be set to 0.
> diff --git a/include/tst_test.h b/include/tst_test.h
> index 60316092d..9aef43581 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -191,17 +191,27 @@ struct tst_test {
> unsigned long min_mem_avail;
>
> /*
> - * If set non-zero number of request_hugepages, test will try to reserve the
> - * expected number of hugepage for testing in setup phase. If system does not
> - * have enough hpage for using, it will try the best to reserve 80% available
> - * number of hpages. With success test stores the reserved hugepage number in
> - * 'tst_hugepages. For the 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 use '.request_hugepages = TST_NO_HUGEPAGES'.
> + * Two policies for hugepage reserving:
> + *
> + * TST_REQUEST:
> + * If set non-zero number in .hugepages, ltp will try to reserve the expected
> + * number of hugepage for testing in setup phase. If system does not have enough
> + * memory for that, it will try the best to reserve 80% available huge pages.
> + *
> + * TST_NEEDS:
> + * This is an enforced requirement for huge page reserve, ltp should strictly do
> + * hpages applying and guarantee the 'HugePages_Free' no less than specified pages
> + * which gives that test can use these specified numbers correctly.
> + *
> + *
> + * With success test stores the reserved hugepage number in 'tst_hugepages. For
> + * the 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
> + * use '.hugepages = {TST_NO_HUGEPAGES, TST_REQUEST}'.
Here as well (update the docs).
> * Also, we do cleanup and restore work for the hpages resetting automatically.
> */
> - unsigned long request_hugepages;
> + struct tst_hugepage hugepages;
>
> /*
> * If set to non-zero, call tst_taint_init(taint_check) during setup
> diff --git a/lib/newlib_tests/test20.c b/lib/newlib_tests/test20.c
> index 5c24770a1..3982ab79f 100644
> --- a/lib/newlib_tests/test20.c
> +++ b/lib/newlib_tests/test20.c
> @@ -4,7 +4,7 @@
> */
>
> /*
> - * Tests .request_hugepages + .save_restore
> + * Tests .hugepages + .save_restore
> */
>
> #include "tst_test.h"
> @@ -18,24 +18,26 @@ static void do_test(void) {
> tst_res(TINFO, "tst_hugepages = %lu", tst_hugepages);
> SAFE_FILE_PRINTF("/proc/sys/kernel/numa_balancing", "1");
>
> - hpages = test.request_hugepages;
> + hpages = test.hugepages.number;
> SAFE_FILE_SCANF(PATH_NR_HPAGES, "%lu", &val);
> if (val != hpages)
> tst_brk(TBROK, "nr_hugepages = %lu, but expect %lu", val, hpages);
> else
> - tst_res(TPASS, "test .needs_hugepges");
> + tst_res(TPASS, "test .hugepges");
> +
> + struct tst_hugepage hp = { 1000000000000, TST_REQUEST };
> + hpages = tst_reserve_hugepages(&hp);
>
> - hpages = tst_request_hugepages(3);
> SAFE_FILE_SCANF(PATH_NR_HPAGES, "%lu", &val);
> if (val != hpages)
> tst_brk(TBROK, "nr_hugepages = %lu, but expect %lu", val, hpages);
> else
> - tst_res(TPASS, "tst_request_hugepages");
> + tst_res(TPASS, "tst_reserve_hugepages");
> }
>
> static struct tst_test test = {
> .test_all = do_test,
> - .request_hugepages = 2,
> + .hugepages = {2, TST_NEEDS},
> .save_restore = (const struct tst_path_val[]) {
> {"!/proc/sys/kernel/numa_balancing", "0"},
> {}
> diff --git a/lib/newlib_tests/test_zero_hugepage.c b/lib/newlib_tests/test_zero_hugepage.c
> index 0d85ce866..ca47ed5a7 100644
> --- a/lib/newlib_tests/test_zero_hugepage.c
> +++ b/lib/newlib_tests/test_zero_hugepage.c
> @@ -19,17 +19,18 @@ static void do_test(void)
> if (val != 0)
> tst_brk(TBROK, "nr_hugepages = %lu, but expect 0", val);
> else
> - tst_res(TPASS, "test .request_hugepages = TST_NO_HUGEPAGES");
> + tst_res(TPASS, "test .hugepages = {TST_NO_HUGEPAGES, TST_REQUEST}");
>
> - hpages = tst_request_hugepages(3);
> + struct tst_hugepage hp = { 3, TST_REQUEST };
> + hpages = tst_reserve_hugepages(&hp);
> SAFE_FILE_SCANF(PATH_NR_HPAGES, "%lu", &val);
> if (val != hpages)
> tst_brk(TBROK, "nr_hugepages = %lu, but expect %lu", val, hpages);
> else
> - tst_res(TPASS, "tst_request_hugepages");
> + tst_res(TPASS, "tst_reserve_hugepages");
> }
>
> static struct tst_test test = {
> .test_all = do_test,
> - .request_hugepages = TST_NO_HUGEPAGES,
> + .hugepages = {TST_NO_HUGEPAGES, TST_REQUEST},
> };
> diff --git a/lib/tst_hugepage.c b/lib/tst_hugepage.c
> index a7585bc3d..e97cc560e 100644
> --- a/lib/tst_hugepage.c
> +++ b/lib/tst_hugepage.c
> @@ -20,11 +20,13 @@ size_t tst_get_hugepage_size(void)
> return SAFE_READ_MEMINFO("Hugepagesize:") * 1024;
> }
>
> -unsigned long tst_request_hugepages(unsigned long hpages)
> +unsigned long tst_reserve_hugepages(struct tst_hugepage *hp)
> {
> unsigned long val, max_hpages;
>
> if (access(PATH_HUGEPAGES, F_OK)) {
> + if (hp->policy == TST_NEEDS)
> + tst_brk(TCONF, "hugetlbfs is not supported");
> tst_hugepages = 0;
> goto out;
> }
> @@ -32,16 +34,20 @@ unsigned long tst_request_hugepages(unsigned long hpages)
> if (nr_opt)
> tst_hugepages = SAFE_STRTOL(nr_opt, 1, LONG_MAX);
> else
> - tst_hugepages = hpages;
> + tst_hugepages = hp->number;
>
> - if (hpages == TST_NO_HUGEPAGES) {
> + if (hp->number == TST_NO_HUGEPAGES) {
> tst_hugepages = 0;
> goto set_hugepages;
> }
>
> SAFE_FILE_PRINTF("/proc/sys/vm/drop_caches", "3");
> - max_hpages = SAFE_READ_MEMINFO("MemFree:") / SAFE_READ_MEMINFO("Hugepagesize:");
> + if (hp->policy == TST_NEEDS) {
> + tst_hugepages += SAFE_READ_MEMINFO("HugePages_Total:");
> + goto set_hugepages;
> + }
>
> + max_hpages = SAFE_READ_MEMINFO("MemFree:") / SAFE_READ_MEMINFO("Hugepagesize:");
> 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",
> @@ -61,6 +67,14 @@ set_hugepages:
> "Not enough hugepages for testing.",
> val, tst_hugepages);
>
> + if (hp->policy == TST_NEEDS) {
> + unsigned long free_hpages = SAFE_READ_MEMINFO("HugePages_Free:");
> + if (hp->number > free_hpages)
> + tst_brk(TCONF, "free_hpages = %lu, but expect %lu. "
> + "Not enough hugepages for testing.",
> + free_hpages, hp->number);
> + }
> +
Minus the minor comments the code looks good, with the small issues fixed:
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
--
Cyril Hrubis
chrubis@suse.cz
More information about the ltp
mailing list