[sysinfo] Fix CPU Frequency reading on AMD Ryzen CPU's (#1117)

Currently, i get: ``` Run on (32 X 7326.56 MHz CPU s) CPU Caches: L1 Data 32 KiB (x16) L1 Instruction 32 KiB (x16) L2 Unified 512 KiB (x16) L3 Unified 32768 KiB (x2) ``` which seems mostly right, except that the frequency is rather bogus. Yes, i guess the CPU could theoretically achieve that, but i have 3.6GHz configured, and scaling disabled. So we clearly read the wrong thing. With this fix, i now get the expected ``` Run on (32 X 3598.53 MHz CPU s) CPU Caches: L1 Data 32 KiB (x16) L1 Instruction 32 KiB (x16) L2 Unified 512 KiB (x16) L3 Unified 32768 KiB (x2) ```

[sysinfo] Fix CPU Frequency reading on AMD Ryzen CPU's (#1117)
c05843a9 · Roman Lebedev · GitHub · 69054ae5 · c05843a9 · c05843a9
Unverified Commit c05843a9 authored Apr 23, 2021 by Roman Lebedev Committed by GitHub Apr 23, 2021
Hide whitespace changes
Inline Side-by-side

Showing with 17 additions and 7 deletions

benchmark.h include/benchmark/benchmark.h +1 -1

sysinfo.cc src/sysinfo.cc +16 -6

No files found.
--- a/include/benchmark/benchmark.h
+++ b/include/benchmark/benchmark.h
@@ -1324,9 +1324,9 @@ struct CPUInfo {
  };

  int num_cpus;
+  Scaling scaling;
  double cycles_per_second;
  std::vector<CacheInfo> caches;
-  Scaling scaling;
  std::vector<double> load_avg;

  static const CPUInfo& Get();

--- a/src/sysinfo.cc
+++ b/src/sysinfo.cc
@@ -530,7 +530,11 @@ int GetNumCPUs() {
  BENCHMARK_UNREACHABLE();
 }

-double GetCPUCyclesPerSecond() {
+double GetCPUCyclesPerSecond(CPUInfo::Scaling scaling) {
+  // Currently, scaling is only used on linux path here,
+  // suppress diagnostics about it being unused on other paths.
+  (void)scaling;
+
 #if defined BENCHMARK_OS_LINUX || defined BENCHMARK_OS_CYGWIN
  long freq;

@@ -541,8 +545,15 @@ double GetCPUCyclesPerSecond() {
  // cannot always be relied upon. The same reasons apply to /proc/cpuinfo as
  // well.
  if (ReadFromFile("/sys/devices/system/cpu/cpu0/tsc_freq_khz", &freq)
-      // If CPU scaling is in effect, we want to use the *maximum* frequency,
-      // not whatever CPU speed some random processor happens to be using now.
+      // If CPU scaling is disabled, use the the *current* frequency.
+      // Note that we specifically don't want to read cpuinfo_cur_freq,
+      // because it is only readable by root.
+      || (scaling == CPUInfo::Scaling::DISABLED &&
+          ReadFromFile("/sys/devices/system/cpu/cpu0/cpufreq/scaling_cur_freq",
+                       &freq))
+      // Otherwise, if CPU scaling may be in effect, we want to use
+      // the *maximum* frequency, not whatever CPU speed some random processor
+      // happens to be using now.
      || ReadFromFile("/sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_max_freq",
                      &freq)) {
    // The value is in kHz (as the file name suggests).  For example, on a
@@ -701,12 +712,11 @@ const CPUInfo& CPUInfo::Get() {

 CPUInfo::CPUInfo()
    : num_cpus(GetNumCPUs()),
-      cycles_per_second(GetCPUCyclesPerSecond()),
-      caches(GetCacheSizes()),
      scaling(CpuScaling(num_cpus)),
+      cycles_per_second(GetCPUCyclesPerSecond(scaling)),
+      caches(GetCacheSizes()),
      load_avg(GetLoadAvg()) {}

-
 const SystemInfo& SystemInfo::Get() {
  static const SystemInfo* info = new SystemInfo();
  return *info;