We currently assume that the hugetlb page size is 2 MiB, which is
why we mmap() a 2 MiB range.

Is the default hugetlb size is larger, mmap() will fail because the
range is not suitable. If the default hugetlb size is smaller (e.g.,
s390x), mmap() will fail because we would need more than one hugetlb
page, but just asserted that we have exactly one.

So let's simply use the default hugetlb page size instead of hard-coded
2 MiB, so the test isn't unconditionally skipped on architectures like
s390x.

Before this patch on s390x:
$ ./hugetlb_fault_after_madv
	1..0 # SKIP Failed to allocated huge page

With this change on s390x:
	$ ./hugetlb_fault_after_madv

While at it, make "huge_ptr" static.

Reported-by: Mario Casquero <mcasquer@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 .../selftests/mm/hugetlb_fault_after_madv.c        | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/mm/hugetlb_fault_after_madv.c b/tools/testing/selftests/mm/hugetlb_fault_after_madv.c
index 73b81c632366..ff3ba675278d 100644
--- a/tools/testing/selftests/mm/hugetlb_fault_after_madv.c
+++ b/tools/testing/selftests/mm/hugetlb_fault_after_madv.c
@@ -9,10 +9,10 @@
 #include "vm_util.h"
 #include "../kselftest.h"
 
-#define MMAP_SIZE (1 << 21)
 #define INLOOP_ITER 100
 
-char *huge_ptr;
+static char *huge_ptr;
+static size_t huge_page_size;
 
 /* Touch the memory while it is being madvised() */
 void *touch(void *unused)
@@ -30,7 +30,7 @@ void *madv(void *unused)
 	usleep(rand() % 10);
 
 	for (int i = 0; i < INLOOP_ITER; i++)
-		madvise(huge_ptr, MMAP_SIZE, MADV_DONTNEED);
+		madvise(huge_ptr, huge_page_size, MADV_DONTNEED);
 
 	return NULL;
 }
@@ -47,6 +47,10 @@ int main(void)
 
 	srand(getpid());
 
+	huge_page_size = default_huge_page_size();
+	if (!huge_page_size)
+		ksft_exit_skip("Could not detect default hugetlb page size.");
+
 	free_hugepages = get_free_hugepages();
 	if (free_hugepages != 1) {
 		ksft_exit_skip("This test needs one and only one page to execute. Got %lu\n",
@@ -54,7 +58,7 @@ int main(void)
 	}
 
 	while (max--) {
-		huge_ptr = mmap(NULL, MMAP_SIZE, PROT_READ | PROT_WRITE,
+		huge_ptr = mmap(NULL, huge_page_size, PROT_READ | PROT_WRITE,
 				MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB,
 				-1, 0);
 
@@ -66,7 +70,7 @@ int main(void)
 
 		pthread_join(thread1, NULL);
 		pthread_join(thread2, NULL);
-		munmap(huge_ptr, MMAP_SIZE);
+		munmap(huge_ptr, huge_page_size);
 	}
 
 	return KSFT_PASS;
-- 
2.46.1

On Thu, Sep 26, 2024 at 05:20:43PM +0200, David Hildenbrand wrote:
> We currently assume that the hugetlb page size is 2 MiB, which is
> why we mmap() a 2 MiB range.
> 
> Is the default hugetlb size is larger, mmap() will fail because the
> range is not suitable. If the default hugetlb size is smaller (e.g.,
> s390x), mmap() will fail because we would need more than one hugetlb
> page, but just asserted that we have exactly one.
> 
> So let's simply use the default hugetlb page size instead of hard-coded
> 2 MiB, so the test isn't unconditionally skipped on architectures like
> s390x.
> 
> Before this patch on s390x:
> $ ./hugetlb_fault_after_madv
> 	1..0 # SKIP Failed to allocated huge page
> 
> With this change on s390x:
> 	$ ./hugetlb_fault_after_madv
> 
> While at it, make "huge_ptr" static.
> 
> Reported-by: Mario Casquero <mcasquer@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>

Reviewed-by: Breno Leitao <leitao@debian.org>

On 9/26/24 09:20, David Hildenbrand wrote:
> We currently assume that the hugetlb page size is 2 MiB, which is
> why we mmap() a 2 MiB range.
> 
> Is the default hugetlb size is larger, mmap() will fail because the
> range is not suitable. If the default hugetlb size is smaller (e.g.,
> s390x), mmap() will fail because we would need more than one hugetlb
> page, but just asserted that we have exactly one.
> 
> So let's simply use the default hugetlb page size instead of hard-coded
> 2 MiB, so the test isn't unconditionally skipped on architectures like
> s390x.
> 
> Before this patch on s390x:
> $ ./hugetlb_fault_after_madv
> 	1..0 # SKIP Failed to allocated huge page
> 
> With this change on s390x:
> 	$ ./hugetlb_fault_after_madv
> 
> While at it, make "huge_ptr" static.
> 
> Reported-by: Mario Casquero <mcasquer@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>   .../selftests/mm/hugetlb_fault_after_madv.c        | 14 +++++++++-----
>   1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/hugetlb_fault_after_madv.c b/tools/testing/selftests/mm/hugetlb_fault_after_madv.c
> index 73b81c632366..ff3ba675278d 100644
> --- a/tools/testing/selftests/mm/hugetlb_fault_after_madv.c
> +++ b/tools/testing/selftests/mm/hugetlb_fault_after_madv.c
> @@ -9,10 +9,10 @@
>   #include "vm_util.h"
>   #include "../kselftest.h"
>   
> -#define MMAP_SIZE (1 << 21)
>   #define INLOOP_ITER 100
>   
> -char *huge_ptr;
> +static char *huge_ptr;
> +static size_t huge_page_size;
>   
>   /* Touch the memory while it is being madvised() */
>   void *touch(void *unused)
> @@ -30,7 +30,7 @@ void *madv(void *unused)
>   	usleep(rand() % 10);
>   
>   	for (int i = 0; i < INLOOP_ITER; i++)
> -		madvise(huge_ptr, MMAP_SIZE, MADV_DONTNEED);
> +		madvise(huge_ptr, huge_page_size, MADV_DONTNEED);

Magical effects of hard-coded values :)

Thank you for fixing this

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

On 26.09.24 17:20, David Hildenbrand wrote:

Of course, just after I sent it:

Subject: s/hguetlb/hugetlb/


-- 
Cheers,

David / dhildenb