All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.