public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
* [LTP] [PATCH 1/3] Adding new function read_maps required for hugemmap40.
@ 2025-11-27  7:22 Pavithra
  2025-11-27  7:22 ` [LTP] [PATCH 2/3] Adding magic definition " Pavithra
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Pavithra @ 2025-11-27  7:22 UTC (permalink / raw)
  To: ltp; +Cc: pavrampu

Function to read and parse the '/proc/self/maps' file to debug memory-related issues.

Signed-off-by: Pavithra <pavrampu@linux.ibm.com>
---
 testcases/kernel/mem/hugetlb/lib/hugetlb.c | 42 ++++++++++++++++++++++
 testcases/kernel/mem/hugetlb/lib/hugetlb.h |  1 +
 2 files changed, 43 insertions(+)

diff --git a/testcases/kernel/mem/hugetlb/lib/hugetlb.c b/testcases/kernel/mem/hugetlb/lib/hugetlb.c
index 6a2976a53..fdd745eda 100644
--- a/testcases/kernel/mem/hugetlb/lib/hugetlb.c
+++ b/testcases/kernel/mem/hugetlb/lib/hugetlb.c
@@ -141,3 +141,45 @@ void update_shm_size(size_t * shm_size)
 		*shm_size = shmmax;
 	}
 }
+
+#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;
+}
diff --git a/testcases/kernel/mem/hugetlb/lib/hugetlb.h b/testcases/kernel/mem/hugetlb/lib/hugetlb.h
index 22975c99a..a59382ab9 100644
--- a/testcases/kernel/mem/hugetlb/lib/hugetlb.h
+++ b/testcases/kernel/mem/hugetlb/lib/hugetlb.h
@@ -57,6 +57,7 @@ int getipckey(void);
 int getuserid(char *user);
 void rm_shm(int shm_id);
 int do_readback(void *p, size_t size, char *desc);
+int read_maps(unsigned long addr, char *buf);
 
 void update_shm_size(size_t *shm_size);
 
-- 
2.43.5


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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [LTP] [PATCH 2/3] Adding magic definition required for hugemmap40.
  2025-11-27  7:22 [LTP] [PATCH 1/3] Adding new function read_maps required for hugemmap40 Pavithra
@ 2025-11-27  7:22 ` Pavithra
  2025-11-27  8:19   ` Petr Vorel
  2025-11-27  7:22 ` [LTP] [PATCH 3/3] [PATCH] [PATCH] Migrating the libhugetlbfs/testcases/straddle_4GB.c v3 Pavithra
  2025-11-27 10:31 ` [LTP] [PATCH 1/3] Adding new function read_maps required for hugemmap40 Petr Vorel
  2 siblings, 1 reply; 6+ messages in thread
From: Pavithra @ 2025-11-27  7:22 UTC (permalink / raw)
  To: ltp; +Cc: pavrampu

Defining HUGETLBFS_MAGIC.

Signed-off-by: Pavithra <pavrampu@linux.ibm.com>
---
 include/tst_fs.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/tst_fs.h b/include/tst_fs.h
index ceae78e7e..c829170fb 100644
--- a/include/tst_fs.h
+++ b/include/tst_fs.h
@@ -33,6 +33,7 @@
 #define TST_FUSE_MAGIC     0x65735546
 #define TST_VFAT_MAGIC     0x4d44 /* AKA MSDOS */
 #define TST_EXFAT_MAGIC    0x2011BAB0UL
+#define HUGETLBFS_MAGIC	   0x958458f6
 
 /* fs/bcachefs/bcachefs_format.h */
 #define TST_BCACHE_MAGIC		0xca451a4e
-- 
2.43.5


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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [LTP] [PATCH 3/3] [PATCH] [PATCH] Migrating the libhugetlbfs/testcases/straddle_4GB.c v3
  2025-11-27  7:22 [LTP] [PATCH 1/3] Adding new function read_maps required for hugemmap40 Pavithra
  2025-11-27  7:22 ` [LTP] [PATCH 2/3] Adding magic definition " Pavithra
@ 2025-11-27  7:22 ` Pavithra
  2025-11-27 11:33   ` Petr Vorel
  2025-11-27 10:31 ` [LTP] [PATCH 1/3] Adding new function read_maps required for hugemmap40 Petr Vorel
  2 siblings, 1 reply; 6+ messages in thread
From: Pavithra @ 2025-11-27  7:22 UTC (permalink / raw)
  To: ltp; +Cc: pavrampu

This testcase attempts to map a memory range that straddles the 4GB boundary using mmap() with and without the MAP_FIXED flag.

Changes in v3:
- Added magic definition to include/tst_fs.h as separate patch.
- Moved CFLAGS to make file.
- Added read_maps definition to separate patch.
- Removed \n from tst_res

Signed-off-by: Pavithra <pavrampu@linux.ibm.com>
---
 runtest/hugetlb                               |   1 +
 testcases/kernel/mem/.gitignore               |   1 +
 .../kernel/mem/hugetlb/hugemmap/Makefile      |   1 +
 .../kernel/mem/hugetlb/hugemmap/hugemmap40.c  | 147 ++++++++++++++++++
 4 files changed, 150 insertions(+)
 create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap40.c

diff --git a/runtest/hugetlb b/runtest/hugetlb
index 0896d3c94..8124ba3e5 100644
--- a/runtest/hugetlb
+++ b/runtest/hugetlb
@@ -36,6 +36,7 @@ hugemmap30 hugemmap30
 hugemmap31 hugemmap31
 hugemmap32 hugemmap32
 hugemmap34 hugemmap34
+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 b4455de51..314396274 100644
--- a/testcases/kernel/mem/.gitignore
+++ b/testcases/kernel/mem/.gitignore
@@ -36,6 +36,7 @@
 /hugetlb/hugemmap/hugemmap31
 /hugetlb/hugemmap/hugemmap32
 /hugetlb/hugemmap/hugemmap34
+/hugetlb/hugemmap/hugemmap40
 /hugetlb/hugeshmat/hugeshmat01
 /hugetlb/hugeshmat/hugeshmat02
 /hugetlb/hugeshmat/hugeshmat03
diff --git a/testcases/kernel/mem/hugetlb/hugemmap/Makefile b/testcases/kernel/mem/hugetlb/hugemmap/Makefile
index 6e72e7009..a1711f978 100644
--- a/testcases/kernel/mem/hugetlb/hugemmap/Makefile
+++ b/testcases/kernel/mem/hugetlb/hugemmap/Makefile
@@ -12,3 +12,4 @@ CFLAGS_no_stack_prot := $(filter-out -fstack-clash-protection, $(CFLAGS))
 
 hugemmap06: CFLAGS+=-pthread
 hugemmap34: CFLAGS=$(CFLAGS_no_stack_prot)
+hugemmap40: CFLAGS += -D_LARGEFILE64_SOURCE
diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap40.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap40.c
new file mode 100644
index 000000000..7b4ad7b05
--- /dev/null
+++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap40.c
@@ -0,0 +1,147 @@
+// 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]
+ *
+ * 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/"
+
+#include "hugetlb.h"
+
+static unsigned 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", 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", 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 (tst_mapping_in_range(below_start, above_end) == 1) {
+			tst_res(TINFO, "region (4G-256M)-1T is not free");
+			tst_res(TINFO, "mmap() failed: %s", strerror(errno));
+			tst_res(TWARN, "Pass Inconclusive!");
+			goto windup;
+		} else
+			tst_res(TFAIL, "mmap() FIXED failed: %s", strerror(errno));
+	}
+
+		if (p != (void *)straddle_addr) {
+			tst_res(TINFO, "got %p instead", 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, 0600);
+	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,
+};
-- 
2.43.5


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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH 2/3] Adding magic definition required for hugemmap40.
  2025-11-27  7:22 ` [LTP] [PATCH 2/3] Adding magic definition " Pavithra
@ 2025-11-27  8:19   ` Petr Vorel
  0 siblings, 0 replies; 6+ messages in thread
From: Petr Vorel @ 2025-11-27  8:19 UTC (permalink / raw)
  To: Pavithra; +Cc: ltp

Hi Pavithra,

...
> +++ b/include/tst_fs.h
> @@ -33,6 +33,7 @@
>  #define TST_FUSE_MAGIC     0x65735546
>  #define TST_VFAT_MAGIC     0x4d44 /* AKA MSDOS */
>  #define TST_EXFAT_MAGIC    0x2011BAB0UL
> +#define HUGETLBFS_MAGIC	   0x958458f6

Could you please use "TST_" prefix for that definition (i.e.
TST_HUGETLBFS_MAGIC) as it is used for other LTP specific definitions?
That way we avoid constant redefinition in case of including <linux/magic.h>.

With that you may add:
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH 1/3] Adding new function read_maps required for hugemmap40.
  2025-11-27  7:22 [LTP] [PATCH 1/3] Adding new function read_maps required for hugemmap40 Pavithra
  2025-11-27  7:22 ` [LTP] [PATCH 2/3] Adding magic definition " Pavithra
  2025-11-27  7:22 ` [LTP] [PATCH 3/3] [PATCH] [PATCH] Migrating the libhugetlbfs/testcases/straddle_4GB.c v3 Pavithra
@ 2025-11-27 10:31 ` Petr Vorel
  2 siblings, 0 replies; 6+ messages in thread
From: Petr Vorel @ 2025-11-27 10:31 UTC (permalink / raw)
  To: Pavithra; +Cc: ltp

Hi Pavithra, Geetika,

> Function to read and parse the '/proc/self/maps' file to debug memory-related issues.

First of all, do you plan to use read_maps() in other hugemmap tests? Otherwise
it does not make sense to add it to hugetlb.c library.

> Signed-off-by: Pavithra <pavrampu@linux.ibm.com>
> ---
>  testcases/kernel/mem/hugetlb/lib/hugetlb.c | 42 ++++++++++++++++++++++
>  testcases/kernel/mem/hugetlb/lib/hugetlb.h |  1 +
>  2 files changed, 43 insertions(+)

> diff --git a/testcases/kernel/mem/hugetlb/lib/hugetlb.c b/testcases/kernel/mem/hugetlb/lib/hugetlb.c
> index 6a2976a53..fdd745eda 100644
> --- a/testcases/kernel/mem/hugetlb/lib/hugetlb.c
> +++ b/testcases/kernel/mem/hugetlb/lib/hugetlb.c
> @@ -141,3 +141,45 @@ void update_shm_size(size_t * shm_size)
>  		*shm_size = shmmax;
>  	}
>  }
> +
> +#define MAPS_BUF_SZ 4096
NOTE: we usually use BUFSIZ (from <stdio.h>) for these purposes.
nit: we usually have definitions at the top. (I know this file has #define
RANDOM_CONSTANT 0x1234ABCD, but this file is outdated, not following LTP
standards.)

> +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) {
nit: we have SAFE_FOPEN(), this check is not needed. See my commit today:
https://github.com/linux-test-project/ltp/commit/f3803d4bfabe4da10d9fc1824df5b10c249dbed6
and please rebase your next version.

> +                tst_res(TFAIL, "Failed to open /proc/self/maps: %s\n", strerror(errno));
Even we did not have not have safe function, failure like this we consider hard
and use tst_brk(TBROK | TERRNO) to quit whole testing.
> +                return -1;
> +        }
> +
> +        while (1) {
> +                unsigned long start, end, off, ino;
> +                int ret;
> +
> +                tmp = fgets(line, MAPS_BUF_SZ, f);
> +                if (!tmp)
> +                        break;

Using
while (fgets(line, BUFSIZ, f) != NULL) {

is much simpler than having char *tmp just for checking.

> +
> +                buf[0] = '\0';
Why this? That's IMHO unnecessary.

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

FYI we can also have FILE_LINES_SCANF() and SAFE_FILE_LINES_SCANF() macros,
which wraps file_lines_scanf() from lib/safe_file_ops.c which uses vsscanf().
I believe it could be used for the task you want to achieve.
> +                }
> +
> +                if ((start <= addr) && (addr < end)) {
> +                        fclose(f);
> +                        return 1;
> +                }
> +        }
> +
> +        fclose(f);
> +        return 0;
> +}

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH 3/3] [PATCH] [PATCH] Migrating the libhugetlbfs/testcases/straddle_4GB.c v3
  2025-11-27  7:22 ` [LTP] [PATCH 3/3] [PATCH] [PATCH] Migrating the libhugetlbfs/testcases/straddle_4GB.c v3 Pavithra
@ 2025-11-27 11:33   ` Petr Vorel
  0 siblings, 0 replies; 6+ messages in thread
From: Petr Vorel @ 2025-11-27 11:33 UTC (permalink / raw)
  To: Pavithra; +Cc: ltp

Hi Pavithra, Geetika,

I very briefly looked into the original source file (I'd personally put that
link into the commit message).

> This testcase attempts to map a memory range that straddles the 4GB boundary using mmap() with and without the MAP_FIXED flag.

> Changes in v3:

FYI I tried to search in the mailing history [2] [3], but I found only v1, sent
by Geetika. Is there any other discussion than I found?

[1] https://github.com/libhugetlbfs/libhugetlbfs/blob/master/tests/straddle_4GB.c
[2] https://lore.kernel.org/ltp/?q=straddle_4GB.c
[3] https://lore.kernel.org/ltp/?q=hugemmap40

> - Added magic definition to include/tst_fs.h as separate patch.
> - Moved CFLAGS to make file.
> - Added read_maps definition to separate patch.
> - Removed \n from tst_res

> diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap40.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap40.c
> new file mode 100644
> index 000000000..7b4ad7b05
> --- /dev/null
> +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap40.c
> @@ -0,0 +1,147 @@
> +// 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]
nit: we don't use "[Description]" any more, because /*\ is enough for parsing
comments.
Please remove the line.

> + *
> + * Test tries to allocate hugepages to cover a memory range that straddles the
> + * 4GB boundary, using mmap(2) with and without MAP_FIXED.
nit: Could you please use :man2:`mmap` (this converts to a man page link [4] in
our docs, see e.g. stack_clash [5]).

[4] https://man7.org/linux/man-pages/man2/mmap.2.html
[5] https://linux-test-project.readthedocs.io/en/latest/users/test_catalog.html#stack-clash

> + */
> +
> +#define MAPS_BUF_SZ 4096
> +#define FOURGB (1UL << 32)
FYI we also have TST_GB in include/tst_fs.h.

> +#define MNTPOINT "hugetlbfs/"
> +
> +#include "hugetlb.h"
> +
> +static unsigned 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", p);
Having this as info message which is later followed by tst_brk(TFAIL) is is not
optimal. I suspect test is unnecessary complicated, but I'm not sure now how to
improve it.

> +		return -1;
> +	}
> +
> +	/* looks like a filename? */
> +	if (name[0] != '/')
> +		return 0;
> +
> +	/* Truncate the filename portion */
> +
nit: empty line above, please remove it.
> +	dirend = strrchr(name, '/');
> +	if (dirend && dirend > name)
> +		*dirend = '\0';
> +
> +	ret = statfs64(name, &sb);
> +	if (ret)
> +		return -1;
I guess we really need statfs64() not just statfs(), right?
In case of the failure test should quit with tst_brk(TBROK | TERRNO, "...").

> +
> +	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;
FYI tst_brk() quits testing (test executes only cleanup function. You need to
use tst_res(TFAIL, ...).

> +		}
> +		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", p);
> +		munmap(p, 2*hpage_size);
We have SAFE_MUNMAP().
> +	}
> +
> +	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 (tst_mapping_in_range(below_start, above_end) == 1) {
> +			tst_res(TINFO, "region (4G-256M)-1T is not free");
> +			tst_res(TINFO, "mmap() failed: %s", strerror(errno));
> +			tst_res(TWARN, "Pass Inconclusive!");
I don't understand "Pass Inconclusive!". Could it be just described what happen?

We have TERRNO which we use instead strerror(). Instead of all 3 tst_res()
mesagess above I would use something like:
			tst_res(TFAIL | TERRNO, "region (4G-256M)-1T is not free");

> +			goto windup;
> +		} else
> +			tst_res(TFAIL, "mmap() FIXED failed: %s", strerror(errno));
I also don't understand "FIXED". I guess it was supposed to be "mmap() with MAP_FIXED failed".
Does it even make sense to continue on mmap() failure? The original test does,
but I'm not sure if that's ok.

very nit: if you use { } on if, they should be also on else.
} else {
> 		tst_res(TFAIL | TERRNO, "mmap() failed: %s", strerror(errno));
}


> +	}
> +
The code below uses wrong indent
> +		if (p != (void *)straddle_addr) {
> +			tst_res(TINFO, "got %p instead", 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);
Why is this memset() for? If needed for testing, shouldn't it be at the
beginning?

> +		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, 0600);

> +	if (hpage_size > FOURGB)
> +		tst_brk(TCONF, "Huge page size is too large!");
I suppose the original check is really needed.

The original test does this check for this:

static inline long check_hugepagesize()
{
	long __hpage_size = gethugepagesize();
	if (__hpage_size < 0) {
		if (errno == ENOSYS)
			CONFIG("No hugepage kernel support\n");
		else if (errno == EOVERFLOW)
			CONFIG("Hugepage size too large");
		else
			CONFIG("Hugepage size (%s)", strerror(errno));
	}
	return __hpage_size;
}

Kind regards,
Petr

[6] https://github.com/libhugetlbfs/libhugetlbfs/blob/master/tests/hugetests.h#L126-L138

> +}
> +
> +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,
> +};

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-11-27 11:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-27  7:22 [LTP] [PATCH 1/3] Adding new function read_maps required for hugemmap40 Pavithra
2025-11-27  7:22 ` [LTP] [PATCH 2/3] Adding magic definition " Pavithra
2025-11-27  8:19   ` Petr Vorel
2025-11-27  7:22 ` [LTP] [PATCH 3/3] [PATCH] [PATCH] Migrating the libhugetlbfs/testcases/straddle_4GB.c v3 Pavithra
2025-11-27 11:33   ` Petr Vorel
2025-11-27 10:31 ` [LTP] [PATCH 1/3] Adding new function read_maps required for hugemmap40 Petr Vorel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox