[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2fdacd57-2314-47c7-9eeb-6928221ce516@redhat.com>
Date: Thu, 17 Jul 2025 13:44:12 +0200
From: David Hildenbrand <david@...hat.com>
To: wang lian <lianux.mm@...il.com>, akpm@...ux-foundation.org,
broonie@...nel.org, lorenzo.stoakes@...cle.com, sj@...nel.org,
ziy@...dia.com, linux-mm@...ck.org, linux-kernel@...r.kernel.org
Cc: 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: refactor common code and improve test
skipping in guard_region
On 17.07.25 13:24, wang lian wrote:
> Move the generic `FORCE_READ` macro from `guard-regions.c` to the shared
> `vm_util.h` header to promote code reuse.
>
> In `guard-regions.c`, replace `ksft_exit_skip()` with the `SKIP()` macro
> to ensure only the current test is skipped on permission failure, instead
> of terminating the entire test binary.
>
> Signed-off-by: wang lian <lianux.mm@...il.com>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
> ---
> tools/testing/selftests/mm/guard-regions.c | 9 +--------
> tools/testing/selftests/mm/vm_util.h | 7 +++++++
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/guard-regions.c b/tools/testing/selftests/mm/guard-regions.c
> index 93af3d3760f9..b0d42eb04e3a 100644
> --- a/tools/testing/selftests/mm/guard-regions.c
> +++ b/tools/testing/selftests/mm/guard-regions.c
> @@ -35,13 +35,6 @@
> static volatile sig_atomic_t signal_jump_set;
> static sigjmp_buf signal_jmp_buf;
>
> -/*
> - * 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)
> -
> /*
> * How is the test backing the mapping being tested?
> */
> @@ -582,7 +575,7 @@ TEST_F(guard_regions, process_madvise)
>
> /* OK we don't have permission to do this, skip. */
> if (count == -1 && errno == EPERM)
> - ksft_exit_skip("No process_madvise() permissions, try running as root.\n");
> + SKIP(return, "No process_madvise() permissions, try running as root.\n");
>
> /* Returns the number of bytes advised. */
> ASSERT_EQ(count, 6 * page_size);
> 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)
> +
> extern unsigned int __page_size;
> extern unsigned int __page_shift;
>
The FORCE_READ() stuff belongs into patch #2.
--
Cheers,
David / dhildenb
Powered by blists - more mailing lists