perf tools build broken for RISCV 32 bit

perf tools build broken for RISCV 32 bit

[Emiliano Ingrassia]

Hi,

When building perf for RISCV 32 bit (v5.10.7) I got the following

| In file included from bench/futex-hash.c:29:
| bench/futex.h: In function ‘futex_wait’:
| bench/futex.h:37:10: error: ‘SYS_futex’ undeclared (first use in this function); did you mean ‘SYS_tee’?

This issue is similar to the one reported in https://lkml.org/lkml/2019/4/19/631

I found that patching tools/arch/riscv/include/uapi/asm/unistd.h as following:

 #ifdef __LP64__
 #define __ARCH_WANT_NEW_STAT
 #define __ARCH_WANT_SET_GET_RLIMIT
+#else
+#define __ARCH_WANT_TIME32_SYSCALLS
 #endif /* __LP64__ */

solved the problem.

I also found that a similar patch for arch/riscv/include/uapi/asm/unistd.h
was removed in commit d4c08b9776b3, so probably this is not the right way(?).

Thank you, best regards.

Re: perf tools build broken for RISCV 32 bit

[Arnd Bergmann]

On Thu, Jan 14, 2021 at 7:38 PM Emiliano Ingrassia
<ingrassia@epigenesys.com> wrote:
>
> Hi,
>
> When building perf for RISCV 32 bit (v5.10.7) I got the following
>
> | In file included from bench/futex-hash.c:29:
> | bench/futex.h: In function ‘futex_wait’:
> | bench/futex.h:37:10: error: ‘SYS_futex’ undeclared (first use in this function); did you mean ‘SYS_tee’?
>
> This issue is similar to the one reported in https://lkml.org/lkml/2019/4/19/631
>
> I found that patching tools/arch/riscv/include/uapi/asm/unistd.h as following:
>
>  #ifdef __LP64__
>  #define __ARCH_WANT_NEW_STAT
>  #define __ARCH_WANT_SET_GET_RLIMIT
> +#else
> +#define __ARCH_WANT_TIME32_SYSCALLS
>  #endif /* __LP64__ */
>
> solved the problem.
>
> I also found that a similar patch for arch/riscv/include/uapi/asm/unistd.h
> was removed in commit d4c08b9776b3, so probably this is not the right way(?).

In short, it won't work, as rv32 does not provide the time32 syscalls.
Your patch will make the application build, but it will not be able to
call futex().

You will in fact run into a related problem on any 32-bit architecture
if CONFIG_COMPAT_32BIT_TIME is disabled, or if you pass a non-NULL
timeout parameter and build with a time64-enabled libc.

The fix in the application is to call either __NR_futex or __NR_futex64
depending on the definition of time_t in the C library. I would recommend
doing it like

#ifdef __NR_futex
#define do_futex (sizeof(time_t) == sizeof(__kernel_long_t)) ? \
         __NR_futex : __NR_futex_time64
#else
#define do_futex __NR_futex
#done

       Arnd

Re: perf tools build broken for RISCV 32 bit

[Emiliano Ingrassia]

Hi Arnd,

thank you for the quick reply and support.

On Thu, Jan 14, 2021 at 08:16:28PM +0100, Arnd Bergmann wrote:
> On Thu, Jan 14, 2021 at 7:38 PM Emiliano Ingrassia
> <ingrassia@epigenesys.com> wrote:
> >
> > Hi,
> >
> > When building perf for RISCV 32 bit (v5.10.7) I got the following
> >
> > | In file included from bench/futex-hash.c:29:
> > | bench/futex.h: In function ‘futex_wait’:
> > | bench/futex.h:37:10: error: ‘SYS_futex’ undeclared (first use in this function); did you mean ‘SYS_tee’?
> >
> > This issue is similar to the one reported in https://lkml.org/lkml/2019/4/19/631
> >
> > I found that patching tools/arch/riscv/include/uapi/asm/unistd.h as following:
> >
> >  #ifdef __LP64__
> >  #define __ARCH_WANT_NEW_STAT
> >  #define __ARCH_WANT_SET_GET_RLIMIT
> > +#else
> > +#define __ARCH_WANT_TIME32_SYSCALLS
> >  #endif /* __LP64__ */
> >
> > solved the problem.
> >
> > I also found that a similar patch for arch/riscv/include/uapi/asm/unistd.h
> > was removed in commit d4c08b9776b3, so probably this is not the right way(?).
>
> In short, it won't work, as rv32 does not provide the time32 syscalls.
> Your patch will make the application build, but it will not be able to
> call futex().
>
> You will in fact run into a related problem on any 32-bit architecture
> if CONFIG_COMPAT_32BIT_TIME is disabled, or if you pass a non-NULL
> timeout parameter and build with a time64-enabled libc.
>

I'm using glibc 2.32 which supports 64 bit time_t on RISCV 32.

In particular, searching for __NR_futex in glibc RISCV source code I got:

sysdeps/unix/sysv/linux/riscv/rv32/arch-syscall.h: #define __NR_futex_time64 422
sysdeps/unix/sysv/linux/riscv/sysdep.h: #define __NR_futex __NR_futex_time64
sysdeps/unix/sysv/linux/riscv/rv64/arch-syscall.h: #define __NR_futex 98

but in the generated bits/syscall.h, included in bench/futex.h, I found:

#ifdef __NR_futex
# define SYS_futex __NR_futex
#endif

#ifdef __NR_futex_time64
# define SYS_futex_time64 __NR_futex_time64
#endif

So the problem is that userspace applications can't see the definition
of __NR_futex which is in sysdep.h, but there are no problems in calling
futex() libc wrapper because glibc syscall.c includes that header.

A possible fix for the perf tool would be to use futex() libc wrapper
in tools/perf/bench/futex.h instead of syscall(), but, if I understand correctly,
there are some drawbacks that does not permit it.

So what should be the right solution?

> The fix in the application is to call either __NR_futex or __NR_futex64
> depending on the definition of time_t in the C library. I would recommend
> doing it like
>
> #ifdef __NR_futex
> #define do_futex (sizeof(time_t) == sizeof(__kernel_long_t)) ? \
>          __NR_futex : __NR_futex_time64
> #else
> #define do_futex __NR_futex
> #done
>
>        Arnd

Where should be this fix applied? To perf code?

Thank you,

Emiliano