Re: [PATCH v3] selftests/futex: Convert 32bit timespec struct to 64bit version for 32bit compatibility mode

[Wei Gao <wegao@suse.com>]

On Wed, Jul 02, 2025 at 10:15:06PM +0200, Thomas Gleixner wrote:
> On Wed, Jul 02 2025 at 11:21, Terry Tritton wrote:
> 
> > Futex_waitv can not accept old_timespec32 struct, so userspace should
> 
> sys_futex_wait()
> 
> > convert it from 32bit to 64bit before syscall in 32bit compatible mode.
> >
> > This fix is based off [1]
> >
> > Link: https://lore.kernel.org/all/20231203235117.29677-1-wegao@suse.com/ [1]
> >
> > Originally-by: Wei Gao <wegao@suse.com>
> > Signed-off-by: Terry Tritton <terry.tritton@linaro.org>
> > ---
> > Changes in v3:
> > - Fix signed-off-by chain but for real this time
> >
> > Changes in v2:
> > - Fix signed-off-by chain
> >
> >  .../testing/selftests/futex/include/futex2test.h  | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/tools/testing/selftests/futex/include/futex2test.h b/tools/testing/selftests/futex/include/futex2test.h
> > index ea79662405bc..6780e51eb2d6 100644
> > --- a/tools/testing/selftests/futex/include/futex2test.h
> > +++ b/tools/testing/selftests/futex/include/futex2test.h
> > @@ -55,6 +55,13 @@ struct futex32_numa {
> >  	futex_t numa;
> >  };
> >  
> > +#if !defined(__LP64__)
> > +struct timespec64 {
> > +	int64_t tv_sec;
> > +	int64_t tv_nsec;
> > +};
> > +#endif
> 
> Why not use __kernel_timespec which is the actual data type that syscall
> requests?
> 
> > +
> >  /**
> >   * futex_waitv - Wait at multiple futexes, wake on any
> >   * @waiters:    Array of waiters
> > @@ -65,7 +72,15 @@ struct futex32_numa {
> >  static inline int futex_waitv(volatile struct futex_waitv *waiters, unsigned long nr_waiters,
> >  			      unsigned long flags, struct timespec *timo, clockid_t clockid)
> >  {
> > +#if !defined(__LP64__)
> > +	struct timespec64 timo64 = {0};
> > +
> > +	timo64.tv_sec = timo->tv_sec;
> > +	timo64.tv_nsec = timo->tv_nsec;
> > +	return syscall(__NR_futex_waitv, waiters, nr_waiters, flags, &timo64, clockid);
> > +#else
> >  	return syscall(__NR_futex_waitv, waiters, nr_waiters, flags, timo, clockid);
> > +#endif
> 
> You simply can do
> 
>         struct __kernel_timespec ts = {
>         	.tv_sec = timo->tv_sec,
>                 .tv_nsec = timo->tv_nsec,
>         };
> 
>   	return syscall(__NR_futex_waitv, waiters, nr_waiters, flags, &ts, clockid);
>         
> unconditionally. No?
Thanks for your valuable comments.
Your change has more directly interacts with system calls and simplifies the code without
conditional compilation. 
Only concern from my side is test on very old kernel(But i think this is not problem for kself test)
> 
> Thanks,
> 
>         tglx