Re: [PATCH 3/3] oslat: Allow for arch specific timer frequency measurements

[Peter Xu <peterx@redhat.com>]

On Wed, Sep 08, 2021 at 12:02:09PM +0200, Nicolas Saenz Julienne wrote:
> Some architectures have special purpose registers to query the system
> timer's frequency. Let's use that when available.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenzju@redhat.com>
> ---
>  src/oslat/oslat.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/src/oslat/oslat.c b/src/oslat/oslat.c
> index bd155a6..23ca9b6 100644
> --- a/src/oslat/oslat.c
> +++ b/src/oslat/oslat.c
> @@ -51,6 +51,9 @@
>  # define atomic_inc(ptr)   __sync_add_and_fetch((ptr), 1)
>  # if defined(__x86_64__)
>  #  define relax()          __asm__ __volatile__("pause" ::: "memory")
> +
> +#define measure_timer_mhz generic_measure_timer_mhz
> +
>  static inline void frc(uint64_t *pval)
>  {
>  	uint32_t low, high;
> @@ -61,12 +64,18 @@ static inline void frc(uint64_t *pval)
>  }
>  # elif defined(__i386__)
>  #  define relax()          __asm__ __volatile__("pause" ::: "memory")
> +
> +#define measure_timer_mhz generic_measure_timer_mhz
> +
>  static inline void frc(uint64_t *pval)
>  {
>  	__asm__ __volatile__("rdtsc" : "=A" (*pval));
>  }
>  # elif defined(__PPC64__)
>  #  define relax()          do { } while (0)
> +
> +#define measure_timer_mhz generic_measure_timer_mhz
> +
>  static inline void frc(uint64_t *pval)
>  {
>  	__asm__ __volatile__("mfspr %0, 268\n" : "=r" (*pval));
> @@ -74,6 +83,15 @@ static inline void frc(uint64_t *pval)
>  # elif defined(__aarch64__)
>  #  define relax()          __asm__ __volatile("yield" : : : "memory")
>  
> +static inline unsigned int measure_timer_mhz(void)
> +{
> +	unsigned int val;
> +
> +	__asm__ __volatile__("mrs %0, cntfrq_el0" : "=r" (val));
> +
> +	return val / 1e6;
> +}
> +
>  static inline void frc(uint64_t *pval)
>  {
>  
> @@ -257,7 +275,7 @@ static cycles_t __measure_timer_hz(void)
>  	return (cycles_t) ((e - s) / sec);
>  }
>  
> -static unsigned int measure_timer_mhz(void)
> +static unsigned int __attribute__((unused)) generic_measure_timer_mhz(void)

This is okay I guess, but does not look nice, as it's marked unused even if
it's used..

How about for any arch that supports a faster version to read the freq, do:

#define measure_timer_mhz_fast
unsigned int measure_timer_mhz_fast(void)
{
    ...
}

Then at entry of measure_timer_mhz():

#ifdef measure_timer_mhz_fast
    return measure_timer_mhz_fast();
#endif

I'm also wondering the diff between the two methods on arm64 and whether
there's a huge difference on the numbers.

Thanks,

-- 
Peter Xu