perf tools build broken for RISCV 32 bit

Tue Jan 19 11:53:31 EST 2021

Hi Arnd,

thank you for the support.

On Mon, Jan 18, 2021 at 08:58:50PM +0100, Arnd Bergmann wrote:
> On Mon, Jan 18, 2021 at 4:33 PM Emiliano Ingrassia
> <ingrassia at epigenesys.com> wrote:
> > On Mon, Jan 18, 2021 at 04:19:47PM +0100, Arnd Bergmann wrote:
> > > On Mon, Jan 18, 2021 at 3:53 PM Emiliano Ingrassia <ingrassia at epigenesys.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > after some searching I found this patch from Yocto project:
> > > >
> > > > https://www.mail-archive.com/linux-yocto@lists.yoctoproject.org/msg01004.html.
> > > >
> > > > I don't know if it's ever been proposed as mainline patch.
> > > >
> > > > What do you think about it? Could it be the right solution?
> > >
> > > No. While this makes it work on rv32, it does not fix it in a
> > > way that makes it work on other architectures with a time64
> > > libc. If you fix it, please do it in a way that works on all architectures.
> >
> > Ok, I totally agree with you on this point.
> >
> > So, it is right to conclude that the problem is not in perf
> > implementation but in the libc implementation?
> > That is, for example, glibc should define SYS_futex as SYS_futex_time64
> > in case of riscv 32 bit because of its support to 64 bit time_t?
>
> Actually, there might be a somewhat less ugly workaround, if we decide
> to put a correct wrapper into the exported kernel headers, e.g. adding
> something along these lines to include/uapi/linux/futex.h or a new header
> from the kernel:
>
> #ifndef __KERNEL__
> #include <linux/futex.h>
> #include <linux/time_types.h>
> #include <asm/unistd.h>
> #include <sys/time.h>
> #include <unistd.h>
> #include <errno.h>
>
> #define __kernel_futex __kernel_futex
> typedef union __attribute__ ((__transparent_union__)) {
>        const struct timespec *timeout;
>        unsigned int intval;
> } __kernel_futex_union;
>
> struct __kernel_old_timespec {
>        __kernel_time_t tv_sec;                 /* seconds */
>        long            tv_nsec;                /* nanoseconds */
> };
>
> static inline int __kernel_futex(int *uaddr, int op, int val,
>                                  __kernel_futex_union val2,
>                                  unsigned *uaddr2, int val3)
> {
>         int cmd = op & FUTEX_CMD_MASK;
>         int ret = -ENOSYS;
> #ifdef __NR_futex64
>         {
>                 if (((cmd != FUTEX_WAIT) && (cmd != FUTEX_WAIT_BITSET)) ||
>                     (sizeof(time_t) == sizeof(__u64))) {
>                         ret = syscall(__NR_futex64, uaddr, op, val,
>                                       val2.timeout, uaddr2, val3);
>                 }
>
>                 if (ret != -ENOSYS && ret != -EPERM)
>                         return ret;
>         }
> #endif
> #ifdef __NR_futex
>         {
>                 if (((cmd != FUTEX_WAIT) && (cmd != FUTEX_WAIT_BITSET)) ||
>                     sizeof(time_t) == sizeof(__kernel_long_t)) {
>                         ret = syscall(__NR_futex, uaddr, op, val,
>                                        val2.timeout, uaddr2, val3);
>                 } else {
>                         struct __kernel_old_timespec oldtimeout = {
>                                 .tv_sec  = val2.timeout->tv_sec,
>                                 .tv_nsec = val2.timeout->tv_nsec,
>                         };
>                         ret = syscall(__NR_futex, uaddr, op, val,
>                                       &oldtimeout, uaddr2, val3);
>                 }
>         }
> #endif
>         return ret;
> }
> #endif /* __KERNEL__
>
> With this, applications can be changed to define their own
>
> #ifdef __kernel_futex
> #define futex(uaddr, futex_op, val, val2, uaddr2, val3) \
>       __kernel_futex(uaddr, futex_op, val, val2, uaddr2, val3)
> #else
> #define futex(uaddr, futex_op, val, val2, uaddr2, val3) \
>        syscall(__NR_futex, uaddr, futex_op, val, val2, uaddr2, val3)
> #endif
>
>          Arnd

I would like to do a recap on what we have so far.

The goal is to compile perf for RISCV 32 bit with glibc 2.32 which has support
for 64 bit time_t. The target kernel is 5.10.7 with option COMPAT_32BIT_TIME
enabled.

Actually in kernel 5.10.7 the file include/uapi/asm-generic/unistd.h contains
the definition of futex syscall based on some macros. In particular:
|...
|#if __BITS_PER_LONG == 32 || defined(__SYSCALL_COMPAT)
|#define __SC_3264(_nr, _32, _64) __SYSCALL(_nr, _32)
|#else
|#define __SC_3264(_nr, _32, _64) __SYSCALL(_nr, _64)
|#endif
|...
|/* kernel/futex.c */
|#if defined(__ARCH_WANT_TIME32_SYSCALLS) || __BITS_PER_LONG != 32
|#define __NR_futex 98
|__SC_3264(__NR_futex, sys_futex_time32, sys_futex)
|#endif
|...
|#define __NR_futex_time64 422
|__SYSCALL(__NR_futex_time64, sys_futex)
|...

So, in case of 64 bit architecture or 32 bit architecture with macro
__ARCH_WANT_TIME32_SYSCALLS defined, __NR_futex is defined and the proper
syscall associated with it.

In other cases, that is 32 bit architecture without support for time32 syscalls,
__NR_futex is not defined. __NR_futex_time64 is defined instead.

The syscall futex() expects timespec kernel structure, while futex_time32()
takes the old_timespec32 kernel structure as timer parameter.

To call futex() syscall in userspace it is necessary to use syscall() and
SYS_futex. In particular this is true in glibc which does not provide a wrapper
for it.

In case of architecture without __NR_futex definition nor futex() wrapper,
the syscall function should be called with SYS_futex_time64 instead of the
classic SYS_futex. This is not backward compatible and also not documented
as far as I know.

How this problem is solved in real scenarios? For example how can pthread
library, which uses futex syscall, be compiled correctly in glibc?

The solution is quite simple, glibc has a file called sysdep.h which in case of
RISCV 32 bit contains the following workarounds:
|...
|#if __WORDSIZE == 32
|
|/* Workarounds for generic code needing to handle 64-bit time_t.  */
|
|/* Fix sysdeps/unix/sysv/linux/clock_getcpuclockid.c.  */
|#define __NR_clock_getres       __NR_clock_getres_time64
|/* Fix sysdeps/nptl/lowlevellock-futex.h.  */
|#define __NR_futex              __NR_futex_time64
|...

So, in that case, __NR_futex is defined as __NR_futex_time64 and so SYS_futex is
defined and usable.

But this file can not be seen and used by userspace application.

So, what should be the right solution here?

Should the kernel export only __NR_futex and then check the timer struct
dimension and distinguish the various cases (time32 vs time64 futex)?

Or should the libc implementation take care to define SYS_futex once and for all
and then use the proper __NR_futex{_time64} based on its definition of time_t?

Considering that this is not only a problem of futex but of many others syscalls
(e.g. __NR_clock_gettime{64}, __NR_clock_settime{64}, and so on), probably
I would argue that the problem should be solved in the libc implementation
in some way.

What do you think?

Thank you, best regards.

Emiliano