OSDN Git Service

Second round of changes to 'perf' profile collection daemon.
authorThan McIntosh <thanm@google.com>
Fri, 17 Apr 2015 19:10:43 +0000 (15:10 -0400)
committerThan McIntosh <thanm@google.com>
Wed, 22 Apr 2015 20:21:24 +0000 (16:21 -0400)
Details:
- avoid use of system() in favor of fork/exec.
- add option to selectively disable/enable mpdecision service around perf
  collection runs to improve profile quality and avoid kernel pmuevents issues.
- default to using 'simpleperf' instead of 'perf'

Change-Id: I27928d8bb647fd852ec944158ebfd8efa38c01b4

perfprofd/Android.mk
perfprofd/cpuconfig.cc [new file with mode: 0644]
perfprofd/cpuconfig.h [new file with mode: 0644]
perfprofd/perfprofdcore.cc
perfprofd/tests/perfprofd_test.cc
perfprofd/tests/perfprofdmockutils.cc

index 4ae7031..3bacc81 100644 (file)
@@ -26,6 +26,7 @@ LOCAL_SRC_FILES :=  \
        quipper/perf_reader.cc \
        quipper/perf_parser.cc \
        perf_data_converter.cc \
+       cpuconfig.cc \
        perfprofdcore.cc \
 
 LOCAL_CPPFLAGS += $(perfprofd_cppflags)
diff --git a/perfprofd/cpuconfig.cc b/perfprofd/cpuconfig.cc
new file mode 100644 (file)
index 0000000..4b3cc36
--- /dev/null
@@ -0,0 +1,105 @@
+/*
+**
+** Copyright 2015, The Android Open Source Project
+**
+** Licensed under the Apache License, Version 2.0 (the "License");
+** you may not use this file except in compliance with the License.
+** You may obtain a copy of the License at
+**
+**     http://www.apache.org/licenses/LICENSE-2.0
+**
+** Unless required by applicable law or agreed to in writing, software
+** distributed under the License is distributed on an "AS IS" BASIS,
+** WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+** See the License for the specific language governing permissions and
+** limitations under the License.
+*/
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <string>
+#include <sstream>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+#include <cutils/properties.h>
+
+#include "cpuconfig.h"
+#include "perfprofdutils.h"
+
+#define SYSFSCPU "/sys/devices/system/cpu"
+
+HardwireCpuHelper::HardwireCpuHelper(bool perform)
+    : mpdecision_stopped_(false)
+{
+  if (perform && GetMpdecisionRunning()) {
+    mpdecision_stopped_ = true;
+    StopMpdecision();
+    int ncores = GetNumCores();
+    for (int i = 0; i < ncores; ++i) {
+      OnlineCore(i, 1);
+    }
+  }
+}
+
+HardwireCpuHelper::~HardwireCpuHelper()
+{
+  if (mpdecision_stopped_) {
+    RestartMpdecision();
+  }
+}
+
+bool HardwireCpuHelper::GetMpdecisionRunning()
+{
+  char propBuf[PROPERTY_VALUE_MAX];
+  property_get("init.svc.mpdecision", propBuf, "");
+  return strcmp(propBuf, "running") == 0;
+}
+
+
+int HardwireCpuHelper::GetNumCores()
+{
+  int ncores = -1;
+  std::string possible(SYSFSCPU "/possible");
+  FILE *fp = fopen(possible.c_str(), "re");
+  if (fp) {
+    unsigned lo = 0, hi = 0;
+    if (fscanf(fp, "%u-%u", &lo, &hi) == 2) {
+      ncores = hi - lo + 1;
+    }
+    fclose(fp);
+  }
+  return ncores;
+}
+
+void HardwireCpuHelper::OnlineCore(int i, int onoff)
+{
+  std::stringstream ss;
+  ss << SYSFSCPU "/cpu" << i << "/online";
+  FILE *fp = fopen(ss.str().c_str(), "we");
+  if (fp) {
+    fprintf(fp, onoff ? "1\n" : "0\n");
+    fclose(fp);
+  } else {
+    W_ALOGW("open failed for %s", ss.str().c_str());
+  }
+}
+
+void HardwireCpuHelper::StopMpdecision()
+{
+  if (property_set("ctl.stop", "mpdecision")) {
+    W_ALOGE("setprop ctl.stop mpdecision failed");
+  }
+}
+
+void HardwireCpuHelper::RestartMpdecision()
+{
+  // Don't try to offline the cores we previously onlined -- let
+  // mpdecision figure out what to do
+
+  if (property_set("ctl.start", "mpdecision")) {
+    W_ALOGE("setprop ctl.start mpdecision failed");
+  }
+}
diff --git a/perfprofd/cpuconfig.h b/perfprofd/cpuconfig.h
new file mode 100644 (file)
index 0000000..11a52f0
--- /dev/null
@@ -0,0 +1,50 @@
+/*
+**
+** Copyright 2015, The Android Open Source Project
+**
+** Licensed under the Apache License, Version 2.0 (the "License");
+** you may not use this file except in compliance with the License.
+** You may obtain a copy of the License at
+**
+**     http://www.apache.org/licenses/LICENSE-2.0
+**
+** Unless required by applicable law or agreed to in writing, software
+** distributed under the License is distributed on an "AS IS" BASIS,
+** WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+** See the License for the specific language governing permissions and
+** limitations under the License.
+*/
+
+//
+// Helper class to perform cpu setup (if needed) prior to a profile collection.
+//
+class HardwireCpuHelper {
+ public:
+
+  // The constructor for this class checks to see if the 'mpdecision'
+  // service is running; if so (and if 'perform' is TRUE), then it
+  // disables the service and on-lines all of the available cores/cpus
+  // (anything listed in /sys/devices/system/cpu/possible). The
+  // destructor will re-enable the mpdecision service if it was
+  // previously disabled.
+  HardwireCpuHelper(bool perform);
+  virtual ~HardwireCpuHelper();
+
+ private:
+  bool mpdecision_stopped_;
+
+  // Collect the number of available cpus/cores from /sys/devices/system/cpu/possible
+  int GetNumCores();
+
+  // Returns TRUE if the system service 'mpdecision' is running
+  bool GetMpdecisionRunning();
+
+  // Online/offline the specified cpu
+  void OnlineCore(int whichCore, int onoff);
+
+  // Enable/disable the mpdecision service via the equivalent of
+  //   setprop ctl.start mpdecision
+  //   setprop ctl.stop mpdecision
+  void StopMpdecision();
+  void RestartMpdecision();
+};
index 62cff18..797ba6f 100644 (file)
 #include <string.h>
 #include <sys/stat.h>
 #include <sys/types.h>
+#include <sys/wait.h>
 #include <time.h>
 #include <unistd.h>
 #include <string>
+#include <sstream>
 #include <map>
 #include <cctype>
 
 #include "perfprofdcore.h"
 #include "perfprofdutils.h"
 #include "perf_data_converter.h"
+#include "cpuconfig.h"
 
 //
 // Perf profiling daemon -- collects system-wide profiles using
 //
-//       /system/bin/perf record -a
+//       simpleperf record -a
 //
 // and encodes them so that they can be uploaded by a separate service.
 //
@@ -48,8 +51,7 @@
 //......................................................................
 
 //
-// Output file from 'perf record'. The linux 'perf' tool by default
-// creates a file with this name.
+// Output file from 'perf record'.
 //
 #define PERF_OUTPUT "perf.data"
 
@@ -119,8 +121,8 @@ class ConfigReader {
   ~ConfigReader();
 
   // Ask for the current setting of a config item
-  unsigned getUnsignedValue(const char *key);
-  std::string getStringValue(const char *key);
+  unsigned getUnsignedValue(const char *key) const;
+  std::string getStringValue(const char *key) const;
 
   // read the specified config file, applying any settings it contains
   void readFile(const char *configFilePath);
@@ -175,8 +177,8 @@ void ConfigReader::addDefaultEntries()
   addStringEntry("destination_directory",
                  "/data/data/com.google.android.gms/files");
 
-  // Path to 'perf' executable.
-  addStringEntry("perf_path", "/system/bin/perf");
+  // Full path to 'perf' executable.
+  addStringEntry("perf_path", "/system/bin/simpleperf");
 
   // Desired sampling period (passed to perf -c option). Small
   // sampling periods can perturb the collected profiles, so enforce
@@ -192,7 +194,15 @@ void ConfigReader::addDefaultEntries()
   // Currently defaults to 1 (true).
   addUnsignedEntry("only_debug_build", 1, 0, 1);
 
-  // If set to 1, pass the -g option when invoking perf (requests
+  // If the "mpdecision" service is running at the point we are ready
+  // to kick off a profiling run, then temporarily disable the service
+  // and hard-wire all cores on prior to the collection run, provided
+  // that the duration of the recording is less than or equal to the value of
+  // 'hardwire_cpus_max_duration'.
+  addUnsignedEntry("hardwire_cpus", 1, 0, 1);
+  addUnsignedEntry("hardwire_cpus_max_duration", 5, 1, UINT32_MAX);
+
+  // If set to 1, pass the -g option when invoking 'perf' (requests
   // stack traces as opposed to flat profile).
   addUnsignedEntry("stack_profile", 0, 0, 1);
 
@@ -227,14 +237,14 @@ void ConfigReader::addStringEntry(const char *key, const char *default_value)
     W_ALOGE("internal error -- duplicate entry for key %s", key);
     exit(9);
   }
-  if (! default_value) {
+  if (default_value == nullptr) {
     W_ALOGE("internal error -- bad default value for key %s", key);
     exit(9);
   }
   s_entries[ks] = std::string(default_value);
 }
 
-unsigned ConfigReader::getUnsignedValue(const char *key)
+unsigned ConfigReader::getUnsignedValue(const char *key) const
 {
   std::string ks(key);
   auto it = u_entries.find(ks);
@@ -242,7 +252,7 @@ unsigned ConfigReader::getUnsignedValue(const char *key)
   return it->second;
 }
 
-std::string ConfigReader::getStringValue(const char *key)
+std::string ConfigReader::getStringValue(const char *key) const
 {
   std::string ks(key);
   auto it = s_entries.find(ks);
@@ -265,7 +275,7 @@ void ConfigReader::parseLine(const char *key,
   auto uit = u_entries.find(key);
   if (uit != u_entries.end()) {
     unsigned uvalue = 0;
-    if (! isdigit(value[0]) || sscanf(value, "%u", &uvalue) != 1) {
+    if (isdigit(value[0]) == 0 || sscanf(value, "%u", &uvalue) != 1) {
       W_ALOGW("line %d: malformed unsigned value (ignored)", linecount);
     } else {
       values vals;
@@ -303,7 +313,7 @@ static bool isblank(const std::string &line)
 {
   for (std::string::const_iterator it = line.begin(); it != line.end(); ++it)
   {
-    if (! isspace(*it)) {
+    if (isspace(*it) == 0) {
       return false;
     }
   }
@@ -384,7 +394,7 @@ static void parse_args(int argc, char** argv)
 //
 const char *ckprofile_result_to_string(CKPROFILE_RESULT result)
 {
-  switch(result) {
+  switch (result) {
     case DO_COLLECT_PROFILE:
       return "DO_COLLECT_PROFILE";
     case DONT_PROFILE_MISSING_DESTINATION_DIR:
@@ -474,7 +484,7 @@ static CKPROFILE_RESULT check_profiling_enabled(ConfigReader &config)
   // Reread aux config file -- it may have changed
   read_aux_config(config);
 
-  // Check for existence of perf executable
+  // Check for existence of simpleperf/perf executable
   std::string pp = config.getStringValue("perf_path");
   if (access(pp.c_str(), R_OK|X_OK) == -1) {
     W_ALOGW("unable to access/execute %s", pp.c_str());
@@ -515,9 +525,6 @@ PROFILE_RESULT encode_to_proto(const std::string &data_file_path,
   const wireless_android_play_playlog::AndroidPerfProfile &encodedProfile =
       wireless_android_logging_awp::RawPerfDataToAndroidPerfProfile(data_file_path);
 
-  fprintf(stderr, "data file path is %s\n", data_file_path.c_str());
-  fprintf(stderr, "encoded file path is %s\n", encoded_file_path.c_str());
-
   //
   // Issue error if no samples
   //
@@ -525,7 +532,6 @@ PROFILE_RESULT encode_to_proto(const std::string &data_file_path,
     return ERR_PERF_ENCODE_FAILED;
   }
 
-
   //
   // Serialize protobuf to array
   //
@@ -554,6 +560,99 @@ PROFILE_RESULT encode_to_proto(const std::string &data_file_path,
 }
 
 //
+// Invoke "perf record". Return value is OK_PROFILE_COLLECTION for
+// success, or some other error code if something went wrong.
+//
+static PROFILE_RESULT invoke_perf(const std::string &perf_path,
+                                  unsigned sampling_period,
+                                  const char *stack_profile_opt,
+                                  unsigned duration,
+                                  const std::string &data_file_path,
+                                  const std::string &perf_stderr_path)
+{
+  pid_t pid = fork();
+
+  if (pid == -1) {
+    return ERR_FORK_FAILED;
+  }
+
+  if (pid == 0) {
+    // child
+
+    // Open file to receive stderr/stdout from perf
+    FILE *efp = fopen(perf_stderr_path.c_str(), "w");
+    if (efp) {
+      dup2(fileno(efp), STDERR_FILENO);
+      dup2(fileno(efp), STDOUT_FILENO);
+    } else {
+      W_ALOGW("unable to open %s for writing", perf_stderr_path.c_str());
+    }
+
+    // marshall arguments
+    constexpr unsigned max_args = 12;
+    const char *argv[max_args];
+    unsigned slot = 0;
+    argv[slot++] = perf_path.c_str();
+    argv[slot++] = "record";
+
+    // -o perf.data
+    argv[slot++] = "-o";
+    argv[slot++] = data_file_path.c_str();
+
+    // -c N
+    argv[slot++] = "-c";
+    char pbuf[64]; snprintf(pbuf, 64, "%u", sampling_period);
+    argv[slot++] = pbuf;
+
+    // -g if desired
+    if (stack_profile_opt)
+      argv[slot++] = stack_profile_opt;
+
+    // system wide profiling
+    argv[slot++] = "-a";
+
+    // sleep <duration>
+    argv[slot++] = "/system/bin/sleep";
+    char dbuf[64]; snprintf(dbuf, 64, "%u", duration);
+    argv[slot++] = dbuf;
+
+    // terminator
+    argv[slot++] = nullptr;
+    assert(slot < max_args);
+
+    // record the final command line in the error output file for
+    // posterity/debugging purposes
+    fprintf(stderr, "perf invocation (pid=%d):\n", getpid());
+    for (unsigned i = 0; argv[i] != nullptr; ++i) {
+      fprintf(stderr, "%s%s", i ? " " : "", argv[i]);
+    }
+    fprintf(stderr, "\n");
+
+    // exec
+    execvp(argv[0], (char * const *)argv);
+    fprintf(stderr, "exec failed: %s\n", strerror(errno));
+    exit(1);
+
+  } else {
+    // parent
+    int st = 0;
+    pid_t reaped = TEMP_FAILURE_RETRY(waitpid(pid, &st, 0));
+
+    if (reaped == -1) {
+      W_ALOGW("waitpid failed: %s", strerror(errno));
+    } else if (WIFSIGNALED(st)) {
+      W_ALOGW("perf killed by signal %d", WTERMSIG(st));
+    } else if (WEXITSTATUS(st) != 0) {
+      W_ALOGW("perf bad exit status %d", WEXITSTATUS(st));
+    } else {
+      return OK_PROFILE_COLLECTION;
+    }
+  }
+
+  return ERR_PERF_RECORD_FAILED;
+}
+
+//
 // Collect a perf profile. Steps for this operation are:
 // - kick off 'perf record'
 // - read perf.data, convert to protocol buf
@@ -582,28 +681,44 @@ static PROFILE_RESULT collect_profile(ConfigReader &config)
   }
 
   //
-  // NB: it would probably be better to use explicit fork/exec with an
-  // alarm timeout in case of funny business. For now, call system().
+  // The "mpdecision" daemon can cause problems for profile
+  // collection: if it decides to online a CPU partway through the
+  // 'perf record' run, the activity on that CPU will be invisible to
+  // perf, and if it offlines a CPU during the recording this can
+  // sometimes leave the PMU in an unusable state (dmesg errors of the
+  // form "perfevents: unable to request IRQXXX for ...").  To avoid
+  // these issues, if "mpdecision" is running the helper below will
+  // stop the service and then online all available CPUs. The object
+  // destructor (invoked when this routine terminates) will then
+  // restart the service again when needed.
   //
-  std::string perf_path = config.getStringValue("perf_path");
   unsigned duration = config.getUnsignedValue("sample_duration");
+  unsigned hardwire = config.getUnsignedValue("hardwire_cpus");
+  unsigned max_duration = config.getUnsignedValue("hardwire_cpus_max_duration");
+  bool take_action = (hardwire && duration <= max_duration);
+  HardwireCpuHelper helper(take_action);
+
+  //
+  // Invoke perf
+  //
+  const char *stack_profile_opt =
+      (config.getUnsignedValue("stack_profile") != 0 ? "-g" : nullptr);
+  std::string perf_path = config.getStringValue("perf_path");
   unsigned period = config.getUnsignedValue("sampling_period");
-  const char *gopt = (config.getUnsignedValue("stack_profile") != 0 ? "-g" : "");
-  char cmd[8192];
-  snprintf(cmd, 8192, "%s record %s -c %u -o %s -a -- sleep %d 1> %s 2>&1 ",
-           perf_path.c_str(), gopt, period, data_file_path.c_str(), duration,
-           perf_stderr_path.c_str());
-  int rc = system(cmd);
-  if (rc == -1) {
-    return ERR_FORK_FAILED;
-  }
-  if (rc != 0) {
-    return ERR_PERF_RECORD_FAILED;
+
+  PROFILE_RESULT ret = invoke_perf(perf_path.c_str(),
+                                  period,
+                                  stack_profile_opt,
+                                  duration,
+                                  data_file_path,
+                                  perf_stderr_path);
+  if (ret != OK_PROFILE_COLLECTION) {
+    return ret;
   }
 
   //
   // Read the resulting perf.data file, encode into protocol buffer, then write
-  // the result to a *.pdb file
+  // the result to the file perf.data.encoded
   //
   std::string encoded_file_path(data_file_path);
   encoded_file_path += ".encoded";
@@ -705,7 +820,7 @@ int perfprofd_main(int argc, char** argv)
   }
 
   unsigned iterations = 0;
-  while(!config.getUnsignedValue("main_loop_iterations") ||
+  while(config.getUnsignedValue("main_loop_iterations") == 0 ||
         iterations < config.getUnsignedValue("main_loop_iterations")) {
 
     // Figure out where in the collection interval we're going to actually
index 94c2814..7e51e0d 100644 (file)
@@ -65,6 +65,7 @@ class PerfProfdTest : public testing::Test {
   virtual void SetUp() {
     mock_perfprofdutils_init();
     create_dest_dir();
+    yesclean();
   }
 
   virtual void TearDown() {
@@ -72,11 +73,24 @@ class PerfProfdTest : public testing::Test {
     remove_dest_dir();
   }
 
+  void noclean() {
+    clean_ = false;
+  }
+  void yesclean() {
+    clean_ = true;
+  }
+
  private:
+  bool clean_;
 
   void create_dest_dir() {
     setup_dirs();
     ASSERT_FALSE(dest_dir == "");
+    if (clean_) {
+      std::string cmd("rm -rf ");
+      cmd += dest_dir;
+      system(cmd.c_str());
+    }
     std::string cmd("mkdir -p ");
     cmd += dest_dir;
     system(cmd.c_str());
@@ -85,9 +99,6 @@ class PerfProfdTest : public testing::Test {
   void remove_dest_dir() {
     setup_dirs();
     ASSERT_FALSE(dest_dir == "");
-    std::string cmd("rm -rf ");
-    cmd += dest_dir;
-    system(cmd.c_str());
   }
 
   void setup_dirs()
@@ -182,6 +193,7 @@ class PerfProfdRunner {
       writeConfigFile(aux_config_path_, aux_config_text_);
     }
 
+
     // execute daemon main
     return perfprofd_main(3, (char **) argv);
   }
@@ -348,9 +360,10 @@ TEST_F(PerfProfdTest, MissingOptInSemaphoreFile)
 TEST_F(PerfProfdTest, MissingPerfExecutable)
 {
   //
-  // AWP currently relies on the 'perf' tool to collect profiles (although
-  // this may conceivably change in the future). This test checks to make
-  // sure that if 'perf' is not present we bail out from collecting profiles.
+  // Perfprofd uses the 'simpleperf' tool to collect profiles
+  // (although this may conceivably change in the future). This test
+  // checks to make sure that if 'simpleperf' is not present we bail out
+  // from collecting profiles.
   //
   PerfProfdRunner runner;
   runner.addToConfig("only_debug_build=0");
@@ -383,10 +396,10 @@ TEST_F(PerfProfdTest, MissingPerfExecutable)
 TEST_F(PerfProfdTest, BadPerfRun)
 {
   //
-  // The linux 'perf' tool tends to be tightly coupled with a
-  // specific kernel version -- if things are out of sync perf could
-  // easily fail or crash. This test makes sure that we detect such a
-  // case and log the error.
+  // Perf tools tend to be tightly coupled with a specific kernel
+  // version -- if things are out of sync perf could fail or
+  // crash. This test makes sure that we detect such a case and log
+  // the error.
   //
   PerfProfdRunner runner;
   runner.addToConfig("only_debug_build=0");
@@ -593,7 +606,6 @@ TEST_F(PerfProfdTest, BasicRunWithLivePerf)
   runner.addToConfig("main_loop_iterations=1");
   runner.addToConfig("use_fixed_seed=12345678");
   runner.addToConfig("collection_interval=9999");
-  runner.addToConfig("stack_profile=1");
   runner.addToConfig("sample_duration=5");
 
   // Create semaphore file
index cc4ea94..5af58c4 100644 (file)
@@ -54,6 +54,7 @@ std::string mock_perfprofdutils_getlogged()
   for (const std::string &s : (*mock_log)) {
     result += s;
   }
+  mock_log->clear();
   return result;
 }