Fix bug in --bionic_cpu option handling.

Make sure that all the variables are properly initialized.

Remove the code that verifies the core to enable using get_schedaffinity
since that make it impossible to change the cpu for different tests.

Change the cpu_to_lock to an int, it really didn't need to be a long.

Fix a few missing tests.

Test: Ran unit tests.
Test: Built the tests and ran on different cpus, verifying that the
Test: chosen cpu was correct.
Test: Created an xml file that had different cpus for different tests
Test: and verified that it locked to each cpu properly.
Change-Id: Ie7b4ad8f306f13d6e968d118e71bb5dc0221552a

benchmarks/bionic_benchmarks.cpp

@@ -139,9 +139,6 @@ bench_opts_t ParseOpts(int argc, char** argv) {
   int opt;
   int option_index = 0;
 
-  opts.cpu_to_lock = LONG_MAX;
-  opts.num_iterations = 0;
-
   // To make this parser handle the benchmark options silently:
   extern int opterr;
   opterr = 0;
@@ -204,8 +201,9 @@ bench_opts_t ParseOpts(int argc, char** argv) {
 }
 
 // This is a wrapper for every function call for per-benchmark cpu pinning.
-void LockAndRun(benchmark::State& state, benchmark_func_t func_to_bench, long cpu_to_lock) {
-  if (cpu_to_lock != LONG_MAX) LockToCPU(cpu_to_lock);
+void LockAndRun(benchmark::State& state, benchmark_func_t func_to_bench, int cpu_to_lock) {
+  if (cpu_to_lock >= 0) LockToCPU(cpu_to_lock);
+
   // To avoid having to link against Google benchmarks in libutil,
   // benchmarks are kept without parameter information, necessitating this cast.
   reinterpret_cast<void(*) (benchmark::State&)>(func_to_bench)(state);
@@ -311,11 +309,11 @@ void RegisterGoogleBenchmarks(bench_opts_t primary_opts, bench_opts_t secondary_
   }
   long iterations_to_use = primary_opts.num_iterations ? primary_opts.num_iterations :
                                                          secondary_opts.num_iterations;
-  int cpu_to_use = INT_MAX;
-  if (primary_opts.cpu_to_lock != INT_MAX) {
+  int cpu_to_use = -1;
+  if (primary_opts.cpu_to_lock >= 0) {
     cpu_to_use = primary_opts.cpu_to_lock;
 
-  } else if (secondary_opts.cpu_to_lock != INT_MAX) {
+  } else if (secondary_opts.cpu_to_lock >= 0) {
     cpu_to_use = secondary_opts.cpu_to_lock;
   }
 
@@ -398,16 +396,12 @@ int RegisterXmlBenchmarks(bench_opts_t cmdline_opts,
       int temp;
       num_iterations_elem->QueryIntText(&temp);
       xml_opts.num_iterations = temp;
-    } else {
-      xml_opts.num_iterations = 0;
     }
     auto* cpu_to_lock_elem = fn->FirstChildElement("cpu");
     if (cpu_to_lock_elem) {
       int temp;
       cpu_to_lock_elem->QueryIntText(&temp);
       xml_opts.cpu_to_lock = temp;
-    } else {
-      xml_opts.cpu_to_lock = INT_MAX;
     }
 
     RegisterGoogleBenchmarks(xml_opts, cmdline_opts, fn_name, run_args);

benchmarks/tests/interface_test.cpp

@@ -272,7 +272,10 @@ TEST_F(SystemTests, all_benchmarks) {
     "BM_pthread_key_delete/iterations:1\n"
     "BM_pthread_mutex_lock/iterations:1\n"
     "BM_pthread_mutex_lock_ERRORCHECK/iterations:1\n"
+    "BM_pthread_mutex_lock_ERRORCHECK_PI/iterations:1\n"
+    "BM_pthread_mutex_lock_PI/iterations:1\n"
     "BM_pthread_mutex_lock_RECURSIVE/iterations:1\n"
+    "BM_pthread_mutex_lock_RECURSIVE_PI/iterations:1\n"
     "BM_pthread_once/iterations:1\n"
     "BM_pthread_rwlock_read/iterations:1\n"
     "BM_pthread_rwlock_write/iterations:1\n"

benchmarks/util.cpp

@@ -71,36 +71,17 @@ bool LockToCPU(int) {
 
 #else
 
-bool LockToCPU(long cpu_to_lock) {
+bool LockToCPU(int cpu_to_lock) {
   cpu_set_t cpuset;
 
-  CPU_ZERO(&cpuset);
-  if (sched_getaffinity(0, sizeof(cpuset), &cpuset) != 0) {
-    perror("sched_getaffinity failed");
-    return false;
-  }
-
-  if (cpu_to_lock < 0) {
-    // Lock to the last active core we find.
-    for (int i = 0; i < CPU_SETSIZE; i++) {
-      if (CPU_ISSET(i, &cpuset)) {
-        cpu_to_lock = i;
-      }
-    }
-  } else if (!CPU_ISSET(cpu_to_lock, &cpuset)) {
-    printf("Cpu %ld does not exist.\n", cpu_to_lock);
-    return false;
-  }
-
-  if (cpu_to_lock < 0) {
-    printf("Cannot find any valid cpu to lock.\n");
-    return false;
-  }
-
   CPU_ZERO(&cpuset);
   CPU_SET(cpu_to_lock, &cpuset);
   if (sched_setaffinity(0, sizeof(cpuset), &cpuset) != 0) {
-    perror("sched_setaffinity failed");
+    if (errno == EINVAL) {
+      printf("Invalid cpu %d\n", cpu_to_lock);
+    } else {
+      perror("sched_setaffinity failed");
+    }
     return false;
   }
 

benchmarks/util.h

@@ -45,8 +45,8 @@ static int  __attribute__((unused)) EmplaceBenchmark(const std::string& fn_name,
 constexpr auto KB = 1024;
 
 typedef struct {
-  long cpu_to_lock;
-  long num_iterations;
+  int cpu_to_lock = -1;
+  long num_iterations = 0;
   std::string xmlpath;
   std::vector<std::string> extra_benchmarks;
 } bench_opts_t;
@@ -60,4 +60,4 @@ wchar_t* GetAlignedPtr(std::vector<wchar_t>* buf, size_t alignment, size_t nbyte
 
 char* GetAlignedPtrFilled(std::vector<char>* buf, size_t alignment, size_t nbytes, char fill_byte);
 
-bool LockToCPU(long cpu_to_lock);
+bool LockToCPU(int cpu_to_lock);