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

[Tarun Sahu <tsahu@linux.ibm.com>]

Hi, 
Thanks for reviewing the patch.

On Nov 02 2022, Cyril Hrubis wrote:
> Hi!
--skip
> > +	 * If set, the test function will create a hugetlbfs mount point
> > +	 * at /tmp/xxxxxx, where xxxxxx is a random string.
> 
> This is no longer up-to-date I guess that this should be:
> 
> "If set hugetlbfs will be moutned at .mntpoint"
Yes, missed it. Will update it.
> 
> > +	 */
> > +	int needs_hugetlbfs:1;
> >  
> > +
--skip
> > +int tst_creat_unlinked(const char *path)
> > +{
> > +	char template[PATH_MAX];
> > +	int fd;
> > +
> > +	snprintf(template, PATH_MAX, "%s/ltp_%.3sXXXXXX",
> > +			path, TCID);
>                                  ^
> 				 Should be tid in new library code.
> 
Ok, I will update it to tid.

> > +	fd = mkstemp(template);
> > +	if (fd < 0)
> > +		tst_brk(TBROK | TERRNO,
> > +			 "%s: mkstemp(%s) failed", __func__, template);
>                                                       ^
> 						      This is not
> 						      necessary
> 
> The tst_brk() prints filename and line number already, there is no need
> to print the function name as well.
> 
Ok.

--skip
> > @@ -1299,8 +1326,12 @@ static void do_cleanup(void)
> >  	if (ovl_mounted)
> >  		SAFE_UMOUNT(OVL_MNT);
> >  
> > -	if (mntpoint_mounted)
> > -		tst_umount(tst_test->mntpoint);
> > +	if (mntpoint_mounted) {
> > +		if (tst_test->needs_hugetlbfs)
> > +			SAFE_UMOUNT(tst_test->mntpoint);
> > +		else
> > +			tst_umount(tst_test->mntpoint);
> > +	}
> 
> And I would just keep this umount part as it was.
> 
Ok, will update it, Anyway, it will throw warning on unsuccessful umount.
> -- 
> Cyril Hrubis
> chrubis@suse.cz
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

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