| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5660b5ea-16f7-485a-8daa-d3d6cb4af310@arm.com>
Date: Mon, 19 Jan 2026 14:29:21 +0100
From: Kevin Brodsky <kevin.brodsky@....com>
To: "David Hildenbrand (Red Hat)" <david@...nel.org>, linux-mm@...ck.org,
linux-kselftest@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, Andrew Morton <akpm@...ux-foundation.org>,
Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, Mark Brown
<broonie@...nel.org>, Ryan Roberts <ryan.roberts@....com>,
Shuah Khan <shuah@...nel.org>
Subject: Re: [PATCH v2 5/8] selftests/mm: introduce helper to read every page
in range
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@....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
Powered by blists - more mailing lists