<div dir="ltr"><div class="gmail_default" style="font-size:small">Hi Tarun,</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">This version is much better, comments are inline below.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Oct 29, 2022 at 3:14 PM Tarun Sahu <<a href="mailto:tsahu@linux.ibm.com" target="_blank">tsahu@linux.ibm.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Most of libhugetlbfs test require mounted hugetlbfs and random opened<br>
unlinked file for follow-up test actions.<br>
<br>
Here, this patch adds two new field in tst_test struct(include/tst_test.h)<br>
which user can set if the test requires mounted hugetlbfs and other one<br>
for if test requires opened unlinked file.<br>
<br>
Signed-off-by: Tarun Sahu <<a href="mailto:tsahu@linux.ibm.com" target="_blank">tsahu@linux.ibm.com</a>><br>
---<br>
include/tst_test.h | 20 +++++++++++++++++-<br>
lib/tst_test.c | 51 +++++++++++++++++++++++++++++++++++++++++++---<br>
2 files changed, 67 insertions(+), 4 deletions(-)<br>
<br>
diff --git a/include/tst_test.h b/include/tst_test.h<br>
index a69965b95..f36382ae9 100644<br>
--- a/include/tst_test.h<br>
+++ b/include/tst_test.h<br>
@@ -131,7 +131,7 @@ struct tst_tag {<br>
};<br>
<br>
extern unsigned int tst_variant;<br>
-<br>
+extern int tst_hugetlb_fd;<br>
#define TST_NO_HUGEPAGES ((unsigned long)-1)<br>
<br>
#define TST_UNLIMITED_RUNTIME (-1)<br>
@@ -176,6 +176,18 @@ struct tst_test {<br>
int all_filesystems:1;<br>
int skip_in_lockdown:1;<br>
int skip_in_compat:1;<br>
+ /*<br>
+ * If set, the test function will create a hugetlbfs mount point<br>
+ * at /tmp/xxxxxx, where xxxxxx is a random string.<br>
+ */<br>
+ int needs_hugetlbfs:1;<br>
+ /*<br>
+ * If set, the test function will create and open a random file<br>
+ * under mounted hugetlbfs. To use this option, needs_hugetlbfs must<br>
+ * be set. The file descriptior will be set in tst_hugetlb_fd.<br>
+ * The close(tst_hugetlb_fd) will be called on test exit(cleanup).<br>
+ */<br>
+ int needs_unlinked_hugetlb_file:1;<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Why not consider encapsulating these two new fields in 'struct tst_hugepage' ?</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">Then the tst_test in the case can simply initialize to:</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">....</div><div class="gmail_default" style="font-size:small">static struct tst_test test = {<br> .needs_root = 1,<br> .taint_check = TST_TAINT_D | TST_TAINT_W,<br> .setup = setup,<br> .test_all = run_test,<br> .hugepages = {1, TST_NEEDS, 1, 1},<br>};<br></div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
/*<br>
* The skip_filesystems is a NULL terminated list of filesystems the<br>
@@ -357,6 +369,12 @@ unsigned int tst_remaining_runtime(void);<br>
*/<br>
void tst_set_max_runtime(int max_runtime);<br>
<br>
+/*<br>
+ * Create and open a random file inside the given dir path.<br>
+ * It unlinks the file after opening and return file descriptor.<br>
+ */<br>
+int tst_create_unlinked_file(const char *path);<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">what about renaming this function to tst_'get|create'_unlinked_file_fd?</div><div class="gmail_default" style="font-size:small">I guess the 'fd' part should be emphasized here.</div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
/*<br>
* Returns path to the test temporary directory in a newly allocated buffer.<br>
*/<br>
diff --git a/lib/tst_test.c b/lib/tst_test.c<br>
index 8ccde1629..43cba1004 100644<br>
--- a/lib/tst_test.c<br>
+++ b/lib/tst_test.c<br>
@@ -925,7 +925,8 @@ static int needs_tmpdir(void)<br>
tst_test->needs_device ||<br>
tst_test->mntpoint ||<br>
tst_test->resource_files ||<br>
- tst_test->needs_checkpoints;<br>
+ tst_test->needs_checkpoints ||<br>
+ tst_test->needs_hugetlbfs;<br>
}<br>
<br>
static void copy_resources(void)<br>
@@ -1021,6 +1022,30 @@ static void prepare_and_mount_dev_fs(const char *mntpoint)<br>
}<br>
}<br>
<br>
+static void prepare_and_mount_hugetlb_fs(void)<br>
+{<br>
+ tst_test->mntpoint = tst_get_tmpdir();<br>
+ SAFE_MOUNT("none", tst_test->mntpoint, "hugetlbfs", 0, NULL);<br>
+ mntpoint_mounted = 1;<br>
+}<br>
+<br>
+int tst_create_unlinked_file(const char *path)<br>
+{<br>
+ char template[PATH_MAX];<br>
+ int fd;<br>
+<br>
+ snprintf(template, PATH_MAX, "%s/ltp_%.3sXXXXXX",<br>
+ path, TCID);<br>
+<br>
+ fd = mkstemp(template);<br>
+ if (fd < 0)<br>
+ tst_brk(TBROK | TERRNO,<br>
+ "%s: mkstemp(%s) failed", __func__, template);<br>
+<br>
+ SAFE_UNLINK(template);<br>
+ return fd;<br>
+}<br>
+<br>
static const char *limit_tmpfs_mount_size(const char *mnt_data,<br>
char *buf, size_t buf_size, const char *fs_type)<br>
{<br>
@@ -1094,6 +1119,8 @@ static void do_cgroup_requires(void)<br>
tst_cg_init();<br>
}<br>
<br>
+int tst_hugetlb_fd = -1;<br>
+<br>
static void do_setup(int argc, char *argv[])<br>
{<br>
if (!tst_test)<br>
@@ -1217,6 +1244,17 @@ static void do_setup(int argc, char *argv[])<br>
}<br>
}<br>
<br>
+ if (tst_test->needs_hugetlbfs)<br>
+ prepare_and_mount_hugetlb_fs();<br>
+<br>
+ if (tst_test->needs_unlinked_hugetlb_file) {<br>
+ if (!(tst_test->needs_hugetlbfs)) {<br>
+ tst_brk(TBROK, "Option needs_unlinked_hugetlb_file "<br>
+ "requires option needs_hugetlbfs");<br>
+ }<br>
+ tst_hugetlb_fd = tst_create_unlinked_file(tst_test->mntpoint);<br>
+ }<br>
+<br></blockquote><div><br></div><div class="gmail_default" style="font-size:small">Seems we have to add a confliction check[1] to avoid multiple mounting</div><div class="gmail_default" style="font-size:small">at 'tst_test->mntpoint'. Or maybe go another method to move all the hugetlbfs</div><div class="gmail_default" style="font-size:small">operations into tst_hugetlb.c to isolate details from the tst_test library, but</div><div class="gmail_default" style="font-size:small">this will require more changes for all preexisting hugetlb tests.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">[1] something like this:</div><div><br></div><div class="gmail_default">@@ -1224,9 +1224,9 @@ static void do_setup(int argc, char *argv[])</div> }<br> <br> if (!!tst_test->needs_rofs + !!tst_test->needs_devfs +<br>- !!tst_test->needs_device > 1) {<br>+ !!tst_test->needs_device + !!tst_test->needs_hugetlbfs > 1) {<br> tst_brk(TBROK,<br>- "Two or more of needs_{rofs, devfs, device} are set");<br>+ "Two or more of needs_{rofs, devfs, device, hugetlb} are set");<br> }<br><div class="gmail_default"><br></div><div class="gmail_default"><br></div><div class="gmail_default" style="font-size:small"></div><div class="gmail_default" style="font-size:small"></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
if (tst_test->needs_device && !mntpoint_mounted) {<br>
<a href="http://tdev.dev" rel="noreferrer" target="_blank">tdev.dev</a> = tst_acquire_device_(NULL, tst_test->dev_min_size);</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
@@ -1299,8 +1337,15 @@ static void do_cleanup(void)<br>
if (ovl_mounted)<br>
SAFE_UMOUNT(OVL_MNT);<br>
<br>
- if (mntpoint_mounted)<br>
- tst_umount(tst_test->mntpoint);<br>
+ if (tst_hugetlb_fd >= 0)<br>
+ SAFE_CLOSE(tst_hugetlb_fd);<br>
+<br>
+ if (mntpoint_mounted) {<br>
+ if (tst_test->needs_hugetlbfs)<br>
+ SAFE_UMOUNT(tst_test->mntpoint);<br>
+ else<br>
+ tst_umount(tst_test->mntpoint);<br>
+ }<br>
<br>
if (tst_test->needs_device && <a href="http://tdev.dev" rel="noreferrer" target="_blank">tdev.dev</a>)<br>
tst_release_device(<a href="http://tdev.dev" rel="noreferrer" target="_blank">tdev.dev</a>);<br>
-- <br>
2.31.1<br>
<br>
<br>
-- <br>
Mailing list info: <a href="https://lists.linux.it/listinfo/ltp" rel="noreferrer" target="_blank">https://lists.linux.it/listinfo/ltp</a><br>
<br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr"><div dir="ltr"><div>Regards,<br></div><div>Li Wang<br></div></div></div></div>