On Mon, Jul 14, 2025 at 09:00:09PM +0800, 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 <[email protected]>
LGTM, so: Reviewed-by: Lorenzo Stoakes <[email protected]> > --- > 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"); Lol oops! :P yes thi is better. > > /* 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) > + Cool makes sense to share this. If this contradicts previous review, take this review to be the correct one :P > extern unsigned int __page_size; > extern unsigned int __page_shift; > > -- > 2.43.0 >
