On Wed, 2021-09-08 at 14:16 -0400, Peter Xu wrote:
> 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@xxxxxxxxxx>
> > ---
> > 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..
Yes the compiler attribute naming is unfortunate. That's why the kernel renamed
it to '__maybe_unused'.
> 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
Sounds good to me.
> I'm also wondering the diff between the two methods on arm64 and whether
> there's a huge difference on the numbers.
There isn't much, after rounding the end result in MHz is the same regardless
of the method used. IIUC, gettimeoftheday() uses the frequency register's value
to scale the counter measurements. So it makes sense for the results to be
coherent.
I figured it's cleaner to get the data from the source instead of inferring it
with extra math.
--
Nicolás Sáenz
