Message-ID: <20251114102555-293562eb-f1f9-47e1-bc2d-59f26a7283fa@linutronix.de>
Date: Fri, 14 Nov 2025 11:02:13 +0100
From: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
To: Arnd Bergmann <arnd@...db.de>
Cc: Andy Lutomirski <luto@...nel.org>, 
	Thomas Gleixner <tglx@...utronix.de>, Vincenzo Frascino <vincenzo.frascino@....com>, 
	shuah <shuah@...nel.org>, linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v2 03/14] selftests: vDSO: Introduce vdso_syscalls.h

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:
> > On Fri, Nov 14, 2025 at 09:13:02AM +0100, Arnd Bergmann wrote:
> >> On Thu, Nov 13, 2025, at 16:30, Thomas Weißschuh wrote:
> >> 
> >> __NR_clock_getres and vdso_clock_getres() both always return a
> >> __kernel_old_timespec, so I now think it's best to return that from
> >> sys_clock_getres() without the __NR_clock_getres_time64 alternative
> >> here and not worry about whether that is a 32-bit or 64-bit type,
> >>
> >> I should have thought this through better in my comments to the
> >> previous version.
> >> 
> >> In kernels without CONFIG_COMPAT_32BIT_TIME, we currently leave
> >> out the clock_getres/clock_gettime/gettimeofday/time syscalls,
> >> but still provide the vdso interfaces. For consistency we should
> >> probably leave out both syscall and vdso in that configuration,
> >> and then we also don't need to compare the vdso_getres result
> >> against sys_getres_time64.
> >
> > That sounds good. But today no vDSO provides clock_getres_time64,
> > so removing clock_getres from the vDSO will affect users.
> 
> In what way? When we introduced the clock_gettime64()
> vdso call, we debated also adding time64(), gettimeofday_time64()
> and clock_getres_time64() but decided against that based on
> the argument that the libc can implement all of these efficiently
> with just clock_gettime64().

clock_getres_time64() can't be implemented with vdso_clock_gettime64().
It could use vdso_clock_getres() as the resolution should never
overflow the type. But nobody seems to do this either.

> 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.

> > So we will end up with some sort of inconsistency in any case.
> > While I agree that it would be nice if the type mangling was unnecessary,
> > I prefer to correctly test what we have today. If we decide to simplify
> > the vDSO itself then we have working tests.
> 
> Sorry, I'm not following. Do you mean we need the mangling since we
> support the vdso for configurations without the direct syscalls, or
> do you mean something else?

Exactly.

> 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.

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.

> > 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.

> > 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.


Thomas

Powered by blists - more mailing lists