[PATCH v1 0/2] selftests/mm: hugetlb_fault_after_madv improvements

[PATCH v1 0/2] selftests/mm: hugetlb_fault_after_madv improvements

[David Hildenbrand]

Mario brought to my attention that the hugetlb_fault_after_madv test
is currently always skipped on s390x.

Let's adjust the test to be independent of the default hugetlb page size
and while at it, also improve the test output.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Mario Casquero <mcasquer@redhat.com>
Cc: Breno Leitao <leitao@debian.org>

David Hildenbrand (2):
  selftests/mm: hugetlb_fault_after_madv: use default hguetlb page size
  selftests/mm: hugetlb_fault_after_madv: improve test output

 .../selftests/mm/hugetlb_fault_after_madv.c   | 48 ++++++++++++++++---
 1 file changed, 42 insertions(+), 6 deletions(-)

-- 
2.46.1

[PATCH v1 1/2] selftests/mm: hugetlb_fault_after_madv: use default hguetlb page size

[David Hildenbrand]

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

Re: [PATCH v1 1/2] selftests/mm: hugetlb_fault_after_madv: use default hguetlb page size

[David Hildenbrand]

On 26.09.24 17:20, David Hildenbrand wrote:

Of course, just after I sent it:

Subject: s/hguetlb/hugetlb/


-- 
Cheers,

David / dhildenb

Re: [PATCH v1 1/2] selftests/mm: hugetlb_fault_after_madv: use default hguetlb page size

[Shuah Khan]

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

Re: [PATCH v1 1/2] selftests/mm: hugetlb_fault_after_madv: use default hguetlb page size

[Breno Leitao]

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>

[PATCH v1 2/2] selftests/mm: hugetlb_fault_after_madv: improve test output

[David Hildenbrand]

Let's improve the test output. For example, print the proper test
result. Install a SIGBUS handler to catch any SIGBUS instead of
crashing the test on failure.

With unsuitable hugetlb page count:
  $ ./hugetlb_fault_after_madv
  TAP version 13
  1..1
  # [INFO] detected default hugetlb page size: 2048 KiB
  ok 2 # SKIP This test needs one and only one page to execute. Got 0
  # Totals: pass:0 fail:0 xfail:0 xpass:0 skip:1 error:0

On a failure:
  $ ./hugetlb_fault_after_madv
  TAP version 13
  1..1
  not ok 1 SIGBUS behavior
  Bail out! 1 out of 1 tests failed

On success:
  $ ./hugetlb_fault_after_madv
  TAP version 13
  1..1
  # [INFO] detected default hugetlb page size: 2048 KiB
  ok 1 SIGBUS behavior
  # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 .../selftests/mm/hugetlb_fault_after_madv.c   | 34 ++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/mm/hugetlb_fault_after_madv.c b/tools/testing/selftests/mm/hugetlb_fault_after_madv.c
index ff3ba675278d..e2640529dbb2 100644
--- a/tools/testing/selftests/mm/hugetlb_fault_after_madv.c
+++ b/tools/testing/selftests/mm/hugetlb_fault_after_madv.c
@@ -5,6 +5,8 @@
 #include <sys/mman.h>
 #include <sys/types.h>
 #include <unistd.h>
+#include <setjmp.h>
+#include <signal.h>
 
 #include "vm_util.h"
 #include "../kselftest.h"
@@ -14,11 +16,25 @@
 static char *huge_ptr;
 static size_t huge_page_size;
 
+static sigjmp_buf sigbuf;
+static bool sigbus_triggered;
+
+static void signal_handler(int signal)
+{
+	if (signal == SIGBUS) {
+		sigbus_triggered = true;
+		siglongjmp(sigbuf, 1);
+	}
+}
+
 /* Touch the memory while it is being madvised() */
 void *touch(void *unused)
 {
 	char *ptr = (char *)huge_ptr;
 
+	if (sigsetjmp(sigbuf, 1))
+		return NULL;
+
 	for (int i = 0; i < INLOOP_ITER; i++)
 		ptr[0] = '.';
 
@@ -44,13 +60,23 @@ int main(void)
 	 * interactions
 	 */
 	int max = 10000;
+	int err;
+
+	ksft_print_header();
+	ksft_set_plan(1);
 
 	srand(getpid());
 
+	if (signal(SIGBUS, signal_handler) == SIG_ERR)
+		ksft_exit_skip("Could not register signal handler.");
+
 	huge_page_size = default_huge_page_size();
 	if (!huge_page_size)
 		ksft_exit_skip("Could not detect default hugetlb page size.");
 
+	ksft_print_msg("[INFO] detected default hugetlb page size: %zu KiB\n",
+		       huge_page_size / 1024);
+
 	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",
@@ -73,5 +99,11 @@ int main(void)
 		munmap(huge_ptr, huge_page_size);
 	}
 
-	return KSFT_PASS;
+	ksft_test_result(!sigbus_triggered, "SIGBUS behavior\n");
+
+	err = ksft_get_fail_cnt();
+	if (err)
+		ksft_exit_fail_msg("%d out of %d tests failed\n",
+				   err, ksft_test_num());
+	ksft_exit_pass();
 }
-- 
2.46.1

Re: [PATCH v1 2/2] selftests/mm: hugetlb_fault_after_madv: improve test output

[Breno Leitao]

On Thu, Sep 26, 2024 at 05:20:44PM +0200, David Hildenbrand wrote:
> Let's improve the test output. For example, print the proper test
> result. Install a SIGBUS handler to catch any SIGBUS instead of
> crashing the test on failure.

That is way better now. Thanks for improving it.

> With unsuitable hugetlb page count:
>   $ ./hugetlb_fault_after_madv
>   TAP version 13
>   1..1
>   # [INFO] detected default hugetlb page size: 2048 KiB
>   ok 2 # SKIP This test needs one and only one page to execute. Got 0
>   # Totals: pass:0 fail:0 xfail:0 xpass:0 skip:1 error:0
> 
> On a failure:
>   $ ./hugetlb_fault_after_madv
>   TAP version 13
>   1..1
>   not ok 1 SIGBUS behavior
>   Bail out! 1 out of 1 tests failed
> 
> On success:
>   $ ./hugetlb_fault_after_madv
>   TAP version 13
>   1..1
>   # [INFO] detected default hugetlb page size: 2048 KiB
>   ok 1 SIGBUS behavior
>   # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>

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

Re: [PATCH v1 2/2] selftests/mm: hugetlb_fault_after_madv: improve test output

[David Hildenbrand]

On 30.09.24 12:05, Breno Leitao wrote:
> On Thu, Sep 26, 2024 at 05:20:44PM +0200, David Hildenbrand wrote:
>> Let's improve the test output. For example, print the proper test
>> result. Install a SIGBUS handler to catch any SIGBUS instead of
>> crashing the test on failure.
> 
> That is way better now. Thanks for improving it.

Thanks for the review!

-- 
Cheers,

David / dhildenb

Re: [PATCH v1 0/2] selftests/mm: hugetlb_fault_after_madv improvements

[Mario Casquero]

This series has been successfully tested. Now when executing the
hugetlb_fault_after_madv selftest the benefits of both patches could
be observed.
# ./hugetlb_fault_after_madv
TAP version 13
1..1
# [INFO] detected default hugetlb page size: 1024 KiB
ok 1 SIGBUS behavior
# Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0

Tested-by: Mario Casquero <mcasquer@redhat.com>


On Thu, Sep 26, 2024 at 5:20 PM David Hildenbrand <david@redhat.com> wrote:
>
> Mario brought to my attention that the hugetlb_fault_after_madv test
> is currently always skipped on s390x.
>
> Let's adjust the test to be independent of the default hugetlb page size
> and while at it, also improve the test output.
>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Shuah Khan <shuah@kernel.org>
> Cc: Mario Casquero <mcasquer@redhat.com>
> Cc: Breno Leitao <leitao@debian.org>
>
> David Hildenbrand (2):
>   selftests/mm: hugetlb_fault_after_madv: use default hguetlb page size
>   selftests/mm: hugetlb_fault_after_madv: improve test output
>
>  .../selftests/mm/hugetlb_fault_after_madv.c   | 48 ++++++++++++++++---
>  1 file changed, 42 insertions(+), 6 deletions(-)
>
> --
> 2.46.1
>