Hi Arnd,
thank you for reviewing my patches.
On 09/11/2018 16:05, Arnd Bergmann wrote:
> On Fri, Nov 9, 2018 at 1:38 PM Vincenzo Frascino
> <vincenzo.frascino@xxxxxxx> wrote:
>> +/*
>> + * To avoid performance issues __u_vdso (unsigned vdso type) depends
>> + * on the architecture:
>> + * 32 bit only __u_vdso is defined as __u32
>> + * 64 bit plus compat __u_vdso is defined as __u64
>> + */
>> +#ifdef CONFIG_GENERIC_VDSO_32
>> +typedef __u32 __u_vdso;
>> +#else
>> +typedef __u64 __u_vdso;
>> +#endif /* CONFIG_GENERIC_VDSO_32 */
>> +
>> struct vdso_data {
>> - __u64 cs_cycle_last; /* Timebase at clocksource init */
>> - __u64 raw_time_sec; /* Raw time */
>> + __u64 cs_cycle_last; /* Timebase at clocksource init */
>> + __u_vdso raw_time_sec; /* Raw time */
>> __u64 raw_time_nsec;
>> - __u64 xtime_clock_sec; /* Kernel time */
>> + __u_vdso xtime_clock_sec; /* Kernel time */
>> __u64 xtime_clock_nsec;
>> - __u64 xtime_coarse_sec; /* Coarse time */
>> + __u_vdso xtime_coarse_sec; /* Coarse time */
>> __u64 xtime_coarse_nsec;
>> - __u64 wtm_clock_sec; /* Wall to monotonic time */
>> + __u_vdso wtm_clock_sec; /* Wall to monotonic time */
>
> It looks like you change the layout in a way that makes it incompatible between
> 32-bit and 64-bit user space. Doesn't that break the compat-vdso code
> for 32-bit?
>
No, it does not since vdso_data structure is not part of the ABI and the userspace can access the vdso library only via function symbols. There was a proposal some time ago to introduce ABI data structures in the vdso data page but it was rejected [1] and [2].
CONFIG_GENERIC_VDSO_32 is meant to be used in 32 bit only platforms and the rationale behind it is to preserve performances were possible.
[1] https://lkml.org/lkml/2015/9/2/460
[2] https://lkml.org/lkml/2015/9/2/534
> It also seems like we need the seconds to be 64-bit wide, at least
> for anything that counts in CLOCK_REALTIME or a related format,
> otherwise we have to change it back for y2038 support.
>
I agree with you, we should address the y2038 support here, even if there are some architectures that currently use u32 for CLOCK_REALTIME sec.
> Arnd
>
--
Regards,
Vincenzo
