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

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> Most of libhugetlbfs test require mounted hugetlbfs.
> 
> Here, this patch adds a new field in tst_test struct(include/tst_test.h)
> which user can set if the test requires mounted hugetlbfs. Also, this
> patch added support to create the unlinked file in the provided dirpath.
> 
> Signed-off-by: Tarun Sahu <tsahu@linux.ibm.com>
> ---
>  include/tst_test.h | 11 +++++++++++
>  lib/tst_test.c     | 41 ++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 47 insertions(+), 5 deletions(-)
> 
> diff --git a/include/tst_test.h b/include/tst_test.h
> index a69965b95..01bc5a05b 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -176,6 +176,11 @@ struct tst_test {
>  	int all_filesystems:1;
>  	int skip_in_lockdown:1;
>  	int skip_in_compat:1;
> +	/*
> +	 * 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"

> +	 */
> +	int needs_hugetlbfs:1;
>  
>  	/*
>  	 * The skip_filesystems is a NULL terminated list of filesystems the
> @@ -357,6 +362,12 @@ unsigned int tst_remaining_runtime(void);
>   */
>  void tst_set_max_runtime(int max_runtime);
>  
> +/*
> + * Create and open a random file inside the given dir path.
> + * It unlinks the file after opening and return file descriptor.
> + */
> +int tst_creat_unlinked(const char *path);
> +
>  /*
>   * Returns path to the test temporary directory in a newly allocated buffer.
>   */
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 8ccde1629..9965706ef 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -1021,6 +1021,29 @@ static void prepare_and_mount_dev_fs(const char *mntpoint)
>  	}
>  }
>  
> +static void prepare_and_mount_hugetlb_fs(void)
> +{
> +	SAFE_MOUNT("none", tst_test->mntpoint, "hugetlbfs", 0, NULL);
> +	mntpoint_mounted = 1;
> +}
> +
> +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.

> +	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.

> +	SAFE_UNLINK(template);
> +	return fd;
> +}
> +
>  static const char *limit_tmpfs_mount_size(const char *mnt_data,
>  		char *buf, size_t buf_size, const char *fs_type)
>  {
> @@ -1191,15 +1214,16 @@ static void do_setup(int argc, char *argv[])
>  		SAFE_MKDIR(tst_test->mntpoint, 0777);
>  
>  	if ((tst_test->needs_devfs || tst_test->needs_rofs ||
> -	     tst_test->mount_device || tst_test->all_filesystems) &&
> +	     tst_test->mount_device || tst_test->all_filesystems ||
> +		 tst_test->needs_hugetlbfs) &&
>  	     !tst_test->mntpoint) {
>  		tst_brk(TBROK, "tst_test->mntpoint must be set!");
>  	}
>  
>  	if (!!tst_test->needs_rofs + !!tst_test->needs_devfs +
> -	    !!tst_test->needs_device > 1) {
> +	    !!tst_test->needs_device + !!tst_test->needs_hugetlbfs > 1) {
>  		tst_brk(TBROK,
> -			"Two or more of needs_{rofs, devfs, device} are set");
> +			"Two or more of needs_{rofs, devfs, device, hugetlbfs} are set");
>  	}
>  
>  	if (tst_test->needs_devfs)
> @@ -1217,6 +1241,9 @@ static void do_setup(int argc, char *argv[])
>  		}
>  	}
>  
> +	if (tst_test->needs_hugetlbfs)
> +		prepare_and_mount_hugetlb_fs();
> +
>  	if (tst_test->needs_device && !mntpoint_mounted) {
>  		tdev.dev = tst_acquire_device_(NULL, tst_test->dev_min_size);
>  
> @@ -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.

-- 
Cyril Hrubis
chrubis@suse.cz

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