Re: [LTP] [PATCH v2] Hugetlb: Test to detect bug with freeing gigantic hugetlb pages

Cyril Hrubis
2023-04-17T10:01:51Z

Hi!
> +#define _GNU_SOURCE
> +#include <dirent.h>
> +
> +#include <stdio.h>
> +
> +#include "hugetlb.h"
> +
> +#define PATH_GIGANTIC_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;
> +
> + dir = SAFE_OPENDIR(PATH_GIGANTIC_HUGEPAGE);
^
This is very minor, but there is
nothing gigantic about the path, that's
just sysfs hugepates directory, so I
suppose that it should be just
PATH_HUGEPAGE

> + while ((ent = SAFE_READDIR(dir))) {
> + if (strstr(ent->d_name, "hugepages-") != NULL) {

Isn't the strstr() here reduntant?

I as far as I can tell if the line in sscanf() will not match the call
will simply return 0.

> + if ((sscanf(ent->d_name, "hugepages-%lukB", &hpage_size) == 1) &&
> + is_hugetlb_gigantic(hpage_size * 1024)) {
> + sprintf(g_hpage_path, "%s/%s/%s", PATH_GIGANTIC_HUGEPAGE,
> + ent->d_name, "nr_hugepages");
> + break;
> + }
> + }
> + }
> + if (!g_hpage_path[0])
> + tst_brk(TCONF, "Gigantic hugepages not supported");
> +
> + SAFE_CLOSEDIR(dir);
> + 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);
> +}
> +
> +static struct tst_test test = {
> + .tags = (struct tst_tag[]) {
> + {"linux-git", "ba9c1201beaa"},
> + {"linux-git", "7118fc2906e9"},
^
This has appears to be wrong. Shouldn't the
last digit be 2 instead of 9?
> + {}
> + },
> + .needs_root = 1,
> + .setup = setup,
> + .cleanup = cleanup,
> + .test_all = run_test,
> + .taint_check = TST_TAINT_B,
> +};
> diff --git a/testcases/kernel/mem/hugetlb/lib/hugetlb.h b/testcases/kernel/mem/hugetlb/lib/hugetlb.h
> index 241dab708..34fe08c24 100644
> --- a/testcases/kernel/mem/hugetlb/lib/hugetlb.h
> +++ b/testcases/kernel/mem/hugetlb/lib/hugetlb.h
> @@ -39,6 +39,12 @@
> # endif
> #endif
>
> +/* Check if hugetlb page is gigantic */
> +static inline int is_hugetlb_gigantic(unsigned long hpage_size)
> +{
> + return (hpage_size / getpagesize()) >> 11;
> +}
> +
> /*
> * to get the lower nine permission bits
> * from shmid_ds.ipc_perm.mode
> --
> 2.31.1
>

--
Cyril Hrubis
chrubis@suse.cz