Hi Arnd,
thank you for your review.
On 22/02/2019 13:49, Arnd Bergmann wrote:
> On Fri, Feb 22, 2019 at 1:25 PM Vincenzo Frascino
> <vincenzo.frascino@xxxxxxx> wrote:
>
>> +/*
>> + * The definitions below are required to overcome the limitations
>> + * of time_t on 32 bit architectures, which overflows in 2038.
>> + * The new code should use the replacements based on time64_t and
>> + * timespec64.
>> + *
>> + * The abstraction below will be updated once the migration to
>> + * time64_t is complete.
>> + */
>> +#ifdef CONFIG_GENERIC_VDSO_32
>> +#define __vdso_timespec old_timespec32
>> +#define __vdso_timeval old_timeval32
>> +#else
>> +#ifdef ENABLE_COMPAT_VDSO
>> +#define __vdso_timespec old_timespec32
>> +#define __vdso_timeval old_timeval32
>> +#else
>> +#define __vdso_timespec __kernel_timespec
>> +#define __vdso_timeval __kernel_old_timeval
>> +#endif /* CONFIG_COMPAT_VDSO */
>> +#endif /* CONFIG_GENERIC_VDSO_32 */
>
> I don't think we need __vdso_timeval at all, just use
> __kernel_old_timeval everywhere. There is no generic
> 64-bit timeval type in the kernel, and there won't be because
> gettimeofday() is deprecated.
>
Ok, I will update my implementation accordingly in v6.
> For __vdso_timespec, I see how you ended up with this
> redefinition, and it makes the current version of your patches
> easier, but I fear it will in turn make it harder to add the
> __kernel_old_timeval based variant.
>
What is __kernel_old_timespec (based on you next email)? Why do you think it
will make harder to add the new variants?
--
Regards,
Vincenzo
> What I'd prefer to see here is to always use the fixed length
> types everywhere in the code, such as
>
> static notrace int __cvdso_clock_gettime64(clockid_t clock,
> struct __kernel_timespec *ts)
> {
> ... /* normal clock_gettime using 64-bit times */
> }
>
> static notrace int __cvdso_clock_gettime32(clockid_t clock,
> struct old_timespec32 *ts32)
> {
> struct __kernel_timespec ts;
> int ret = __cvdso_clock_gettime64(clock, &ts);
>
> ts32->tv_sec = ts->tv_sec;
> ts32->tv_nsec = ts->tv_nsec;
>
> return ret;
> }
>
> and then have two different versions for 32-bit and 64-bit
> architectures, calling one or the other function of the
> generic code.
>
> Arnd
>
