Re: [PATCH] selftests: hugetlb_dio: Fixup Check for initial conditions to skip in the start
On 11/10/24 12:19, Donet Tom wrote:
This test verifies that a hugepage, used as a user buffer for
DIO operations, is correctly freed upon unmapping. To test this,
we read the count of free hugepages before and after the mmap,
DIO, and munmap operations, then check if the free hugepage count
is the same.
Reading free hugepages before the test was removed by commit
0268d4579901 ('selftests: hugetlb_dio: check for initial conditions
to skip at the start'), causing the test to always fail.
This patch adds back reading the free hugepages before starting
the test. With this patch, the tests are now passing.
Test Results without this patch
./tools/testing/selftests/mm/hugetlb_dio
TAP version 13
1..4
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 1 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 2 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 3 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 4 : Huge pages not freed!
# Totals: pass:0 fail:4 xfail:0 xpass:0 skip:0 error:0
Test results with this patch.
Sorry the test results with this patch is below.
Test results With this patch
/tools/testing/selftests/mm/hugetlb_dio
TAP version 13
1..4
# No. Free pages before allocation : 100
# No. Free pages after munmap : 100
ok 1 : Huge pages freed successfully !
# No. Free pages before allocation : 100
# No. Free pages after munmap : 100
ok 2 : Huge pages freed successfully !
# No. Free pages before allocation : 100
# No. Free pages after munmap : 100
ok 3 : Huge pages freed successfully !
# No. Free pages before allocation : 100
# No. Free pages after munmap : 100
ok 4 : Huge pages freed successfully !
# Totals: pass:4 fail:0 xfail:0 xpass:0 skip:0 error:0
Thank
Donet
./tools/testing/selftests/mm/hugetlb_dio
TAP version 13
1..4
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 1 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 2 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 3 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 4 : Huge pages not freed!
# Totals: pass:0 fail:4 xfail:0 xpass:0 skip:0 error:0
Fixes: 0268d4579901 ("selftests: hugetlb_dio: check for initial conditions to skip
in the start")
Signed-off-by: Donet Tom
---
tools/testing/selftests/mm/hugetlb_dio.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/tools/testing/selftests/mm/hugetlb_dio.c
b/tools/testing/selftests/mm/hugetlb_dio.c
index 60001c142ce9..432d5af15e66 100644
--- a/tools/testing/selftests/mm/hugetlb_dio.c
+++ b/tools/testing/selftests/mm/hugetlb_dio.c
@@ -44,6 +44,13 @@ void run_dio_using_hugetlb(unsigned int start_off, unsigned
int end_off)
if (fd < 0)
ksft_exit_fail_perror("Error opening file\n");
+ /* Get the free huge pages before allocation */
+ free_hpage_b = get_free_hugepages();
+ if (free_hpage_b == 0) {
+ close(fd);
+ ksft_exit_skip("No free hugepage, exiting!\n");
+ }
+
/* Allocate a hugetlb page */
orig_buffer = mmap(NULL, h_pagesize, mmap_prot, mmap_flags, -1, 0);
if (orig_buffer == MAP_FAILED) {
[PATCH] selftests: hugetlb_dio: Fixup Check for initial conditions to skip in the start
This test verifies that a hugepage, used as a user buffer for
DIO operations, is correctly freed upon unmapping. To test this,
we read the count of free hugepages before and after the mmap,
DIO, and munmap operations, then check if the free hugepage count
is the same.
Reading free hugepages before the test was removed by commit
0268d4579901 ('selftests: hugetlb_dio: check for initial conditions
to skip at the start'), causing the test to always fail.
This patch adds back reading the free hugepages before starting
the test. With this patch, the tests are now passing.
Test Results without this patch
./tools/testing/selftests/mm/hugetlb_dio
TAP version 13
1..4
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 1 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 2 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 3 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 4 : Huge pages not freed!
# Totals: pass:0 fail:4 xfail:0 xpass:0 skip:0 error:0
Test results with this patch.
./tools/testing/selftests/mm/hugetlb_dio
TAP version 13
1..4
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 1 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 2 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 3 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 4 : Huge pages not freed!
# Totals: pass:0 fail:4 xfail:0 xpass:0 skip:0 error:0
Fixes: 0268d4579901 ("selftests: hugetlb_dio: check for initial conditions to
skip in the start")
Signed-off-by: Donet Tom
---
tools/testing/selftests/mm/hugetlb_dio.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/tools/testing/selftests/mm/hugetlb_dio.c
b/tools/testing/selftests/mm/hugetlb_dio.c
index 60001c142ce9..432d5af15e66 100644
--- a/tools/testing/selftests/mm/hugetlb_dio.c
+++ b/tools/testing/selftests/mm/hugetlb_dio.c
@@ -44,6 +44,13 @@ void run_dio_using_hugetlb(unsigned int start_off, unsigned
int end_off)
if (fd < 0)
ksft_exit_fail_perror("Error opening file\n");
+ /* Get the free huge pages before allocation */
+ free_hpage_b = get_free_hugepages();
+ if (free_hpage_b == 0) {
+ close(fd);
+ ksft_exit_skip("No free hugepage, exiting!\n");
+ }
+
/* Allocate a hugetlb page */
orig_buffer = mmap(NULL, h_pagesize, mmap_prot, mmap_flags, -1, 0);
if (orig_buffer == MAP_FAILED) {
--
2.43.5
Re: [PATCH] selftests: hugetlb_dio: Fixup Check for initial conditions to skip in the start
On 11/10/24 12:19, Donet Tom wrote:
This test verifies that a hugepage, used as a user buffer for
DIO operations, is correctly freed upon unmapping. To test this,
we read the count of free hugepages before and after the mmap,
DIO, and munmap operations, then check if the free hugepage count
is the same.
Reading free hugepages before the test was removed by commit
0268d4579901 ('selftests: hugetlb_dio: check for initial conditions
to skip at the start'), causing the test to always fail.
This patch adds back reading the free hugepages before starting
the test. With this patch, the tests are now passing.
Test Results without this patch
./tools/testing/selftests/mm/hugetlb_dio
TAP version 13
1..4
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 1 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 2 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 3 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 4 : Huge pages not freed!
# Totals: pass:0 fail:4 xfail:0 xpass:0 skip:0 error:0
Test results with this patch.
./tools/testing/selftests/mm/hugetlb_dio
TAP version 13
1..4
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 1 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 2 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 3 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 4 : Huge pages not freed!
# Totals: pass:0 fail:4 xfail:0 xpass:0 skip:0 error:0
Fixes: 0268d4579901 ("selftests: hugetlb_dio: check for initial conditions to skip
in the start")
Signed-off-by: Donet Tom
---
tools/testing/selftests/mm/hugetlb_dio.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/tools/testing/selftests/mm/hugetlb_dio.c
b/tools/testing/selftests/mm/hugetlb_dio.c
index 60001c142ce9..432d5af15e66 100644
--- a/tools/testing/selftests/mm/hugetlb_dio.c
+++ b/tools/testing/selftests/mm/hugetlb_dio.c
@@ -44,6 +44,13 @@ void run_dio_using_hugetlb(unsigned int start_off, unsigned
int end_off)
if (fd < 0)
ksft_exit_fail_perror("Error opening file\n");
+ /* Get the free huge pages before allocation */
+ free_hpage_b = get_free_hugepages();
+ if (free_hpage_b == 0) {
+ close(fd);
+ ksft_exit_skip("No free hugepage, exiting!\n");
+ }
+
/* Allocate a hugetlb page */
orig_buffer = mmap(NULL, h_pagesize, mmap_prot, mmap_flags, -1, 0);
if (orig_buffer == MAP_FAILED) {
Hi Andrew
Would you prefer I send this fixup patch as a new series, or is it okay as is?
Thanks
Donet
[PATCH] selftests/mm: Fixed incorrect buffer->mirror size in hmm2 double_map test
The hmm2 double_map test was failing due to an incorrect buffer->mirror size. The buffer->mirror size was 6, while buffer->ptr size was 6 * PAGE_SIZE. The test failed because the kernel's copy_to_user function was attempting to copy a 6 * PAGE_SIZE buffer to buffer->mirror. Since the size of buffer->mirror was incorrect, copy_to_user failed. This patch corrects the buffer->mirror size to 6 * PAGE_SIZE. Test Result without this patch == # RUN hmm2.hmm2_device_private.double_map ... # hmm-tests.c:1680:double_map:Expected ret (-14) == 0 (0) # double_map: Test terminated by assertion # FAIL hmm2.hmm2_device_private.double_map not ok 53 hmm2.hmm2_device_private.double_map Test Result with this patch === # RUN hmm2.hmm2_device_private.double_map ... #OK hmm2.hmm2_device_private.double_map ok 53 hmm2.hmm2_device_private.double_map Signed-off-by: Donet Tom --- tools/testing/selftests/mm/hmm-tests.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/mm/hmm-tests.c b/tools/testing/selftests/mm/hmm-tests.c index d2cfc9b494a0..141bf63cbe05 100644 --- a/tools/testing/selftests/mm/hmm-tests.c +++ b/tools/testing/selftests/mm/hmm-tests.c @@ -1657,7 +1657,7 @@ TEST_F(hmm2, double_map) buffer->fd = -1; buffer->size = size; - buffer->mirror = malloc(npages); + buffer->mirror = malloc(size); ASSERT_NE(buffer->mirror, NULL); /* Reserve a range of addresses. */ -- 2.43.5
Re: [PATCH] selftests/mm: Fixed incorrect buffer->mirror size in hmm2 double_map test
On 9/27/24 12:48, Muhammad Usama Anjum wrote: On 9/27/24 10:07 AM, Donet Tom wrote: The hmm2 double_map test was failing due to an incorrect buffer->mirror size. The buffer->mirror size was 6, while buffer->ptr size was 6 * PAGE_SIZE. The test failed because the kernel's copy_to_user function was attempting to copy a 6 * PAGE_SIZE buffer to buffer->mirror. Since the size of buffer->mirror was incorrect, copy_to_user failed. This patch corrects the buffer->mirror size to 6 * PAGE_SIZE. Test Result without this patch == # RUN hmm2.hmm2_device_private.double_map ... # hmm-tests.c:1680:double_map:Expected ret (-14) == 0 (0) # double_map: Test terminated by assertion # FAIL hmm2.hmm2_device_private.double_map not ok 53 hmm2.hmm2_device_private.double_map Test Result with this patch === # RUN hmm2.hmm2_device_private.double_map ... #OK hmm2.hmm2_device_private.double_map ok 53 hmm2.hmm2_device_private.double_map Signed-off-by: Donet Tom Please add Fixes-by tag. Other than this, LGTM Fixes-by : Donet Tom I have added the Fixes-by tag here. Please let me know if you would prefer that I send a V2 with this tag. Reviewed-by: Muhammad Usama Anjum Thank you Donet --- tools/testing/selftests/mm/hmm-tests.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/mm/hmm-tests.c b/tools/testing/selftests/mm/hmm-tests.c index d2cfc9b494a0..141bf63cbe05 100644 --- a/tools/testing/selftests/mm/hmm-tests.c +++ b/tools/testing/selftests/mm/hmm-tests.c @@ -1657,7 +1657,7 @@ TEST_F(hmm2, double_map) buffer->fd = -1; buffer->size = size; - buffer->mirror = malloc(npages); + buffer->mirror = malloc(size); ASSERT_NE(buffer->mirror, NULL); /* Reserve a range of addresses. */
Re: [PATCH] selftests: hugetlb_dio: Check for initial conditions to skip in the start
On 11/1/24 19:45, Muhammad Usama Anjum wrote:
The test should be skipped if initial conditions aren't fulfilled in
the start instead of failing and outputting non-compliant TAP logs. This
kind of failure pollutes the results. The initial conditions are:
- The test should only execute if /tmp file can be allocated.
- The test should only execute if huge pages are free.
Signed-off-by: Muhammad Usama Anjum
---
Before:
TAP version 13
1..4
Bail out! Error opening file
: Read-only file system (30)
# Planned tests != run tests (4 != 0)
# Totals: pass:0 fail:0 xfail:0 xpass:0 skip:0 error:0
After:
TAP version 13
1..0 # SKIP Unable to allocate file: Read-only file system
---
tools/testing/selftests/mm/hugetlb_dio.c | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/mm/hugetlb_dio.c
b/tools/testing/selftests/mm/hugetlb_dio.c
index f9ac20c657ec6..60001c142ce99 100644
--- a/tools/testing/selftests/mm/hugetlb_dio.c
+++ b/tools/testing/selftests/mm/hugetlb_dio.c
@@ -44,13 +44,6 @@ void run_dio_using_hugetlb(unsigned int start_off, unsigned
int end_off)
if (fd < 0)
ksft_exit_fail_perror("Error opening file\n");
- /* Get the free huge pages before allocation */
- free_hpage_b = get_free_hugepages();
Hi Muhammed Usman Anjum
Reading the free pages is required before starting the test. This value will be
compared to the free pages after the test. If they are not the same, the test
will be considered a failure.
Since reading the free pages before the test was removed,|free_hpage_b| is
always 0, causing the test to fail.
./tools/testing/selftests/mm/hugetlb_dio TAP version 13 1..4 # No. Free
pages before allocation : 0 # No. Free pages after munmap : 100 not ok 1
: Huge pages not freed! # No. Free pages before allocation : 0 # No.
Free pages after munmap : 100 not ok 2 : Huge pages not freed! # No.
Free pages before allocation : 0 # No. Free pages after munmap : 100 not
ok 3 : Huge pages not freed! # No. Free pages before allocation : 0 #
No. Free pages after munmap : 100 not ok 4 : Huge pages not freed! #
Totals: pass:0 fail:4 xfail:0 xpass:0 skip:0 error:0 I think below
changes are required. --- a/tools/testing/selftests/mm/hugetlb_dio.c +++
b/tools/testing/selftests/mm/hugetlb_dio.c @@ -44,6 +44,9 @@ void
run_dio_using_hugetlb(unsigned int start_off, unsigned int end_off) if
(fd < 0) ksft_exit_fail_perror("Error opening file\n"); + /* Get the
free huge pages before allocation */ + free_hpage_b =
get_free_hugepages(); + /* Allocate a hugetlb page */ orig_buffer =
mmap(NULL, h_pagesize, mmap_prot, mmap_flags, -1, 0); if (orig_buffer ==
MAP_FAILED) { With this change the tests are passing.
./tools/testing/selftests/mm/hugetlb_dio TAP version 13 1..4 # No. Free
pages before allocation : 100 # No. Free pages after munmap : 100 ok 1 :
Huge pages freed successfully ! # No. Free pages before allocation : 100
# No. Free pages after munmap : 100 ok 2 : Huge pages freed successfully
! # No. Free pages before allocation : 100 # No. Free pages after munmap
: 100 ok 3 : Huge pages freed successfully ! # No. Free pages before
allocation : 100 # No. Free pages after munmap : 100 ok 4 : Huge pages
freed successfully ! # Totals: pass:4 fail:0 xfail:0 xpass:0 skip:0
error:0 Thank
Donet
- if (free_hpage_b == 0) {
- close(fd);
- ksft_exit_skip("No free hugepage, exiting!\n");
- }
-
/* Allocate a hugetlb page */
orig_buffer = mmap(NULL, h_pagesize, mmap_prot, mmap_flags, -1, 0);
if (orig_buffer == MAP_FAILED) {
@@ -94,8 +87,20 @@ void run_dio_using_hugetlb(unsigned int start_off, unsigned
int end_off)
int main(void)
{
size_t pagesize = 0;
+ int fd;
ksft_print_header();
+
+ /* Open the file to DIO */
+ fd = open("/tmp", O_TMPFILE | O_RDWR | O_DIRECT, 0664);
+ if (fd < 0)
+ ksft_exit_skip("Unable to allocate file: %s\n",
strerror(errno));
+ close(fd);
+
+ /* Check if huge pages are free */
+ if (!get_free_hugepages())
+ ksft_exit_skip("No free hugepage, exiting\n");
+
ksft_set_plan(4);
/* Get base page size */
Re: [PATCH] selftests: hugetlb_dio: Check for initial conditions to skip in the start
On 11/8/24 16:05, Donet Tom wrote:
On 11/1/24 19:45, Muhammad Usama Anjum wrote:
The test should be skipped if initial conditions aren't fulfilled in
the start instead of failing and outputting non-compliant TAP logs. This
kind of failure pollutes the results. The initial conditions are:
- The test should only execute if /tmp file can be allocated.
- The test should only execute if huge pages are free.
Signed-off-by: Muhammad Usama Anjum
---
Before:
TAP version 13
1..4
Bail out! Error opening file
: Read-only file system (30)
# Planned tests != run tests (4 != 0)
# Totals: pass:0 fail:0 xfail:0 xpass:0 skip:0 error:0
After:
TAP version 13
1..0 # SKIP Unable to allocate file: Read-only file system
---
tools/testing/selftests/mm/hugetlb_dio.c | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/mm/hugetlb_dio.c
b/tools/testing/selftests/mm/hugetlb_dio.c
index f9ac20c657ec6..60001c142ce99 100644
--- a/tools/testing/selftests/mm/hugetlb_dio.c
+++ b/tools/testing/selftests/mm/hugetlb_dio.c
@@ -44,13 +44,6 @@ void run_dio_using_hugetlb(unsigned int start_off,
unsigned int end_off)
if (fd < 0)
ksft_exit_fail_perror("Error opening file\n");
- /* Get the free huge pages before allocation */
- free_hpage_b = get_free_hugepages();
Hi Muhammed Usman Anjum
Reading the free pages is required before starting the test. This
value will be compared to the free pages after the test. If they are
not the same, the test will be considered a failure.
Since reading the free pages before the test was
removed,|free_hpage_b| is always 0, causing the test to fail.
./tools/testing/selftests/mm/hugetlb_dio TAP version 13 1..4 # No.
Free pages before allocation : 0 # No. Free pages after munmap : 100
not ok 1 : Huge pages not freed! # No. Free pages before allocation :
0 # No. Free pages after munmap : 100 not ok 2 : Huge pages not freed!
# No. Free pages before allocation : 0 # No. Free pages after munmap :
100 not ok 3 : Huge pages not freed! # No. Free pages before
allocation : 0 # No. Free pages after munmap : 100 not ok 4 : Huge
pages not freed! # Totals: pass:0 fail:4 xfail:0 xpass:0 skip:0
error:0 I think below changes are required. ---
a/tools/testing/selftests/mm/hugetlb_dio.c +++
b/tools/testing/selftests/mm/hugetlb_dio.c @@ -44,6 +44,9 @@ void
run_dio_using_hugetlb(unsigned int start_off, unsigned int end_off) if
(fd < 0) ksft_exit_fail_perror("Error opening file\n"); + /* Get the
free huge pages before allocation */ + free_hpage_b =
get_free_hugepages(); + /* Allocate a hugetlb page */ orig_buffer =
mmap(NULL, h_pagesize, mmap_prot, mmap_flags, -1, 0); if (orig_buffer
== MAP_FAILED) { With this change the tests are passing.
./tools/testing/selftests/mm/hugetlb_dio TAP version 13 1..4 # No.
Free pages before allocation : 100 # No. Free pages after munmap : 100
ok 1 : Huge pages freed successfully ! # No. Free pages before
allocation : 100 # No. Free pages after munmap : 100 ok 2 : Huge pages
freed successfully ! # No. Free pages before allocation : 100 # No.
Free pages after munmap : 100 ok 3 : Huge pages freed successfully ! #
No. Free pages before allocation : 100 # No. Free pages after munmap :
100 ok 4 : Huge pages freed successfully ! # Totals: pass:4 fail:0
xfail:0 xpass:0 skip:0 error:0 Thank
Donet
Sorry. Please ignore above mail.
Reading the free pages is required before starting the test. This
value will be compared to the free pages after the test. If they are not the
same, the test will be considered a failure.
Since reading the free pages before the test was removed,free_hpage_b is always
0, causing the test to fail.
./tools/testing/selftests/mm/hugetlb_dio
TAP version 13
1..4
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 1 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 2 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 3 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 4 : Huge pages not freed!
# Totals: pass:0 fail:4 xfail:0 xpass:0 skip:0 error:0
I think below changes are required.
iff --git a/tools/testing/selftests/mm/hugetlb_dio.c
b/tools/testing/selftests/mm/hugetlb_dio.c
index 60001c142ce9..4b52106b8124 100644
--- a/tools/testing/selftests/mm/hugetlb_dio.c
+++ b/tools/testing/selftests/mm/hugetlb_dio.c
@@ -44,6 +44,9 @@ void run_dio_using_hugetlb(unsigned int start_off, unsigned
int end_off)
if (fd < 0)
ksft_exit_fail_perror("Error opening file\n");
+ /* Get the free huge pages before allocation */
+ free_hpage_b = get_free_hugepages();
+
/* Allocate a hugetlb page */
orig_buffer = mmap(NULL, h_pagesize, mmap_prot, mmap_flags, -1, 0);
if (ori
Re: [PATCH] selftest: hugetlb_dio: Fix test naming
On 11/28/24 18:14, Mark Brown wrote: On Thu, Nov 28, 2024 at 10:46:56AM +0530, Donet Tom wrote: On 11/27/24 21:44, Mark Brown wrote: + ksft_test_result(free_hpage_a == free_hpage_b, +"free huge pages from %u-%u\n", start_off, end_off); This test allocates a hugetlb buffer and adjusts the start and end offsets of the buffer based on|start_off| and|end_off|. The adjusted buffer is then used for Direct I/O (DIO). If I understand correctly,|start_off| and|end_off| are not free huge pages but rather DIO buffer offsets. Should we change this message to "Hugetlb DIO buffer offset"? Sure, so long as the message is consistent it doesn't really matter what it is from the point of view of the tooling. I also noticed while doing this that the test doesn't verify that a huge page is actually used at any point, I was thinking about doing an incremental change for that too. Sure. Thank you. Please fix your mail client to word wrap within paragraphs at something substantially less than 80 columns. Doing this makes your messages much easier to read and reply to.
Re: [PATCH] selftest: hugetlb_dio: Fix test naming
On 11/27/24 21:44, Mark Brown wrote:
The string logged when a test passes or fails is used by the selftest
framework to identify which test is being reported. The hugetlb_dio test
not only uses the same strings for every test that is run but it also uses
different strings for test passes and failures which means that test
automation is unable to follow what the test is doing at all.
Pull the existing duplicated logging of the number of free huge pages
before and after the test out of the conditional and replace that and the
logging of the result with a single ksft_print_result() which incorporates
the parameters passed into the test into the output.
Fixes: fae1980347bf ("selftests: hugetlb_dio: fixup check for initial conditions to
skip in the start")
Signed-off-by: Mark Brown
---
tools/testing/selftests/mm/hugetlb_dio.c | 14 +-
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/mm/hugetlb_dio.c
b/tools/testing/selftests/mm/hugetlb_dio.c
index
432d5af15e66b7d6cac0273fb244d6696d7c9ddc..db63abe5ee5e85ff7795d3ea176c3ac47184bf4f
100644
--- a/tools/testing/selftests/mm/hugetlb_dio.c
+++ b/tools/testing/selftests/mm/hugetlb_dio.c
@@ -76,19 +76,15 @@ void run_dio_using_hugetlb(unsigned int start_off, unsigned
int end_off)
/* Get the free huge pages after unmap*/
free_hpage_a = get_free_hugepages();
+ ksft_print_msg("No. Free pages before allocation : %d\n", free_hpage_b);
+ ksft_print_msg("No. Free pages after munmap : %d\n", free_hpage_a);
+
/*
* If the no. of free hugepages before allocation and after unmap does
* not match - that means there could still be a page which is pinned.
*/
- if (free_hpage_a != free_hpage_b) {
- ksft_print_msg("No. Free pages before allocation : %d\n",
free_hpage_b);
- ksft_print_msg("No. Free pages after munmap : %d\n",
free_hpage_a);
- ksft_test_result_fail(": Huge pages not freed!\n");
- } else {
- ksft_print_msg("No. Free pages before allocation : %d\n",
free_hpage_b);
- ksft_print_msg("No. Free pages after munmap : %d\n",
free_hpage_a);
- ksft_test_result_pass(": Huge pages freed successfully !\n");
- }
+ ksft_test_result(free_hpage_a == free_hpage_b,
+"free huge pages from %u-%u\n", start_off, end_off);
Hi Mark
This test allocates a hugetlb buffer and adjusts the start and end offsets of
the buffer based
on|start_off| and|end_off|. The adjusted buffer is then used for Direct I/O
(DIO). If I understand
correctly,|start_off| and|end_off| are not free huge pages but rather DIO
buffer offsets. Should we
change this message to "Hugetlb DIO buffer offset"?
Thanks
Donet
}
int main(void)
---
base-commit: 6f3d2b5299b0a8bcb8a9405a8d3fceb24f79c4f0
change-id: 20241127-kselftest-mm-hugetlb-dio-names-1ebccbe8183d
Best regards,
[PATCH] selftests/mm: Added new test cases to the migration test
Added three new test cases to the migration tests:
1. Shared anon THP migration test
This test will mmap shared anon memory, madvise it to
MADV_HUGEPAGE, then do migration entry testing. One thread
will move pages back and forth between nodes whilst other
threads try and access them.
2. Private anon hugetlb migration test
This test will mmap private anon hugetlb memory and then
do the migration entry testing.
3. Shared anon hugetlb migration test
This test will mmap shared anon hugetlb memory and then
do the migration entry testing.
Test results
# ./tools/testing/selftests/mm/migration
TAP version 13
1..6
# Starting 6 tests from 1 test cases.
# RUN migration.private_anon ...
#OK migration.private_anon
ok 1 migration.private_anon
# RUN migration.shared_anon ...
#OK migration.shared_anon
ok 2 migration.shared_anon
# RUN migration.private_anon_thp ...
#OK migration.private_anon_thp
ok 3 migration.private_anon_thp
# RUN migration.shared_anon_thp ...
#OK migration.shared_anon_thp
ok 4 migration.shared_anon_thp
# RUN migration.private_anon_htlb ...
#OK migration.private_anon_htlb
ok 5 migration.private_anon_htlb
# RUN migration.shared_anon_htlb ...
#OK migration.shared_anon_htlb
ok 6 migration.shared_anon_htlb
# PASSED: 6 / 6 tests passed.
# Totals: pass:6 fail:0 xfail:0 xpass:0 skip:0 error:0
#
Signed-off-by: Donet Tom
---
tools/testing/selftests/mm/migration.c | 99 ++
1 file changed, 99 insertions(+)
diff --git a/tools/testing/selftests/mm/migration.c
b/tools/testing/selftests/mm/migration.c
index 64bcbb7151cf..1e3a595fbf01 100644
--- a/tools/testing/selftests/mm/migration.c
+++ b/tools/testing/selftests/mm/migration.c
@@ -204,4 +204,103 @@ TEST_F_TIMEOUT(migration, private_anon_thp, 2*RUNTIME)
ASSERT_EQ(pthread_cancel(self->threads[i]), 0);
}
+/*
+ * migration test with shared anon THP page
+ */
+
+TEST_F_TIMEOUT(migration, shared_anon_thp, 2*RUNTIME)
+{
+ pid_t pid;
+ uint64_t *ptr;
+ int i;
+
+ if (self->nthreads < 2 || self->n1 < 0 || self->n2 < 0)
+ SKIP(return, "Not enough threads or NUMA nodes available");
+
+ ptr = mmap(NULL, 2 * TWOMEG, PROT_READ | PROT_WRITE,
+ MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+ ASSERT_NE(ptr, MAP_FAILED);
+
+ ptr = (uint64_t *) ALIGN((uintptr_t) ptr, TWOMEG);
+ ASSERT_EQ(madvise(ptr, TWOMEG, MADV_HUGEPAGE), 0);
+
+ memset(ptr, 0xde, TWOMEG);
+ for (i = 0; i < self->nthreads - 1; i++) {
+ pid = fork();
+ if (!pid) {
+ prctl(PR_SET_PDEATHSIG, SIGHUP);
+ /* Parent may have died before prctl so check now. */
+ if (getppid() == 1)
+ kill(getpid(), SIGHUP);
+ access_mem(ptr);
+ } else {
+ self->pids[i] = pid;
+ }
+ }
+
+ ASSERT_EQ(migrate(ptr, self->n1, self->n2), 0);
+ for (i = 0; i < self->nthreads - 1; i++)
+ ASSERT_EQ(kill(self->pids[i], SIGTERM), 0);
+}
+
+/*
+ * migration test with private anon hugetlb page
+ */
+TEST_F_TIMEOUT(migration, private_anon_htlb, 2*RUNTIME)
+{
+ uint64_t *ptr;
+ int i;
+
+ if (self->nthreads < 2 || self->n1 < 0 || self->n2 < 0)
+ SKIP(return, "Not enough threads or NUMA nodes available");
+
+ ptr = mmap(NULL, TWOMEG, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
+ ASSERT_NE(ptr, MAP_FAILED);
+
+ memset(ptr, 0xde, TWOMEG);
+ for (i = 0; i < self->nthreads - 1; i++)
+ if (pthread_create(&self->threads[i], NULL, access_mem, ptr))
+ perror("Couldn't create thread");
+
+ ASSERT_EQ(migrate(ptr, self->n1, self->n2), 0);
+ for (i = 0; i < self->nthreads - 1; i++)
+ ASSERT_EQ(pthread_cancel(self->threads[i]), 0);
+}
+
+/*
+ * migration test with shared anon hugetlb page
+ */
+TEST_F_TIMEOUT(migration, shared_anon_htlb, 2*RUNTIME)
+{
+ pid_t pid;
+ uint64_t *ptr;
+ int i;
+
+ if (self->nthreads < 2 || self->n1 < 0 || self->n2 < 0)
+ SKIP(return, "Not enough threads or NUMA nodes available");
+
+ ptr = mmap(NULL, TWOMEG, PROT_READ | PROT_WRITE,
+ MAP_SHARED | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
+ ASSERT_NE(ptr, MAP_FAILED);
+
+ memset(ptr, 0xde, TWOMEG);
+ for (i = 0; i < self->nthreads - 1; i++) {
+ pid = fork();
+ if (!pid) {
+ prctl(PR_SET_PDEATHSIG, SIGHUP);
+
Re: [PATCH] selftest/mm: Make hugetlb_reparenting_test tolerant to async reparenting
On 4/7/25 2:12 PM, Li Wang wrote:
In cgroup v2, memory and hugetlb usage reparenting is asynchronous.
This can cause test flakiness when immediately asserting usage after
deleting a child cgroup. To address this, add a helper function
`assert_with_retry()` that checks usage values with a timeout-based retry.
This improves test stability without relying on fixed sleep delays.
Also bump up the tolerance size to 7MB.
To avoid False Positives:
...
# Assert memory charged correctly for child only use.
# actual a = 11 MB
# expected a = 0 MB
# fail
# cleanup
# [FAIL]
not ok 11 hugetlb_reparenting_test.sh -cgroup-v2 # exit=1
# 0
# SUMMARY: PASS=10 SKIP=0 FAIL=1
I was also seeing this failure. I have tested this patch on my powerPC
setup and it is passing now.
./hugetlb_reparenting_test.sh -cgroup-v2
cleanup
Test charge, rmdir, uncharge
mkdir
write
Writing to this path: /mnt/huge/test
Writing this size: 52428800
Populating.
Not writing to memory.
Using method=0
Shared mapping.
RESERVE mapping.
Allocating using HUGETLBFS.
rmdir
uncharge
cleanup
done
Test child only hugetlb usage
setup
write
Writing to this path: /mnt/huge/test2
Writing this size: 52428800
Populating.
Not writing to memory.
Using method=0
Shared mapping.
RESERVE mapping.
Allocating using HUGETLBFS.
Assert memory charged correctly for child only use.
actual = 10 MB
expected = 0 MB
cleanup
Feel free to add
Tested-by Donet Tom
Signed-off-by: Li Wang
Cc: Waiman Long
Cc: Anshuman Khandual
Cc: Dev Jain
Cc: Kirill A. Shuemov
Cc: Shuah Khan
---
.../selftests/mm/hugetlb_reparenting_test.sh | 96 ---
1 file changed, 41 insertions(+), 55 deletions(-)
diff --git a/tools/testing/selftests/mm/hugetlb_reparenting_test.sh
b/tools/testing/selftests/mm/hugetlb_reparenting_test.sh
index 11f9bbe7dc22..1c172c6999f4 100755
--- a/tools/testing/selftests/mm/hugetlb_reparenting_test.sh
+++ b/tools/testing/selftests/mm/hugetlb_reparenting_test.sh
@@ -36,7 +36,7 @@ else
do_umount=1
fi
fi
-MNT='/mnt/huge/'
+MNT='/mnt/huge'
function get_machine_hugepage_size() {
hpz=$(grep -i hugepagesize /proc/meminfo)
@@ -60,6 +60,41 @@ function cleanup() {
set -e
}
+function assert_with_retry() {
+ local actual_path="$1"
+ local expected="$2"
+ local tolerance=$((7 * 1024 * 1024))
+ local timeout=20
+ local interval=1
+ local start_time
+ local now
+ local elapsed
+ local actual
+
+ start_time=$(date +%s)
+
+ while true; do
+actual="$(cat "$actual_path")"
+
+if [[ $actual -ge $(($expected - $tolerance)) ]] &&
+[[ $actual -le $(($expected + $tolerance)) ]]; then
+ return 0
+fi
+
+now=$(date +%s)
+elapsed=$((now - start_time))
+
+if [[ $elapsed -ge $timeout ]]; then
+ echo "actual = $((${actual%% *} / 1024 / 1024)) MB"
+ echo "expected = $((${expected%% *} / 1024 / 1024)) MB"
+ cleanup
+ exit 1
+fi
+
+sleep $interval
+ done
+}
+
function assert_state() {
local expected_a="$1"
local expected_a_hugetlb="$2"
@@ -70,58 +105,13 @@ function assert_state() {
expected_b="$3"
expected_b_hugetlb="$4"
fi
- local tolerance=$((5 * 1024 * 1024))
-
- local actual_a
- actual_a="$(cat "$CGROUP_ROOT"/a/memory.$usage_file)"
- if [[ $actual_a -lt $(($expected_a - $tolerance)) ]] ||
-[[ $actual_a -gt $(($expected_a + $tolerance)) ]]; then
-echo actual a = $((${actual_a%% *} / 1024 / 1024)) MB
-echo expected a = $((${expected_a%% *} / 1024 / 1024)) MB
-echo fail
-
-cleanup
-exit 1
- fi
-
- local actual_a_hugetlb
- actual_a_hugetlb="$(cat "$CGROUP_ROOT"/a/hugetlb.${MB}MB.$usage_file)"
- if [[ $actual_a_hugetlb -lt $(($expected_a_hugetlb - $tolerance)) ]] ||
-[[ $actual_a_hugetlb -gt $(($expected_a_hugetlb + $tolerance)) ]]; then
-echo actual a hugetlb = $((${actual_a_hugetlb%% *} / 1024 / 1024)) MB
-echo expected a hugetlb = $((${expected_a_hugetlb%% *} / 1024 / 1024)) MB
-echo fail
-
-cleanup
-exit 1
- fi
-
- if [[ -z "$expected_b" || -z "$expected_b_hugetlb" ]]; then
-return
- fi
-
- local actual_b
- actual_b="$(cat "$CGROUP_ROOT"/a/b/memory.$usage_file)"
- if [[ $actual_b -lt $(($expected_b - $tolerance)) ]] ||
-[[ $actual_b -gt $(($expected_b + $tolerance)) ]]; then
-echo actual b = $((${actual_b%% *} / 1024 / 1024)) MB
-echo expected b = $((${expected_b%% *} / 1024 / 1024)) MB
-echo fail
-
-cleanup
-exit 1
- fi
- local actual_b_hugetlb
- actual_b_hugetlb="$(cat "$CGROUP_ROOT"/a/b/hugetlb.${MB}MB.$usage_file)"
- if [[ $actual_b_hugetlb -lt $(($expected_b_hugetlb - $tolerance)) ]] ||
-[[ $actual_b_hugetlb -gt $(($expected_b_hugetlb + $tolerance)) ]]; then
-echo
Re: [PATCH 2/2] selftests/mm: Fix a build failure on powerpc
On 4/28/25 6:49 PM, Nysal Jan K.A. wrote:
The compiler is unaware of the size of code generated by the ".rept"
assembler directive. This results in the compiler emitting branch
instructions where the offset to branch to exceeds the maximum allowed
value, resulting in build failures like the following:
CC protection_keys
/tmp/ccypKWAE.s: Assembler messages:
/tmp/ccypKWAE.s:2073: Error: operand out of range (0x00020158
is not between 0x8000 and 0x7ffc)
/tmp/ccypKWAE.s:2509: Error: operand out of range (0x00020130
is not between 0x8000 and 0x7ffc)
Fix the issue by manually adding nop instructions using the preprocessor.
Fixes: 46036188ea1f5 ("selftests/mm: build with -O2")
Reported-by: Madhavan Srinivasan
Signed-off-by: Nysal Jan K.A.
---
tools/testing/selftests/mm/pkey-powerpc.h | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/mm/pkey-powerpc.h
b/tools/testing/selftests/mm/pkey-powerpc.h
index d8ec906b8120..17bf2d1b0192 100644
--- a/tools/testing/selftests/mm/pkey-powerpc.h
+++ b/tools/testing/selftests/mm/pkey-powerpc.h
@@ -104,8 +104,18 @@ static inline void expect_fault_on_read_execonly_key(void
*p1, int pkey)
return;
}
+#define REPEAT_8(s) s s s s s s s s
+#define REPEAT_64(s) REPEAT_8(s) REPEAT_8(s) REPEAT_8(s) REPEAT_8(s) \
+REPEAT_8(s) REPEAT_8(s) REPEAT_8(s) REPEAT_8(s)
+#define REPEAT_512(s) REPEAT_64(s) REPEAT_64(s) REPEAT_64(s) REPEAT_64(s) \
+ REPEAT_64(s) REPEAT_64(s) REPEAT_64(s) REPEAT_64(s)
+#define REPEAT_4096(s) REPEAT_512(s) REPEAT_512(s) REPEAT_512(s) REPEAT_512(s)
\
+ REPEAT_512(s) REPEAT_512(s) REPEAT_512(s) REPEAT_512(s)
+#define REPEAT_16384(s) REPEAT_4096(s) REPEAT_4096(s) \
+ REPEAT_4096(s) REPEAT_4096(s)
+
Hi Nysal,
This change looks good to me. I tested in on power and the error is not seen.
CC droppable
CC guard-regions
CC soft-dirty
CC protection_keys
CC va_high_addr_switch
CC virtual_address_range
CC write_to_hugetlbfs
Reviewed-by:Donet Tom
Tested-by: Donet Tom
/* 4-byte instructions * 16384 = 64K page */
-#define __page_o_noops() asm(".rept 16384 ; nop; .endr")
+#define __page_o_noops() asm(REPEAT_16384("nop\n"))
static inline void *malloc_pkey_with_mprotect_subpage(long size, int prot, u16 pkey)
{
Re: [PATCH 1/6] mm/selftests: Fix virtual_address_range test issues.
On Mon, Jun 16, 2025 at 09:57:10PM +0530, Dev Jain wrote: > Hi Dev > On 16/06/25 9:36 pm, Aboorva Devarajan wrote: > > From: Donet Tom > > > > In this patch, we are fixing three issues in the virtual_address_range > > test. > > > > 1. validate_addr() checks if the allocated address is within the range. > > In the current implementation, if addr is greater than HIGH_ADDR_MARK, > > the test fails. However, addr will be greater than HIGH_ADDR_MARK if > > high_addr is set. Therefore, if high_addr is set, we should not check > > the (addr > HIGH_ADDR_MARK) condition. > > > > 2.In main(), the high address is stored in hptr, but for mark_range(), > > the address passed is ptr, not hptr. Fixed this by changing ptr[i] to > > hptr[i] in mark_range() function call. > > > > 3./proc/self/maps may not always have gaps smaller than MAP_CHUNK_SIZE. > > The gap between the first high address mapping and the previous mapping > > is not smaller than MAP_CHUNK_SIZE. > > For this, can't we just elide the check when we cross the high boundary? > As I see it you are essentially nullifying the purpose of > validate_complete_va_space; > I had written that function so as to have an alternate way of checking VA > exhaustion > without relying on mmap correctness in a circular way. > In this test, we first allocate 128TB of low memory and verify that the allocated area falls within the expected range. Next, we allocate memory at high addresses and check whether the returned addresses are within the specified limits. To allocate memory at a high address, we pass a hint address. This hint address is derived using HIGH_ADDR_SHIFT, which is set to 48 — corresponding to 256TB. So, we are requesting allocation at the 256TB address, and the memory is successfully allocated there. Since the low address region is allocated up to 128TB, there is a gap between the low address VMA and the high address VMA . Additionally, we use a random number to generate the hint address, so the actual allocated address will vary but will always be above 256TB. 1000-1001 r-xp fd:05 13429 1001-1002 r--p fd:05 13429 1002-1003 rw-p 0001 fd:05 13429 10022b8-10022bb rw-p 00:00 0 7fff5c00-7fff9c00 r--p 00:00 0 7fff9cb3-7fff9ce4 rw-p 00:00 0 7fff9ce4-7fff9d07 r-xp fd:00 792355 7fff9d07-7fff9d08 r--p 0023 fd:00 792355 7fff9d08-7fff9d09 rw-p 0024 fd:00 792355 7fff9d09-7fff9d17 r-xp fd:00 792358 7fff9d17-7fff9d18 r--p 000d fd:00 792358 7fff9d18-7fff9d19 rw-p 000e fd:00 792358 7fff9d1a-7fff9d1e r--p 00:00 0 7fff9d1e-7fff9d1f r-xp 00:00 0 7fff9d1f-7fff9d24 r-xp fd:00 792351 7fff9d24-7fff9d25 r--p 0004 fd:00 792351 7fff9d25-7fff9d26 rw-p 0005 fd:00 792351 7fffecfa-7fffecfd rw-p 00:00 0 1-14000 r--p 00:00 0 -> High address 2-24000 r--p 00:00 0 4-44000 r--p 00:00 0 8-84000 r--p 00:00 0 e80009d26-f9d26 r--p 00:00 0 The high address we are getting is at 256TB because we are explicitly requesting it. The gap between the high address VMA and the previous VMA is large because the low memory allocation goes up to 128TB. If I understand correctly, this test is verifying whether the allocated address falls within the expected range, and validate_complete_va_space() is validating the allocated virtual address space. Why do we need to check whether the gap between two VMAs is within MAP_CHUNK_SIZE? Should it validate only the allocated VMAs? Thanks Donet > Btw @Lorenzo, validate_complete_va_space was written by me as my first patch > ever for > the Linux kernel : ) from the limited knowledge I have of VMA stuff, I guess > the > only requirement for VMA alignment is PAGE_SIZE in this test, therefore, the > only > check required is that the gap between two VMAs should be at least > MAP_CHUNK_SIZE? > Or can such a gap still exist even when the VA has been exhausted? > > > > > $cat /proc/3713/maps > > 1000-1001 r-xp fd:00 36140094 >
Re: [PATCH 1/6] mm/selftests: Fix virtual_address_range test issues.
eOn Thu, Jun 19, 2025 at 02:32:19PM +0530, Dev Jain wrote: > > On 19/06/25 1:53 pm, Donet Tom wrote: > > On Wed, Jun 18, 2025 at 08:13:54PM +0530, Dev Jain wrote: > > > On 18/06/25 8:05 pm, Lorenzo Stoakes wrote: > > > > On Wed, Jun 18, 2025 at 07:47:18PM +0530, Dev Jain wrote: > > > > > On 18/06/25 7:37 pm, Lorenzo Stoakes wrote: > > > > > > On Wed, Jun 18, 2025 at 07:28:16PM +0530, Dev Jain wrote: > > > > > > > On 18/06/25 5:27 pm, Lorenzo Stoakes wrote: > > > > > > > > On Wed, Jun 18, 2025 at 05:15:50PM +0530, Dev Jain wrote: > > > > > > > > Are you accounting for sys.max_map_count? If not, then you'll > > > > > > > > be hitting that > > > > > > > > first. > > > > > > > run_vmtests.sh will run the test in overcommit mode so that won't > > > > > > > be an issue. > > > > > > Umm, what? You mean overcommit all mode, and that has no bearing on > > > > > > the max > > > > > > mapping count check. > > > > > > > > > > > > In do_mmap(): > > > > > > > > > > > > /* Too many mappings? */ > > > > > > if (mm->map_count > sysctl_max_map_count) > > > > > > return -ENOMEM; > > > > > > > > > > > > > > > > > > As well as numerous other checks in mm/vma.c. > > > > > Ah sorry, didn't look at the code properly just assumed that > > > > > overcommit_always meant overriding > > > > > this. > > > > No problem! It's hard to be aware of everything in mm :) > > > > > > > > > > I'm not sure why an overcommit toggle is even necessary when you > > > > > > could use > > > > > > MAP_NORESERVE or simply map PROT_NONE to avoid the OVERCOMMIT_GUESS > > > > > > limits? > > > > > > > > > > > > I'm pretty confused as to what this test is really achieving > > > > > > honestly. This > > > > > > isn't a useful way of asserting mmap() behaviour as far as I can > > > > > > tell. > > > > > Well, seems like a useful way to me at least : ) Not sure if you are > > > > > in the mood > > > > > to discuss that but if you'd like me to explain from start to end > > > > > what the test > > > > > is doing, I can do that : ) > > > > > > > > > I just don't have time right now, I guess I'll have to come back to it > > > > later... it's not the end of the world for it to be iffy in my view as > > > > long as > > > > it passes, but it might just not be of great value. > > > > > > > > Philosophically I'd rather we didn't assert internal implementation > > > > details like > > > > where we place mappings in userland memory. At no point do we promise > > > > to not > > > > leave larger gaps if we feel like it :) > > > You have a fair point. Anyhow a debate for another day. > > > > > > > I'm guessing, reading more, the _real_ test here is some mathematical > > > > assertion > > > > about layout from HIGH_ADDR_SHIFT -> end of address space when using > > > > hints. > > > > > > > > But again I'm not sure that achieves much and again also is asserting > > > > internal > > > > implementation details. > > > > > > > > Correct behaviour of this kind of thing probably better belongs to > > > > tests in the > > > > userland VMA testing I'd say. > > > > > > > > Sorry I don't mean to do down work you've done before, just giving an > > > > honest > > > > technical appraisal! > > > Nah, it will be rather hilarious to see it all go down the drain xD > > > > > > > Anyway don't let this block work to fix the test if it's failing. We > > > > can revisit > > > > this later. > > > Sure. @Aboorva and Donet, I still believe that the correct approach is to > > > elide > > > the gap check at the crossing boundary. What do you think? > > > > > One problem I am seeing with this approach is that, since the hint address > > is generated randomly, the VM
Re: [PATCH 1/6] mm/selftests: Fix virtual_address_range test issues.
On Wed, Jun 18, 2025 at 08:13:54PM +0530, Dev Jain wrote: > > On 18/06/25 8:05 pm, Lorenzo Stoakes wrote: > > On Wed, Jun 18, 2025 at 07:47:18PM +0530, Dev Jain wrote: > > > On 18/06/25 7:37 pm, Lorenzo Stoakes wrote: > > > > On Wed, Jun 18, 2025 at 07:28:16PM +0530, Dev Jain wrote: > > > > > On 18/06/25 5:27 pm, Lorenzo Stoakes wrote: > > > > > > On Wed, Jun 18, 2025 at 05:15:50PM +0530, Dev Jain wrote: > > > > > > Are you accounting for sys.max_map_count? If not, then you'll be > > > > > > hitting that > > > > > > first. > > > > > run_vmtests.sh will run the test in overcommit mode so that won't be > > > > > an issue. > > > > Umm, what? You mean overcommit all mode, and that has no bearing on the > > > > max > > > > mapping count check. > > > > > > > > In do_mmap(): > > > > > > > > /* Too many mappings? */ > > > > if (mm->map_count > sysctl_max_map_count) > > > > return -ENOMEM; > > > > > > > > > > > > As well as numerous other checks in mm/vma.c. > > > Ah sorry, didn't look at the code properly just assumed that > > > overcommit_always meant overriding > > > this. > > No problem! It's hard to be aware of everything in mm :) > > > > > > I'm not sure why an overcommit toggle is even necessary when you could > > > > use > > > > MAP_NORESERVE or simply map PROT_NONE to avoid the OVERCOMMIT_GUESS > > > > limits? > > > > > > > > I'm pretty confused as to what this test is really achieving honestly. > > > > This > > > > isn't a useful way of asserting mmap() behaviour as far as I can tell. > > > Well, seems like a useful way to me at least : ) Not sure if you are in > > > the mood > > > to discuss that but if you'd like me to explain from start to end what > > > the test > > > is doing, I can do that : ) > > > > > I just don't have time right now, I guess I'll have to come back to it > > later... it's not the end of the world for it to be iffy in my view as long > > as > > it passes, but it might just not be of great value. > > > > Philosophically I'd rather we didn't assert internal implementation details > > like > > where we place mappings in userland memory. At no point do we promise to not > > leave larger gaps if we feel like it :) > > You have a fair point. Anyhow a debate for another day. > > > > > I'm guessing, reading more, the _real_ test here is some mathematical > > assertion > > about layout from HIGH_ADDR_SHIFT -> end of address space when using hints. > > > > But again I'm not sure that achieves much and again also is asserting > > internal > > implementation details. > > > > Correct behaviour of this kind of thing probably better belongs to tests in > > the > > userland VMA testing I'd say. > > > > Sorry I don't mean to do down work you've done before, just giving an honest > > technical appraisal! > > Nah, it will be rather hilarious to see it all go down the drain xD > > > > > Anyway don't let this block work to fix the test if it's failing. We can > > revisit > > this later. > > Sure. @Aboorva and Donet, I still believe that the correct approach is to > elide > the gap check at the crossing boundary. What do you think? > One problem I am seeing with this approach is that, since the hint address is generated randomly, the VMAs are also being created at randomly based on the hint address.So, for the VMAs created at high addresses, we cannot guarantee that the gaps between them will be aligned to MAP_CHUNK_SIZE. High address VMAs - 1-14000 r--p 00:00 0 2-24000 r--p 00:00 0 4-44000 r--p 00:00 0 8-84000 r--p 00:00 0 e80009d26-f9d26 r--p 00:00 0 I have a different approach to solve this issue. >From 0 to 128TB, we map memory directly without using any hint. For the range >above 256TB up to 512TB, we perform the mapping using hint addresses. In the current test, we use random hint addresses, but I have modified it to generate hint addresses linearly starting from 128TB. With this change: The 0–128TB range is mapped without hints and verified accordingly. The 128TB–512TB range is mapped using linear hint addresses and then verified. Below are the VMAs obtained with this approach: 1000-1001 r-xp fd:05 135019531 1001-1002 r--p fd:05 135019531 1002-1003 rw-p 0001 fd:05 135019531 2000-1002000 r--p 00:00 0 1002080-1002083 rw-p 00:00 0 1004bcf-1004c00 rw-p 00:00 0 1004c00-7fff8c00 r--p 00:00 0 7fff8c13-7fff8c36 r-xp fd:00 792355 7fff8c36-7fff8c37 r--p 0023 fd:00 792
Re: [PATCH] selftests/mm: Fix validate_addr helper
On 6/20/25 4:41 PM, Dev Jain wrote:
validate_addr() function checks whether the address returned by mmap()
lies in the low or high VA space, according to whether a high addr hint
was passed or not. The fix commit mentioned below changed the code in
such a way that this function will always return failure when passed
high_addr == 1; addr will be >= HIGH_ADDR_MARK always, we will fall
down to "if (addr < HIGH_ADDR_MARK)" and return failure. Fix this.
Fixes: d1d86ce28d0f ("selftests/mm: virtual_address_range: conform to TAP format
output")
Signed-off-by: Dev Jain
---
tools/testing/selftests/mm/virtual_address_range.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/mm/virtual_address_range.c
b/tools/testing/selftests/mm/virtual_address_range.c
index b380e102b22f..169dbd692bf5 100644
--- a/tools/testing/selftests/mm/virtual_address_range.c
+++ b/tools/testing/selftests/mm/virtual_address_range.c
@@ -77,8 +77,11 @@ static void validate_addr(char *ptr, int high_addr)
{
unsigned long addr = (unsigned long) ptr;
- if (high_addr && addr < HIGH_ADDR_MARK)
- ksft_exit_fail_msg("Bad address %lx\n", addr);
+ if (high_addr) {
+ if (addr < HIGH_ADDR_MARK)
+ ksft_exit_fail_msg("Bad address %lx\n", addr);
+ return;
+ }
if (addr > HIGH_ADDR_MARK)
ksft_exit_fail_msg("Bad address %lx\n", addr);
This looks good to me. Feel free to add
Reviewed-by: Donet Tom
Re: [PATCH 1/6] mm/selftests: Fix virtual_address_range test issues.
eOn Tue, Jun 24, 2025 at 11:45:09AM +0530, Dev Jain wrote: > > On 23/06/25 11:02 pm, Donet Tom wrote: > > On Mon, Jun 23, 2025 at 10:23:02AM +0530, Dev Jain wrote: > > > On 21/06/25 11:25 pm, Donet Tom wrote: > > > > On Fri, Jun 20, 2025 at 08:15:25PM +0530, Dev Jain wrote: > > > > > On 19/06/25 1:53 pm, Donet Tom wrote: > > > > > > On Wed, Jun 18, 2025 at 08:13:54PM +0530, Dev Jain wrote: > > > > > > > On 18/06/25 8:05 pm, Lorenzo Stoakes wrote: > > > > > > > > On Wed, Jun 18, 2025 at 07:47:18PM +0530, Dev Jain wrote: > > > > > > > > > On 18/06/25 7:37 pm, Lorenzo Stoakes wrote: > > > > > > > > > > On Wed, Jun 18, 2025 at 07:28:16PM +0530, Dev Jain wrote: > > > > > > > > > > > On 18/06/25 5:27 pm, Lorenzo Stoakes wrote: > > > > > > > > > > > > On Wed, Jun 18, 2025 at 05:15:50PM +0530, Dev Jain > > > > > > > > > > > > wrote: > > > > > > > > > > > > Are you accounting for sys.max_map_count? If not, then > > > > > > > > > > > > you'll be hitting that > > > > > > > > > > > > first. > > > > > > > > > > > run_vmtests.sh will run the test in overcommit mode so > > > > > > > > > > > that won't be an issue. > > > > > > > > > > Umm, what? You mean overcommit all mode, and that has no > > > > > > > > > > bearing on the max > > > > > > > > > > mapping count check. > > > > > > > > > > > > > > > > > > > > In do_mmap(): > > > > > > > > > > > > > > > > > > > > /* Too many mappings? */ > > > > > > > > > > if (mm->map_count > sysctl_max_map_count) > > > > > > > > > > return -ENOMEM; > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > As well as numerous other checks in mm/vma.c. > > > > > > > > > Ah sorry, didn't look at the code properly just assumed that > > > > > > > > > overcommit_always meant overriding > > > > > > > > > this. > > > > > > > > No problem! It's hard to be aware of everything in mm :) > > > > > > > > > > > > > > > > > > I'm not sure why an overcommit toggle is even necessary > > > > > > > > > > when you could use > > > > > > > > > > MAP_NORESERVE or simply map PROT_NONE to avoid the > > > > > > > > > > OVERCOMMIT_GUESS limits? > > > > > > > > > > > > > > > > > > > > I'm pretty confused as to what this test is really > > > > > > > > > > achieving honestly. This > > > > > > > > > > isn't a useful way of asserting mmap() behaviour as far as > > > > > > > > > > I can tell. > > > > > > > > > Well, seems like a useful way to me at least : ) Not sure if > > > > > > > > > you are in the mood > > > > > > > > > to discuss that but if you'd like me to explain from start to > > > > > > > > > end what the test > > > > > > > > > is doing, I can do that : ) > > > > > > > > > > > > > > > > > I just don't have time right now, I guess I'll have to come > > > > > > > > back to it > > > > > > > > later... it's not the end of the world for it to be iffy in my > > > > > > > > view as long as > > > > > > > > it passes, but it might just not be of great value. > > > > > > > > > > > > > > > > Philosophically I'd rather we didn't assert internal > > > > > > > > implementation details like > > > > > > > > where we place mappings in userland memory. At no point do we > > > > > > > > promise to not > > > > > > > > leave larger gaps if we feel like it :) > > > > > > > You have a fair point.
Re: [PATCH 1/6] mm/selftests: Fix virtual_address_range test issues.
On Mon, Jun 23, 2025 at 10:23:02AM +0530, Dev Jain wrote: > > On 21/06/25 11:25 pm, Donet Tom wrote: > > On Fri, Jun 20, 2025 at 08:15:25PM +0530, Dev Jain wrote: > > > On 19/06/25 1:53 pm, Donet Tom wrote: > > > > On Wed, Jun 18, 2025 at 08:13:54PM +0530, Dev Jain wrote: > > > > > On 18/06/25 8:05 pm, Lorenzo Stoakes wrote: > > > > > > On Wed, Jun 18, 2025 at 07:47:18PM +0530, Dev Jain wrote: > > > > > > > On 18/06/25 7:37 pm, Lorenzo Stoakes wrote: > > > > > > > > On Wed, Jun 18, 2025 at 07:28:16PM +0530, Dev Jain wrote: > > > > > > > > > On 18/06/25 5:27 pm, Lorenzo Stoakes wrote: > > > > > > > > > > On Wed, Jun 18, 2025 at 05:15:50PM +0530, Dev Jain wrote: > > > > > > > > > > Are you accounting for sys.max_map_count? If not, then > > > > > > > > > > you'll be hitting that > > > > > > > > > > first. > > > > > > > > > run_vmtests.sh will run the test in overcommit mode so that > > > > > > > > > won't be an issue. > > > > > > > > Umm, what? You mean overcommit all mode, and that has no > > > > > > > > bearing on the max > > > > > > > > mapping count check. > > > > > > > > > > > > > > > > In do_mmap(): > > > > > > > > > > > > > > > > /* Too many mappings? */ > > > > > > > > if (mm->map_count > sysctl_max_map_count) > > > > > > > > return -ENOMEM; > > > > > > > > > > > > > > > > > > > > > > > > As well as numerous other checks in mm/vma.c. > > > > > > > Ah sorry, didn't look at the code properly just assumed that > > > > > > > overcommit_always meant overriding > > > > > > > this. > > > > > > No problem! It's hard to be aware of everything in mm :) > > > > > > > > > > > > > > I'm not sure why an overcommit toggle is even necessary when > > > > > > > > you could use > > > > > > > > MAP_NORESERVE or simply map PROT_NONE to avoid the > > > > > > > > OVERCOMMIT_GUESS limits? > > > > > > > > > > > > > > > > I'm pretty confused as to what this test is really achieving > > > > > > > > honestly. This > > > > > > > > isn't a useful way of asserting mmap() behaviour as far as I > > > > > > > > can tell. > > > > > > > Well, seems like a useful way to me at least : ) Not sure if you > > > > > > > are in the mood > > > > > > > to discuss that but if you'd like me to explain from start to end > > > > > > > what the test > > > > > > > is doing, I can do that : ) > > > > > > > > > > > > > I just don't have time right now, I guess I'll have to come back to > > > > > > it > > > > > > later... it's not the end of the world for it to be iffy in my view > > > > > > as long as > > > > > > it passes, but it might just not be of great value. > > > > > > > > > > > > Philosophically I'd rather we didn't assert internal implementation > > > > > > details like > > > > > > where we place mappings in userland memory. At no point do we > > > > > > promise to not > > > > > > leave larger gaps if we feel like it :) > > > > > You have a fair point. Anyhow a debate for another day. > > > > > > > > > > > I'm guessing, reading more, the _real_ test here is some > > > > > > mathematical assertion > > > > > > about layout from HIGH_ADDR_SHIFT -> end of address space when > > > > > > using hints. > > > > > > > > > > > > But again I'm not sure that achieves much and again also is > > > > > > asserting internal > > > > > > implementation details. > > > > > > > > > > > > Correct behaviour of this kind of thing probably better belongs to > > > > > > tests in the &g
Re: [PATCH 1/6] mm/selftests: Fix virtual_address_range test issues.
On Fri, Jun 20, 2025 at 08:15:25PM +0530, Dev Jain wrote: > > On 19/06/25 1:53 pm, Donet Tom wrote: > > On Wed, Jun 18, 2025 at 08:13:54PM +0530, Dev Jain wrote: > > > On 18/06/25 8:05 pm, Lorenzo Stoakes wrote: > > > > On Wed, Jun 18, 2025 at 07:47:18PM +0530, Dev Jain wrote: > > > > > On 18/06/25 7:37 pm, Lorenzo Stoakes wrote: > > > > > > On Wed, Jun 18, 2025 at 07:28:16PM +0530, Dev Jain wrote: > > > > > > > On 18/06/25 5:27 pm, Lorenzo Stoakes wrote: > > > > > > > > On Wed, Jun 18, 2025 at 05:15:50PM +0530, Dev Jain wrote: > > > > > > > > Are you accounting for sys.max_map_count? If not, then you'll > > > > > > > > be hitting that > > > > > > > > first. > > > > > > > run_vmtests.sh will run the test in overcommit mode so that won't > > > > > > > be an issue. > > > > > > Umm, what? You mean overcommit all mode, and that has no bearing on > > > > > > the max > > > > > > mapping count check. > > > > > > > > > > > > In do_mmap(): > > > > > > > > > > > > /* Too many mappings? */ > > > > > > if (mm->map_count > sysctl_max_map_count) > > > > > > return -ENOMEM; > > > > > > > > > > > > > > > > > > As well as numerous other checks in mm/vma.c. > > > > > Ah sorry, didn't look at the code properly just assumed that > > > > > overcommit_always meant overriding > > > > > this. > > > > No problem! It's hard to be aware of everything in mm :) > > > > > > > > > > I'm not sure why an overcommit toggle is even necessary when you > > > > > > could use > > > > > > MAP_NORESERVE or simply map PROT_NONE to avoid the OVERCOMMIT_GUESS > > > > > > limits? > > > > > > > > > > > > I'm pretty confused as to what this test is really achieving > > > > > > honestly. This > > > > > > isn't a useful way of asserting mmap() behaviour as far as I can > > > > > > tell. > > > > > Well, seems like a useful way to me at least : ) Not sure if you are > > > > > in the mood > > > > > to discuss that but if you'd like me to explain from start to end > > > > > what the test > > > > > is doing, I can do that : ) > > > > > > > > > I just don't have time right now, I guess I'll have to come back to it > > > > later... it's not the end of the world for it to be iffy in my view as > > > > long as > > > > it passes, but it might just not be of great value. > > > > > > > > Philosophically I'd rather we didn't assert internal implementation > > > > details like > > > > where we place mappings in userland memory. At no point do we promise > > > > to not > > > > leave larger gaps if we feel like it :) > > > You have a fair point. Anyhow a debate for another day. > > > > > > > I'm guessing, reading more, the _real_ test here is some mathematical > > > > assertion > > > > about layout from HIGH_ADDR_SHIFT -> end of address space when using > > > > hints. > > > > > > > > But again I'm not sure that achieves much and again also is asserting > > > > internal > > > > implementation details. > > > > > > > > Correct behaviour of this kind of thing probably better belongs to > > > > tests in the > > > > userland VMA testing I'd say. > > > > > > > > Sorry I don't mean to do down work you've done before, just giving an > > > > honest > > > > technical appraisal! > > > Nah, it will be rather hilarious to see it all go down the drain xD > > > > > > > Anyway don't let this block work to fix the test if it's failing. We > > > > can revisit > > > > this later. > > > Sure. @Aboorva and Donet, I still believe that the correct approach is to > > > elide > > > the gap check at the crossing boundary. What do you think? > > > > > One problem I am seeing with this approach is that, since the hint address > > is generated randomly, the VMAs ar
Re: [PATCH 1/6] mm/selftests: Fix virtual_address_range test issues.
On Thu, Jun 26, 2025 at 09:27:30AM +0530, Dev Jain wrote: > > On 25/06/25 10:47 pm, Donet Tom wrote: > > On Wed, Jun 25, 2025 at 06:22:53PM +0530, Dev Jain wrote: > > > On 19/06/25 1:53 pm, Donet Tom wrote: > > > > On Wed, Jun 18, 2025 at 08:13:54PM +0530, Dev Jain wrote: > > > > > On 18/06/25 8:05 pm, Lorenzo Stoakes wrote: > > > > > > On Wed, Jun 18, 2025 at 07:47:18PM +0530, Dev Jain wrote: > > > > > > > On 18/06/25 7:37 pm, Lorenzo Stoakes wrote: > > > > > > > > On Wed, Jun 18, 2025 at 07:28:16PM +0530, Dev Jain wrote: > > > > > > > > > On 18/06/25 5:27 pm, Lorenzo Stoakes wrote: > > > > > > > > > > On Wed, Jun 18, 2025 at 05:15:50PM +0530, Dev Jain wrote: > > > > > > > > > > Are you accounting for sys.max_map_count? If not, then > > > > > > > > > > you'll be hitting that > > > > > > > > > > first. > > > > > > > > > run_vmtests.sh will run the test in overcommit mode so that > > > > > > > > > won't be an issue. > > > > > > > > Umm, what? You mean overcommit all mode, and that has no > > > > > > > > bearing on the max > > > > > > > > mapping count check. > > > > > > > > > > > > > > > > In do_mmap(): > > > > > > > > > > > > > > > > /* Too many mappings? */ > > > > > > > > if (mm->map_count > sysctl_max_map_count) > > > > > > > > return -ENOMEM; > > > > > > > > > > > > > > > > > > > > > > > > As well as numerous other checks in mm/vma.c. > > > > > > > Ah sorry, didn't look at the code properly just assumed that > > > > > > > overcommit_always meant overriding > > > > > > > this. > > > > > > No problem! It's hard to be aware of everything in mm :) > > > > > > > > > > > > > > I'm not sure why an overcommit toggle is even necessary when > > > > > > > > you could use > > > > > > > > MAP_NORESERVE or simply map PROT_NONE to avoid the > > > > > > > > OVERCOMMIT_GUESS limits? > > > > > > > > > > > > > > > > I'm pretty confused as to what this test is really achieving > > > > > > > > honestly. This > > > > > > > > isn't a useful way of asserting mmap() behaviour as far as I > > > > > > > > can tell. > > > > > > > Well, seems like a useful way to me at least : ) Not sure if you > > > > > > > are in the mood > > > > > > > to discuss that but if you'd like me to explain from start to end > > > > > > > what the test > > > > > > > is doing, I can do that : ) > > > > > > > > > > > > > I just don't have time right now, I guess I'll have to come back to > > > > > > it > > > > > > later... it's not the end of the world for it to be iffy in my view > > > > > > as long as > > > > > > it passes, but it might just not be of great value. > > > > > > > > > > > > Philosophically I'd rather we didn't assert internal implementation > > > > > > details like > > > > > > where we place mappings in userland memory. At no point do we > > > > > > promise to not > > > > > > leave larger gaps if we feel like it :) > > > > > You have a fair point. Anyhow a debate for another day. > > > > > > > > > > > I'm guessing, reading more, the _real_ test here is some > > > > > > mathematical assertion > > > > > > about layout from HIGH_ADDR_SHIFT -> end of address space when > > > > > > using hints. > > > > > > > > > > > > But again I'm not sure that achieves much and again also is > > > > > > asserting internal > > > > > > implementation details. > > > > > > > > > > > > Correct behaviour of this kind of thing probably better belongs to > > > > > > tests in the
Re: [PATCH 1/6] mm/selftests: Fix virtual_address_range test issues.
On Thu, Jun 26, 2025 at 12:05:11PM +0530, Dev Jain wrote: > > On 26/06/25 11:12 am, Donet Tom wrote: > > On Thu, Jun 26, 2025 at 09:27:30AM +0530, Dev Jain wrote: > > > On 25/06/25 10:47 pm, Donet Tom wrote: > > > > On Wed, Jun 25, 2025 at 06:22:53PM +0530, Dev Jain wrote: > > > > > On 19/06/25 1:53 pm, Donet Tom wrote: > > > > > > On Wed, Jun 18, 2025 at 08:13:54PM +0530, Dev Jain wrote: > > > > > > > On 18/06/25 8:05 pm, Lorenzo Stoakes wrote: > > > > > > > > On Wed, Jun 18, 2025 at 07:47:18PM +0530, Dev Jain wrote: > > > > > > > > > On 18/06/25 7:37 pm, Lorenzo Stoakes wrote: > > > > > > > > > > On Wed, Jun 18, 2025 at 07:28:16PM +0530, Dev Jain wrote: > > > > > > > > > > > On 18/06/25 5:27 pm, Lorenzo Stoakes wrote: > > > > > > > > > > > > On Wed, Jun 18, 2025 at 05:15:50PM +0530, Dev Jain > > > > > > > > > > > > wrote: > > > > > > > > > > > > Are you accounting for sys.max_map_count? If not, then > > > > > > > > > > > > you'll be hitting that > > > > > > > > > > > > first. > > > > > > > > > > > run_vmtests.sh will run the test in overcommit mode so > > > > > > > > > > > that won't be an issue. > > > > > > > > > > Umm, what? You mean overcommit all mode, and that has no > > > > > > > > > > bearing on the max > > > > > > > > > > mapping count check. > > > > > > > > > > > > > > > > > > > > In do_mmap(): > > > > > > > > > > > > > > > > > > > > /* Too many mappings? */ > > > > > > > > > > if (mm->map_count > sysctl_max_map_count) > > > > > > > > > > return -ENOMEM; > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > As well as numerous other checks in mm/vma.c. > > > > > > > > > Ah sorry, didn't look at the code properly just assumed that > > > > > > > > > overcommit_always meant overriding > > > > > > > > > this. > > > > > > > > No problem! It's hard to be aware of everything in mm :) > > > > > > > > > > > > > > > > > > I'm not sure why an overcommit toggle is even necessary > > > > > > > > > > when you could use > > > > > > > > > > MAP_NORESERVE or simply map PROT_NONE to avoid the > > > > > > > > > > OVERCOMMIT_GUESS limits? > > > > > > > > > > > > > > > > > > > > I'm pretty confused as to what this test is really > > > > > > > > > > achieving honestly. This > > > > > > > > > > isn't a useful way of asserting mmap() behaviour as far as > > > > > > > > > > I can tell. > > > > > > > > > Well, seems like a useful way to me at least : ) Not sure if > > > > > > > > > you are in the mood > > > > > > > > > to discuss that but if you'd like me to explain from start to > > > > > > > > > end what the test > > > > > > > > > is doing, I can do that : ) > > > > > > > > > > > > > > > > > I just don't have time right now, I guess I'll have to come > > > > > > > > back to it > > > > > > > > later... it's not the end of the world for it to be iffy in my > > > > > > > > view as long as > > > > > > > > it passes, but it might just not be of great value. > > > > > > > > > > > > > > > > Philosophically I'd rather we didn't assert internal > > > > > > > > implementation details like > > > > > > > > where we place mappings in userland memory. At no point do we > > > > > > > > promise to not > > > > > > > > leave larger gaps if we feel like it :) > > > > > > > You have a fair point. A
Re: [PATCH v2 5/7] selftests/mm: Fix child process exit codes in ksm_functional_tests
On 7/3/25 2:44 PM, David Hildenbrand wrote:
On 03.07.25 10:51, Donet Tom wrote:
Hi David
On 7/3/25 2:03 PM, David Hildenbrand wrote:
On 03.07.25 08:06, Aboorva Devarajan wrote:
In ksm_functional_tests, test_child_ksm() returned negative values
to indicate errors. However, when passed to exit(), these were
interpreted as large unsigned values (e.g, -2 became 254), leading to
incorrect handling in the parent process. As a result, some tests
appeared to be skipped or silently failed.
This patch changes test_child_ksm() to return positive error codes
(1, 2, 3) and updates test_child_ksm_err() to interpret them
correctly.
This ensures the parent accurately detects and reports child process
failures.
--
Before patch:
--
- [RUN] test_unmerge
ok 1 Pages were unmerged
...
- [RUN] test_prctl_fork
- No pages got merged
- [RUN] test_prctl_fork_exec
ok 7 PR_SET_MEMORY_MERGE value is inherited
...
Bail out! 1 out of 8 tests failed
- Planned tests != run tests (9 != 8)
- Totals: pass:7 fail:1 xfail:0 xpass:0 skip:0 error:0
--
After patch:
--
- [RUN] test_unmerge
ok 1 Pages were unmerged
...
- [RUN] test_prctl_fork
- No pages got merged
not ok 7 Merge in child failed
- [RUN] test_prctl_fork_exec
ok 8 PR_SET_MEMORY_MERGE value is inherited
...
Bail out! 2 out of 9 tests failed
- Totals: pass:7 fail:2 xfail:0 xpass:0 skip:0 error:0
Fixes: 6c47de3be3a0 ("selftest/mm: ksm_functional_tests: extend test
case for ksm fork/exec")
Signed-off-by: Aboorva Devarajan
BTW, when I run the test, I get this weird output
TAP version 13
1..9
# [RUN] test_unmerge
ok 1 Pages were unmerged
# [RUN] test_unmerge_zero_pages
ok 2 KSM zero pages were unmerged
# [RUN] test_unmerge_discarded
ok 3 Pages were unmerged
# [RUN] test_unmerge_uffd_wp
ok 4 Pages were unmerged
# [RUN] test_prot_none
ok 5 Pages were unmerged
# [RUN] test_prctl
ok 6 Setting/clearing PR_SET_MEMORY_MERGE works
# [RUN] test_prctl_fork
ok 7 PR_SET_MEMORY_MERGE value is inherited
# [RUN] test_prctl_fork_exec
^ where is the test?
# [RUN] test_prctl_unmerge
ok 8 Pages were unmerged
# Planned tests != run tests (9 != 8)
# Totals: pass:8 fail:0 xfail:0 xpass:0 skip:0 error:0
^ what?
ok 8 PR_SET_MEMORY_MERGE value is inherited
# [RUN] test_prctl_unmerge
ok 9 Pages were unmerged
# Totals: pass:9 fail:0 xfail:0 xpass:0 skip:0 error:0
^ huh, what now?
The problem with the exec test is that it uses its own binary to exec.
} else if (child_pid == 0) {
char *prg_name = "./ksm_functional_tests";
char *argv_for_program[] = { prg_name,
FORK_EXEC_CHILD_PRG_NAME, NULL };
execv(prg_name, argv_for_program);
return;
}
> > So we should run it on the same directory where the binary present.
So, I assume the execv fails. We should handle that, and figure out
why it fails.
diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c
b/tools/testing/selftests/mm/ksm_functional_tests.c
index d8bd1911dfc0a..0ddbb390df33b 100644
--- a/tools/testing/selftests/mm/ksm_functional_tests.c
+++ b/tools/testing/selftests/mm/ksm_functional_tests.c
@@ -527,6 +527,8 @@ static void test_child_ksm_err(int status)
ksft_test_result_fail("Merge in child failed\n");
else if (status == -3)
ksft_test_result_skip("Merge in child skipped\n");
+ else if (status == 4)
+ ksft_test_result_fail("Binary not found\n");
}
/* Verify that prctl ksm flag is inherited. */
@@ -598,7 +600,7 @@ static void test_prctl_fork_exec(void)
char *argv_for_program[] = { prg_name,
FORK_EXEC_CHILD_PRG_NAME };
execv(prg_name, argv_for_program);
- return;
+ exit(4);
}
if (waitpid(child_pid, &status, 0) > 0) {
results in
TAP version 13
1..9
# [RUN] test_unmerge
ok 1 Pages were unmerged
# [RUN] test_unmerge_zero_pages
ok 2 KSM zero pages were unmerged
# [RUN] test_unmerge_discarded
ok 3 Pages were unmerged
# [RUN] test_unmerge_uffd_wp
ok 4 Pages were unmerged
# [RUN] test_prot_none
ok 5 Pages were unmerged
# [RUN] test_prctl
ok 6 Setting/clearing PR_SET_MEMORY_MERGE works
# [RUN] test_prctl_fork
ok 7 PR_SET_MEMORY_MERGE value is inherited
# [RUN] test_prctl_fork_exec
not ok 8 Binary not found
# [RUN] test_prctl_unmerge
ok 9 Pages were unmerged
Bail out! 1 out of 9 tests failed
# Totals: pass:8 fail:1 xfail:0 xpass:0 skip:0 error:0
Thanks David.
We will add this in next version.
Re: [PATCH v2 4/7] mm/selftests: Fix split_huge_page_test failure on systems with 64KB page size
On 7/3/25 7:51 PM, Zi Yan wrote:
On 3 Jul 2025, at 4:58, Donet Tom wrote:
On 7/3/25 1:52 PM, David Hildenbrand wrote:
On 03.07.25 08:06, Aboorva Devarajan wrote:
From: Donet Tom
The split_huge_page_test fails on systems with a 64KB base page size.
This is because the order of a 2MB huge page is different:
On 64KB systems, the order is 5.
On 4KB systems, it's 9.
The test currently assumes a maximum huge page order of 9, which is only
valid for 4KB base page systems. On systems with 64KB pages, attempting
to split huge pages beyond their actual order (5) causes the test to fail.
In this patch, we calculate the huge page order based on the system's base
page size. With this change, the tests now run successfully on both 64KB
and 4KB page size systems.
Fixes: fa6c02315f745 ("mm: huge_memory: a new debugfs interface for splitting THP
tests")
Signed-off-by: Donet Tom
Signed-off-by: Aboorva Devarajan
---
.../selftests/mm/split_huge_page_test.c | 23 ++-
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/mm/split_huge_page_test.c
b/tools/testing/selftests/mm/split_huge_page_test.c
index aa7400ed0e99..38296a758330 100644
--- a/tools/testing/selftests/mm/split_huge_page_test.c
+++ b/tools/testing/selftests/mm/split_huge_page_test.c
@@ -514,6 +514,15 @@ void split_thp_in_pagecache_to_order_at(size_t fd_size,
const char *fs_loc,
}
}
+static unsigned int get_order(unsigned int pages)
+{
+ unsigned int order = 0;
+
+ while ((1U << order) < pages)
+ order++;
+ return order;
+}
I think this can simply be
return 32 - __builtin_clz(pages - 1);
That mimics what get_order() in the kernel does for BITS_PER_LONG == 32
or simpler
return 31 - __builtin_clz(pages);
E.g., if pages=512, you get 31-22=9
Sure David, We will change it.
+
int main(int argc, char **argv)
{
int i;
@@ -523,6 +532,7 @@ int main(int argc, char **argv)
const char *fs_loc;
bool created_tmp;
int offset;
+ unsigned int max_order;
ksft_print_header();
@@ -534,32 +544,33 @@ int main(int argc, char **argv)
if (argc > 1)
optional_xfs_path = argv[1];
- ksft_set_plan(1+8+1+9+9+8*4+2);
-
pagesize = getpagesize();
pageshift = ffs(pagesize) - 1;
pmd_pagesize = read_pmd_pagesize();
if (!pmd_pagesize)
ksft_exit_fail_msg("Reading PMD pagesize failed\n");
+ max_order = get_order(pmd_pagesize/pagesize);
+ ksft_set_plan(1+(max_order-1)+1+max_order+max_order+(max_order-1)*4+2);
Wow. Can we simplify that in any sane way?
It is counting test by test. Let me try to simplify it and send the next
version.
Yeah, I did that (ksft_set_plan(1+8+1+9+9+8*4+2);) to count different tests
separately and in the same order as the following tests, so that I could
get ksft_set_plan number right when adding or removing tests. Maybe it is
fine to just sum them up now.
Sure. Thank you
Best Regards,
Yan, Zi
Re: [PATCH] selftests/mm: fix split_huge_page_test for folio_split() tests.
On 7/9/25 6:57 AM, Zi Yan wrote:
Hi Zi Yan
PID_FMT does not have an offset field, so folio_split() tests are not
performed. Add PID_FMT_OFFSET with an offset field and use it to perform
folio_split() tests.
Fixes: 80a5c494c89f ("selftests/mm: add tests for folio_split(), buddy allocator
like split")
Signed-off-by: Zi Yan
---
tools/testing/selftests/mm/split_huge_page_test.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/mm/split_huge_page_test.c
b/tools/testing/selftests/mm/split_huge_page_test.c
index aa7400ed0e99..f0d9c035641d 100644
--- a/tools/testing/selftests/mm/split_huge_page_test.c
+++ b/tools/testing/selftests/mm/split_huge_page_test.c
@@ -31,6 +31,7 @@ uint64_t pmd_pagesize;
#define INPUT_MAX 80
#define PID_FMT "%d,0x%lx,0x%lx,%d"
+#define PID_FMT_OFFSET "%d,0x%lx,0x%lx,%d,%d"
#define PATH_FMT "%s,0x%lx,0x%lx,%d"
#define PFN_MASK ((1UL<<55)-1)
@@ -483,7 +484,7 @@ void split_thp_in_pagecache_to_order_at(size_t fd_size,
const char *fs_loc,
write_debugfs(PID_FMT, getpid(), (uint64_t)addr,
(uint64_t)addr + fd_size, order);
else
- write_debugfs(PID_FMT, getpid(), (uint64_t)addr,
+ write_debugfs(PID_FMT_OFFSET, getpid(), (uint64_t)addr,
(uint64_t)addr + fd_size, order, offset);
for (i = 0; i < fd_size; i++)
This looks good to me.
I tested it on my system, and the test is passing.
ok 17 Split PMD-mapped pagecache folio to order 0 at in-folio offset 0
passed
ok 18 Split PMD-mapped pagecache folio to order 0 at in-folio offset 8
passed
ok 19 Split PMD-mapped pagecache folio to order 0 at in-folio offset 16
passed
ok 20 Split PMD-mapped pagecache folio to order 0 at in-folio offset 24
passed
ok 21 Split PMD-mapped pagecache folio to order 1 at in-folio offset 0
passed
ok 22 Split PMD-mapped pagecache folio to order 1 at in-folio offset 8
passed
ok 23 Split PMD-mapped pagecache folio to order 1 at in-folio offset 16
passed
ok 24 Split PMD-mapped pagecache folio to order 1 at in-folio offset 24
passed
ok 25 Split PMD-mapped pagecache folio to order 2 at in-folio offset 0
passed
ok 26 Split PMD-mapped pagecache folio to order 2 at in-folio offset 8
passed
ok 27 Split PMD-mapped pagecache folio to order 2 at in-folio offset 16
passed
ok 28 Split PMD-mapped pagecache folio to order 2 at in-folio offset 24
passed
ok 29 Split PMD-mapped pagecache folio to order 3 at in-folio offset 0
passed
ok 30 Split PMD-mapped pagecache folio to order 3 at in-folio offset 8
passed
ok 31 Split PMD-mapped pagecache folio to order 3 at in-folio offset 16
passed
ok 32 Split PMD-mapped pagecache folio to order 3 at in-folio offset 24
passed
ok 33 Split PMD-mapped pagecache folio to order 4 at in-folio offset 0
passed
ok 34 Split PMD-mapped pagecache folio to order 4 at in-folio offset 16
passed
Feel free to add:
Reviewed-by: Donet Tom
Tested-by : Donet Tom
Re: [PATCH v2 5/7] selftests/mm: Fix child process exit codes in ksm_functional_tests
Hi David
On 7/3/25 2:03 PM, David Hildenbrand wrote:
On 03.07.25 08:06, Aboorva Devarajan wrote:
In ksm_functional_tests, test_child_ksm() returned negative values
to indicate errors. However, when passed to exit(), these were
interpreted as large unsigned values (e.g, -2 became 254), leading to
incorrect handling in the parent process. As a result, some tests
appeared to be skipped or silently failed.
This patch changes test_child_ksm() to return positive error codes
(1, 2, 3) and updates test_child_ksm_err() to interpret them correctly.
This ensures the parent accurately detects and reports child process
failures.
--
Before patch:
--
- [RUN] test_unmerge
ok 1 Pages were unmerged
...
- [RUN] test_prctl_fork
- No pages got merged
- [RUN] test_prctl_fork_exec
ok 7 PR_SET_MEMORY_MERGE value is inherited
...
Bail out! 1 out of 8 tests failed
- Planned tests != run tests (9 != 8)
- Totals: pass:7 fail:1 xfail:0 xpass:0 skip:0 error:0
--
After patch:
--
- [RUN] test_unmerge
ok 1 Pages were unmerged
...
- [RUN] test_prctl_fork
- No pages got merged
not ok 7 Merge in child failed
- [RUN] test_prctl_fork_exec
ok 8 PR_SET_MEMORY_MERGE value is inherited
...
Bail out! 2 out of 9 tests failed
- Totals: pass:7 fail:2 xfail:0 xpass:0 skip:0 error:0
Fixes: 6c47de3be3a0 ("selftest/mm: ksm_functional_tests: extend test
case for ksm fork/exec")
Signed-off-by: Aboorva Devarajan
BTW, when I run the test, I get this weird output
TAP version 13
1..9
# [RUN] test_unmerge
ok 1 Pages were unmerged
# [RUN] test_unmerge_zero_pages
ok 2 KSM zero pages were unmerged
# [RUN] test_unmerge_discarded
ok 3 Pages were unmerged
# [RUN] test_unmerge_uffd_wp
ok 4 Pages were unmerged
# [RUN] test_prot_none
ok 5 Pages were unmerged
# [RUN] test_prctl
ok 6 Setting/clearing PR_SET_MEMORY_MERGE works
# [RUN] test_prctl_fork
ok 7 PR_SET_MEMORY_MERGE value is inherited
# [RUN] test_prctl_fork_exec
^ where is the test?
# [RUN] test_prctl_unmerge
ok 8 Pages were unmerged
# Planned tests != run tests (9 != 8)
# Totals: pass:8 fail:0 xfail:0 xpass:0 skip:0 error:0
^ what?
ok 8 PR_SET_MEMORY_MERGE value is inherited
# [RUN] test_prctl_unmerge
ok 9 Pages were unmerged
# Totals: pass:9 fail:0 xfail:0 xpass:0 skip:0 error:0
^ huh, what now?
The problem with the exec test is that it uses its own binary to exec.
} else if (child_pid == 0) {
char *prg_name = "./ksm_functional_tests";
char *argv_for_program[] = { prg_name,
FORK_EXEC_CHILD_PRG_NAME, NULL };
execv(prg_name, argv_for_program);
return;
}
So we should run it on the same directory where the binary present.
Re: [PATCH v2 2/7] selftests/mm: Add support to test 4PB VA on PPC64
On 7/3/25 8:11 PM, Zi Yan wrote: On 3 Jul 2025, at 2:06, Aboorva Devarajan wrote: From: Donet Tom PowerPC64 supports a 4PB virtual address space, but this test was previously limited to 512TB. This patch extends the coverage up to the full 4PB VA range on PowerPC64. Memory from 0 to 128TB is allocated without an address hint, while allocations from 128TB to 4PB use a hint address. Signed-off-by: Donet Tom Signed-off-by: Aboorva Devarajan --- tools/testing/selftests/mm/virtual_address_range.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/tools/testing/selftests/mm/virtual_address_range.c b/tools/testing/selftests/mm/virtual_address_range.c index e24c36a39f22..619acf0b9239 100644 --- a/tools/testing/selftests/mm/virtual_address_range.c +++ b/tools/testing/selftests/mm/virtual_address_range.c @@ -50,6 +50,7 @@ #define NR_CHUNKS_256TB (NR_CHUNKS_128TB * 2UL) #define NR_CHUNKS_384TB (NR_CHUNKS_128TB * 3UL) #define NR_CHUNKS_3840TB (NR_CHUNKS_128TB * 30UL) +#define NR_CHUNKS_3968TB (NR_CHUNKS_128TB * 31UL) #define ADDR_MARK_128TB (1UL << 47) /* First address beyond 128TB */ #define ADDR_MARK_256TB (1UL << 48) /* First address beyond 256TB */ @@ -59,6 +60,11 @@ #define HIGH_ADDR_SHIFT 49 #define NR_CHUNKS_LOW NR_CHUNKS_256TB #define NR_CHUNKS_HIGH NR_CHUNKS_3840TB +#elif defined(__PPC64__) +#define HIGH_ADDR_MARK ADDR_MARK_128TB +#define HIGH_ADDR_SHIFT 48 +#define NR_CHUNKS_LOW NR_CHUNKS_128TB +#define NR_CHUNKS_HIGH NR_CHUNKS_3968TB #else #define HIGH_ADDR_MARK ADDR_MARK_128TB #define HIGH_ADDR_SHIFT 48 Could you also update the comment above this code to say PowerPC64 also supports 4PB virtual address space? Sure. I will add From the comment, arm64 supports 4PB but its NR_CHUNKS_HIGH is only 3840TB, whereas PowerPC64 here can get to 3968TB. I do not know why arm64’s 4PB is smaller. ;) Otherwise, the patch looks good to me. Reviewed-by: Zi Yan Best Regards, Yan, Zi
Re: [PATCH v2 4/7] mm/selftests: Fix split_huge_page_test failure on systems with 64KB page size
On 7/3/25 8:00 PM, Zi Yan wrote:
On 3 Jul 2025, at 2:06, Aboorva Devarajan wrote:
From: Donet Tom
The split_huge_page_test fails on systems with a 64KB base page size.
This is because the order of a 2MB huge page is different:
On 64KB systems, the order is 5.
On 4KB systems, it's 9.
The test currently assumes a maximum huge page order of 9, which is only
valid for 4KB base page systems. On systems with 64KB pages, attempting
to split huge pages beyond their actual order (5) causes the test to fail.
In this patch, we calculate the huge page order based on the system's base
page size. With this change, the tests now run successfully on both 64KB
and 4KB page size systems.
Fixes: fa6c02315f745 ("mm: huge_memory: a new debugfs interface for splitting THP
tests")
Signed-off-by: Donet Tom
Signed-off-by: Aboorva Devarajan
---
.../selftests/mm/split_huge_page_test.c | 23 ++-
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/mm/split_huge_page_test.c
b/tools/testing/selftests/mm/split_huge_page_test.c
index aa7400ed0e99..38296a758330 100644
--- a/tools/testing/selftests/mm/split_huge_page_test.c
+++ b/tools/testing/selftests/mm/split_huge_page_test.c
@@ -514,6 +514,15 @@ void split_thp_in_pagecache_to_order_at(size_t fd_size,
const char *fs_loc,
}
}
+static unsigned int get_order(unsigned int pages)
+{
+ unsigned int order = 0;
+
+ while ((1U << order) < pages)
+ order++;
+ return order;
+}
+
int main(int argc, char **argv)
{
int i;
@@ -523,6 +532,7 @@ int main(int argc, char **argv)
const char *fs_loc;
bool created_tmp;
int offset;
+ unsigned int max_order;
ksft_print_header();
@@ -534,32 +544,33 @@ int main(int argc, char **argv)
if (argc > 1)
optional_xfs_path = argv[1];
- ksft_set_plan(1+8+1+9+9+8*4+2);
-
pagesize = getpagesize();
pageshift = ffs(pagesize) - 1;
pmd_pagesize = read_pmd_pagesize();
if (!pmd_pagesize)
ksft_exit_fail_msg("Reading PMD pagesize failed\n");
+ max_order = get_order(pmd_pagesize/pagesize);
pmd_pagesize/pagesize is reused below, a tmp variable would be good.
Thank you. I will add it in next version.
+ ksft_set_plan(1+(max_order-1)+1+max_order+max_order+(max_order-1)*4+2);
+
fd_size = 2 * pmd_pagesize;
split_pmd_zero_pages();
- for (i = 0; i < 9; i++)
+ for (i = 0; i < max_order; i++)
if (i != 1)
split_pmd_thp_to_order(i);
split_pte_mapped_thp();
- for (i = 0; i < 9; i++)
+ for (i = 0; i < max_order; i++)
split_file_backed_thp(i);
created_tmp = prepare_thp_fs(optional_xfs_path, fs_loc_template,
&fs_loc);
- for (i = 8; i >= 0; i--)
+ for (i = (max_order-1); i >= 0; i--)
split_thp_in_pagecache_to_order_at(fd_size, fs_loc, i, -1);
- for (i = 0; i < 9; i++)
+ for (i = 0; i < max_order; i++)
for (offset = 0;
offset < pmd_pagesize / pagesize;
offset += MAX(pmd_pagesize / pagesize / 4, 1 << i))
With the change to get_order() proposed by David and ksft_set_plan()
simplification, Reviewed-by: Zi Yan
Best Regards,
Yan, Zi
Re: [PATCH 1/6] mm/selftests: Fix virtual_address_range test issues.
On Wed, Jun 25, 2025 at 06:22:53PM +0530, Dev Jain wrote: > > On 19/06/25 1:53 pm, Donet Tom wrote: > > On Wed, Jun 18, 2025 at 08:13:54PM +0530, Dev Jain wrote: > > > On 18/06/25 8:05 pm, Lorenzo Stoakes wrote: > > > > On Wed, Jun 18, 2025 at 07:47:18PM +0530, Dev Jain wrote: > > > > > On 18/06/25 7:37 pm, Lorenzo Stoakes wrote: > > > > > > On Wed, Jun 18, 2025 at 07:28:16PM +0530, Dev Jain wrote: > > > > > > > On 18/06/25 5:27 pm, Lorenzo Stoakes wrote: > > > > > > > > On Wed, Jun 18, 2025 at 05:15:50PM +0530, Dev Jain wrote: > > > > > > > > Are you accounting for sys.max_map_count? If not, then you'll > > > > > > > > be hitting that > > > > > > > > first. > > > > > > > run_vmtests.sh will run the test in overcommit mode so that won't > > > > > > > be an issue. > > > > > > Umm, what? You mean overcommit all mode, and that has no bearing on > > > > > > the max > > > > > > mapping count check. > > > > > > > > > > > > In do_mmap(): > > > > > > > > > > > > /* Too many mappings? */ > > > > > > if (mm->map_count > sysctl_max_map_count) > > > > > > return -ENOMEM; > > > > > > > > > > > > > > > > > > As well as numerous other checks in mm/vma.c. > > > > > Ah sorry, didn't look at the code properly just assumed that > > > > > overcommit_always meant overriding > > > > > this. > > > > No problem! It's hard to be aware of everything in mm :) > > > > > > > > > > I'm not sure why an overcommit toggle is even necessary when you > > > > > > could use > > > > > > MAP_NORESERVE or simply map PROT_NONE to avoid the OVERCOMMIT_GUESS > > > > > > limits? > > > > > > > > > > > > I'm pretty confused as to what this test is really achieving > > > > > > honestly. This > > > > > > isn't a useful way of asserting mmap() behaviour as far as I can > > > > > > tell. > > > > > Well, seems like a useful way to me at least : ) Not sure if you are > > > > > in the mood > > > > > to discuss that but if you'd like me to explain from start to end > > > > > what the test > > > > > is doing, I can do that : ) > > > > > > > > > I just don't have time right now, I guess I'll have to come back to it > > > > later... it's not the end of the world for it to be iffy in my view as > > > > long as > > > > it passes, but it might just not be of great value. > > > > > > > > Philosophically I'd rather we didn't assert internal implementation > > > > details like > > > > where we place mappings in userland memory. At no point do we promise > > > > to not > > > > leave larger gaps if we feel like it :) > > > You have a fair point. Anyhow a debate for another day. > > > > > > > I'm guessing, reading more, the _real_ test here is some mathematical > > > > assertion > > > > about layout from HIGH_ADDR_SHIFT -> end of address space when using > > > > hints. > > > > > > > > But again I'm not sure that achieves much and again also is asserting > > > > internal > > > > implementation details. > > > > > > > > Correct behaviour of this kind of thing probably better belongs to > > > > tests in the > > > > userland VMA testing I'd say. > > > > > > > > Sorry I don't mean to do down work you've done before, just giving an > > > > honest > > > > technical appraisal! > > > Nah, it will be rather hilarious to see it all go down the drain xD > > > > > > > Anyway don't let this block work to fix the test if it's failing. We > > > > can revisit > > > > this later. > > > Sure. @Aboorva and Donet, I still believe that the correct approach is to > > > elide > > > the gap check at the crossing boundary. What do you think? > > > > > One problem I am seeing with this approach is that, since the hint address > > is generated randomly, the VMAs are
Re: [PATCH v2 4/7] mm/selftests: Fix split_huge_page_test failure on systems with 64KB page size
On 7/3/25 1:52 PM, David Hildenbrand wrote:
On 03.07.25 08:06, Aboorva Devarajan wrote:
From: Donet Tom
The split_huge_page_test fails on systems with a 64KB base page size.
This is because the order of a 2MB huge page is different:
On 64KB systems, the order is 5.
On 4KB systems, it's 9.
The test currently assumes a maximum huge page order of 9, which is only
valid for 4KB base page systems. On systems with 64KB pages, attempting
to split huge pages beyond their actual order (5) causes the test to
fail.
In this patch, we calculate the huge page order based on the system's
base
page size. With this change, the tests now run successfully on both 64KB
and 4KB page size systems.
Fixes: fa6c02315f745 ("mm: huge_memory: a new debugfs interface for
splitting THP tests")
Signed-off-by: Donet Tom
Signed-off-by: Aboorva Devarajan
---
.../selftests/mm/split_huge_page_test.c | 23 ++-
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/mm/split_huge_page_test.c
b/tools/testing/selftests/mm/split_huge_page_test.c
index aa7400ed0e99..38296a758330 100644
--- a/tools/testing/selftests/mm/split_huge_page_test.c
+++ b/tools/testing/selftests/mm/split_huge_page_test.c
@@ -514,6 +514,15 @@ void split_thp_in_pagecache_to_order_at(size_t
fd_size, const char *fs_loc,
}
}
+static unsigned int get_order(unsigned int pages)
+{
+ unsigned int order = 0;
+
+ while ((1U << order) < pages)
+ order++;
+ return order;
+}
I think this can simply be
return 32 - __builtin_clz(pages - 1);
That mimics what get_order() in the kernel does for BITS_PER_LONG == 32
or simpler
return 31 - __builtin_clz(pages);
E.g., if pages=512, you get 31-22=9
Sure David, We will change it.
+
int main(int argc, char **argv)
{
int i;
@@ -523,6 +532,7 @@ int main(int argc, char **argv)
const char *fs_loc;
bool created_tmp;
int offset;
+ unsigned int max_order;
ksft_print_header();
@@ -534,32 +544,33 @@ int main(int argc, char **argv)
if (argc > 1)
optional_xfs_path = argv[1];
- ksft_set_plan(1+8+1+9+9+8*4+2);
-
pagesize = getpagesize();
pageshift = ffs(pagesize) - 1;
pmd_pagesize = read_pmd_pagesize();
if (!pmd_pagesize)
ksft_exit_fail_msg("Reading PMD pagesize failed\n");
+ max_order = get_order(pmd_pagesize/pagesize);
+
ksft_set_plan(1+(max_order-1)+1+max_order+max_order+(max_order-1)*4+2);
Wow. Can we simplify that in any sane way?
It is counting test by test. Let me try to simplify it and send the next
version.
+
fd_size = 2 * pmd_pagesize;
split_pmd_zero_pages();
- for (i = 0; i < 9; i++)
+ for (i = 0; i < max_order; i++)
if (i != 1)
split_pmd_thp_to_order(i);
split_pte_mapped_thp();
- for (i = 0; i < 9; i++)
+ for (i = 0; i < max_order; i++)
split_file_backed_thp(i);
created_tmp = prepare_thp_fs(optional_xfs_path, fs_loc_template,
&fs_loc);
- for (i = 8; i >= 0; i--)
+ for (i = (max_order-1); i >= 0; i--)
"i = max_order - 1"
I will change it.
split_thp_in_pagecache_to_order_at(fd_size, fs_loc, i, -1);
- for (i = 0; i < 9; i++)
+ for (i = 0; i < max_order; i++)
for (offset = 0;
offset < pmd_pagesize / pagesize;
offset += MAX(pmd_pagesize / pagesize / 4, 1 << i))
Re: [PATCH v3 3/7] selftest/mm: Fix ksm_funtional_test failures
On 8/4/25 2:41 PM, Wei Yang wrote:
On Tue, Jul 29, 2025 at 11:03:59AM +0530, Aboorva Devarajan wrote:
From: Donet Tom
This patch fixed 2 issues.
1) After fork() in test_prctl_fork, the child process uses the file
descriptors from the parent process to read ksm_stat and
ksm_merging_pages. This results in incorrect values being read (parent
process ksm_stat and ksm_merging_pages will be read in child), causing
the test to fail.
This patch calls init_global_file_handles() in the child process to
ensure that the current process's file descriptors are used to read
ksm_stat and ksm_merging_pages.
2) All tests currently call ksm_merge to trigger page merging.
To ensure the system remains in a consistent state for subsequent
tests, it is better to call ksm_unmerge during the test cleanup phase.
In the test_prctl_fork test, after a fork(), reading ksm_merging_pages
in the child process returns a non-zero value because a previous test
performed a merge, and the child's memory state is inherited from the
parent.
Although the child process calls ksm_unmerge, the ksm_merging_pages
counter in the parent is reset to zero, while the child's counter
remains unchanged. This discrepancy causes the test to fail.
To avoid this issue, each test should call ksm_unmerge during cleanup
to ensure the counter is reset and the system is in a clean state for
subsequent tests.
execv argument is an array of pointers to null-terminated strings.
In this patch we also added NULL in the execv argument.
Fixes: 6c47de3be3a0 ("selftest/mm: ksm_functional_tests: extend test case for ksm
fork/exec")
Co-developed-by: Aboorva Devarajan
Signed-off-by: Aboorva Devarajan
Signed-off-by: Donet Tom
---
tools/testing/selftests/mm/ksm_functional_tests.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c
b/tools/testing/selftests/mm/ksm_functional_tests.c
index d8bd1911dfc0..996dc6645570 100644
--- a/tools/testing/selftests/mm/ksm_functional_tests.c
+++ b/tools/testing/selftests/mm/ksm_functional_tests.c
@@ -46,6 +46,8 @@ static int ksm_use_zero_pages_fd;
static int pagemap_fd;
static size_t pagesize;
+static void init_global_file_handles(void);
+
static bool range_maps_duplicates(char *addr, unsigned long size)
{
unsigned long offs_a, offs_b, pfn_a, pfn_b;
@@ -274,6 +276,7 @@ static void test_unmerge(void)
ksft_test_result(!range_maps_duplicates(map, size),
"Pages were unmerged\n");
unmap:
+ ksm_unmerge();
In __mmap_and_merge_range(), we call ksm_unmerge(). Why this one not help?
Not very familiar with ksm stuff. Would you mind giving more on how this fix
the failure you see?
The issue I was facing here was test_prctl_fork was failing.
# [RUN] test_prctl_fork
# Still pages merged
#
This issue occurred because the previous test performed a merge, causing
the value of /proc/self/ksm_merging_pages to reflect the number of
deduplicated pages. After that, a fork() was called. Post-fork, the
child process
inherited the parent's ksm_merging_pages value.
Then, the child process invoked __mmap_and_merge_range(), which resulted
in unmerging the pages and resetting the value. However, since the
parent process
had performed the merge, its ksm_merging_pages value also got reset to 0.
Meanwhile, the child process had not performed any merge itself, so the
inherited
value remained unchanged. That’s why get_my_merging_page() in the child was
returning a non-zero value.
Initially, I fixed the issue by calling ksm_unmerge() before the fork(),
and that
resolved the problem. Later, I decided it would be cleaner to move the
ksm_unmerge() call to the test cleanup phase.
munmap(map, size);
}
@@ -338,6 +341,7 @@ static void test_unmerge_zero_pages(void)
ksft_test_result(!range_maps_duplicates(map, size),
"KSM zero pages were unmerged\n");
unmap:
+ ksm_unmerge();
munmap(map, size);
}
@@ -366,6 +370,7 @@ static void test_unmerge_discarded(void)
ksft_test_result(!range_maps_duplicates(map, size),
"Pages were unmerged\n");
unmap:
+ ksm_unmerge();
munmap(map, size);
}
@@ -452,6 +457,7 @@ static void test_unmerge_uffd_wp(void)
close_uffd:
close(uffd);
unmap:
+ ksm_unmerge();
munmap(map, size);
}
#endif
@@ -515,6 +521,7 @@ static int test_child_ksm(void)
else if (map == MAP_MERGE_SKIP)
return -3;
+ ksm_unmerge();
munmap(map, size);
return 0;
}
@@ -548,6 +555,7 @@ static void test_prctl_fork(void)
child_pid = fork();
if (!child_pid) {
+ init_global_file_handles();
Would this leave fd in parent as orphan?
exit(test_child_ksm());
} else if (child_pid < 0) {
ksft_test_result_fail("fork() failed\n");
@@ -595,
Re: [PATCH v3 4/7] mm/selftests: Fix split_huge_page_test failure on systems with 64KB page size
On 8/4/25 2:34 PM, Wei Yang wrote:
On Tue, Jul 29, 2025 at 11:04:00AM +0530, Aboorva Devarajan wrote:
From: Donet Tom
The split_huge_page_test fails on systems with a 64KB base page size.
This is because the order of a 2MB huge page is different:
On 64KB systems, the order is 5.
On 4KB systems, it's 9.
The test currently assumes a maximum huge page order of 9, which is only
valid for 4KB base page systems. On systems with 64KB pages, attempting
to split huge pages beyond their actual order (5) causes the test to fail.
In this patch, we calculate the huge page order based on the system's base
page size. With this change, the tests now run successfully on both 64KB
and 4KB page size systems.
Fixes: fa6c02315f745 ("mm: huge_memory: a new debugfs interface for splitting THP
tests")
Reviewed-by: Dev Jain
Reviewed-by: Zi Yan
Co-developed-by: Aboorva Devarajan
Signed-off-by: Aboorva Devarajan
Signed-off-by: Donet Tom
---
.../selftests/mm/split_huge_page_test.c | 23 ---
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/mm/split_huge_page_test.c
b/tools/testing/selftests/mm/split_huge_page_test.c
index 05de1fc0005b..718daceb5282 100644
--- a/tools/testing/selftests/mm/split_huge_page_test.c
+++ b/tools/testing/selftests/mm/split_huge_page_test.c
@@ -36,6 +36,7 @@ uint64_t pmd_pagesize;
#define PFN_MASK ((1UL<<55)-1)
#define KPF_THP (1UL<<22)
+#define GET_ORDER(nr_pages)(31 - __builtin_clz(nr_pages))
int is_backed_by_thp(char *vaddr, int pagemap_file, int kpageflags_file)
{
@@ -522,6 +523,9 @@ int main(int argc, char **argv)
const char *fs_loc;
bool created_tmp;
int offset;
+ unsigned int max_order;
+ unsigned int nr_pages;
+ unsigned int tests;
ksft_print_header();
@@ -533,35 +537,38 @@ int main(int argc, char **argv)
if (argc > 1)
optional_xfs_path = argv[1];
- ksft_set_plan(1+8+1+9+9+8*4+2);
-
pagesize = getpagesize();
pageshift = ffs(pagesize) - 1;
pmd_pagesize = read_pmd_pagesize();
if (!pmd_pagesize)
ksft_exit_fail_msg("Reading PMD pagesize failed\n");
+ nr_pages = pmd_pagesize / pagesize;
+ max_order = GET_ORDER(nr_pages);
There is a sz2ord() in cow.c and uffd-wp-mremap.c.
Maybe we can factor it into vm_util.h and use it here.
Sure, I will make the change and send a new version.
