[LTP] [PATCH] [PATCH] Migrating the libhugetlbfs/testcases/straddle_4GB.c

[LTP] [PATCH] [PATCH] Migrating the libhugetlbfs/testcases/straddle_4GB.c

[Geetika]

Test Description:
This test tries to allocate hugepages to cover a memory range
that straddles the 4GB boundary.

Signed-off-by: Geetika <geetika@linux.ibm.com>
---
 runtest/hugetlb                               |   1 +
 testcases/kernel/mem/.gitignore               |   1 +
 .../kernel/mem/hugetlb/hugemmap/hugemmap40.c  | 145 ++++++++++++++++++
 testcases/kernel/mem/hugetlb/lib/hugetlb.c    |  42 +++++
 4 files changed, 189 insertions(+)
 create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap40.c

diff --git a/runtest/hugetlb b/runtest/hugetlb
index 299c07ac9..da37dc815 100644
--- a/runtest/hugetlb
+++ b/runtest/hugetlb
@@ -35,6 +35,7 @@ hugemmap29 hugemmap29
 hugemmap30 hugemmap30
 hugemmap31 hugemmap31
 hugemmap32 hugemmap32
+hugemmap40 hugemmap40
 hugemmap05_1 hugemmap05 -m
 hugemmap05_2 hugemmap05 -s
 hugemmap05_3 hugemmap05 -s -m
diff --git a/testcases/kernel/mem/.gitignore b/testcases/kernel/mem/.gitignore
index 7258489ed..dd1858a4a 100644
--- a/testcases/kernel/mem/.gitignore
+++ b/testcases/kernel/mem/.gitignore
@@ -34,6 +34,7 @@
 /hugetlb/hugemmap/hugemmap30
 /hugetlb/hugemmap/hugemmap31
 /hugetlb/hugemmap/hugemmap32
+/hugetlb/hugemmap/hugemmap40
 /hugetlb/hugeshmat/hugeshmat01
 /hugetlb/hugeshmat/hugeshmat02
 /hugetlb/hugeshmat/hugeshmat03
diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap40.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap40.c
new file mode 100644
index 000000000..1ba070831
--- /dev/null
+++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap40.c
@@ -0,0 +1,145 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+/*
+ * Copyright (C) 2005-2006 David Gibson & Adam Litke, IBM Corporation.
+ * Copyright (C) 2006 Hugh Dickins <hugh@veritas.com>
+ */
+
+/*\
+ * [Description]
+ * This test tries to allocate hugepages to cover a memory range
+ * that straddles the 4GB boundary.
+ * Scenario 1 : mmap without MAP_FIXED
+ * Scenario 2 : mmap with MAP_FIXED
+ *
+ */
+
+#define MAPS_BUF_SZ 4096
+#define FOURGB (1UL << 32)
+#define MNTPOINT "hugetlbfs/"
+#define HUGETLBFS_MAGIC	0x958458f6
+#define _LARGEFILE64_SOURCE /* Need this for statfs64 */
+
+#include "hugetlb.h"
+
+static long hpage_size;
+static int fd = -1;
+static unsigned long straddle_addr;
+
+static int test_addr_huge(void *p)
+{
+	char name[256];
+	char *dirend;
+	int ret;
+	struct statfs64 sb;
+
+	ret = read_maps((unsigned long)p, name);
+	if (ret < 0)
+		return ret;
+	if (ret == 0) {
+		tst_res(TINFO, "Couldn't find address %p in /proc/self/maps\n", p);
+		return -1;
+	}
+	/* looks like a filename? */
+	if (name[0] != '/')
+		return 0;
+
+	/* Truncate the filename portion */
+
+	dirend = strrchr(name, '/');
+	if (dirend && dirend > name)
+		*dirend = '\0';
+
+	ret = statfs64(name, &sb);
+	if (ret)
+		return -1;
+
+	return (sb.f_type == HUGETLBFS_MAGIC);
+}
+
+static void run_test(void)
+{
+	void *p;
+
+	/* We first try to get the mapping without MAP_FIXED */
+	tst_res(TINFO, "Mapping without MAP_FIXED at %lx...", straddle_addr);
+	p = mmap((void *)straddle_addr, 2*hpage_size, PROT_READ|PROT_WRITE,
+			MAP_SHARED, fd, 0);
+	if (p == (void *)straddle_addr) {
+		/* These tests irrelevant if we didn't get the straddle address*/
+		if (test_addr_huge(p) != 1) {
+			tst_brk(TFAIL, "1st Mapped address is not hugepage");
+			goto windup;
+		}
+		if (test_addr_huge(p + hpage_size) != 1) {
+			tst_brk(TFAIL, "2nd Mapped address is not hugepage");
+			goto windup;
+		}
+		memset(p, 0, hpage_size);
+		memset(p + hpage_size, 0, hpage_size);
+		tst_res(TPASS, "Mapping without MAP_FIXED at %lx... completed", straddle_addr);
+	} else {
+		tst_res(TINFO, "Got %p instead, never mind, let's move to mapping with MAP_FIXED\n", p);
+		munmap(p, 2*hpage_size);
+	}
+	tst_res(TINFO, "Mapping with MAP_FIXED at %lx...", straddle_addr);
+	p = mmap((void *)straddle_addr, 2*hpage_size, PROT_READ|PROT_WRITE,
+				MAP_SHARED|MAP_FIXED, fd, 0);
+	if (p == MAP_FAILED) {
+		/* this area crosses last low slice and first high slice */
+		unsigned long below_start = FOURGB - 256L*1024*1024;
+		unsigned long above_end = 1024L*1024*1024*1024;
+
+		if (range_is_mapped(below_start, above_end) == 1) {
+			tst_res(TINFO, "region (4G-256M)-1T is not free");
+			tst_res(TINFO, "mmap() failed: %s\n", strerror(errno));
+			tst_res(TWARN, "Pass Inconclusive!");
+			goto windup;
+		} else
+			tst_res(TFAIL, "mmap() FIXED failed: %s\n", strerror(errno));
+	}
+		if (p != (void *)straddle_addr) {
+			tst_res(TINFO, "got %p instead\n", p);
+			tst_res(TFAIL, "Wrong address with MAP_FIXED");
+			goto windup;
+		}
+
+		if (test_addr_huge(p) != 1) {
+			tst_brk(TFAIL, "1st Mapped address is not hugepage");
+			goto windup;
+		}
+		if (test_addr_huge(p + hpage_size) != 1) {
+			tst_brk(TFAIL, "2nd Mapped address is not hugepage");
+			goto windup;
+		}
+		memset(p, 0, hpage_size);
+		memset(p + hpage_size, 0, hpage_size);
+		tst_res(TPASS, "Mapping with MAP_FIXED at %lx... completed", straddle_addr);
+
+windup:
+	SAFE_MUNMAP(p, 2*hpage_size);
+}
+
+static void setup(void)
+{
+	straddle_addr = FOURGB - hpage_size;
+	hpage_size = tst_get_hugepage_size();
+	fd = tst_creat_unlinked(MNTPOINT, 0);
+	if (hpage_size > FOURGB)
+		tst_brk(TCONF, "Huge page size is too large!");
+}
+
+static void cleanup(void)
+{
+	if (fd >= 0)
+		SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.mntpoint = MNTPOINT,
+	.needs_hugetlbfs = 1,
+	.hugepages = {2, TST_NEEDS},
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run_test,
+};
diff --git a/testcases/kernel/mem/hugetlb/lib/hugetlb.c b/testcases/kernel/mem/hugetlb/lib/hugetlb.c
index 43a677ce9..39f101517 100644
--- a/testcases/kernel/mem/hugetlb/lib/hugetlb.c
+++ b/testcases/kernel/mem/hugetlb/lib/hugetlb.c
@@ -130,3 +130,45 @@ int do_readback(void *p, size_t size, char *desc)
 	}
 	return 0;
 }
+
+#define MAPS_BUF_SZ 4096
+int read_maps(unsigned long addr, char *buf)
+{
+        FILE *f;
+        char line[MAPS_BUF_SZ];
+        char *tmp;
+
+        f = fopen("/proc/self/maps", "r");
+        if (!f) {
+                tst_res(TFAIL, "Failed to open /proc/self/maps: %s\n", strerror(errno));
+                return -1;
+        }
+
+        while (1) {
+                unsigned long start, end, off, ino;
+                int ret;
+
+                tmp = fgets(line, MAPS_BUF_SZ, f);
+                if (!tmp)
+                        break;
+
+                buf[0] = '\0';
+                ret = sscanf(line, "%lx-%lx %*s %lx %*s %ld %255s",
+                             &start, &end, &off, &ino,
+                             buf);
+                if ((ret < 4) || (ret > 5)) {
+                        tst_res(TFAIL, "Couldn't parse /proc/self/maps line: %s\n",
+                                        line);
+                        fclose(f);
+                        return -1;
+                }
+
+                if ((start <= addr) && (addr < end)) {
+                        fclose(f);
+                        return 1;
+                }
+        }
+
+        fclose(f);
+        return 0;
+}
-- 
2.39.2 (Apple Git-143)


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

Re: [LTP] [PATCH] [PATCH] Migrating the libhugetlbfs/testcases/straddle_4GB.c

[Petr Vorel]

Hi Geetika,

Please have look at my comments at your previous patch [1] [2], these changes
apply a lot for your patches as well.

[1] https://lore.kernel.org/ltp/20231128111024.GA364870@pevik/
[2] https://lore.kernel.org/ltp/20231128112254.GA367506@pevik/


> Test Description:
> This test tries to allocate hugepages to cover a memory range
> that straddles the 4GB boundary.

> Signed-off-by: Geetika <geetika@linux.ibm.com>
> ---
>  runtest/hugetlb                               |   1 +
>  testcases/kernel/mem/.gitignore               |   1 +
>  .../kernel/mem/hugetlb/hugemmap/hugemmap40.c  | 145 ++++++++++++++++++
>  testcases/kernel/mem/hugetlb/lib/hugetlb.c    |  42 +++++
>  4 files changed, 189 insertions(+)
>  create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap40.c

> diff --git a/runtest/hugetlb b/runtest/hugetlb
> index 299c07ac9..da37dc815 100644
> --- a/runtest/hugetlb
> +++ b/runtest/hugetlb
> @@ -35,6 +35,7 @@ hugemmap29 hugemmap29
>  hugemmap30 hugemmap30
>  hugemmap31 hugemmap31
>  hugemmap32 hugemmap32
> +hugemmap40 hugemmap40
>  hugemmap05_1 hugemmap05 -m
>  hugemmap05_2 hugemmap05 -s
>  hugemmap05_3 hugemmap05 -s -m
> diff --git a/testcases/kernel/mem/.gitignore b/testcases/kernel/mem/.gitignore
> index 7258489ed..dd1858a4a 100644
> --- a/testcases/kernel/mem/.gitignore
> +++ b/testcases/kernel/mem/.gitignore
> @@ -34,6 +34,7 @@
>  /hugetlb/hugemmap/hugemmap30
>  /hugetlb/hugemmap/hugemmap31
>  /hugetlb/hugemmap/hugemmap32
> +/hugetlb/hugemmap/hugemmap40
>  /hugetlb/hugeshmat/hugeshmat01
>  /hugetlb/hugeshmat/hugeshmat02
>  /hugetlb/hugeshmat/hugeshmat03
> diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap40.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap40.c
> new file mode 100644
> index 000000000..1ba070831
> --- /dev/null
> +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap40.c
> @@ -0,0 +1,145 @@
> +// SPDX-License-Identifier: LGPL-2.1-or-later
> +/*
> + * Copyright (C) 2005-2006 David Gibson & Adam Litke, IBM Corporation.
> + * Copyright (C) 2006 Hugh Dickins <hugh@veritas.com>
> + */
> +
> +/*\
> + * [Description]
> + * This test tries to allocate hugepages to cover a memory range
> + * that straddles the 4GB boundary.
> + * Scenario 1 : mmap without MAP_FIXED
> + * Scenario 2 : mmap with MAP_FIXED
This will be badly formatted (it will not be list, but inline).

How about:

/*\
 * [Description]
 *
 * Test tries to allocate hugepages to cover a memory range that straddles the
 * 4GB boundary, using mmap(2) with and without MAP_FIXED.
 */
> + */
> +
> +#define MAPS_BUF_SZ 4096
> +#define FOURGB (1UL << 32)
> +#define MNTPOINT "hugetlbfs/"
> +#define HUGETLBFS_MAGIC	0x958458f6

Could you please add this magic definition to include/tst_fs.h
(as a separate patch), we store all magic there.

> +#define _LARGEFILE64_SOURCE /* Need this for statfs64 */
We would probably define it in Makefile

hugemmap40: CFLAGS += -D_LARGEFILE64_SOURCE

> +
> +#include "hugetlb.h"
> +
> +static long hpage_size;
> +static int fd = -1;
> +static unsigned long straddle_addr;
> +
> +static int test_addr_huge(void *p)
> +{
> +	char name[256];
> +	char *dirend;
> +	int ret;
> +	struct statfs64 sb;
> +
> +	ret = read_maps((unsigned long)p, name);
> +	if (ret < 0)
> +		return ret;
> +	if (ret == 0) {
> +		tst_res(TINFO, "Couldn't find address %p in /proc/self/maps\n", p);
Please remove "\n", it's already added in tst_res().
> +		return -1;
> +	}
> +	/* looks like a filename? */
> +	if (name[0] != '/')
> +		return 0;
> +
> +	/* Truncate the filename portion */
> +
> +	dirend = strrchr(name, '/');
> +	if (dirend && dirend > name)
> +		*dirend = '\0';
> +
> +	ret = statfs64(name, &sb);
> +	if (ret)
> +		return -1;
> +
> +	return (sb.f_type == HUGETLBFS_MAGIC);
> +}
> +
> +static void run_test(void)
> +{
> +	void *p;
> +
> +	/* We first try to get the mapping without MAP_FIXED */
> +	tst_res(TINFO, "Mapping without MAP_FIXED at %lx...", straddle_addr);
> +	p = mmap((void *)straddle_addr, 2*hpage_size, PROT_READ|PROT_WRITE,
> +			MAP_SHARED, fd, 0);
> +	if (p == (void *)straddle_addr) {
> +		/* These tests irrelevant if we didn't get the straddle address*/
> +		if (test_addr_huge(p) != 1) {
> +			tst_brk(TFAIL, "1st Mapped address is not hugepage");
> +			goto windup;
> +		}
> +		if (test_addr_huge(p + hpage_size) != 1) {
> +			tst_brk(TFAIL, "2nd Mapped address is not hugepage");
> +			goto windup;
> +		}
> +		memset(p, 0, hpage_size);
> +		memset(p + hpage_size, 0, hpage_size);
> +		tst_res(TPASS, "Mapping without MAP_FIXED at %lx... completed", straddle_addr);
> +	} else {
> +		tst_res(TINFO, "Got %p instead, never mind, let's move to mapping with MAP_FIXED\n", p);
> +		munmap(p, 2*hpage_size);
> +	}
> +	tst_res(TINFO, "Mapping with MAP_FIXED at %lx...", straddle_addr);
Maybe use .tcnt = 2, in struct tst_test and separate these 2 cases into it's
wonf functions?

You would either use:
static void run_test(unsigned int n)

With that you would reduce code duplicity and make test function smaller.

Also, sometimes we use test struct (pointer to the function and description, see
e.g. testcases/kernel/mem/hugetlb/hugeshmctl/hugeshmctl02.c).

...
> +++ b/testcases/kernel/mem/hugetlb/lib/hugetlb.c
Maybe add this change as a separate commit?

Kind regards,
Petr

> @@ -130,3 +130,45 @@ int do_readback(void *p, size_t size, char *desc)
>  	}
>  	return 0;
>  }
> +
> +#define MAPS_BUF_SZ 4096
> +int read_maps(unsigned long addr, char *buf)
> +{
> +        FILE *f;
> +        char line[MAPS_BUF_SZ];
> +        char *tmp;
> +
> +        f = fopen("/proc/self/maps", "r");
> +        if (!f) {
> +                tst_res(TFAIL, "Failed to open /proc/self/maps: %s\n", strerror(errno));
> +                return -1;
> +        }
> +
> +        while (1) {
> +                unsigned long start, end, off, ino;
> +                int ret;
> +
> +                tmp = fgets(line, MAPS_BUF_SZ, f);
> +                if (!tmp)
> +                        break;
> +
> +                buf[0] = '\0';
> +                ret = sscanf(line, "%lx-%lx %*s %lx %*s %ld %255s",
> +                             &start, &end, &off, &ino,
> +                             buf);
> +                if ((ret < 4) || (ret > 5)) {
> +                        tst_res(TFAIL, "Couldn't parse /proc/self/maps line: %s\n",
> +                                        line);
> +                        fclose(f);
> +                        return -1;
> +                }
> +
> +                if ((start <= addr) && (addr < end)) {
> +                        fclose(f);
> +                        return 1;
> +                }
> +        }
> +
> +        fclose(f);
> +        return 0;
> +}

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

Re: [LTP] [PATCH] [PATCH] Migrating the libhugetlbfs/testcases/straddle_4GB.c

[Geetika M]

Hi Petr,

On 28/11/23 5:20 pm, Petr Vorel wrote:
> Hi Geetika,
>
> Please have look at my comments at your previous patch [1] [2], these changes
> apply a lot for your patches as well.
>
> [1]https://lore.kernel.org/ltp/20231128111024.GA364870@pevik/
> [2]https://lore.kernel.org/ltp/20231128112254.GA367506@pevik/
>
Noted.
I will apply whatever changes are applicable to this testcase in the 
next version.

...
>> +
>> +/*\
>> + * [Description]
>> + * This test tries to allocate hugepages to cover a memory range
>> + * that straddles the 4GB boundary.
>> + * Scenario 1 : mmap without MAP_FIXED
>> + * Scenario 2 : mmap with MAP_FIXED
> This will be badly formatted (it will not be list, but inline).
>
> How about:
>
> /*\
>   * [Description]
>   *
>   * Test tries to allocate hugepages to cover a memory range that straddles the
>   * 4GB boundary, using mmap(2) with and without MAP_FIXED.
>   */
>> + */
>> +
>> +#define MAPS_BUF_SZ 4096
>> +#define FOURGB (1UL << 32)
>> +#define MNTPOINT "hugetlbfs/"
>> +#define HUGETLBFS_MAGIC	0x958458f6
> Could you please add this magic definition to include/tst_fs.h
> (as a separate patch), we store all magic there.
>
>> +#define _LARGEFILE64_SOURCE /* Need this for statfs64 */
> We would probably define it in Makefile
>
> hugemmap40: CFLAGS += -D_LARGEFILE64_SOURCE
Noted.
I will apply above changes with the next version.

...

> +
> +static void run_test(void)
> +{
> +	void *p;
> +
> +	/* We first try to get the mapping without MAP_FIXED */
> +	tst_res(TINFO, "Mapping without MAP_FIXED at %lx...", straddle_addr);
> +	p = mmap((void *)straddle_addr, 2*hpage_size, PROT_READ|PROT_WRITE,
> +			MAP_SHARED, fd, 0);
> +	if (p == (void *)straddle_addr) {
> +		/* These tests irrelevant if we didn't get the straddle address*/
> +		if (test_addr_huge(p) != 1) {
> +			tst_brk(TFAIL, "1st Mapped address is not hugepage");
> +			goto windup;
> +		}
> +		if (test_addr_huge(p + hpage_size) != 1) {
> +			tst_brk(TFAIL, "2nd Mapped address is not hugepage");
> +			goto windup;
> +		}
> +		memset(p, 0, hpage_size);
> +		memset(p + hpage_size, 0, hpage_size);
> +		tst_res(TPASS, "Mapping without MAP_FIXED at %lx... completed", straddle_addr);
> +	} else {
> +		tst_res(TINFO, "Got %p instead, never mind, let's move to mapping with MAP_FIXED\n", p);
> +		munmap(p, 2*hpage_size);
> +	}
> +	tst_res(TINFO, "Mapping with MAP_FIXED at %lx...", straddle_addr);
> Maybe use .tcnt = 2, in struct tst_test and separate these 2 cases into it's
> wonf functions?
>
> You would either use:
> static void run_test(unsigned int n)
>
> With that you would reduce code duplicity and make test function smaller.
>
> Also, sometimes we use test struct (pointer to the function and description, see
> e.g. testcases/kernel/mem/hugetlb/hugeshmctl/hugeshmctl02.c).
>
> ...

Can you suggest how we can handle the test exit upon failure inside one 
of test cases then?

Currently I am handling it by using label windup placed at the end of 
function.
In one of the other tests there was feedback to not use exit(0); then 
how can we exit
easily upon failure from one of the testcase?


Thanks & Regards,
Geetika

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