Re: [LTP] [PATCH v3 1/4] Hugetlb: Add new tst_test options for hugeltb test support

Li Wang
2022-11-01T02:06:08Z

[-- Attachment #1.1: Type: text/plain, Size: 1245 bytes --]

On Mon, Oct 31, 2022 at 10:54 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> > Why not consider encapsulating these two new fields in 'struct
> > tst_hugepage' ?
> >
> > Then the tst_test in the case can simply initialize to:
> >
> > ....
> > static struct tst_test test = {
> > .needs_root = 1,
> > .taint_check = TST_TAINT_D | TST_TAINT_W,
> > .setup = setup,
> > .test_all = run_test,
> > .hugepages = {1, TST_NEEDS, 1, 1},
> > };
>
> I do not like that we have magic constants in the .hugepages that are
> not self describing. I would treat the hugetltbfs just as we treat
> devfs, that would be:
>
> #define MNTPOINT "hugetlbfs/"
> #define HUGEFILE MNTPOINT "hugefile"
>
> static int huge_fd;
>
> static void setup(void)
> {
> huge_fd = tst_creat_unlinked(HUGEFILE);
> ...
> }
>
> static void cleanup(void)
> {
> if (huge_fd > 0)
> SAFE_CLOSE(huge_fd);
> }
>
> static struct tst_test test = {
> ...
> .mntpoint = MNTPOINT,
> .needs_hugetlbfs = 1,
> .setup = setup,
> .cleanup = cleanup,
> ...
> }
>
>
> What do you think?
>

+1 Looks good, this treats it as an FS and is separated from the hugepage
usage.

--
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 2121 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]