Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 19 Oct 2021 13:56:32 -0300
From:   Arnaldo Carvalho de Melo <acme@kernel.org>
To:     Alistair Francis <alistair.francis@opensource.wdc.com>
Cc:     linux-riscv@lists.infradead.org, linux-perf-users@vger.kernel.org,
        linux-kernel@vger.kernel.org, alistair23@gmail.com,
        namhyung@kernel.org, jolsa@redhat.com,
        alexander.shishkin@linux.intel.com, mark.rutland@arm.com,
        dave@stgolabs.net, dvhart@infradead.org, peterz@infradead.org,
        mingo@redhat.com, tglx@linutronix.de, atish.patra@wdc.com,
        arnd@arndb.de, Alistair Francis <alistair.francis@wdc.com>
Subject: Re: [PATCH v2 1/2] perf bench futex: Use a 64-bit time_t
Message-ID: <YW74wK03ibOS3jVZ@kernel.org>
References: <20211015005634.2658470-1-alistair.francis@opensource.wdc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211015005634.2658470-1-alistair.francis@opensource.wdc.com>
Precedence: bulk

Em Fri, Oct 15, 2021 at 10:56:33AM +1000, Alistair Francis escreveu:
> From: Alistair Francis <alistair.francis@wdc.com>
> 
> Convert tools/perf to only use a 64-bit time_t. On 64-bit architectures
> this isn't a functional change. On 32-bit architectures we now only
> perform 64-bit time_t syscalls (__NR_futex_time64) and use a struct
> timespec64.
> 
> This won't work on kernels before 5.1, but as perf is tied to the kernel
> that's ok.

No, perf is not tied to the kernel, one can use a new perf tool on any
previous kernel, and an old perf tool should work on new kernels as
well.

- Arnaldo
 
> This allows us to build perf for 32-bit architectures with 64-bit time_t
> like RISC-V 32-bit.
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  tools/perf/bench/futex.h | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/bench/futex.h b/tools/perf/bench/futex.h
> index b3853aac3021c..b9665d43d2988 100644
> --- a/tools/perf/bench/futex.h
> +++ b/tools/perf/bench/futex.h
> @@ -12,6 +12,7 @@
>  #include <sys/syscall.h>
>  #include <sys/types.h>
>  #include <linux/futex.h>
> +#include <linux/time_types.h>
>  
>  struct bench_futex_parameters {
>  	bool silent;
> @@ -27,12 +28,14 @@ struct bench_futex_parameters {
>  	unsigned int nrequeue;
>  };
>  
> +#define timespec64 __kernel_timespec
> +
>  /**
>   * futex() - SYS_futex syscall wrapper
>   * @uaddr:	address of first futex
>   * @op:		futex op code
>   * @val:	typically expected value of uaddr, but varies by op
> - * @timeout:	typically an absolute struct timespec (except where noted
> + * @timeout:	typically an absolute struct timespec64 (except where noted
>   *		otherwise). Overloaded by some ops
>   * @uaddr2:	address of second futex for some ops
>   * @val3:	varies by op
> @@ -47,15 +50,26 @@ struct bench_futex_parameters {
>   * These argument descriptions are the defaults for all
>   * like-named arguments in the following wrappers except where noted below.
>   */
> -#define futex(uaddr, op, val, timeout, uaddr2, val3, opflags) \
> -	syscall(SYS_futex, uaddr, op | opflags, val, timeout, uaddr2, val3)
> +/**
> + * We only support 64-bit time_t for the timeout.
> + * On 64-bit architectures we can use __NR_futex
> + * On 32-bit architectures we use __NR_futex_time64. This only works on kernel
> + * versions 5.1+.
> + */
> +#if __BITS_PER_LONG == 64
> +# define futex(uaddr, op, val, timeout, uaddr2, val3, opflags) \
> +	syscall(__NR_futex, uaddr, op | opflags, val, timeout, uaddr2, val3)
> +#else
> +# define futex(uaddr, op, val, timeout, uaddr2, val3, opflags) \
> +	syscall(__NR_futex_time64, uaddr, op | opflags, val, timeout, uaddr2, val3)
> +#endif
>  
>  /**
>   * futex_wait() - block on uaddr with optional timeout
>   * @timeout:	relative timeout
>   */
>  static inline int
> -futex_wait(u_int32_t *uaddr, u_int32_t val, struct timespec *timeout, int opflags)
> +futex_wait(u_int32_t *uaddr, u_int32_t val, struct timespec64 *timeout, int opflags)
>  {
>  	return futex(uaddr, FUTEX_WAIT, val, timeout, NULL, 0, opflags);
>  }
> @@ -74,7 +88,7 @@ futex_wake(u_int32_t *uaddr, int nr_wake, int opflags)
>   * futex_lock_pi() - block on uaddr as a PI mutex
>   */
>  static inline int
> -futex_lock_pi(u_int32_t *uaddr, struct timespec *timeout, int opflags)
> +futex_lock_pi(u_int32_t *uaddr, struct timespec64 *timeout, int opflags)
>  {
>  	return futex(uaddr, FUTEX_LOCK_PI, 0, timeout, NULL, 0, opflags);
>  }
> @@ -111,7 +125,7 @@ futex_cmp_requeue(u_int32_t *uaddr, u_int32_t val, u_int32_t *uaddr2, int nr_wak
>   */
>  static inline int
>  futex_wait_requeue_pi(u_int32_t *uaddr, u_int32_t val, u_int32_t *uaddr2,
> -		      struct timespec *timeout, int opflags)
> +		      struct timespec64 *timeout, int opflags)
>  {
>  	return futex(uaddr, FUTEX_WAIT_REQUEUE_PI, val, timeout, uaddr2, 0,
>  		     opflags);
> -- 
> 2.31.1

-- 

- Arnaldo