Re: [LTP] [PATCH] mm: hugemmap05: fix hardcoded hugepagesize

Cyril Hrubis
2011-03-17T16:29:27Z

Hi!
Here comes some minor notes.

> +static int hugepagesize; /* in Byte */

Correct spelling should be "in Bytes"

> +
> +static void get_hugepagesize(void)
> +{
> + FILE *fp;
> +
> + memset(buf, -1, BUFSIZ);
> + if ((fp = fopen(PATH_MEMINFO, "r")) == NULL)
> + tst_brkm(TBROK, NULL, "can't open %s", PATH_MEMINFO);
> + while (fgets(line, BUFSIZ, fp) != NULL) {
> + if (lookup(line, "Hugepagesize")) {
> + tst_resm(TINFO, "Hugepagesize is %d kB", atoi(buf));

It's kind of useless to convert string into integer and then integer
into string again. ;)

> + hugepagesize = atoi(buf) * 1024;
> + return;
> + }
> + }
> + tst_brkm(TBROK, NULL, "get Hugepagesize failed.");
> +}
> +
> +/*
> + * It's not easy to #define tunable file paths via sysfs,
> + * use function get_hugepagesize and global variable instead.
> + */
> +static void get_sys_sz_paths(void)
> +{
> + sprintf(path_sys_sz, "/sys/kernel/mm/hugepages/hugepages-%dkB",
> + hugepagesize / 1024);
> + sprintf(path_sys_sz_over, "%s/nr_overcommit_hugepages",
> + path_sys_sz);
> + sprintf(path_sys_sz_free, "%s/free_hugepages",
> + path_sys_sz);
> + sprintf(path_sys_sz_resv, "%s/resv_hugepages",
> + path_sys_sz);
> + sprintf(path_sys_sz_surp, "%s/surplus_hugepages",
> + path_sys_sz);
> + sprintf(path_sys_sz_huge, "%s/nr_hugepages",
> + path_sys_sz);
> +}

It's kind of confusing to name the function get_sys_sz_paths() as it's
not returning anything, I would rather name it as init_sys_sz_paths().
The same goes for get_hugepagesize().

--
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Colocation vs. Managed Hosting
A question and answer guide to determining the best fit
for your organization - today and in the future.