[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <22169C82-5701-4ABB-811F-075D22CE6FCD@nvidia.com>
Date: Tue, 05 Aug 2025 10:26:17 -0400
From: Zi Yan <ziy@...dia.com>
To: wang lian <lianux.mm@...il.com>
Cc: akpm@...ux-foundation.org, broonie@...nel.org, david@...hat.com,
lorenzo.stoakes@...cle.com, sj@...nel.org, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, brauner@...nel.org, gkwang@...x-info.com,
jannh@...gle.com, Liam.Howlett@...cle.com, ludovico.zy.wu@...il.com,
p1ucky0923@...il.com, richard.weiyang@...il.com, ryncsn@...il.com,
shuah@...nel.org, vbabka@...e.cz, zijing.zhang@...ton.me
Subject: Re: [PATCH 1/2] selftests/mm: reuse FORCE_READ to replace "asm
volatile("" : "+r" (XXX));"
On 17 Jul 2025, at 9:18, wang lian wrote:
> Several mm selftests use the `asm volatile("" : "+r" (variable));`
> construct to force a read of a variable, preventing the compiler from
> optimizing away the memory access. This idiom is cryptic and duplicated
> across multiple test files.
>
> Following a suggestion from David[1], this patch refactors this
> common pattern into a FORCE_READ() macro
>
> [1] https://lore.kernel.org/lkml/4a3e0759-caa1-4cfa-bc3f-402593f1eee3@redhat.com/
>
> Signed-off-by: wang lian <lianux.mm@...il.com>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
> ---
> tools/testing/selftests/mm/cow.c | 30 +++++++++----------
> tools/testing/selftests/mm/guard-regions.c | 7 -----
> tools/testing/selftests/mm/hugetlb-madvise.c | 5 +---
> tools/testing/selftests/mm/migration.c | 13 ++++----
> tools/testing/selftests/mm/pagemap_ioctl.c | 4 +--
> .../selftests/mm/split_huge_page_test.c | 4 +--
> tools/testing/selftests/mm/vm_util.h | 7 +++++
> 7 files changed, 31 insertions(+), 39 deletions(-)
>
<snip>
> diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c
> index f0d9c035641d..05de1fc0005b 100644
> --- a/tools/testing/selftests/mm/split_huge_page_test.c
> +++ b/tools/testing/selftests/mm/split_huge_page_test.c
> @@ -399,7 +399,6 @@ int create_pagecache_thp_and_fd(const char *testfile, size_t fd_size, int *fd,
> char **addr)
> {
> size_t i;
> - int dummy = 0;
> unsigned char buf[1024];
>
> srand(time(NULL));
> @@ -441,8 +440,7 @@ int create_pagecache_thp_and_fd(const char *testfile, size_t fd_size, int *fd,
> madvise(*addr, fd_size, MADV_HUGEPAGE);
>
> for (size_t i = 0; i < fd_size; i++)
> - dummy += *(*addr + i);
> - asm volatile("" : "+r" (dummy));
> + FORCE_READ((*addr + i));
I encountered a segfault when running the test on x86_64.
i is 4194297 and fd_size is 4194304.
It seems that FORCE_READ() is reading (*addr + i) in 8 byte size
and i is only 7 bytes away from the end of the memory address.
This led to segfault.
(*(volatile char*)(*addr + i)); works fine.
Both gcc-12 and gcc-14 have the issue.
>
> 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 2b154c287591..c20298ae98ea 100644
> --- a/tools/testing/selftests/mm/vm_util.h
> +++ b/tools/testing/selftests/mm/vm_util.h
> @@ -18,6 +18,13 @@
> #define PM_SWAP BIT_ULL(62)
> #define PM_PRESENT BIT_ULL(63)
>
> +/*
> + * Ignore the checkpatch warning, we must read from x but don't want to do
> + * anything with it in order to trigger a read page fault. We therefore must use
> + * volatile to stop the compiler from optimising this away.
> + */
> +#define FORCE_READ(x) (*(volatile typeof(x) *)x)
> +
Also, look at FORCE_READ again, it converts x to a pointer to x and
deferences x as a point. It does not seem right to me.
Best Regards,
Yan, Zi
Powered by blists - more mailing lists