Re: [PATCH 09/13] tools/nolibc: sys_poll: riscv: use __NR_ppoll_time64 for rv32

From: Willy Tarreau <w@1wt.eu>
To: Arnd Bergmann <arnd@arndb.de>
Cc: "Zhangjin Wu" <falcon@tinylab.org>,
	"Thomas Weißschuh" <thomas@t-8ch.de>,
	linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
	linux-riscv@lists.infradead.org,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Paul Walmsley" <paul.walmsley@sifive.com>
Subject: Re: [PATCH 09/13] tools/nolibc: sys_poll: riscv: use __NR_ppoll_time64 for rv32
Date: Sun, 28 May 2023 13:03:05 +0200	[thread overview]
Message-ID: <20230528110305.GI1956@1wt.eu> (raw)
In-Reply-To: <afc4944f-9494-4367-906d-06ac47648ab7@app.fastmail.com>

On Sun, May 28, 2023 at 12:55:47PM +0200, Arnd Bergmann wrote:
> On Sun, May 28, 2023, at 12:29, Willy Tarreau wrote:
> > On Sun, May 28, 2023 at 04:25:09PM +0800, Zhangjin Wu wrote:
> >> 
> >> * Use __kernel_timespec as timespec
> >> * Use 64bit time_t based struct timeval
> >>     * Disable gettimeofday syscall completely for 32bit platforms
> >>         * And disable the gettimeofday_bad1/2 test case too
> >
> > When you say "disable", you mean "remap", right ? Or do you mean
> > "break in 2023 code that was expected to break only in 2038 after
> 
> clock_gettime() has been supported for a very long time, so both
> time() and gettimeofday() can be trivial wrappers around that.

OK, that's what I wanted to clarify. I understood "drop" in the sense
of, well, "drop" :-)

> Nothing really should be using the timezone argument, so I'd
> just ignore that in nolibc. (it's a little trickier for /sbin/init
> setting the initial timezone, but I hope we can ignore that here).

Yes I'm fine with this approach.

> clock_gettime() as a function call that takes a timespec argument
> in turn should be a wrapper around either sys_clock_gettime64 (on
> 32-bit architectures) or sys_clock_gettime_old() (on 64-bit
> architectures, or as a fallback on old 32-bit kernels after
> clock_gettime64 fails).

Sounds good to me.

> On normal libc implementations, the low-level
> sys_clock_gettime64() and sys_clock_gettime_old(), whatever
> they are named, would call vdso first and then fall back
> to the syscall, but I don't think that's necessary for nolibc.

Indeed, we don't exploit the VDSO here since it's essentially useful
for performance and that's not what we're seeking.

> I'd define them the same as the kernel, with
> sys_clock_gettime64() taking a __kernel_timespec, and
> sys_clock_gettime_old() takeing a __kernel_old_timespec.

Sounds good, thanks Arnd!
Willy