+cc Pedro
On Tue, Aug 05, 2025 at 03:09:54PM -0400, Zi Yan wrote:
On 5 Aug 2025, at 15:00, Lorenzo Stoakes wrote:
On Tue, Aug 05, 2025 at 01:51:40PM -0400, Zi Yan wrote:
diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h index c20298ae98ea..b55d1809debc 100644 --- a/tools/testing/selftests/mm/vm_util.h +++ b/tools/testing/selftests/mm/vm_util.h @@ -23,7 +23,7 @@
- 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) +#define FORCE_READ(x) (*(const volatile typeof(x) *)&(x))
NIT: but wonder if const is necessary, and also (as discussed off-list
I just used READ_ONCE() code, but it is not necessary.
It's not end of the world though.
again :) will this work with a (void) prefixed, just to a. make it clear we're reading but discarding and b. to avoid any possible compiler warning on this?
Adding (void) makes no difference, at least from godbolt.
Yeah I won't pretend to understand _exactly_ what the compiler is doing here, if this is working in practice across multiple compilers and read-faulting the page that's good enough for me :)
I know for some reason this form doesn't generate one currently (not sure why), but we may hit that in future.
Neither gcc nor clang complains without (void). My guess is that volatile is doing something there.
Indeed possibly, be interesting if you or Pedro who's also playing with this could nail down exactly what's going on here.
Best Regards, Yan, Zi
But from my point of view this patch is fine - ship it! :)
Cheers, Lorenzo