On Thu, Nov 29, 2018 at 11:12 PM Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
> On Thu, 29 Nov 2018, Vincenzo Frascino wrote:
> > +if HAVE_GENERIC_VDSO
> > +
> > +config GENERIC_GETTIMEOFDAY
> > + bool
> > + help
> > + This is a generic implementation of gettimeofday vdso.
> > + Each architecture that enables this feature has to
> > + provide the fallback implementation.
> > +
> > +config GENERIC_VDSO_32
> > + bool
> > + depends on GENERIC_GETTIMEOFDAY && !64BIT
> > + help
> > + This config option helps to avoid possible performance issues
> > + in 32 bit only architectures.
> > +
> > +config HAVE_ARCH_TIMER
> > + bool
> > + depends on GENERIC_GETTIMEOFDAY
> > + help
> > + Select this configuration option if the architecture has an arch
> > + timer.
>
> Please don't add code which is architecture specific to the generic
> implementation. There is really no reason to make ARCH_TIMER special.
I think it's just unfortunate naming based on what arm64 had, but
conceptually it does the right thing, and just disable the clock_gettime
fastpath on kernel builds that never have access to a clocksource
from user space.
> > +static notrace int __cvdso_clock_getres(clockid_t clock_id,
> > + struct __vdso_timespec *res)
> > +{
> > + u64 ns;
> > +
> > + if (!__arch_valid_arg(res))
> > + return -EFAULT;
> > +
> > + if (clock_id == CLOCK_REALTIME ||
> > + clock_id == CLOCK_TAI ||
> > + clock_id == CLOCK_BOOTTIME ||
> > + clock_id == CLOCK_MONOTONIC ||
> > + clock_id == CLOCK_MONOTONIC_RAW)
> > + ns = MONOTONIC_RES_NSEC;
>
> This is wrong. You cannot unconditionally set that. See what the syscall
> based version does. You always need to verify that the syscall version and
> the vdso version return the same information and not something randomly
> different.
Do you think we actually need the fastpath here? If not, the
easy way to do it would be to always fall back to calling
the syscall based version. Or was this originally added
to deal with the syscall and vdso clock_gettime having
different resolutions (which would sound like a bad idea, but
might have to stay for compatibility)?
Arnd
