| Message ID | 20210908100209.118609-3-nsaenzju@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] oslat: rename cpu_mhz/cpu_hz to timer_mhz/cpu_hz | expand |
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@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.. 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
On Thu, Sep 09, 2021 at 12:29:17PM +0200, nsaenzju@redhat.com wrote: > > 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. Fair enough. Thanks, -- Peter Xu
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) { cycles_t m, mprev, d;
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(-)