1. Patchew
  2. linux
  3. [PATCH v2 0/8] Various mm kselftests improvements/fixes
  4. View patch

[PATCH v2 5/8] selftests/mm: introduce helper to read every page in range

Kevin Brodsky posted 8 patches 1 month ago
There is a newer version of this series
[PATCH v2 5/8] selftests/mm: introduce helper to read every page in range
Posted by Kevin Brodsky 1 month ago 
FORCE_READ(*addr) ensures that the compiler will emit a load from
addr. Several tests need to trigger such a load for every page in
the range [addr, addr + len), ensuring that every page is faulted
in, if it wasn't already.

Introduce a new helper force_read_pages_in_range() that does exactly
that and replace existing loops with a call to it. Some of those
loops have a different step size, but reading from every page is
appropriate in all cases.

Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
---
 tools/testing/selftests/mm/hugetlb-madvise.c     |  9 +--------
 tools/testing/selftests/mm/pfnmap.c              | 16 ++++++----------
 .../testing/selftests/mm/split_huge_page_test.c  |  6 +-----
 tools/testing/selftests/mm/vm_util.h             |  6 ++++++
 4 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/tools/testing/selftests/mm/hugetlb-madvise.c b/tools/testing/selftests/mm/hugetlb-madvise.c
index 05d9d2805ae4..1f82568ae262 100644
--- a/tools/testing/selftests/mm/hugetlb-madvise.c
+++ b/tools/testing/selftests/mm/hugetlb-madvise.c
@@ -47,14 +47,7 @@ void write_fault_pages(void *addr, unsigned long nr_pages)
 
 void read_fault_pages(void *addr, unsigned long nr_pages)
 {
-	unsigned long i;
-
-	for (i = 0; i < nr_pages; i++) {
-		unsigned long *addr2 =
-			((unsigned long *)(addr + (i * huge_page_size)));
-		/* Prevent the compiler from optimizing out the entire loop: */
-		FORCE_READ(*addr2);
-	}
+	force_read_pages_in_range(addr, nr_pages * huge_page_size);
 }
 
 int main(int argc, char **argv)
diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/pfnmap.c
index f546dfb10cae..35b0e3ed54cd 100644
--- a/tools/testing/selftests/mm/pfnmap.c
+++ b/tools/testing/selftests/mm/pfnmap.c
@@ -33,20 +33,17 @@ static void signal_handler(int sig)
 	siglongjmp(sigjmp_buf_env, -EFAULT);
 }
 
-static int test_read_access(char *addr, size_t size, size_t pagesize)
+static int test_read_access(char *addr, size_t size)
 {
-	size_t offs;
 	int ret;
 
 	if (signal(SIGSEGV, signal_handler) == SIG_ERR)
 		return -EINVAL;
 
 	ret = sigsetjmp(sigjmp_buf_env, 1);
-	if (!ret) {
-		for (offs = 0; offs < size; offs += pagesize)
-			/* Force a read that the compiler cannot optimize out. */
-			*((volatile char *)(addr + offs));
-	}
+	if (!ret)
+		force_read_pages_in_range(addr, size);
+
 	if (signal(SIGSEGV, SIG_DFL) == SIG_ERR)
 		return -EINVAL;
 
@@ -138,7 +135,7 @@ FIXTURE_SETUP(pfnmap)
 		SKIP(return, "Invalid file: '%s'. Not pfnmap'ed\n", file);
 
 	/* ... and want to be able to read from them. */
-	if (test_read_access(self->addr1, self->size1, self->pagesize))
+	if (test_read_access(self->addr1, self->size1))
 		SKIP(return, "Cannot read-access mmap'ed '%s'\n", file);
 
 	self->size2 = 0;
@@ -243,8 +240,7 @@ TEST_F(pfnmap, fork)
 	ASSERT_GE(pid, 0);
 
 	if (!pid) {
-		EXPECT_EQ(test_read_access(self->addr1, self->size1,
-					   self->pagesize), 0);
+		EXPECT_EQ(test_read_access(self->addr1, self->size1), 0);
 		exit(0);
 	}
 
diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c
index 40799f3f0213..65a89ceca4a5 100644
--- a/tools/testing/selftests/mm/split_huge_page_test.c
+++ b/tools/testing/selftests/mm/split_huge_page_test.c
@@ -652,11 +652,7 @@ static int create_pagecache_thp_and_fd(const char *testfile, size_t fd_size,
 	}
 	madvise(*addr, fd_size, MADV_HUGEPAGE);
 
-	for (size_t i = 0; i < fd_size; i++) {
-		char *addr2 = *addr + i;
-
-		FORCE_READ(*addr2);
-	}
+	force_read_pages_in_range(*addr, fd_size);
 
 	if (!check_huge_file(*addr, fd_size / pmd_pagesize, pmd_pagesize)) {
 		ksft_print_msg("No large pagecache folio generated, please provide a filesystem supporting large folio\n");
diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h
index 6ad32b1830f1..74bdf96161d7 100644
--- a/tools/testing/selftests/mm/vm_util.h
+++ b/tools/testing/selftests/mm/vm_util.h
@@ -54,6 +54,12 @@ static inline unsigned int pshift(void)
 	return __page_shift;
 }
 
+static inline void force_read_pages_in_range(char *addr, size_t len)
+{
+	for (size_t i = 0; i < len; i += psize())
+		FORCE_READ(addr[i]);
+}
+
 bool detect_huge_zeropage(void);
 
 /*
-- 
2.51.2
Re: [PATCH v2 5/8] selftests/mm: introduce helper to read every page in range
Posted by Dev Jain 2 weeks, 4 days ago 
On 07/01/26 10:18 pm, Kevin Brodsky wrote:
> FORCE_READ(*addr) ensures that the compiler will emit a load from
> addr. Several tests need to trigger such a load for every page in
> the range [addr, addr + len), ensuring that every page is faulted
> in, if it wasn't already.
>
> Introduce a new helper force_read_pages_in_range() that does exactly
> that and replace existing loops with a call to it. Some of those
> loops have a different step size, but reading from every page is
> appropriate in all cases.
>
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
> ---
>  tools/testing/selftests/mm/hugetlb-madvise.c     |  9 +--------
>  tools/testing/selftests/mm/pfnmap.c              | 16 ++++++----------
>  .../testing/selftests/mm/split_huge_page_test.c  |  6 +-----
>  tools/testing/selftests/mm/vm_util.h             |  6 ++++++
>  4 files changed, 14 insertions(+), 23 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/hugetlb-madvise.c b/tools/testing/selftests/mm/hugetlb-madvise.c
> index 05d9d2805ae4..1f82568ae262 100644
> --- a/tools/testing/selftests/mm/hugetlb-madvise.c
> +++ b/tools/testing/selftests/mm/hugetlb-madvise.c
> @@ -47,14 +47,7 @@ void write_fault_pages(void *addr, unsigned long nr_pages)
>  
>  void read_fault_pages(void *addr, unsigned long nr_pages)
>  {
> -	unsigned long i;
> -
> -	for (i = 0; i < nr_pages; i++) {
> -		unsigned long *addr2 =
> -			((unsigned long *)(addr + (i * huge_page_size)));
> -		/* Prevent the compiler from optimizing out the entire loop: */
> -		FORCE_READ(*addr2);
> -	}
> +	force_read_pages_in_range(addr, nr_pages * huge_page_size);
>  }

Yeah this should be fine to do.

>  
>  int main(int argc, char **argv)
> diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/pfnmap.c
> index f546dfb10cae..35b0e3ed54cd 100644
> --- a/tools/testing/selftests/mm/pfnmap.c
> +++ b/tools/testing/selftests/mm/pfnmap.c
> @@ -33,20 +33,17 @@ static void signal_handler(int sig)
>  	siglongjmp(sigjmp_buf_env, -EFAULT);
>  }
>  
> -static int test_read_access(char *addr, size_t size, size_t pagesize)
> +static int test_read_access(char *addr, size_t size)
>  {
> -	size_t offs;
>  	int ret;
>  
>  	if (signal(SIGSEGV, signal_handler) == SIG_ERR)
>  		return -EINVAL;
>  
>  	ret = sigsetjmp(sigjmp_buf_env, 1);
> -	if (!ret) {
> -		for (offs = 0; offs < size; offs += pagesize)
> -			/* Force a read that the compiler cannot optimize out. */
> -			*((volatile char *)(addr + offs));
> -	}
> +	if (!ret)
> +		force_read_pages_in_range(addr, size);
> +

LGTM

>  	if (signal(SIGSEGV, SIG_DFL) == SIG_ERR)
>  		return -EINVAL;
>  
> @@ -138,7 +135,7 @@ FIXTURE_SETUP(pfnmap)
>  		SKIP(return, "Invalid file: '%s'. Not pfnmap'ed\n", file);
>  
>  	/* ... and want to be able to read from them. */
> -	if (test_read_access(self->addr1, self->size1, self->pagesize))
> +	if (test_read_access(self->addr1, self->size1))
>  		SKIP(return, "Cannot read-access mmap'ed '%s'\n", file);
>  
>  	self->size2 = 0;
> @@ -243,8 +240,7 @@ TEST_F(pfnmap, fork)
>  	ASSERT_GE(pid, 0);
>  
>  	if (!pid) {
> -		EXPECT_EQ(test_read_access(self->addr1, self->size1,
> -					   self->pagesize), 0);
> +		EXPECT_EQ(test_read_access(self->addr1, self->size1), 0);
>  		exit(0);
>  	}
>  
> diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c
> index 40799f3f0213..65a89ceca4a5 100644
> --- a/tools/testing/selftests/mm/split_huge_page_test.c
> +++ b/tools/testing/selftests/mm/split_huge_page_test.c
> @@ -652,11 +652,7 @@ static int create_pagecache_thp_and_fd(const char *testfile, size_t fd_size,
>  	}
>  	madvise(*addr, fd_size, MADV_HUGEPAGE);
>  
> -	for (size_t i = 0; i < fd_size; i++) {
> -		char *addr2 = *addr + i;
> -
> -		FORCE_READ(*addr2);
> -	}
> +	force_read_pages_in_range(*addr, fd_size);

LGTM

>  
>  	if (!check_huge_file(*addr, fd_size / pmd_pagesize, pmd_pagesize)) {
>  		ksft_print_msg("No large pagecache folio generated, please provide a filesystem supporting large folio\n");
> diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h
> index 6ad32b1830f1..74bdf96161d7 100644
> --- a/tools/testing/selftests/mm/vm_util.h
> +++ b/tools/testing/selftests/mm/vm_util.h
> @@ -54,6 +54,12 @@ static inline unsigned int pshift(void)
>  	return __page_shift;
>  }
>  
> +static inline void force_read_pages_in_range(char *addr, size_t len)
> +{
> +	for (size_t i = 0; i < len; i += psize())
> +		FORCE_READ(addr[i]);
> +}
> +

Reviewed-by: Dev Jain <dev.jain@arm.com>

>  bool detect_huge_zeropage(void);
>  
>  /*
Re: [PATCH v2 5/8] selftests/mm: introduce helper to read every page in range
Posted by David Hildenbrand (Red Hat) 3 weeks ago 
On 1/7/26 17:48, Kevin Brodsky wrote:
> FORCE_READ(*addr) ensures that the compiler will emit a load from
> addr. Several tests need to trigger such a load for every page in
> the range [addr, addr + len), ensuring that every page is faulted
> in, if it wasn't already.
> 
> Introduce a new helper force_read_pages_in_range() that does exactly
> that and replace existing loops with a call to it. Some of those
> loops have a different step size, but reading from every page is
> appropriate in all cases.
> 
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
> ---
>   tools/testing/selftests/mm/hugetlb-madvise.c     |  9 +--------
>   tools/testing/selftests/mm/pfnmap.c              | 16 ++++++----------
>   .../testing/selftests/mm/split_huge_page_test.c  |  6 +-----
>   tools/testing/selftests/mm/vm_util.h             |  6 ++++++
>   4 files changed, 14 insertions(+), 23 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/hugetlb-madvise.c b/tools/testing/selftests/mm/hugetlb-madvise.c
> index 05d9d2805ae4..1f82568ae262 100644
> --- a/tools/testing/selftests/mm/hugetlb-madvise.c
> +++ b/tools/testing/selftests/mm/hugetlb-madvise.c
> @@ -47,14 +47,7 @@ void write_fault_pages(void *addr, unsigned long nr_pages)
>   
>   void read_fault_pages(void *addr, unsigned long nr_pages)
>   {
> -	unsigned long i;
> -
> -	for (i = 0; i < nr_pages; i++) {
> -		unsigned long *addr2 =
> -			((unsigned long *)(addr + (i * huge_page_size)));
> -		/* Prevent the compiler from optimizing out the entire loop: */
> -		FORCE_READ(*addr2);
> -	}
> +	force_read_pages_in_range(addr, nr_pages * huge_page_size);
>   }
>   
>   int main(int argc, char **argv)
> diff --git a/tools/testing/selftests/mm/pfnmap.c b/tools/testing/selftests/mm/pfnmap.c
> index f546dfb10cae..35b0e3ed54cd 100644
> --- a/tools/testing/selftests/mm/pfnmap.c
> +++ b/tools/testing/selftests/mm/pfnmap.c
> @@ -33,20 +33,17 @@ static void signal_handler(int sig)
>   	siglongjmp(sigjmp_buf_env, -EFAULT);
>   }
>   
> -static int test_read_access(char *addr, size_t size, size_t pagesize)
> +static int test_read_access(char *addr, size_t size)
>   {
> -	size_t offs;
>   	int ret;
>   
>   	if (signal(SIGSEGV, signal_handler) == SIG_ERR)
>   		return -EINVAL;
>   
>   	ret = sigsetjmp(sigjmp_buf_env, 1);
> -	if (!ret) {
> -		for (offs = 0; offs < size; offs += pagesize)
> -			/* Force a read that the compiler cannot optimize out. */
> -			*((volatile char *)(addr + offs));
> -	}
> +	if (!ret)
> +		force_read_pages_in_range(addr, size);
> +
>   	if (signal(SIGSEGV, SIG_DFL) == SIG_ERR)
>   		return -EINVAL;
>   
> @@ -138,7 +135,7 @@ FIXTURE_SETUP(pfnmap)
>   		SKIP(return, "Invalid file: '%s'. Not pfnmap'ed\n", file);
>   
>   	/* ... and want to be able to read from them. */
> -	if (test_read_access(self->addr1, self->size1, self->pagesize))
> +	if (test_read_access(self->addr1, self->size1))
>   		SKIP(return, "Cannot read-access mmap'ed '%s'\n", file);
>   
>   	self->size2 = 0;
> @@ -243,8 +240,7 @@ TEST_F(pfnmap, fork)
>   	ASSERT_GE(pid, 0);
>   
>   	if (!pid) {
> -		EXPECT_EQ(test_read_access(self->addr1, self->size1,
> -					   self->pagesize), 0);
> +		EXPECT_EQ(test_read_access(self->addr1, self->size1), 0);
>   		exit(0);
>   	}
>   
> diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c
> index 40799f3f0213..65a89ceca4a5 100644
> --- a/tools/testing/selftests/mm/split_huge_page_test.c
> +++ b/tools/testing/selftests/mm/split_huge_page_test.c
> @@ -652,11 +652,7 @@ static int create_pagecache_thp_and_fd(const char *testfile, size_t fd_size,
>   	}
>   	madvise(*addr, fd_size, MADV_HUGEPAGE);
>   
> -	for (size_t i = 0; i < fd_size; i++) {
> -		char *addr2 = *addr + i;
> -
> -		FORCE_READ(*addr2);
> -	}
> +	force_read_pages_in_range(*addr, fd_size);
>   
>   	if (!check_huge_file(*addr, fd_size / pmd_pagesize, pmd_pagesize)) {
>   		ksft_print_msg("No large pagecache folio generated, please provide a filesystem supporting large folio\n");
> diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h
> index 6ad32b1830f1..74bdf96161d7 100644
> --- a/tools/testing/selftests/mm/vm_util.h
> +++ b/tools/testing/selftests/mm/vm_util.h
> @@ -54,6 +54,12 @@ static inline unsigned int pshift(void)
>   	return __page_shift;
>   }
>   
> +static inline void force_read_pages_in_range(char *addr, size_t len)
> +{
> +	for (size_t i = 0; i < len; i += psize())
> +		FORCE_READ(addr[i]);
> +}
> +

Of course, we could also just pass the pagesize

static inline void force_read_pages_in_range(char *addr, size_t len,
	size_t pagesize)
{
	for (size_t i = 0; i < len; i += pagesize)
		FORCE_READ(addr[i]);
}


Or alternatively even better:

static inline void force_read_pages(char *addr, unsigned int nr_pages,
	size_t pagesize)
{
	for (size_t i = 0; i < nr_pages; i++)
		FORCE_READ(addr[i * pagesize]);
}

Then there is no change at all and we avoid the repeated psize() naturally.

Thoughts?

-- 
Cheers

David
Re: [PATCH v2 5/8] selftests/mm: introduce helper to read every page in range
Posted by Kevin Brodsky 3 weeks ago 
On 19/01/2026 11:55, David Hildenbrand (Red Hat) wrote:
> On 1/7/26 17:48, Kevin Brodsky wrote:
>> FORCE_READ(*addr) ensures that the compiler will emit a load from
>> addr. Several tests need to trigger such a load for every page in
>> the range [addr, addr + len), ensuring that every page is faulted
>> in, if it wasn't already.
>>
>> Introduce a new helper force_read_pages_in_range() that does exactly
>> that and replace existing loops with a call to it. Some of those
>> loops have a different step size, but reading from every page is
>> appropriate in all cases.
>>
>> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
>> ---
>>   tools/testing/selftests/mm/hugetlb-madvise.c     |  9 +--------
>>   tools/testing/selftests/mm/pfnmap.c              | 16 ++++++----------
>>   .../testing/selftests/mm/split_huge_page_test.c  |  6 +-----
>>   tools/testing/selftests/mm/vm_util.h             |  6 ++++++
>>   4 files changed, 14 insertions(+), 23 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mm/hugetlb-madvise.c
>> b/tools/testing/selftests/mm/hugetlb-madvise.c
>> index 05d9d2805ae4..1f82568ae262 100644
>> --- a/tools/testing/selftests/mm/hugetlb-madvise.c
>> +++ b/tools/testing/selftests/mm/hugetlb-madvise.c
>> @@ -47,14 +47,7 @@ void write_fault_pages(void *addr, unsigned long
>> nr_pages)
>>     void read_fault_pages(void *addr, unsigned long nr_pages)
>>   {
>> -    unsigned long i;
>> -
>> -    for (i = 0; i < nr_pages; i++) {
>> -        unsigned long *addr2 =
>> -            ((unsigned long *)(addr + (i * huge_page_size)));
>> -        /* Prevent the compiler from optimizing out the entire loop: */
>> -        FORCE_READ(*addr2);
>> -    }
>> +    force_read_pages_in_range(addr, nr_pages * huge_page_size);
>>   }
>>     int main(int argc, char **argv)
>> diff --git a/tools/testing/selftests/mm/pfnmap.c
>> b/tools/testing/selftests/mm/pfnmap.c
>> index f546dfb10cae..35b0e3ed54cd 100644
>> --- a/tools/testing/selftests/mm/pfnmap.c
>> +++ b/tools/testing/selftests/mm/pfnmap.c
>> @@ -33,20 +33,17 @@ static void signal_handler(int sig)
>>       siglongjmp(sigjmp_buf_env, -EFAULT);
>>   }
>>   -static int test_read_access(char *addr, size_t size, size_t pagesize)
>> +static int test_read_access(char *addr, size_t size)
>>   {
>> -    size_t offs;
>>       int ret;
>>         if (signal(SIGSEGV, signal_handler) == SIG_ERR)
>>           return -EINVAL;
>>         ret = sigsetjmp(sigjmp_buf_env, 1);
>> -    if (!ret) {
>> -        for (offs = 0; offs < size; offs += pagesize)
>> -            /* Force a read that the compiler cannot optimize out. */
>> -            *((volatile char *)(addr + offs));
>> -    }
>> +    if (!ret)
>> +        force_read_pages_in_range(addr, size);
>> +
>>       if (signal(SIGSEGV, SIG_DFL) == SIG_ERR)
>>           return -EINVAL;
>>   @@ -138,7 +135,7 @@ FIXTURE_SETUP(pfnmap)
>>           SKIP(return, "Invalid file: '%s'. Not pfnmap'ed\n", file);
>>         /* ... and want to be able to read from them. */
>> -    if (test_read_access(self->addr1, self->size1, self->pagesize))
>> +    if (test_read_access(self->addr1, self->size1))
>>           SKIP(return, "Cannot read-access mmap'ed '%s'\n", file);
>>         self->size2 = 0;
>> @@ -243,8 +240,7 @@ TEST_F(pfnmap, fork)
>>       ASSERT_GE(pid, 0);
>>         if (!pid) {
>> -        EXPECT_EQ(test_read_access(self->addr1, self->size1,
>> -                       self->pagesize), 0);
>> +        EXPECT_EQ(test_read_access(self->addr1, self->size1), 0);
>>           exit(0);
>>       }
>>   diff --git a/tools/testing/selftests/mm/split_huge_page_test.c
>> b/tools/testing/selftests/mm/split_huge_page_test.c
>> index 40799f3f0213..65a89ceca4a5 100644
>> --- a/tools/testing/selftests/mm/split_huge_page_test.c
>> +++ b/tools/testing/selftests/mm/split_huge_page_test.c
>> @@ -652,11 +652,7 @@ static int create_pagecache_thp_and_fd(const
>> char *testfile, size_t fd_size,
>>       }
>>       madvise(*addr, fd_size, MADV_HUGEPAGE);
>>   -    for (size_t i = 0; i < fd_size; i++) {
>> -        char *addr2 = *addr + i;
>> -
>> -        FORCE_READ(*addr2);
>> -    }
>> +    force_read_pages_in_range(*addr, fd_size);
>>         if (!check_huge_file(*addr, fd_size / pmd_pagesize,
>> pmd_pagesize)) {
>>           ksft_print_msg("No large pagecache folio generated, please
>> provide a filesystem supporting large folio\n");
>> diff --git a/tools/testing/selftests/mm/vm_util.h
>> b/tools/testing/selftests/mm/vm_util.h
>> index 6ad32b1830f1..74bdf96161d7 100644
>> --- a/tools/testing/selftests/mm/vm_util.h
>> +++ b/tools/testing/selftests/mm/vm_util.h
>> @@ -54,6 +54,12 @@ static inline unsigned int pshift(void)
>>       return __page_shift;
>>   }
>>   +static inline void force_read_pages_in_range(char *addr, size_t len)
>> +{
>> +    for (size_t i = 0; i < len; i += psize())
>> +        FORCE_READ(addr[i]);
>> +}
>> +
>
> Of course, we could also just pass the pagesize
>
> static inline void force_read_pages_in_range(char *addr, size_t len,
>     size_t pagesize)
> {
>     for (size_t i = 0; i < len; i += pagesize)
>         FORCE_READ(addr[i]);
> }
>
>
> Or alternatively even better:
>
> static inline void force_read_pages(char *addr, unsigned int nr_pages,
>     size_t pagesize)
> {
>     for (size_t i = 0; i < nr_pages; i++)
>         FORCE_READ(addr[i * pagesize]);
> }
>
> Then there is no change at all and we avoid the repeated psize()
> naturally.
>
> Thoughts?

Sure we can do that. I concluded that all tests fetch the pagesize in a
different way and some aren't using psize() sparingly either (e.g.
gup_test) but I suppose that avoiding some ugliness doesn't hurt :)

Of course this especially makes sense for the hugepage tests (then we
can still read hugepage by hugepage).

- Kevin
Re: [PATCH v2 5/8] selftests/mm: introduce helper to read every page in range
Posted by SeongJae Park 1 month ago 
On Wed,  7 Jan 2026 16:48:39 +0000 Kevin Brodsky <kevin.brodsky@arm.com> wrote:

> FORCE_READ(*addr) ensures that the compiler will emit a load from
> addr. Several tests need to trigger such a load for every page in
> the range [addr, addr + len), ensuring that every page is faulted
> in, if it wasn't already.
> 
> Introduce a new helper force_read_pages_in_range() that does exactly
> that and replace existing loops with a call to it.

Seems like a good cleanup to me.

> Some of those
> loops have a different step size, but reading from every page is
> appropriate in all cases.

So the test program's behavior is slightly be changed.  I believe that
shouldn't be problem, but I'm not that familiar with the test code, so not very
sure.  I'd like to listen voices from people more familiar with those.

Meanwhile, I'm curious what do you think about making the helper function
receives the step size together, and let the callers just pass their current
step size.


Thanks,
SJ

[...]
Re: [PATCH v2 5/8] selftests/mm: introduce helper to read every page in range
Posted by Kevin Brodsky 4 weeks ago 
On 09/01/2026 02:30, SeongJae Park wrote:
> On Wed,  7 Jan 2026 16:48:39 +0000 Kevin Brodsky <kevin.brodsky@arm.com> wrote:
>
>> FORCE_READ(*addr) ensures that the compiler will emit a load from
>> addr. Several tests need to trigger such a load for every page in
>> the range [addr, addr + len), ensuring that every page is faulted
>> in, if it wasn't already.
>>
>> Introduce a new helper force_read_pages_in_range() that does exactly
>> that and replace existing loops with a call to it.
> Seems like a good cleanup to me.

Thanks for having a look at this series!

>> Some of those
>> loops have a different step size, but reading from every page is
>> appropriate in all cases.
> So the test program's behavior is slightly be changed.  I believe that
> shouldn't be problem, but I'm not that familiar with the test code, so not very
> sure.  I'd like to listen voices from people more familiar with those.
>
> Meanwhile, I'm curious what do you think about making the helper function
> receives the step size together, and let the callers just pass their current
> step size.

That's what I initially considered, but considering this discussion on
v1 [1] this doesn't seem to be justified. In hugetlb-madvise, reading
every page instead of every hugepage is unnecessary but still correct
and the overhead should be negligible. In split_huge_page_test, I don't
think there's any justification for reading every byte - the intention
is to fault in pages, like all the other cases this patch touches.

- Kevin

[1]
https://lore.kernel.org/all/a3ca6293-8f85-4489-a48e-eb8d0d3792c5@kernel.org/
Re: [PATCH v2 5/8] selftests/mm: introduce helper to read every page in range
Posted by SeongJae Park 3 weeks, 6 days ago 
On Mon, 12 Jan 2026 10:37:26 +0100 Kevin Brodsky <kevin.brodsky@arm.com> wrote:

> On 09/01/2026 02:30, SeongJae Park wrote:
> > On Wed,  7 Jan 2026 16:48:39 +0000 Kevin Brodsky <kevin.brodsky@arm.com> wrote:
> >
> >> FORCE_READ(*addr) ensures that the compiler will emit a load from
> >> addr. Several tests need to trigger such a load for every page in
> >> the range [addr, addr + len), ensuring that every page is faulted
> >> in, if it wasn't already.
> >>
> >> Introduce a new helper force_read_pages_in_range() that does exactly
> >> that and replace existing loops with a call to it.
> > Seems like a good cleanup to me.
> 
> Thanks for having a look at this series!

My pleasure!

> 
> >> Some of those
> >> loops have a different step size, but reading from every page is
> >> appropriate in all cases.
> > So the test program's behavior is slightly be changed.  I believe that
> > shouldn't be problem, but I'm not that familiar with the test code, so not very
> > sure.  I'd like to listen voices from people more familiar with those.
> >
> > Meanwhile, I'm curious what do you think about making the helper function
> > receives the step size together, and let the callers just pass their current
> > step size.
> 
> That's what I initially considered, but considering this discussion on
> v1 [1] this doesn't seem to be justified. In hugetlb-madvise, reading
> every page instead of every hugepage is unnecessary but still correct
> and the overhead should be negligible. In split_huge_page_test, I don't
> think there's any justification for reading every byte - the intention
> is to fault in pages, like all the other cases this patch touches.
> 
> - Kevin
> 
> [1]
> https://lore.kernel.org/all/a3ca6293-8f85-4489-a48e-eb8d0d3792c5@kernel.org/

Makes sense, thank you for the link!

Please feel free to add

Reviewed-by: SeongJae Park <sj@kernel.org>


Thanks,
SJ

[...]

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.