Re: [RFC PATCH v2 07/10] lib: vdso: don't use READ_ONCE() in __c_kernel_time()
From: Thomas Gleixner <hidden>
Date: 2020-01-11 11:07:56
Also in:
linux-arm-kernel, linux-mips, lkml
Christophe Leroy [off-list ref] writes:
With READ_ONCE() the 64 bits are being read: Without the READ_ONCE() only 32 bits are read. That's the most optimal. Without READ_ONCE() but with a barrier() after the read, we should get the same result but GCC (GCC 8.1) does less good: Assuming both part of the 64 bits data will fall into a single cacheline, the second read is in the noise.
They definitely are in the same cacheline.
So agreed to drop this change.
We could be smart about this and force the compiler to issue a 32bit
read for 32bit builds. See below. Not sure whether it's worth it, but
OTOH it will take quite a while until the 32bit time interfaces die
completely.
Thanks,
tglx
8<--------------- a/include/vdso/datapage.h
+++ b/include/vdso/datapage.h@@ -21,6 +21,18 @@ #define CS_RAW 1 #define CS_BASES (CS_RAW + 1) +#ifdef __LITTLE_ENDIAN +struct sec_hl { + u32 sec_l; + u32 sec_h; +}; +#else +struct sec_hl { + u32 sec_h; + u32 sec_l; +}; +#endif + /** * struct vdso_timestamp - basetime per clock_id * @sec: seconds
@@ -35,7 +47,10 @@ * vdso_data.cs[x].shift. */ struct vdso_timestamp { - u64 sec; + union { + u64 sec; + struct sec_hl sec_hl; + }; u64 nsec; }; --- a/lib/vdso/gettimeofday.c +++ b/lib/vdso/gettimeofday.c
@@ -165,8 +165,13 @@ static __maybe_unused int static __maybe_unused __kernel_old_time_t __cvdso_time(__kernel_old_time_t *time) { const struct vdso_data *vd = __arch_get_vdso_data(); - __kernel_old_time_t t = READ_ONCE(vd[CS_HRES_COARSE].basetime[CLOCK_REALTIME].sec); + __kernel_old_time_t t; +#if BITS_PER_LONG == 32 + t = READ_ONCE(vd[CS_HRES_COARSE].basetime[CLOCK_REALTIME].sec_hl.sec_l); +#else + t = READ_ONCE(vd[CS_HRES_COARSE].basetime[CLOCK_REALTIME].sec); +#endif if (time) *time = t;