Re: [LTP] [PATCH v2 1/3] Hugetlb: Migrating libhugetlbfs brk_near_huge

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

Tarun Sahu <tsahu@linux.ibm.com> writes:

>
>> 
>> > +	snprintf(hfile, sizeof(hfile), "%s/ltp_hugetlbfile%d", Hopt,
>> > getpid());
>> > +	hpage_size = SAFE_READ_MEMINFO("Hugepagesize:")*1024;
>> > +
>> > +	fd = SAFE_OPEN(hfile, O_RDWR | O_CREAT, 0600);
>> > +	SAFE_UNLINK(hfile);
>> 
>> I'm guessing opening this file and using it with mmap is a very
>> common
>> pattern. If so, it should be encapsulated in tst_hugepage.c.
>> 
> Yeah I agree. But the change in tst_hugepage.c will require change in
> prexisting hugetlb tests.

You don't need to update existing LTP tests if you add a new flag to
.tst_test I guess.

>
>> > +}
>> > +
>> > +static void cleanup(void)
>> > +{
>> > +	if (fd >= 0)
>> > +		SAFE_CLOSE(fd);
>> > +	SAFE_UMOUNT(Hopt);
>> > +}
>> > +
>> > +static struct tst_test test = {
>> > +	.needs_root = 1,
>> > +	.needs_tmpdir = 1,
>> > +	.options = (struct tst_option[]) {
>> > +		{"H:", &Hopt,   "Location of hugetlbfs, i.e.  -H
>> > /var/hugetlbfs"},
>> 
>> This is a source of confusion. This description suggests we pass in
>> an
>> existing hugetlb mount. However it's actually where it will be
>> mounted.
>> 
>> Perhaps instead we could use set/getmntent to find an existing
>> hugetlb
>> mount?  Then if it is not there, try mounting it. This is what we do
>> for
>> CGroups.
>> 
>> I'm not sure what difference it makes where the FS is mounted
>> anyway. Why is it even an option?
>
> Not sure, If user is ok for using premounted fs without their
> permission. So instead of searching, it will mount where user will
> explicitly asked for. Or otherwise if not provided, it will mount at
> temp location under /tmp.

Does it actually create a new FS or is it just remounting the existing
hugetlb interface in a new place?

Also it requires privileges to mount an FS. It seems unlikely that some
database wanting to use hugepages would mount it themselves.

From the kernel docs:

"If the user applications are going to request huge pages using mmap system
call, then it is required that system administrator mount a file system of
type hugetlbfs"

>
> I had taken this option from hugemmap01 test. Thinking, it might be
> to provide user the flexibility incase user doesnt want test to mount
> fs at random location by default.
>
> I will change the description to "Provide the location to mount the
> hugetlbfs or default '/tmp/xxxxxxx'"
>
>> 
>> > +		{"s:", &nr_opt, "Set the number of the been allocated
>> > hugepages"},
>> > +		{}
>> > +	},
>> > +	.setup = setup,
>> > +	.cleanup = cleanup,
>> > +	.test_all = run_test,
>> > +	.hugepages = {1, TST_NEEDS},
>> 
>> When we set this tst_hugepages.c could fill Hopt (which should be
>> something like tst_hugepages_mount) with the location of the mount
>> point. It could also open the hfile fd and store it in a static
>> variable like tst_hugepage_fd.
> Yeah, It will move the repeated actions to single location.
>
> This will
> require change in lib/tst_hugepage.c and which will require
> change in already existing test under hugetlb/ . Which will be like a
> new separate change patch series.

If it results in too many complicated changes, then converting all the
existing tests could be defered to a later time by creating new
variables.

-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp