Re: [PATCH v2 03/14] selftests: vDSO: Introduce vdso_syscalls.h

["Arnd Bergmann" <arnd@arndb.de>]

On Fri, Nov 14, 2025, at 11:02, Thomas Weißschuh wrote:
> On Fri, Nov 14, 2025 at 10:16:01AM +0100, Arnd Bergmann wrote:
>> On Fri, Nov 14, 2025, at 09:48, Thomas Weißschuh wrote:
>> If you think that clock_getres_time64() is important, I don't
>> mind changing that, especially now that we have a shared vdso
>> for all architectures. The arguments here is a bit different,
>> since an efficient clock_getres() function in libc requires
>> caching the values in userspace, while an efficient gettimeofday()
>> is much simpler, by calling vdso_clock_gettime_time64()
>
> I don't think it is important. For my SPARC vDSO series I even
> dropped the regular clock_getres() after your request. But because it
> doesn't exist we need to handle the presence of vdso_clock_getres() and
> the simultaneous absence of sys_clock_getres() in the test.

But that is the other way round, right? On sparc32 we have
(optionally) sys_clock_getres() but never vdso_clock_getres().

>> I don't think we can actually build a full userspace (other than nolibc)
>> that works with CONFIG_COMPAT_32BIT_TIME=n, so I'm not particularly
>> worried about testing the vdso for that case.
>
> musl 1.2 started to always use 64-bit times. Looking at both the musl and glibc
> code, they always try the 64-bit variant first.
> I think they should work fine.

No, musl only uses the time64 syscalls when it actually passes
a 64-bit time value, but e.g. still uses __NR_futex instead of
__NR_futex_time64 when waiting for a futex without a timeout, and it uses 
__NR_clock_settime instead of __NR_clock_settime_time64 when setting a
time within the 32-bit time_t range (1902..2037).

> Personally I'd like to have tests for the functionality that exists.
> Even if there are currently no users.
>
>> You already skip testing vdso_time() if sys_time() is unavailable, and I
>> think we can do it the exact same way for all five vdso calls.
>
> That was an oversight.

Ok. So you'd want to check all the time32 and time64 vdso calls
against the __kernel_timespec values returned from
sys_clock_get{res_time64,time64} and their 64-bit equivalents?

I think in this case we have to actually address the inconsistency
in the rounding between the interfaces, which I don't think is
well documented and possibly differs across implementations.

As far as I can tell, gettimeofday() always returns the
CLOCK_REALTIME value rounded down to full microseconds and
truncated to signed 'long' seconds, while time() returns the
CLOCK_REALTIME_COARSE value rounded down to full seconds.
This can be a second earlier than a previous CLOCK_REALTIME
value.

I see that glibc's time() function uses CLOCK_REALTIME_COARSE
to be consistent with the Linux sys_time() and vdso_time(),
while musl's time() uses CLOCK_REALTIME for consistency with
gettimeofday() and sensible user expectations.

>> > sys_clock_gettime() should probably be called sys_clock_gettime64(),
>> > as that is what it actually is.
>> 
>> That also seems wrong, as there is no clock_gettime64 on 64-bit
>> architectures, only clock_gettime.
>
> I referred to the type that it returns, which is always 64-bit.
> Another name, without the sys_ prefix, would be better.

Right, but then I would make it return 'struct timespec', not
'struct __kernel_timespec', because it's no longer the kernel
interface.

>> > FYI: gettimeoday() seems to be available even in kernels without
>> > CONFIG_COMPAT_32BIT_TIME.
>> 
>> I see, that does sound like a mistake. It's relatively harmless,
>> but I think it would be safe to change this along with changing
>> the vdso to only expose the time32 interfaces when COMPAT_32BIT_TIME
>> is enabled.
>
> IMO that would need to be another series with its own discussion.

Sure.

      Arnd