From d0d7695ecbfd12aaecc8aec66aacb487b116ac0b Mon Sep 17 00:00:00 2001 From: Andreas Gampe Date: Tue, 22 Aug 2017 13:08:37 -0700 Subject: Dumpstate: Add tombstone filtering Only package the ten latest tombstones. This recovers the old behavior when tombstones were limited to ten by tombstoned, and ensures that bugreports stay small in size. It is future work to optimize this, e.g., by packaging as many as possible. Bug: 64290162 Test: m Test: adb root && for ((i=0;i<50;i++)) ; do adb shell touch /data/tombstones/tombstone_$i ; done ; adb bugreport test.zip ; unzip -l test.zip | grep tomb Change-Id: I4072b5fbcf1e0314aa3eebeefbadc61d5ec10787 --- cmds/dumpstate/dumpstate.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 9e77e8fbcb..b3d628cac5 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -144,10 +144,12 @@ static const CommandOptions AS_ROOT_20 = CommandOptions::WithTimeout(20).AsRoot( * Returns a vector of dump fds under |dir_path| with a given |file_prefix|. * The returned vector is sorted by the mtimes of the dumps. If |limit_by_mtime| * is set, the vector only contains files that were written in the last 30 minutes. + * If |limit_by_count| is set, the vector only contains the ten latest files. */ static std::vector* GetDumpFds(const std::string& dir_path, const std::string& file_prefix, - bool limit_by_mtime) { + bool limit_by_mtime, + bool limit_by_count = true) { const time_t thirty_minutes_ago = ds.now_ - 60 * 30; std::unique_ptr> dump_data(new std::vector()); @@ -190,6 +192,10 @@ static std::vector* GetDumpFds(const std::string& dir_path, std::sort(dump_data->begin(), dump_data->end()); + if (limit_by_count && dump_data->size() > 10) { + dump_data->erase(dump_data->begin() + 10, dump_data->end()); + } + return dump_data.release(); } -- cgit v1.2.3-59-g8ed1b From 3cd671e868a3648a4ca00c3e0b063e983d34b702 Mon Sep 17 00:00:00 2001 From: Jin Qian Date: Wed, 27 Sep 2017 18:50:04 -0700 Subject: dumpstate: dump iotop to show io threads Also fix title for storaged -u. Test: adb bugreport Bug: 63629306 Change-Id: I31a6225c19cd86bfe1c55d71c2ef3f02802c3b93 --- cmds/dumpstate/dumpstate.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index b3d628cac5..c41edf5518 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1135,7 +1135,7 @@ static void dumpstate() { RunCommand("VOLD DUMP", {"vdc", "dump"}); RunCommand("SECURE CONTAINERS", {"vdc", "asec", "list"}); - RunCommand("STORAGED TASKIOINFO", {"storaged", "-u"}, CommandOptions::WithTimeout(10).Build()); + RunCommand("STORAGED UID IO INFO", {"storaged", "-u"}); RunCommand("FILESYSTEMS & FREE SPACE", {"df"}); @@ -1850,6 +1850,9 @@ int main(int argc, char *argv[]) { RunCommand("DETAILED SOCKET STATE", {"ss", "-eionptu"}, CommandOptions::WithTimeout(10).Build()); + // Run iotop as root to show top 100 IO threads + RunCommand("IOTOP", {"iotop", "-n", "1", "-m", "100"}); + if (!DropRootUser()) { return -1; } -- cgit v1.2.3-59-g8ed1b From 591a72da3ec97e986f9149d0b0d851bf27f6665d Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Thu, 5 Oct 2017 21:36:35 -0700 Subject: dumpstate: dump CPU time consumed for each thread This should be useful to diagnose battery drain bugs due to accidental semi-infinite busy-loop in processes that merely get killed by the system (e.g. IMEs). Bug: 67330309 Fixes: 67482875 Test: Manually verified by checking 'adb bugreport report.zip' Change-Id: I1fedaf2518dd65dac105b29a74d50b51eb633cd7 --- cmds/dumpstate/dumpstate.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index b6b34fbaa9..5b0c1550d2 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1062,7 +1062,7 @@ static void dumpstate() { DumpFile("KERNEL SYNC", "/d/sync"); RunCommand("PROCESSES AND THREADS", - {"ps", "-A", "-T", "-Z", "-O", "pri,nice,rtprio,sched,pcy"}); + {"ps", "-A", "-T", "-Z", "-O", "pri,nice,rtprio,sched,pcy,time"}); RunCommand("LIBRANK", {"librank"}, CommandOptions::AS_ROOT); if (ds.IsZipping()) { -- cgit v1.2.3-59-g8ed1b From 780b1283e6d219e1ef3cb061f5096dcb359d88ca Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Tue, 10 Oct 2017 13:57:24 -0700 Subject: Add priority based dumpsys support to dumpstate Adds a new version of dumpstate which calls dumpsys with different priorities. Current version will remain INITIAL_VERSION (1.0) until tools support the new version and services register with supported priorities. Modified dumpsys to pass prioirty args to services. BugReport format changes: - removed service specific dumps and dump order changed - Start of dumpsys section changed to DUMPSYS CRITICAL/HIGH/NORMAL - Start of service dump changed to DUMP OF SERVICE CRITICAL/HIGH/NORMAL Bug: 27429130 Test: adb shell setprop dumpstate.version "1.0" && \ adb bugreport ~/tmp_old.zip Test: adb shell setprop dumpstate.version "2.0-dev-split-anr" && \ adb bugreport ~/tmp_anr.zip Test: adb shell setprop dumpstate.version "2.0-dev-priority-dumps" && \ adb bugreport ~/tmp_new.zip Change-Id: I9fd0f2d0e6d73b36cc8ee0f8239092ce83da9560 --- cmds/dumpstate/bugreport-format.md | 16 +++++++++-- cmds/dumpstate/dumpstate.cpp | 55 +++++++++++++++++++++++++++++--------- cmds/dumpstate/dumpstate.h | 13 +++++++-- cmds/dumpstate/utils.cpp | 4 +++ cmds/dumpsys/Android.bp | 4 +++ cmds/dumpsys/dumpsys.cpp | 40 ++++++++++++++++++++------- 6 files changed, 107 insertions(+), 25 deletions(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/bugreport-format.md b/cmds/dumpstate/bugreport-format.md index b995b808c1..dee89cf174 100644 --- a/cmds/dumpstate/bugreport-format.md +++ b/cmds/dumpstate/bugreport-format.md @@ -56,8 +56,20 @@ files upon the end user’s request: - `description.txt`: whose value is a multi-line, detailed description of the problem. ## Android O versions -On _Android O (OhMightyAndroidWhatsYourNextReleaseName?)_, the following changes were made: -- The ANR traces are added to the `FS` folder, typically under `FS/data/anr` (version `2.0-dev-1`). +On _Android O (Oreo)_, the following changes were made: +- The ANR traces are added to the `FS` folder, typically under `FS/data/anr` (version `2.0-dev-split-anr`). + +## Android P versions +On _Android P (PleaseMightyAndroidWhatsYourNextReleaseName?)_, the following changes were made: +- Dumpsys sections are dumped by priority (version `2.0-dev-priority-dumps`). + Supported priorities can be specified when registering framework services. Section headers are + changed to contain priority info. + `DUMPSYS` -> `DUMPSYS CRITICAL/HIGH/NORMAL` + `DUMP OF SERVICE ` -> `DUMP OF SERVICE CRITICAL/HIGH/NORMAL ` + Supported Priorities: + - CRITICAL - services that must dump first, and fast (under 100ms). Ex: cpuinfo. + - HIGH - services that also must dump first, but can take longer (under 250ms) to dump. Ex: meminfo. + - NORMAL - services that have no rush to dump and can take a long time (under 10s). ## Intermediate versions During development, the versions will be suffixed with _-devX_ or diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 5b0c1550d2..335d25cfb5 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1038,6 +1038,40 @@ static void DumpIpAddrAndRules() { RunCommand("IP RULES v6", {"ip", "-6", "rule", "show"}); } +// Runs dumpsys on services that must dump first and and will take less than 100ms to dump. +static void RunDumpsysCritical() { + if (ds.CurrentVersionSupportsPriorityDumps()) { + RunDumpsys("DUMPSYS CRITICAL", {"--priority", "CRITICAL"}, + CommandOptions::WithTimeout(5).DropRoot().Build()); + } else { + RunDumpsys("DUMPSYS MEMINFO", {"meminfo", "-a"}, + CommandOptions::WithTimeout(90).DropRoot().Build()); + RunDumpsys("DUMPSYS CPUINFO", {"cpuinfo", "-a"}, + CommandOptions::WithTimeout(10).DropRoot().Build()); + } +} + +// Runs dumpsys on services that must dump first but can take up to 250ms to dump. +static void RunDumpsysHigh() { + if (ds.CurrentVersionSupportsPriorityDumps()) { + RunDumpsys("DUMPSYS HIGH", {"--priority", "HIGH"}, + CommandOptions::WithTimeout(20).DropRoot().Build()); + } else { + RunDumpsys("NETWORK DIAGNOSTICS", {"connectivity", "--diag"}); + } +} + +// Runs dumpsys on services that must dump but can take up to 10s to dump. +static void RunDumpsysNormal() { + if (ds.CurrentVersionSupportsPriorityDumps()) { + RunDumpsys("DUMPSYS NORMAL", {"--priority", "NORMAL"}, + CommandOptions::WithTimeout(90).DropRoot().Build()); + } else { + RunDumpsys("DUMPSYS", {"--skip", "meminfo", "cpuinfo"}, + CommandOptions::WithTimeout(90).Build(), 10); + } +} + static void dumpstate() { DurationReporter duration_reporter("DUMPSTATE"); @@ -1128,8 +1162,7 @@ static void dumpstate() { RunCommand("IPv6 ND CACHE", {"ip", "-6", "neigh", "show"}); RunCommand("MULTICAST ADDRESSES", {"ip", "maddr"}); - RunDumpsys("NETWORK DIAGNOSTICS", {"connectivity", "--diag"}, - CommandOptions::WithTimeout(10).Build()); + RunDumpsysHigh(); RunCommand("SYSTEM PROPERTIES", {"getprop"}); @@ -1182,8 +1215,7 @@ static void dumpstate() { printf("== Android Framework Services\n"); printf("========================================================\n"); - RunDumpsys("DUMPSYS", {"--skip", "meminfo", "cpuinfo"}, CommandOptions::WithTimeout(90).Build(), - 10); + RunDumpsysNormal(); printf("========================================================\n"); printf("== Checkins\n"); @@ -1624,10 +1656,12 @@ int main(int argc, char *argv[]) { ds.version_ = VERSION_CURRENT; } - if (ds.version_ != VERSION_CURRENT && ds.version_ != VERSION_SPLIT_ANR) { - MYLOGE("invalid version requested ('%s'); suppported values are: ('%s', '%s', '%s')\n", - ds.version_.c_str(), VERSION_DEFAULT.c_str(), VERSION_CURRENT.c_str(), - VERSION_SPLIT_ANR.c_str()); + if (ds.version_ != VERSION_CURRENT && ds.version_ != VERSION_SPLIT_ANR && + ds.version_ != VERSION_PRIORITY_DUMPS) { + MYLOGE( + "invalid version requested ('%s'); suppported values are: ('%s', '%s', '%s', '%s')\n", + ds.version_.c_str(), VERSION_DEFAULT.c_str(), VERSION_CURRENT.c_str(), + VERSION_SPLIT_ANR.c_str(), VERSION_PRIORITY_DUMPS.c_str()); exit(1); } @@ -1818,10 +1852,7 @@ int main(int argc, char *argv[]) { // Invoking the following dumpsys calls before dump_traces() to try and // keep the system stats as close to its initial state as possible. - RunDumpsys("DUMPSYS MEMINFO", {"meminfo", "-a"}, - CommandOptions::WithTimeout(90).DropRoot().Build()); - RunDumpsys("DUMPSYS CPUINFO", {"cpuinfo", "-a"}, - CommandOptions::WithTimeout(10).DropRoot().Build()); + RunDumpsysCritical(); // TODO: Drop root user and move into dumpstate() once b/28633932 is fixed. dump_raft(); diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h index 7757c1e445..1bfafbaffe 100644 --- a/cmds/dumpstate/dumpstate.h +++ b/cmds/dumpstate/dumpstate.h @@ -149,9 +149,15 @@ static std::string VERSION_CURRENT = "1.0"; /* * Temporary version that adds a anr-traces.txt entry. Once tools support it, the current version - * will be bumped to 2.0-dev-1. + * will be bumped to 2.0. */ -static std::string VERSION_SPLIT_ANR = "2.0-dev-1"; +static std::string VERSION_SPLIT_ANR = "2.0-dev-split-anr"; + +/* + * Temporary version that adds priority based dumps. Once tools support it, the current version + * will be bumped to 2.0. + */ +static std::string VERSION_PRIORITY_DUMPS = "2.0-dev-priority-dumps"; /* * "Alias" for the current version. @@ -266,6 +272,9 @@ class Dumpstate { /* Gets the path of a bugreport file with the given suffix. */ std::string GetPath(const std::string& suffix) const; + /* Returns true if the current version supports priority dump feature. */ + bool CurrentVersionSupportsPriorityDumps() const; + // TODO: initialize fields on constructor // dumpstate id - unique after each device reboot. diff --git a/cmds/dumpstate/utils.cpp b/cmds/dumpstate/utils.cpp index 93f4c22e43..c2c9071812 100644 --- a/cmds/dumpstate/utils.cpp +++ b/cmds/dumpstate/utils.cpp @@ -257,6 +257,10 @@ std::string Dumpstate::GetPath(const std::string& suffix) const { name_.c_str(), suffix.c_str()); } +bool Dumpstate::CurrentVersionSupportsPriorityDumps() const { + return (version_ == VERSION_PRIORITY_DUMPS); +} + void Dumpstate::SetProgress(std::unique_ptr progress) { progress_ = std::move(progress); } diff --git a/cmds/dumpsys/Android.bp b/cmds/dumpsys/Android.bp index 34769644d5..f68b862f24 100644 --- a/cmds/dumpsys/Android.bp +++ b/cmds/dumpsys/Android.bp @@ -17,6 +17,10 @@ cc_defaults { "libbinder", ], + static_libs: [ + "libserviceutils", + ], + clang: true, } diff --git a/cmds/dumpsys/dumpsys.cpp b/cmds/dumpsys/dumpsys.cpp index 8fe246b609..239a2d5e69 100644 --- a/cmds/dumpsys/dumpsys.cpp +++ b/cmds/dumpsys/dumpsys.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include @@ -76,9 +77,26 @@ static bool IsSkipped(const Vector& skipped, const String16& service) return false; } +static bool ConvertPriorityTypeToBitmask(String16& type, int& bitmask) { + if (type == PRIORITY_ARG_CRITICAL) { + bitmask = IServiceManager::DUMP_PRIORITY_CRITICAL; + return true; + } + if (type == PRIORITY_ARG_HIGH) { + bitmask = IServiceManager::DUMP_PRIORITY_HIGH; + return true; + } + if (type == PRIORITY_ARG_NORMAL) { + bitmask = IServiceManager::DUMP_PRIORITY_NORMAL; + return true; + } + return false; +} + int Dumpsys::main(int argc, char* const argv[]) { Vector services; Vector args; + String16 priorityType; Vector skippedServices; bool showListOnly = false; bool skipServices = false; @@ -110,13 +128,8 @@ int Dumpsys::main(int argc, char* const argv[]) { usage(); return 0; } else if (!strcmp(longOptions[optionIndex].name, "priority")) { - if (!strcmp(optarg, "CRITICAL")) { - dumpPriority = IServiceManager::DUMP_PRIORITY_CRITICAL; - } else if (!strcmp(optarg, "HIGH")) { - dumpPriority = IServiceManager::DUMP_PRIORITY_HIGH; - } else if (!strcmp(optarg, "NORMAL")) { - dumpPriority = IServiceManager::DUMP_PRIORITY_NORMAL; - } else { + priorityType = String16(String8(optarg)); + if (!ConvertPriorityTypeToBitmask(priorityType, dumpPriority)) { fprintf(stderr, "\n"); usage(); return -1; @@ -168,7 +181,12 @@ int Dumpsys::main(int argc, char* const argv[]) { // gets all services services = sm_->listServices(dumpPriority); services.sort(sort_func); - args.add(String16("-a")); + if (dumpPriority != IServiceManager::DUMP_PRIORITY_ALL) { + args.insertAt(String16(PRIORITY_ARG), 0); + args.insertAt(priorityType, 1); + } else { + args.add(String16("-a")); + } } const size_t N = services.size(); @@ -212,7 +230,11 @@ int Dumpsys::main(int argc, char* const argv[]) { if (N > 1) { aout << "------------------------------------------------------------" "-------------------" << endl; - aout << "DUMP OF SERVICE " << service_name << ":" << endl; + if (dumpPriority == IServiceManager::DUMP_PRIORITY_ALL) { + aout << "DUMP OF SERVICE " << service_name << ":" << endl; + } else { + aout << "DUMP OF SERVICE " << priorityType << " " << service_name << ":" << endl; + } } // dump blocks until completion, so spawn a thread.. -- cgit v1.2.3-59-g8ed1b From f334d669dcd9ce322dd5a997c3b5a392b9152d3b Mon Sep 17 00:00:00 2001 From: Jin Qian Date: Tue, 10 Oct 2017 14:41:37 -0700 Subject: dumpstate: dump IO perf history from storaged Test: adb bugreport Bug: 63629306 Change-Id: I3126ce30e263bcb6e7760aa913b3461954a3719d --- cmds/dumpstate/dumpstate.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 5b0c1550d2..d3c21c9014 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1136,7 +1136,7 @@ static void dumpstate() { RunCommand("VOLD DUMP", {"vdc", "dump"}); RunCommand("SECURE CONTAINERS", {"vdc", "asec", "list"}); - RunCommand("STORAGED UID IO INFO", {"storaged", "-u"}); + RunCommand("STORAGED IO INFO", {"storaged", "-u", "-p"}); RunCommand("FILESYSTEMS & FREE SPACE", {"df"}); -- cgit v1.2.3-59-g8ed1b From 46edde3cd8d4bb4eb3765ba5760591127f2e57fd Mon Sep 17 00:00:00 2001 From: Alain Vongsouvanh Date: Wed, 25 Oct 2017 16:16:24 -0700 Subject: dumpstate: zip bugreport when bugreportwear is in the extra_options. BUG: 67464962 Change-Id: Ic95c558a425c2ad64b33b519cd6b0b21b0fd15e7 (cherry picked from commit 50a79132548ff57be5f0e1e03e12e507fe207314) --- cmds/dumpstate/dumpstate.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index b1a03fa0f0..0d3cb329e4 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1613,6 +1613,7 @@ int main(int argc, char *argv[]) { do_fb = 0; } else if (ds.extra_options_ == "bugreportwear") { ds.update_progress_ = true; + do_zip_file = 1; } else if (ds.extra_options_ == "bugreporttelephony") { telephony_only = true; } else { -- cgit v1.2.3-59-g8ed1b From 6b9516ced325e93880e9c8ef2793380866a1c579 Mon Sep 17 00:00:00 2001 From: Narayan Kamath Date: Fri, 27 Oct 2017 11:15:51 +0100 Subject: dumpstate: always include last ANR as a separate entry. For consistency, as well as to make tooling easier. Most of this change is an extended yak-shave to try and close DumpData FDs in a consistent manner. Several layers of abstraction were assuming that it was their job to close input FDs, leading to double closes in some cases. These double closes are benign prior to this change to dump a given DumpData twice, given that dumpstate is single threaded. Bug: 68188758 Test: adb bugreport Change-Id: I6302f4cb553139c0c26defb4859c4eca5b18ec93 --- cmds/dumpstate/DumpstateInternal.cpp | 1 - cmds/dumpstate/DumpstateUtil.cpp | 7 +++--- cmds/dumpstate/dumpstate.cpp | 45 +++++++++++++++++++++++++----------- cmds/dumpstate/utils.cpp | 12 ++++------ 4 files changed, 41 insertions(+), 24 deletions(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/DumpstateInternal.cpp b/cmds/dumpstate/DumpstateInternal.cpp index 5149b7fc4f..83e30a22ff 100644 --- a/cmds/dumpstate/DumpstateInternal.cpp +++ b/cmds/dumpstate/DumpstateInternal.cpp @@ -176,7 +176,6 @@ int DumpFileFromFdToFd(const std::string& title, const std::string& path_string, } } } - close(fd); if (!newline) dprintf(out_fd, "\n"); if (!title.empty()) dprintf(out_fd, "\n"); diff --git a/cmds/dumpstate/DumpstateUtil.cpp b/cmds/dumpstate/DumpstateUtil.cpp index ea7109bacd..ede4254a9b 100644 --- a/cmds/dumpstate/DumpstateUtil.cpp +++ b/cmds/dumpstate/DumpstateUtil.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include "DumpstateInternal.h" @@ -182,8 +183,8 @@ bool PropertiesHelper::IsDryRun() { } int DumpFileToFd(int out_fd, const std::string& title, const std::string& path) { - int fd = TEMP_FAILURE_RETRY(open(path.c_str(), O_RDONLY | O_NONBLOCK | O_CLOEXEC)); - if (fd < 0) { + android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(path.c_str(), O_RDONLY | O_NONBLOCK | O_CLOEXEC))); + if (fd.get() < 0) { int err = errno; if (title.empty()) { dprintf(out_fd, "*** Error dumping %s: %s\n", path.c_str(), strerror(err)); @@ -194,7 +195,7 @@ int DumpFileToFd(int out_fd, const std::string& title, const std::string& path) fsync(out_fd); return -1; } - return DumpFileFromFdToFd(title, path, fd, out_fd, PropertiesHelper::IsDryRun()); + return DumpFileFromFdToFd(title, path, fd.get(), out_fd, PropertiesHelper::IsDryRun()); } int RunCommandToFd(int fd, const std::string& title, const std::vector& full_command, diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index b1a03fa0f0..66536035d3 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -207,20 +207,36 @@ static bool AddDumps(const std::vector::const_iterator start, const std::string& name = it->name; const int fd = it->fd; dumped = true; + + // Seek to the beginning of the file before dumping any data. A given + // DumpData entry might be dumped multiple times in the report. + // + // For example, the most recent ANR entry is dumped to the body of the + // main entry and it also shows up as a separate entry in the bugreport + // ZIP file. + if (lseek(fd, 0, SEEK_SET) != static_cast(0)) { + MYLOGE("Unable to add %s to zip file, lseek failed: %s\n", name.c_str(), + strerror(errno)); + } + if (ds.IsZipping() && add_to_zip) { if (!ds.AddZipEntryFromFd(ZIP_ROOT_DIR + name, fd)) { - MYLOGE("Unable to add %s %s to zip file\n", name.c_str(), type_name); + MYLOGE("Unable to add %s to zip file, addZipEntryFromFd failed\n", name.c_str()); } } else { dump_file_from_fd(type_name, name.c_str(), fd); } - - close(fd); } return dumped; } +static void CloseDumpFds(const std::vector* dumps) { + for (auto it = dumps->begin(); it != dumps->end(); ++it) { + close(it->fd); + } +} + // for_each_pid() callback to get mount info about a process. void do_mountinfo(int pid, const char* name __attribute__((unused))) { char path[PATH_MAX]; @@ -887,9 +903,9 @@ static void AddGlobalAnrTraceFile(const bool add_to_zip, const std::string& anr_ MYLOGD("AddGlobalAnrTraceFile(): dump_traces_dir=%s, anr_traces_dir=%s, already_dumped=%d\n", dump_traces_dir.c_str(), anr_traces_dir.c_str(), already_dumped); - int fd = TEMP_FAILURE_RETRY( - open(anr_traces_file.c_str(), O_RDONLY | O_CLOEXEC | O_NOFOLLOW | O_NONBLOCK)); - if (fd < 0) { + android::base::unique_fd fd(TEMP_FAILURE_RETRY( + open(anr_traces_file.c_str(), O_RDONLY | O_CLOEXEC | O_NOFOLLOW | O_NONBLOCK))); + if (fd.get() < 0) { printf("*** NO ANR VM TRACES FILE (%s): %s\n\n", anr_traces_file.c_str(), strerror(errno)); } else { if (add_to_zip) { @@ -901,7 +917,7 @@ static void AddGlobalAnrTraceFile(const bool add_to_zip, const std::string& anr_ } else { MYLOGD("Dumping last ANR traces (%s) to the main bugreport entry\n", anr_traces_file.c_str()); - dump_file_from_fd("VM TRACES AT LAST ANR", anr_traces_file.c_str(), fd); + dump_file_from_fd("VM TRACES AT LAST ANR", anr_traces_file.c_str(), fd.get()); } } } @@ -934,12 +950,12 @@ static void AddAnrTraceDir(const bool add_to_zip, const std::string& anr_traces_ AddDumps(anr_data->begin(), anr_data->begin() + 1, "VM TRACES AT LAST ANR", add_to_zip); - if (anr_data->size() > 1) { - // NOTE: Historical ANRs are always added as separate entries in the - // bugreport zip file. - AddDumps(anr_data->begin() + 1, anr_data->end(), - "HISTORICAL ANR", true /* add_to_zip */); - } + // The "last" ANR will always be included as separate entry in the zip file. In addition, + // it will be present in the body of the main entry if |add_to_zip| == false. + // + // Historical ANRs are always included as separate entries in the bugreport zip file. + AddDumps(anr_data->begin() + ((add_to_zip) ? 1 : 0), anr_data->end(), + "HISTORICAL ANR", true /* add_to_zip */); } else { printf("*** NO ANRs to dump in %s\n\n", ANR_DIR.c_str()); } @@ -2033,5 +2049,8 @@ int main(int argc, char *argv[]) { close(ds.control_socket_fd_); } + CloseDumpFds(tombstone_data.get()); + CloseDumpFds(anr_data.get()); + return 0; } diff --git a/cmds/dumpstate/utils.cpp b/cmds/dumpstate/utils.cpp index c2c9071812..6ff0dae01c 100644 --- a/cmds/dumpstate/utils.cpp +++ b/cmds/dumpstate/utils.cpp @@ -632,7 +632,7 @@ int dump_files(const std::string& title, const char* dir, bool (*skip)(const cha struct dirent *d; char *newpath = NULL; const char *slash = "/"; - int fd, retval = 0; + int retval = 0; if (!title.empty()) { printf("------ %s (%s) ------\n", title.c_str(), dir); @@ -674,13 +674,13 @@ int dump_files(const std::string& title, const char* dir, bool (*skip)(const cha } continue; } - fd = TEMP_FAILURE_RETRY(open(newpath, O_RDONLY | O_NONBLOCK | O_CLOEXEC)); - if (fd < 0) { - retval = fd; + android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(newpath, O_RDONLY | O_NONBLOCK | O_CLOEXEC))); + if (fd.get() < 0) { + retval = -1; printf("*** %s: %s\n", newpath, strerror(errno)); continue; } - (*dump_from_fd)(NULL, newpath, fd); + (*dump_from_fd)(NULL, newpath, fd.get()); } closedir(dirp); if (!title.empty()) { @@ -699,11 +699,9 @@ int dump_file_from_fd(const char *title, const char *path, int fd) { int flags = fcntl(fd, F_GETFL); if (flags == -1) { printf("*** %s: failed to get flags on fd %d: %s\n", path, fd, strerror(errno)); - close(fd); return -1; } else if (!(flags & O_NONBLOCK)) { printf("*** %s: fd must have O_NONBLOCK set.\n", path); - close(fd); return -1; } return DumpFileFromFdToFd(title, path, fd, STDOUT_FILENO, PropertiesHelper::IsDryRun()); -- cgit v1.2.3-59-g8ed1b From f3700c39eda134651b609f6f38173b7636d14330 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Thu, 9 Nov 2017 14:56:17 -0700 Subject: Remove "vdc" commands from bugreports. We moved vold to Binder, and it no longer has a "dump" command. Test: builds, boots Bug: 69116075 Change-Id: Iff8cdf45e15acac9207a3d2eafaed6cb144d26e1 --- cmds/dumpstate/dumpstate.cpp | 3 --- 1 file changed, 3 deletions(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index b94d3d5367..201b5a3496 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1184,9 +1184,6 @@ static void dumpstate() { RunCommand("SYSTEM PROPERTIES", {"getprop"}); - RunCommand("VOLD DUMP", {"vdc", "dump"}); - RunCommand("SECURE CONTAINERS", {"vdc", "asec", "list"}); - RunCommand("STORAGED IO INFO", {"storaged", "-u", "-p"}); RunCommand("FILESYSTEMS & FREE SPACE", {"df"}); -- cgit v1.2.3-59-g8ed1b From 36b4cdb2ddec610e52ab60ae1f6497ae3739f496 Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Fri, 17 Nov 2017 10:27:05 -0800 Subject: Add window and surface trace files to bug report - rename surface trace path to use common location Bug: 64831661 Test: adb shell su root service call SurfaceFlinger 1025 i32 1 >/dev/null && adb shell su root service call SurfaceFlinger 1025 i32 0 >/dev/null && adb bugreport ~/tmp.zip Change-Id: I0b166a1098158a12c1da192e38c4d3cf011710a6 --- cmds/dumpstate/dumpstate.cpp | 6 ++++++ services/surfaceflinger/SurfaceTracing.h | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 201b5a3496..364ead0d18 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -81,6 +81,7 @@ void add_mountinfo(); #define PROFILE_DATA_DIR_CUR "/data/misc/profiles/cur" #define PROFILE_DATA_DIR_REF "/data/misc/profiles/ref" #define WLUTIL "/vendor/xbin/wlutil" +#define WMTRACE_DATA_DIR "/data/misc/wmtrace" // TODO(narayan): Since this information has to be kept in sync // with tombstoned, we should just put it in a common header. @@ -1210,6 +1211,11 @@ static void dumpstate() { DumpFile("BINDER STATS", "/sys/kernel/debug/binder/stats"); DumpFile("BINDER STATE", "/sys/kernel/debug/binder/state"); + /* Add window and surface trace files. */ + if (!PropertiesHelper::IsUserBuild()) { + ds.AddDir(WMTRACE_DATA_DIR, false); + } + ds.DumpstateBoard(); /* Migrate the ril_dumpstate to a device specific dumpstate? */ diff --git a/services/surfaceflinger/SurfaceTracing.h b/services/surfaceflinger/SurfaceTracing.h index 9b219890a1..590ab9680f 100644 --- a/services/surfaceflinger/SurfaceTracing.h +++ b/services/surfaceflinger/SurfaceTracing.h @@ -37,7 +37,7 @@ public: void traceLayers(const char* where, LayersProto); private: - static constexpr auto DEFAULT_FILENAME = "/data/misc/trace/layerstrace.pb"; + static constexpr auto DEFAULT_FILENAME = "/data/misc/wmtrace/layers_trace.pb"; status_t writeProtoFileLocked(); -- cgit v1.2.3-59-g8ed1b From 6921f80f26cb779d2982d2e37e14aeadbc8230b7 Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Wed, 22 Nov 2017 09:17:23 -0800 Subject: Support dumpsys timeouts in milliseconds - add new dumpsys argument to specify timeouts in milliseconds - use milliseconds to define timeouts in dumpstate - minor dumpsys output format changes: "SERVICE '' DUMP TIMEOUT (1s) EXPIRED" -> "SERVICE '' DUMP TIMEOUT (1000ms) EXPIRED" Bug: 27429130 Test: mmm -j32 frameworks/native/cmds/dumpsys && \ mmm -j32 frameworks/native/cmds/dumpstate && \ adb sync data && adb shell /data/nativetest/dumpsys_test/dumpsys_test && \ adb shell /data/nativetest64/dumpsys_test/dumpsys_test && \ adb shell /data/nativetest/dumpstate_test/dumpstate_test && \ adb shell /data/nativetest64/dumpstate_test/dumpstate_test && \ printf "\n\n#### ALL TESTS PASSED ####\n" Change-Id: Ibc96ad030bb2c6d880b8201c9b6241fce20b284f Change-Id: I6ef2ff19787f2b6d940d56e453a1a7462a8c854a --- cmds/dumpstate/DumpstateUtil.cpp | 33 +++++++++++++++++++++------------ cmds/dumpstate/DumpstateUtil.h | 27 +++++++++++++++++++++------ cmds/dumpstate/dumpstate.cpp | 33 ++++++++++++++++----------------- cmds/dumpstate/dumpstate.h | 6 +++--- cmds/dumpstate/tests/dumpstate_test.cpp | 13 ++++++++++++- cmds/dumpstate/utils.cpp | 16 ++++++++-------- cmds/dumpsys/dumpsys.cpp | 33 +++++++++++++++++++++++---------- cmds/dumpsys/tests/dumpsys_test.cpp | 17 +++++++++++++++-- 8 files changed, 119 insertions(+), 59 deletions(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/DumpstateUtil.cpp b/cmds/dumpstate/DumpstateUtil.cpp index ede4254a9b..85eb464104 100644 --- a/cmds/dumpstate/DumpstateUtil.cpp +++ b/cmds/dumpstate/DumpstateUtil.cpp @@ -43,7 +43,7 @@ namespace { static constexpr const char* kSuPath = "/system/xbin/su"; -static bool waitpid_with_timeout(pid_t pid, int timeout_seconds, int* status) { +static bool waitpid_with_timeout(pid_t pid, int timeout_ms, int* status) { sigset_t child_mask, old_mask; sigemptyset(&child_mask); sigaddset(&child_mask, SIGCHLD); @@ -54,10 +54,11 @@ static bool waitpid_with_timeout(pid_t pid, int timeout_seconds, int* status) { } timespec ts; - ts.tv_sec = timeout_seconds; - ts.tv_nsec = 0; + ts.tv_sec = MSEC_TO_SEC(timeout_ms); + ts.tv_nsec = (timeout_ms % 1000) * 1000000; int ret = TEMP_FAILURE_RETRY(sigtimedwait(&child_mask, NULL, &ts)); int saved_errno = errno; + // Set the signals back the way they were. if (sigprocmask(SIG_SETMASK, &old_mask, NULL) == -1) { printf("*** sigprocmask failed: %s\n", strerror(errno)); @@ -91,7 +92,7 @@ static bool waitpid_with_timeout(pid_t pid, int timeout_seconds, int* status) { CommandOptions CommandOptions::DEFAULT = CommandOptions::WithTimeout(10).Build(); CommandOptions CommandOptions::AS_ROOT = CommandOptions::WithTimeout(10).AsRoot().Build(); -CommandOptions::CommandOptionsBuilder::CommandOptionsBuilder(int64_t timeout) : values(timeout) { +CommandOptions::CommandOptionsBuilder::CommandOptionsBuilder(int64_t timeout_ms) : values(timeout_ms) { } CommandOptions::CommandOptionsBuilder& CommandOptions::CommandOptionsBuilder::Always() { @@ -130,8 +131,8 @@ CommandOptions CommandOptions::CommandOptionsBuilder::Build() { return CommandOptions(values); } -CommandOptions::CommandOptionsValues::CommandOptionsValues(int64_t timeout) - : timeout_(timeout), +CommandOptions::CommandOptionsValues::CommandOptionsValues(int64_t timeout_ms) + : timeout_ms_(timeout_ms), always_(false), account_mode_(DONT_DROP_ROOT), output_mode_(NORMAL_OUTPUT), @@ -142,7 +143,11 @@ CommandOptions::CommandOptions(const CommandOptionsValues& values) : values(valu } int64_t CommandOptions::Timeout() const { - return values.timeout_; + return MSEC_TO_SEC(values.timeout_ms_); +} + +int64_t CommandOptions::TimeoutInMs() const { + return values.timeout_ms_; } bool CommandOptions::Always() const { @@ -161,8 +166,12 @@ std::string CommandOptions::LoggingMessage() const { return values.logging_message_; } -CommandOptions::CommandOptionsBuilder CommandOptions::WithTimeout(int64_t timeout) { - return CommandOptions::CommandOptionsBuilder(timeout); +CommandOptions::CommandOptionsBuilder CommandOptions::WithTimeout(int64_t timeout_sec) { + return CommandOptions::CommandOptionsBuilder(SEC_TO_MSEC(timeout_sec)); +} + +CommandOptions::CommandOptionsBuilder CommandOptions::WithTimeoutInMs(int64_t timeout_ms) { + return CommandOptions::CommandOptionsBuilder(timeout_ms); } std::string PropertiesHelper::build_type_ = ""; @@ -314,7 +323,7 @@ int RunCommandToFd(int fd, const std::string& title, const std::vector(elapsed) / NANOS_PER_SEC, pid); } kill(pid, SIGTERM); - if (!waitpid_with_timeout(pid, 5, nullptr)) { + if (!waitpid_with_timeout(pid, 5000, nullptr)) { kill(pid, SIGKILL); - if (!waitpid_with_timeout(pid, 5, nullptr)) { + if (!waitpid_with_timeout(pid, 5000, nullptr)) { if (!silent) dprintf(fd, "could not kill command '%s' (pid %d) even with SIGKILL.\n", command, pid); diff --git a/cmds/dumpstate/DumpstateUtil.h b/cmds/dumpstate/DumpstateUtil.h index 698ceffcc4..8342099821 100644 --- a/cmds/dumpstate/DumpstateUtil.h +++ b/cmds/dumpstate/DumpstateUtil.h @@ -19,6 +19,16 @@ #include #include +/* + * Converts seconds to milliseconds. + */ +#define SEC_TO_MSEC(second) (second * 1000) + +/* + * Converts milliseconds to seconds. + */ +#define MSEC_TO_SEC(millisecond) (millisecond / 1000) + namespace android { namespace os { namespace dumpstate { @@ -66,9 +76,9 @@ class CommandOptions { private: class CommandOptionsValues { private: - CommandOptionsValues(int64_t timeout); + CommandOptionsValues(int64_t timeout_ms); - int64_t timeout_; + int64_t timeout_ms_; bool always_; PrivilegeMode account_mode_; OutputMode output_mode_; @@ -102,13 +112,15 @@ class CommandOptions { CommandOptions Build(); private: - CommandOptionsBuilder(int64_t timeout); + CommandOptionsBuilder(int64_t timeout_ms); CommandOptionsValues values; friend class CommandOptions; }; - /** Gets the command timeout, in seconds. */ + /** Gets the command timeout in seconds. */ int64_t Timeout() const; + /** Gets the command timeout in milliseconds. */ + int64_t TimeoutInMs() const; /* Checks whether the command should always be run, even on dry-run mode. */ bool Always() const; /** Gets the PrivilegeMode of the command. */ @@ -118,8 +130,11 @@ class CommandOptions { /** Gets the logging message header, it any. */ std::string LoggingMessage() const; - /** Creates a builder with the requied timeout. */ - static CommandOptionsBuilder WithTimeout(int64_t timeout); + /** Creates a builder with the requied timeout in seconds. */ + static CommandOptionsBuilder WithTimeout(int64_t timeout_sec); + + /** Creates a builder with the requied timeout in milliseconds. */ + static CommandOptionsBuilder WithTimeoutInMs(int64_t timeout_ms); // Common options. static CommandOptions DEFAULT; diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 364ead0d18..aba08d9de9 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -113,8 +113,8 @@ static int RunCommand(const std::string& title, const std::vector& } static void RunDumpsys(const std::string& title, const std::vector& dumpsysArgs, const CommandOptions& options = Dumpstate::DEFAULT_DUMPSYS, - long dumpsysTimeout = 0) { - return ds.RunDumpsys(title, dumpsysArgs, options, dumpsysTimeout); + long dumpsysTimeoutMs = 0) { + return ds.RunDumpsys(title, dumpsysArgs, options, dumpsysTimeoutMs); } static int DumpFile(const std::string& title, const std::string& path) { return ds.DumpFile(title, path); @@ -830,33 +830,32 @@ static void DoKmsg() { } static void DoLogcat() { - unsigned long timeout; + unsigned long timeout_ms; // DumpFile("EVENT LOG TAGS", "/etc/event-log-tags"); // calculate timeout - timeout = logcat_timeout("main") + logcat_timeout("system") + logcat_timeout("crash"); - if (timeout < 20000) { - timeout = 20000; + timeout_ms = logcat_timeout("main") + logcat_timeout("system") + logcat_timeout("crash"); + if (timeout_ms < 20000) { + timeout_ms = 20000; } RunCommand("SYSTEM LOG", - {"logcat", "-v", "threadtime", "-v", "printable", "-v", "uid", - "-d", "*:v"}, - CommandOptions::WithTimeout(timeout / 1000).Build()); - timeout = logcat_timeout("events"); - if (timeout < 20000) { - timeout = 20000; + {"logcat", "-v", "threadtime", "-v", "printable", "-v", "uid", "-d", "*:v"}, + CommandOptions::WithTimeoutInMs(timeout_ms).Build()); + timeout_ms = logcat_timeout("events"); + if (timeout_ms < 20000) { + timeout_ms = 20000; } RunCommand("EVENT LOG", {"logcat", "-b", "events", "-v", "threadtime", "-v", "printable", "-v", "uid", "-d", "*:v"}, - CommandOptions::WithTimeout(timeout / 1000).Build()); - timeout = logcat_timeout("radio"); - if (timeout < 20000) { - timeout = 20000; + CommandOptions::WithTimeoutInMs(timeout_ms).Build()); + timeout_ms = logcat_timeout("radio"); + if (timeout_ms < 20000) { + timeout_ms = 20000; } RunCommand("RADIO LOG", {"logcat", "-b", "radio", "-v", "threadtime", "-v", "printable", "-v", "uid", "-d", "*:v"}, - CommandOptions::WithTimeout(timeout / 1000).Build()); + CommandOptions::WithTimeoutInMs(timeout_ms).Build()); RunCommand("LOG STATISTICS", {"logcat", "-b", "all", "-S"}); diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h index 1bfafbaffe..8db23a94f4 100644 --- a/cmds/dumpstate/dumpstate.h +++ b/cmds/dumpstate/dumpstate.h @@ -196,19 +196,19 @@ class Dumpstate { /* * Runs `dumpsys` with the given arguments, automatically setting its timeout - * (`-t` argument) + * (`-T` argument) * according to the command options. * * |title| description of the command printed on `stdout` (or empty to skip * description). * |dumpsys_args| `dumpsys` arguments (except `-t`). * |options| optional argument defining the command's behavior. - * |dumpsys_timeout| when > 0, defines the value passed to `dumpsys -t` (otherwise it uses the + * |dumpsys_timeout| when > 0, defines the value passed to `dumpsys -T` (otherwise it uses the * timeout from `options`) */ void RunDumpsys(const std::string& title, const std::vector& dumpsys_args, const android::os::dumpstate::CommandOptions& options = DEFAULT_DUMPSYS, - long dumpsys_timeout = 0); + long dumpsys_timeout_ms = 0); /* * Prints the contents of a file. diff --git a/cmds/dumpstate/tests/dumpstate_test.cpp b/cmds/dumpstate/tests/dumpstate_test.cpp index 92b0c0d8bc..a2e94538c2 100644 --- a/cmds/dumpstate/tests/dumpstate_test.cpp +++ b/cmds/dumpstate/tests/dumpstate_test.cpp @@ -1001,7 +1001,7 @@ TEST_F(DumpstateUtilTest, RunCommandCrashes) { err, StartsWith("stderr\n*** command '" + kSimpleCommand + " --crash' failed: exit code")); } -TEST_F(DumpstateUtilTest, RunCommandTimesout) { +TEST_F(DumpstateUtilTest, RunCommandTimesoutWithSec) { CreateFd("RunCommandTimesout.txt"); EXPECT_EQ(-1, RunCommand("", {kSimpleCommand, "--sleep", "2"}, CommandOptions::WithTimeout(1).Build())); @@ -1011,6 +1011,17 @@ TEST_F(DumpstateUtilTest, RunCommandTimesout) { " --sleep 2' timed out after 1")); } +TEST_F(DumpstateUtilTest, RunCommandTimesoutWithMsec) { + CreateFd("RunCommandTimesout.txt"); + EXPECT_EQ(-1, RunCommand("", {kSimpleCommand, "--sleep", "2"}, + CommandOptions::WithTimeoutInMs(1000).Build())); + EXPECT_THAT(out, StartsWith("stdout line1\n*** command '" + kSimpleCommand + + " --sleep 2' timed out after 1")); + EXPECT_THAT(err, StartsWith("sleeping for 2s\n*** command '" + kSimpleCommand + + " --sleep 2' timed out after 1")); +} + + TEST_F(DumpstateUtilTest, RunCommandIsKilled) { CreateFd("RunCommandIsKilled.txt"); CaptureStderr(); diff --git a/cmds/dumpstate/utils.cpp b/cmds/dumpstate/utils.cpp index 6ff0dae01c..ac48041f6d 100644 --- a/cmds/dumpstate/utils.cpp +++ b/cmds/dumpstate/utils.cpp @@ -215,10 +215,10 @@ int32_t Progress::Get() const { return progress_; } -bool Progress::Inc(int32_t delta) { +bool Progress::Inc(int32_t delta_sec) { bool changed = false; - if (delta >= 0) { - progress_ += delta; + if (delta_sec >= 0) { + progress_ += delta_sec; if (progress_ > max_) { int32_t old_max = max_; max_ = floor((float)progress_ * growth_factor_); @@ -723,9 +723,9 @@ int Dumpstate::RunCommand(const std::string& title, const std::vector& dumpsys_args, - const CommandOptions& options, long dumpsysTimeout) { - long timeout = dumpsysTimeout > 0 ? dumpsysTimeout : options.Timeout(); - std::vector dumpsys = {"/system/bin/dumpsys", "-t", std::to_string(timeout)}; + const CommandOptions& options, long dumpsysTimeoutMs) { + long timeout_ms = dumpsysTimeoutMs > 0 ? dumpsysTimeoutMs : options.TimeoutInMs(); + std::vector dumpsys = {"/system/bin/dumpsys", "-T", std::to_string(timeout_ms)}; dumpsys.insert(dumpsys.end(), dumpsys_args.begin(), dumpsys_args.end()); RunCommand(title, dumpsys, options); } @@ -1165,14 +1165,14 @@ void dump_route_tables() { } // TODO: make this function thread safe if sections are generated in parallel. -void Dumpstate::UpdateProgress(int32_t delta) { +void Dumpstate::UpdateProgress(int32_t delta_sec) { if (progress_ == nullptr) { MYLOGE("UpdateProgress: progress_ not set\n"); return; } // Always update progess so stats can be tuned... - bool max_changed = progress_->Inc(delta); + bool max_changed = progress_->Inc(delta_sec); // ...but only notifiy listeners when necessary. if (!update_progress_) return; diff --git a/cmds/dumpsys/dumpsys.cpp b/cmds/dumpsys/dumpsys.cpp index c36ab0872a..0862a40734 100644 --- a/cmds/dumpsys/dumpsys.cpp +++ b/cmds/dumpsys/dumpsys.cpp @@ -61,7 +61,8 @@ static void usage() { "SERVICE [ARGS]]\n" " --help: shows this help\n" " -l: only list services, do not dump them\n" - " -t TIMEOUT: TIMEOUT to use in seconds instead of default 10 seconds\n" + " -t TIMEOUT_SEC: TIMEOUT to use in seconds instead of default 10 seconds\n" + " -T TIMEOUT_MS: TIMEOUT to use in milliseconds instead of default 10 seconds\n" " --proto: filter services that support dumping data in proto format. Dumps" " will be in proto format.\n" " --priority LEVEL: filter services based on specified priority\n" @@ -104,7 +105,7 @@ int Dumpsys::main(int argc, char* const argv[]) { bool showListOnly = false; bool skipServices = false; bool filterByProto = false; - int timeoutArg = 10; + int timeoutArgMs = 10000; int dumpPriorityFlags = IServiceManager::DUMP_FLAG_PRIORITY_ALL; static struct option longOptions[] = {{"priority", required_argument, 0, 0}, {"proto", no_argument, 0, 0}, @@ -119,7 +120,7 @@ int Dumpsys::main(int argc, char* const argv[]) { int c; int optionIndex = 0; - c = getopt_long(argc, argv, "+t:l", longOptions, &optionIndex); + c = getopt_long(argc, argv, "+t:T:l", longOptions, &optionIndex); if (c == -1) { break; @@ -146,10 +147,22 @@ int Dumpsys::main(int argc, char* const argv[]) { case 't': { - char *endptr; - timeoutArg = strtol(optarg, &endptr, 10); - if (*endptr != '\0' || timeoutArg <= 0) { - fprintf(stderr, "Error: invalid timeout number: '%s'\n", optarg); + char* endptr; + timeoutArgMs = strtol(optarg, &endptr, 10); + timeoutArgMs = timeoutArgMs * 1000; + if (*endptr != '\0' || timeoutArgMs <= 0) { + fprintf(stderr, "Error: invalid timeout(seconds) number: '%s'\n", optarg); + return -1; + } + } + break; + + case 'T': + { + char* endptr; + timeoutArgMs = strtol(optarg, &endptr, 10); + if (*endptr != '\0' || timeoutArgMs <= 0) { + fprintf(stderr, "Error: invalid timeout(milliseconds) number: '%s'\n", optarg); return -1; } } @@ -269,7 +282,7 @@ int Dumpsys::main(int argc, char* const argv[]) { } }); - auto timeout = std::chrono::seconds(timeoutArg); + auto timeout = std::chrono::milliseconds(timeoutArgMs); auto start = std::chrono::steady_clock::now(); auto end = start + timeout; @@ -321,8 +334,8 @@ int Dumpsys::main(int argc, char* const argv[]) { if (timed_out) { aout << endl - << "*** SERVICE '" << service_name << "' DUMP TIMEOUT (" << timeoutArg - << "s) EXPIRED ***" << endl + << "*** SERVICE '" << service_name << "' DUMP TIMEOUT (" << timeoutArgMs + << "ms) EXPIRED ***" << endl << endl; } diff --git a/cmds/dumpsys/tests/dumpsys_test.cpp b/cmds/dumpsys/tests/dumpsys_test.cpp index 18a4da9739..bdb0a9ad8f 100644 --- a/cmds/dumpsys/tests/dumpsys_test.cpp +++ b/cmds/dumpsys/tests/dumpsys_test.cpp @@ -299,12 +299,25 @@ TEST_F(DumpsysTest, DumpRunningService) { } // Tests 'dumpsys -t 1 service_name' on a service that times out after 2s -TEST_F(DumpsysTest, DumpRunningServiceTimeout) { +TEST_F(DumpsysTest, DumpRunningServiceTimeoutInSec) { sp binder_mock = ExpectDumpAndHang("Valet", 2, "Here's your car"); CallMain({"-t", "1", "Valet"}); - AssertOutputContains("SERVICE 'Valet' DUMP TIMEOUT (1s) EXPIRED"); + AssertOutputContains("SERVICE 'Valet' DUMP TIMEOUT (1000ms) EXPIRED"); + AssertNotDumped("Here's your car"); + + // TODO(b/65056227): BinderMock is not destructed because thread is detached on dumpsys.cpp + Mock::AllowLeak(binder_mock.get()); +} + +// Tests 'dumpsys -T 500 service_name' on a service that times out after 2s +TEST_F(DumpsysTest, DumpRunningServiceTimeoutInMs) { + sp binder_mock = ExpectDumpAndHang("Valet", 2, "Here's your car"); + + CallMain({"-T", "500", "Valet"}); + + AssertOutputContains("SERVICE 'Valet' DUMP TIMEOUT (500ms) EXPIRED"); AssertNotDumped("Here's your car"); // TODO(b/65056227): BinderMock is not destructed because thread is detached on dumpsys.cpp -- cgit v1.2.3-59-g8ed1b From 652cc80fcffda5a80b468b822cdddd69b28696a3 Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Thu, 30 Nov 2017 15:18:30 -0800 Subject: Fix dumpsys timeout regression Bug: 69974956 Test: adb bugreport ~/tmp.zip Change-Id: I833937de55c780773d946ef36a5105e544940bfa --- cmds/dumpstate/dumpstate.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index aba08d9de9..4b67d8f1a0 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1085,7 +1085,7 @@ static void RunDumpsysNormal() { CommandOptions::WithTimeout(90).DropRoot().Build()); } else { RunDumpsys("DUMPSYS", {"--skip", "meminfo", "cpuinfo"}, - CommandOptions::WithTimeout(90).Build(), 10); + CommandOptions::WithTimeout(90).Build(), SEC_TO_MSEC(10)); } } @@ -1307,8 +1307,10 @@ static void DumpstateTelephonyOnly() { printf("== Android Framework Services\n"); printf("========================================================\n"); - RunDumpsys("DUMPSYS", {"connectivity"}, CommandOptions::WithTimeout(90).Build(), 10); - RunDumpsys("DUMPSYS", {"carrier_config"}, CommandOptions::WithTimeout(90).Build(), 10); + RunDumpsys("DUMPSYS", {"connectivity"}, CommandOptions::WithTimeout(90).Build(), + SEC_TO_MSEC(10)); + RunDumpsys("DUMPSYS", {"carrier_config"}, CommandOptions::WithTimeout(90).Build(), + SEC_TO_MSEC(10)); printf("========================================================\n"); printf("== Running Application Services\n"); -- cgit v1.2.3-59-g8ed1b From 7709f8a5e62a67c6df06f68bddcc2a1036bd62bb Mon Sep 17 00:00:00 2001 From: Felipe Leme Date: Tue, 5 Dec 2017 09:30:09 -0800 Subject: Added -p to uptime command on header (and changed it to run always) Test: manual verification Test: mmm -j32 frameworks/native/cmds/dumpstate/ && adb sync && \ adb shell /data/nativetest/dumpstate_test/dumpstate_test && \ adb shell /data/nativetest64/dumpstate_test/dumpstate_test && \ printf "\n\n#### GOOD NEWS, EVERYONE: ALL TESTS PASSED! ####\n" Bug: 65205261 Change-Id: Ide1aa2dc9f3446f9597433867445dd07731ae563 --- cmds/dumpstate/dumpstate.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 4b67d8f1a0..2e3e3caeb0 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -691,7 +691,9 @@ void Dumpstate::PrintHeader() const { printf("Kernel: "); DumpFileToFd(STDOUT_FILENO, "", "/proc/version"); printf("Command line: %s\n", strtok(cmdline_buf, "\n")); - ds.RunCommand("UPTIME", {"uptime"}, CommandOptions::DEFAULT); + printf("Uptime: "); + RunCommandToFd(STDOUT_FILENO, "", {"uptime", "-p"}, + CommandOptions::WithTimeout(1).Always().Build()); printf("Bugreport format version: %s\n", version_.c_str()); printf("Dumpstate info: id=%d pid=%d dry_run=%d args=%s extra_options=%s\n", id_, pid_, PropertiesHelper::IsDryRun(), args_.c_str(), extra_options_.c_str()); @@ -1093,7 +1095,6 @@ static void dumpstate() { DurationReporter duration_reporter("DUMPSTATE"); dump_dev_files("TRUSTY VERSION", "/sys/bus/platform/drivers/trusty", "trusty_version"); - /* TODO: Remove duplicate uptime call when tools use it from header */ RunCommand("UPTIME", {"uptime"}); DumpBlockStatFiles(); dump_emmc_ecsd("/d/mmc0/mmc0:0001/ext_csd"); -- cgit v1.2.3-59-g8ed1b From 5ec5888cf406e206bbffa9fed01202a80606f2b4 Mon Sep 17 00:00:00 2001 From: Felipe Leme Date: Mon, 18 Dec 2017 11:55:57 -0800 Subject: Increased minimum logcat timeout from 20s to 50s... ...as 20s is often not enough when the device has larger logcat buffer sizes. Test: mmm -j32 frameworks/native/cmds/dumpstate/ && adb sync && \ adb shell /data/nativetest/dumpstate_test/dumpstate_test && \ adb shell /data/nativetest64/dumpstate_test/dumpstate_test && \ printf "\n\n#### GOOD NEWS, EVERYONE: ALL TESTS PASSED! ####\n" Fixes: 70597931 Change-Id: Ie302f46ed405d33bc7411dce28a93949183d4bac --- cmds/dumpstate/dumpstate.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 2e3e3caeb0..2176522834 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -831,28 +831,30 @@ static void DoKmsg() { } } +static const long MINIMUM_LOGCAT_TIMEOUT_MS = 50000; + static void DoLogcat() { unsigned long timeout_ms; // DumpFile("EVENT LOG TAGS", "/etc/event-log-tags"); // calculate timeout timeout_ms = logcat_timeout("main") + logcat_timeout("system") + logcat_timeout("crash"); - if (timeout_ms < 20000) { - timeout_ms = 20000; + if (timeout_ms < MINIMUM_LOGCAT_TIMEOUT_MS) { + timeout_ms = MINIMUM_LOGCAT_TIMEOUT_MS; } RunCommand("SYSTEM LOG", {"logcat", "-v", "threadtime", "-v", "printable", "-v", "uid", "-d", "*:v"}, CommandOptions::WithTimeoutInMs(timeout_ms).Build()); timeout_ms = logcat_timeout("events"); - if (timeout_ms < 20000) { - timeout_ms = 20000; + if (timeout_ms < MINIMUM_LOGCAT_TIMEOUT_MS) { + timeout_ms = MINIMUM_LOGCAT_TIMEOUT_MS; } RunCommand("EVENT LOG", {"logcat", "-b", "events", "-v", "threadtime", "-v", "printable", "-v", "uid", "-d", "*:v"}, CommandOptions::WithTimeoutInMs(timeout_ms).Build()); timeout_ms = logcat_timeout("radio"); - if (timeout_ms < 20000) { - timeout_ms = 20000; + if (timeout_ms < MINIMUM_LOGCAT_TIMEOUT_MS) { + timeout_ms = MINIMUM_LOGCAT_TIMEOUT_MS; } RunCommand("RADIO LOG", {"logcat", "-b", "radio", "-v", "threadtime", "-v", "printable", "-v", "uid", -- cgit v1.2.3-59-g8ed1b From 44cd9480005ed5a5b1b3530f44335ba400055de3 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Thu, 4 Jan 2018 16:24:13 -0800 Subject: Break up hal debug files into multiple files. Full list of hals installed on the device is preserved in the main bugreport file. Now, for hals which implement IBase::debug, their debug data is saved in one file per hal. This is required so that hals can dump a substantial amount of data (this change in particular is motivated by wifi). Fixes: 71597580 Test: bugreport on walleye shows data in the main list and in individual files. Change-Id: I0bab88b2a98ec50f0c03eafd7e1e20a223e14bcc --- cmds/dumpstate/dumpstate.cpp | 69 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 56 insertions(+), 13 deletions(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 143192eba0..665695cfb0 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -43,8 +43,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -131,8 +133,6 @@ static const std::string kDumpstateBoardFiles[] = { }; static const int NUM_OF_DUMPS = arraysize(kDumpstateBoardFiles); -static const std::string kLsHalDebugPath = "/bugreports/dumpstate_lshal.txt"; - static constexpr char PROPERTY_EXTRA_OPTIONS[] = "dumpstate.options"; static constexpr char PROPERTY_LAST_ID[] = "dumpstate.last_id"; static constexpr char PROPERTY_VERSION[] = "dumpstate.version"; @@ -1093,6 +1093,57 @@ static void RunDumpsysNormal() { } } +static void DumpHals() { + using android::sp; + using android::hidl::manager::V1_0::IServiceManager; + using android::hardware::defaultServiceManager; + + sp sm = defaultServiceManager(); + if (sm == nullptr) { + MYLOGE("Could not retrieve hwservicemanager to dump hals.\n"); + return; + } + + auto ret = sm->list([&](const auto& interfaces) { + for (const std::string& interface : interfaces) { + std::string cleanName = interface; + std::replace_if(cleanName.begin(), + cleanName.end(), + [](char c) { + return !isalnum(c) && + std::string("@-_:.").find(c) == std::string::npos; + }, '_'); + const std::string path = kDumpstateBoardPath + "lshal_debug_" + cleanName; + + { + auto fd = android::base::unique_fd( + TEMP_FAILURE_RETRY(open(path.c_str(), + O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW, + S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH))); + if (fd < 0) { + MYLOGE("Could not open %s to dump additional hal information.\n", path.c_str()); + continue; + } + RunCommandToFd(fd, + "", + {"lshal", "debug", interface}, + CommandOptions::WithTimeout(2).AsRootIfAvailable().Build()); + + bool empty = 0 == lseek(fd, 0, SEEK_END); + if (!empty) { + ds.AddZipEntry("lshal-debug/" + cleanName + ".txt", path); + } + } + + unlink(path.c_str()); + } + }); + + if (!ret.isOk()) { + MYLOGE("Could not list hals from hwservicemanager.\n"); + } +} + static void dumpstate() { DurationReporter duration_reporter("DUMPSTATE"); @@ -1121,18 +1172,10 @@ static void dumpstate() { RunCommand("LIBRANK", {"librank"}, CommandOptions::AS_ROOT); if (ds.IsZipping()) { - RunCommand( - "HARDWARE HALS", - {"lshal", std::string("--debug=") + kLsHalDebugPath}, - CommandOptions::WithTimeout(10).AsRootIfAvailable().Build()); - - ds.AddZipEntry("lshal-debug.txt", kLsHalDebugPath); - - unlink(kLsHalDebugPath.c_str()); + RunCommand("HARDWARE HALS", {"lshal"}, CommandOptions::WithTimeout(2).AsRootIfAvailable().Build()); + DumpHals(); } else { - RunCommand( - "HARDWARE HALS", {"lshal", "--debug"}, - CommandOptions::WithTimeout(10).AsRootIfAvailable().Build()); + RunCommand("HARDWARE HALS", {"lshal", "--debug"}, CommandOptions::WithTimeout(10).AsRootIfAvailable().Build()); } RunCommand("PRINTENV", {"printenv"}); -- cgit v1.2.3-59-g8ed1b From 20cf5036c1f373c1acfbb95295f118b7ff6c2227 Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Fri, 5 Jan 2018 13:15:49 -0800 Subject: Add bugreport section progress reporter - Allow dumpstatelisteners to monitor section size and duration and errors while the bugreport is generated. Data will be used to write smoke tests for bugreport. - Refactor main function to allow test to call dumpstate main function. Required until bugreport api is completed - Restore stdout and stderr fds before exiting dumpstate Bug: 70154685 Test: mmm -j56 frameworks/native/cmds/dumpstate && \ adb sync data && \ adb shell /data/nativetest64/dumpstate_test/dumpstate_test && \ printf "\n\n#### ALL TESTS PASSED ####\n" Change-Id: I7e0938baf6e055f14dce2348d0fe99f261870bf1 --- cmds/dumpstate/Android.bp | 2 + cmds/dumpstate/DumpstateSectionReporter.cpp | 42 ++++++++++++++ cmds/dumpstate/DumpstateSectionReporter.h | 65 ++++++++++++++++++++++ cmds/dumpstate/DumpstateService.cpp | 2 + cmds/dumpstate/DumpstateService.h | 1 + cmds/dumpstate/binder/android/os/IDumpstate.aidl | 5 +- .../binder/android/os/IDumpstateListener.aidl | 12 ++++ cmds/dumpstate/dumpstate.cpp | 11 +++- cmds/dumpstate/dumpstate.h | 6 +- cmds/dumpstate/main.cpp | 21 +++++++ cmds/dumpstate/tests/dumpstate_test.cpp | 26 +++++++-- 11 files changed, 184 insertions(+), 9 deletions(-) create mode 100644 cmds/dumpstate/DumpstateSectionReporter.cpp create mode 100644 cmds/dumpstate/DumpstateSectionReporter.h create mode 100644 cmds/dumpstate/main.cpp (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/Android.bp b/cmds/dumpstate/Android.bp index ce3a6aad7a..35cff5f62e 100644 --- a/cmds/dumpstate/Android.bp +++ b/cmds/dumpstate/Android.bp @@ -83,7 +83,9 @@ cc_binary { ], srcs: [ "DumpstateInternal.cpp", + "DumpstateSectionReporter.cpp", "DumpstateService.cpp", + "main.cpp", "utils.cpp", "dumpstate.cpp", ], diff --git a/cmds/dumpstate/DumpstateSectionReporter.cpp b/cmds/dumpstate/DumpstateSectionReporter.cpp new file mode 100644 index 0000000000..f814bde26d --- /dev/null +++ b/cmds/dumpstate/DumpstateSectionReporter.cpp @@ -0,0 +1,42 @@ +/* + * Copyright (C) 2018 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. + */ + +#define LOG_TAG "dumpstate" + +#include "DumpstateSectionReporter.h" + +namespace android { +namespace os { +namespace dumpstate { + +DumpstateSectionReporter::DumpstateSectionReporter(const std::string& title, + sp listener, + bool sendReport) + : title_(title), listener_(listener), sendReport_(sendReport), status_(OK), size_(-1) { + started_ = std::chrono::steady_clock::now(); +} + +DumpstateSectionReporter::~DumpstateSectionReporter() { + if ((listener_ != nullptr) && (sendReport_)) { + auto elapsed = std::chrono::duration_cast( + std::chrono::steady_clock::now() - started_); + listener_->onSectionComplete(title_, status_, size_, (int32_t)elapsed.count()); + } +} + +} // namespace dumpstate +} // namespace os +} // namespace android diff --git a/cmds/dumpstate/DumpstateSectionReporter.h b/cmds/dumpstate/DumpstateSectionReporter.h new file mode 100644 index 0000000000..e971de84c5 --- /dev/null +++ b/cmds/dumpstate/DumpstateSectionReporter.h @@ -0,0 +1,65 @@ +/* + * Copyright (C) 2018 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. + */ + +#ifndef ANDROID_OS_DUMPSTATESECTIONREPORTER_H_ +#define ANDROID_OS_DUMPSTATESECTIONREPORTER_H_ + +#include +#include + +namespace android { +namespace os { +namespace dumpstate { + + +/* + * Helper class used to report per section details to a listener. + * + * Typical usage: + * + * DumpstateSectionReporter sectionReporter(title, listener, sendReport); + * sectionReporter.setSize(5000); + * + */ +class DumpstateSectionReporter { + public: + DumpstateSectionReporter(const std::string& title, sp listener, + bool sendReport); + + ~DumpstateSectionReporter(); + + void setStatus(status_t status) { + status_ = status; + } + + void setSize(int size) { + size_ = size; + } + + private: + std::string title_; + android::sp listener_; + bool sendReport_; + status_t status_; + int size_; + std::chrono::time_point started_; +}; + +} // namespace dumpstate +} // namespace os +} // namespace android + +#endif // ANDROID_OS_DUMPSTATESECTIONREPORTER_H_ diff --git a/cmds/dumpstate/DumpstateService.cpp b/cmds/dumpstate/DumpstateService.cpp index efe0466d07..49a78e751b 100644 --- a/cmds/dumpstate/DumpstateService.cpp +++ b/cmds/dumpstate/DumpstateService.cpp @@ -52,6 +52,7 @@ status_t DumpstateService::Start() { binder::Status DumpstateService::setListener(const std::string& name, const sp& listener, + bool getSectionDetails, sp* returned_token) { *returned_token = nullptr; if (name.empty()) { @@ -70,6 +71,7 @@ binder::Status DumpstateService::setListener(const std::string& name, ds_.listener_name_ = name; ds_.listener_ = listener; + ds_.report_section_ = getSectionDetails; *returned_token = new DumpstateToken(); return binder::Status::ok(); diff --git a/cmds/dumpstate/DumpstateService.h b/cmds/dumpstate/DumpstateService.h index 4352d3dacf..7bca24ae33 100644 --- a/cmds/dumpstate/DumpstateService.h +++ b/cmds/dumpstate/DumpstateService.h @@ -38,6 +38,7 @@ class DumpstateService : public BinderService, public BnDumpst status_t dump(int fd, const Vector& args) override; binder::Status setListener(const std::string& name, const sp& listener, + bool getSectionDetails, sp* returned_token) override; private: diff --git a/cmds/dumpstate/binder/android/os/IDumpstate.aidl b/cmds/dumpstate/binder/android/os/IDumpstate.aidl index 4becccfc6d..9b11b960c5 100644 --- a/cmds/dumpstate/binder/android/os/IDumpstate.aidl +++ b/cmds/dumpstate/binder/android/os/IDumpstate.aidl @@ -30,6 +30,9 @@ interface IDumpstate { * * Returns a token used to monitor dumpstate death, or `nullptr` if the listener was already * set (the listener behaves like a Highlander: There Can be Only One). + * Set {@code getSectionDetails} to true in order to receive callbacks with per section + * progress details */ - IDumpstateToken setListener(@utf8InCpp String name, IDumpstateListener listener); + IDumpstateToken setListener(@utf8InCpp String name, IDumpstateListener listener, + boolean getSectionDetails); } diff --git a/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl b/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl index 32717f4f87..030d69d16e 100644 --- a/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl +++ b/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl @@ -24,4 +24,16 @@ package android.os; interface IDumpstateListener { void onProgressUpdated(int progress); void onMaxProgressUpdated(int maxProgress); + + /** + * Called after every section is complete. + * @param name section name + * @param status values from status_t + * {@code OK} section completed successfully + * {@code TIMEOUT} dump timed out + * {@code != OK} error + * @param size size in bytes, may be invalid if status != OK + * @param durationMs duration in ms + */ + void onSectionComplete(@utf8InCpp String name, int status, int size, int durationMs); } diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 143192eba0..eb9079b0f1 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1545,7 +1545,8 @@ static void Vibrate(int duration_ms) { // clang-format on } -int main(int argc, char *argv[]) { +/** Main entry point for dumpstate. */ +int run_main(int argc, char* argv[]) { int do_add_date = 0; int do_zip_file = 0; int do_vibrate = 1; @@ -1558,6 +1559,8 @@ int main(int argc, char *argv[]) { bool show_header_only = false; bool do_start_service = false; bool telephony_only = false; + int dup_stdout_fd; + int dup_stderr_fd; /* set as high priority, and protect from OOM killer */ setpriority(PRIO_PROCESS, 0, -20); @@ -1829,11 +1832,13 @@ int main(int argc, char *argv[]) { } if (is_redirecting) { + TEMP_FAILURE_RETRY(dup_stderr_fd = dup(fileno(stderr))); redirect_to_file(stderr, const_cast(ds.log_path_.c_str())); if (chown(ds.log_path_.c_str(), AID_SHELL, AID_SHELL)) { MYLOGE("Unable to change ownership of dumpstate log file %s: %s\n", ds.log_path_.c_str(), strerror(errno)); } + TEMP_FAILURE_RETRY(dup_stdout_fd = dup(fileno(stdout))); /* TODO: rather than generating a text file now and zipping it later, it would be more efficient to redirect stdout to the zip entry directly, but the libziparchive doesn't support that option yet. */ @@ -1907,7 +1912,7 @@ int main(int argc, char *argv[]) { /* close output if needed */ if (is_redirecting) { - fclose(stdout); + TEMP_FAILURE_RETRY(dup2(dup_stdout_fd, fileno(stdout))); } /* rename or zip the (now complete) .tmp file to its final location */ @@ -2038,7 +2043,7 @@ int main(int argc, char *argv[]) { MYLOGI("done (id %d)\n", ds.id_); if (is_redirecting) { - fclose(stderr); + TEMP_FAILURE_RETRY(dup2(dup_stderr_fd, fileno(stderr))); } if (use_control_socket && ds.control_socket_fd_ != -1) { diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h index 8db23a94f4..843c545e50 100644 --- a/cmds/dumpstate/dumpstate.h +++ b/cmds/dumpstate/dumpstate.h @@ -341,9 +341,10 @@ class Dumpstate { // Pointer to the zip structure. std::unique_ptr zip_writer_; - // Binder object listing to progress. + // Binder object listening to progress. android::sp listener_; std::string listener_name_; + bool report_section_; // Notification title and description std::string notification_title; @@ -433,6 +434,9 @@ void dump_emmc_ecsd(const char *ext_csd_path); /** Gets command-line arguments. */ void format_args(int argc, const char *argv[], std::string *args); +/** Main entry point for dumpstate. */ +int run_main(int argc, char* argv[]); + #ifdef __cplusplus } #endif diff --git a/cmds/dumpstate/main.cpp b/cmds/dumpstate/main.cpp new file mode 100644 index 0000000000..78aad1137b --- /dev/null +++ b/cmds/dumpstate/main.cpp @@ -0,0 +1,21 @@ +/* + * Copyright (C) 2018 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 "dumpstate.h" + +int main(int argc, char* argv[]) { + return run_main(argc, argv); +} diff --git a/cmds/dumpstate/tests/dumpstate_test.cpp b/cmds/dumpstate/tests/dumpstate_test.cpp index a2e94538c2..838b385b1b 100644 --- a/cmds/dumpstate/tests/dumpstate_test.cpp +++ b/cmds/dumpstate/tests/dumpstate_test.cpp @@ -58,6 +58,8 @@ class DumpstateListenerMock : public IDumpstateListener { public: MOCK_METHOD1(onProgressUpdated, binder::Status(int32_t progress)); MOCK_METHOD1(onMaxProgressUpdated, binder::Status(int32_t max_progress)); + MOCK_METHOD4(onSectionComplete, binder::Status(const ::std::string& name, int32_t status, + int32_t size, int32_t durationMs)); protected: MOCK_METHOD0(onAsBinder, IBinder*()); @@ -601,27 +603,43 @@ class DumpstateServiceTest : public DumpstateBaseTest { TEST_F(DumpstateServiceTest, SetListenerNoName) { sp listener(new DumpstateListenerMock()); sp token; - EXPECT_TRUE(dss.setListener("", listener, &token).isOk()); + EXPECT_TRUE(dss.setListener("", listener, /* getSectionDetails = */ false, &token).isOk()); ASSERT_THAT(token, IsNull()); } TEST_F(DumpstateServiceTest, SetListenerNoPointer) { sp token; - EXPECT_TRUE(dss.setListener("whatever", nullptr, &token).isOk()); + EXPECT_TRUE( + dss.setListener("whatever", nullptr, /* getSectionDetails = */ false, &token).isOk()); ASSERT_THAT(token, IsNull()); } TEST_F(DumpstateServiceTest, SetListenerTwice) { sp listener(new DumpstateListenerMock()); sp token; - EXPECT_TRUE(dss.setListener("whatever", listener, &token).isOk()); + EXPECT_TRUE( + dss.setListener("whatever", listener, /* getSectionDetails = */ false, &token).isOk()); ASSERT_THAT(token, NotNull()); EXPECT_THAT(Dumpstate::GetInstance().listener_name_, StrEq("whatever")); + EXPECT_FALSE(Dumpstate::GetInstance().report_section_); token.clear(); - EXPECT_TRUE(dss.setListener("whatsoever", listener, &token).isOk()); + EXPECT_TRUE( + dss.setListener("whatsoever", listener, /* getSectionDetails = */ false, &token).isOk()); ASSERT_THAT(token, IsNull()); EXPECT_THAT(Dumpstate::GetInstance().listener_name_, StrEq("whatever")); + EXPECT_FALSE(Dumpstate::GetInstance().report_section_); +} + +TEST_F(DumpstateServiceTest, SetListenerWithSectionDetails) { + sp listener(new DumpstateListenerMock()); + sp token; + Dumpstate::GetInstance().listener_ = nullptr; + EXPECT_TRUE( + dss.setListener("whatever", listener, /* getSectionDetails = */ true, &token).isOk()); + ASSERT_THAT(token, NotNull()); + EXPECT_THAT(Dumpstate::GetInstance().listener_name_, StrEq("whatever")); + EXPECT_TRUE(Dumpstate::GetInstance().report_section_); } class ProgressTest : public DumpstateBaseTest { -- cgit v1.2.3-59-g8ed1b From 4a0a877c6d9b8aa2225a791329c2da0f7e8bdbe9 Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Tue, 5 Dec 2017 16:22:49 -0800 Subject: Use logcat -b kernel for bug reports One of the main advantages of the kernel log buffer is that _spammy_ selinux violations are suppressed there, and instead land in events and main log buffers for all to view. This gives the kernel log buffer a longer logspan and better signal to noise ratio than an equivalently sized dmesg buffer. The other advantage is the CLOCK_REALTIME usage, that makes it easier to correlate kernel and user space activities in the bugreport. Bug: 30736473 Test: 1) adb bugreport 2) adb shell setprop ro.logd.kernel false 3) adb bugreport Then compare the two resulting bug reports (open both in Android Bug Tool). Observe that timestamps are seconds since bootup in the second bugreport, and actual time (human-readable) in the first bugreport. Change-Id: I968de323833dda97ded4ecc454e12220d4bd3021 --- cmds/dumpstate/dumpstate.cpp | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 6f78141752..93f8d4354b 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -833,6 +833,17 @@ static void DoKmsg() { static const long MINIMUM_LOGCAT_TIMEOUT_MS = 50000; +static void DoKernelLogcat() { + unsigned long timeout_ms = logcat_timeout("kernel"); + if (timeout_ms < MINIMUM_LOGCAT_TIMEOUT_MS) { + timeout_ms = MINIMUM_LOGCAT_TIMEOUT_MS; + } + RunCommand( + "KERNEL LOG", + {"logcat", "-b", "kernel", "-v", "threadtime", "-v", "printable", "-v", "uid", "-d", "*:v"}, + CommandOptions::WithTimeoutInMs(timeout_ms).Build()); +} + static void DoLogcat() { unsigned long timeout_ms; // DumpFile("EVENT LOG TAGS", "/etc/event-log-tags"); @@ -848,25 +859,24 @@ static void DoLogcat() { if (timeout_ms < MINIMUM_LOGCAT_TIMEOUT_MS) { timeout_ms = MINIMUM_LOGCAT_TIMEOUT_MS; } - RunCommand("EVENT LOG", - {"logcat", "-b", "events", "-v", "threadtime", "-v", "printable", "-v", "uid", - "-d", "*:v"}, - CommandOptions::WithTimeoutInMs(timeout_ms).Build()); + RunCommand( + "EVENT LOG", + {"logcat", "-b", "events", "-v", "threadtime", "-v", "printable", "-v", "uid", "-d", "*:v"}, + CommandOptions::WithTimeoutInMs(timeout_ms).Build()); timeout_ms = logcat_timeout("radio"); if (timeout_ms < MINIMUM_LOGCAT_TIMEOUT_MS) { timeout_ms = MINIMUM_LOGCAT_TIMEOUT_MS; } - RunCommand("RADIO LOG", - {"logcat", "-b", "radio", "-v", "threadtime", "-v", "printable", "-v", "uid", - "-d", "*:v"}, - CommandOptions::WithTimeoutInMs(timeout_ms).Build()); + RunCommand( + "RADIO LOG", + {"logcat", "-b", "radio", "-v", "threadtime", "-v", "printable", "-v", "uid", "-d", "*:v"}, + CommandOptions::WithTimeoutInMs(timeout_ms).Build()); RunCommand("LOG STATISTICS", {"logcat", "-b", "all", "-S"}); /* kernels must set CONFIG_PSTORE_PMSG, slice up pstore with device tree */ - RunCommand("LAST LOGCAT", - {"logcat", "-L", "-b", "all", "-v", "threadtime", "-v", "printable", "-v", "uid", - "-d", "*:v"}); + RunCommand("LAST LOGCAT", {"logcat", "-L", "-b", "all", "-v", "threadtime", "-v", "printable", + "-v", "uid", "-d", "*:v"}); } static void DumpIpTablesAsRoot() { @@ -1187,7 +1197,12 @@ static void dumpstate() { RunCommand("LSMOD", {"lsmod"}); } - do_dmesg(); + if (__android_logger_property_get_bool( + "ro.logd.kernel", BOOL_DEFAULT_TRUE | BOOL_DEFAULT_FLAG_ENG | BOOL_DEFAULT_FLAG_SVELTE)) { + DoKernelLogcat(); + } else { + do_dmesg(); + } RunCommand("LIST OF OPEN FILES", {"lsof"}, CommandOptions::AS_ROOT); for_each_pid(do_showmap, "SMAPS OF ALL PROCESSES"); -- cgit v1.2.3-59-g8ed1b From c81cd3c115443fdd162446992e040ca45e7e4c51 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Thu, 18 Jan 2018 14:36:26 -0800 Subject: Dumpstate: don't dump HAL super classes. Previously, if 'a.h.f@1.2::IFoo' was registered, dumpstate woudl dump it three times as 1.0, 1.1, 1.2. Fixes: 72123369 Test: bugreport Change-Id: I121cf5b6ca5d3fe65e3922b07883320975fe5e0c --- cmds/dumpstate/dumpstate.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 93f8d4354b..1a0e4569a9 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1136,7 +1136,7 @@ static void DumpHals() { } RunCommandToFd(fd, "", - {"lshal", "debug", interface}, + {"lshal", "debug", "-E", interface}, CommandOptions::WithTimeout(2).AsRootIfAvailable().Build()); bool empty = 0 == lseek(fd, 0, SEEK_END); -- cgit v1.2.3-59-g8ed1b From be3bbc17db0fd09de2b2134a2a9c32ea7357db0e Mon Sep 17 00:00:00 2001 From: Yao Chen Date: Wed, 17 Jan 2018 16:31:10 -0800 Subject: Add STATS LOG section to dumpstate/bugreport "stats" is a new logd buffer for computing android metrics by statsd (go/westworld-design). The default size of stats buffer is 64K. Bug: 72123656 Test: manual Change-Id: I71611b26458b77deb39954d27da1c5f4cf3bcfb0 --- cmds/dumpstate/dumpstate.cpp | 45 +++++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 24 deletions(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 93f8d4354b..fcf763b730 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -657,12 +657,18 @@ static int dump_stat_from_fd(const char *title __unused, const char *path, int f return 0; } -/* timeout in ms */ -static unsigned long logcat_timeout(const char *name) { - log_id_t id = android_name_to_log_id(name); - unsigned long property_size = __android_logger_get_buffer_size(id); - /* Engineering margin is ten-fold our guess */ - return 10 * (property_size + worst_write_perf) / worst_write_perf; +static const long MINIMUM_LOGCAT_TIMEOUT_MS = 50000; + +/* timeout in ms to read a list of buffers */ +static unsigned long logcat_timeout(const std::vector& buffers) { + unsigned long timeout_ms = 0; + for (const auto& buffer : buffers) { + log_id_t id = android_name_to_log_id(buffer.c_str()); + unsigned long property_size = __android_logger_get_buffer_size(id); + /* Engineering margin is ten-fold our guess */ + timeout_ms += 10 * (property_size + worst_write_perf) / worst_write_perf; + } + return timeout_ms > MINIMUM_LOGCAT_TIMEOUT_MS ? timeout_ms : MINIMUM_LOGCAT_TIMEOUT_MS; } void Dumpstate::PrintHeader() const { @@ -831,13 +837,8 @@ static void DoKmsg() { } } -static const long MINIMUM_LOGCAT_TIMEOUT_MS = 50000; - static void DoKernelLogcat() { - unsigned long timeout_ms = logcat_timeout("kernel"); - if (timeout_ms < MINIMUM_LOGCAT_TIMEOUT_MS) { - timeout_ms = MINIMUM_LOGCAT_TIMEOUT_MS; - } + unsigned long timeout_ms = logcat_timeout({"kernel"}); RunCommand( "KERNEL LOG", {"logcat", "-b", "kernel", "-v", "threadtime", "-v", "printable", "-v", "uid", "-d", "*:v"}, @@ -848,25 +849,21 @@ static void DoLogcat() { unsigned long timeout_ms; // DumpFile("EVENT LOG TAGS", "/etc/event-log-tags"); // calculate timeout - timeout_ms = logcat_timeout("main") + logcat_timeout("system") + logcat_timeout("crash"); - if (timeout_ms < MINIMUM_LOGCAT_TIMEOUT_MS) { - timeout_ms = MINIMUM_LOGCAT_TIMEOUT_MS; - } + timeout_ms = logcat_timeout({"main", "system", "crash"}); RunCommand("SYSTEM LOG", {"logcat", "-v", "threadtime", "-v", "printable", "-v", "uid", "-d", "*:v"}, CommandOptions::WithTimeoutInMs(timeout_ms).Build()); - timeout_ms = logcat_timeout("events"); - if (timeout_ms < MINIMUM_LOGCAT_TIMEOUT_MS) { - timeout_ms = MINIMUM_LOGCAT_TIMEOUT_MS; - } + timeout_ms = logcat_timeout({"events"}); RunCommand( "EVENT LOG", {"logcat", "-b", "events", "-v", "threadtime", "-v", "printable", "-v", "uid", "-d", "*:v"}, CommandOptions::WithTimeoutInMs(timeout_ms).Build()); - timeout_ms = logcat_timeout("radio"); - if (timeout_ms < MINIMUM_LOGCAT_TIMEOUT_MS) { - timeout_ms = MINIMUM_LOGCAT_TIMEOUT_MS; - } + timeout_ms = logcat_timeout({"stats"}); + RunCommand( + "STATS LOG", + {"logcat", "-b", "stats", "-v", "threadtime", "-v", "printable", "-v", "uid", "-d", "*:v"}, + CommandOptions::WithTimeoutInMs(timeout_ms).Build()); + timeout_ms = logcat_timeout({"radio"}); RunCommand( "RADIO LOG", {"logcat", "-b", "radio", "-v", "threadtime", "-v", "printable", "-v", "uid", "-d", "*:v"}, -- cgit v1.2.3-59-g8ed1b From e97d6127dd73f8c9f4a60e33dd9a701ece47716d Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Thu, 18 Jan 2018 13:58:56 -0800 Subject: Add support for proto dumps in dumpstate - Call dumpsys library directly from dumpstate. - Send section generation status (name, size and duration) via section progress reporter. - Add end to end smoke test for bugreport: - Checks if zipped bug report was generated without errors within a reasonable time of a reasonable size. - Checks if zipped bug report contains version file, main entry and some selected files from the device file system. - Checks if all sections in the bug report were generated without errors. - Checks if some sections were generated with a reasonable size. - Changes are gated by Bugreport version. Version will be updated in a subsequent cl. Bug: 67716082, 70154685 Test: Manually generate bugrepot (default version) and check for any issues Test: mmm -j56 frameworks/native/cmds/dumpstate && \ adb shell setprop dumpstate.version "2.0-dev-priority-dumps" && \ adb sync data && adb shell /data/nativetest/dumpstate_test/dumpstate_test && \ adb shell /data/nativetest64/dumpstate_test/dumpstate_test && \ adb shell /data/nativetest64/dumpstate_smoke_test/dumpstate_smoke_test && \ printf "\n\n#### ALL TESTS PASSED ####\n" Change-Id: If036699d0588f74ef8e84c56323126214857dbdd --- cmds/dumpstate/Android.bp | 52 ++--- cmds/dumpstate/dumpstate.cpp | 173 ++++++++++++++-- cmds/dumpstate/dumpstate.h | 8 +- cmds/dumpstate/tests/dumpstate_smoke_test.cpp | 286 ++++++++++++++++++++++++++ cmds/dumpsys/dumpsys.cpp | 2 +- cmds/dumpsys/dumpsys.h | 2 +- 6 files changed, 478 insertions(+), 45 deletions(-) create mode 100644 cmds/dumpstate/tests/dumpstate_smoke_test.cpp (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/Android.bp b/cmds/dumpstate/Android.bp index 35cff5f62e..562898dd48 100644 --- a/cmds/dumpstate/Android.bp +++ b/cmds/dumpstate/Android.bp @@ -14,7 +14,7 @@ // limitations under the License. cc_defaults { - name: "dumpstate_defaults", + name: "dumpstate_cflag_defaults", cflags: [ "-Wall", "-Werror", @@ -26,7 +26,7 @@ cc_defaults { cc_library_shared { name: "libdumpstateutil", - defaults: ["dumpstate_defaults"], + defaults: ["dumpstate_cflag_defaults"], vendor_available: true, vndk: { enabled: true, @@ -47,7 +47,7 @@ cc_library_shared { cc_library_shared { name: "libdumpstateaidl", - defaults: ["dumpstate_defaults"], + defaults: ["dumpstate_cflag_defaults"], shared_libs: [ "libbinder", "libutils", @@ -63,9 +63,9 @@ cc_library_shared { ], } -cc_binary { - name: "dumpstate", - defaults: ["dumpstate_defaults"], +cc_defaults { + name: "dumpstate_defaults", + defaults: ["dumpstate_cflag_defaults"], shared_libs: [ "android.hardware.dumpstate@1.0", "libziparchive", @@ -82,12 +82,22 @@ cc_binary { "libutils", ], srcs: [ - "DumpstateInternal.cpp", "DumpstateSectionReporter.cpp", "DumpstateService.cpp", - "main.cpp", "utils.cpp", + ], + static_libs: [ + "libdumpsys", + "libserviceutils" + ], +} + +cc_binary { + name: "dumpstate", + defaults: ["dumpstate_defaults"], + srcs: [ "dumpstate.cpp", + "main.cpp", ], init_rc: ["dumpstate.rc"], } @@ -95,24 +105,18 @@ cc_binary { cc_test { name: "dumpstate_test", defaults: ["dumpstate_defaults"], - shared_libs: [ - "libziparchive", - "libbase", - "libbinder", - "libcutils", - "libdebuggerd_client", - "libdumpstateaidl", - "libdumpstateutil", - "libhidlbase", - "libhidltransport", - "liblog", - "libutils", - ], srcs: [ - "DumpstateInternal.cpp", - "DumpstateService.cpp", - "utils.cpp", "tests/dumpstate_test.cpp", ], static_libs: ["libgmock"], } + +cc_test { + name: "dumpstate_smoke_test", + defaults: ["dumpstate_defaults"], + srcs: [ + "dumpstate.cpp", + "tests/dumpstate_smoke_test.cpp", + ], + static_libs: ["libgmock"], +} \ No newline at end of file diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 93f8d4354b..1a25335861 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -46,22 +47,39 @@ #include #include #include +#include #include #include #include #include - +#include #include "DumpstateInternal.h" +#include "DumpstateSectionReporter.h" #include "DumpstateService.h" #include "dumpstate.h" using ::android::hardware::dumpstate::V1_0::IDumpstateDevice; +using ::std::literals::chrono_literals::operator""ms; +using ::std::literals::chrono_literals::operator""s; // TODO: remove once moved to namespace +using android::defaultServiceManager; +using android::Dumpsys; +using android::INVALID_OPERATION; +using android::IServiceManager; +using android::OK; +using android::sp; +using android::status_t; +using android::String16; +using android::String8; +using android::TIMED_OUT; +using android::UNKNOWN_ERROR; +using android::Vector; using android::os::dumpstate::CommandOptions; using android::os::dumpstate::DumpFileToFd; -using android::os::dumpstate::PropertiesHelper; +using android::os::dumpstate::DumpstateSectionReporter; using android::os::dumpstate::GetPidByName; +using android::os::dumpstate::PropertiesHelper; /* read before root is shed */ static char cmdline_buf[16384] = "(unknown)"; @@ -127,6 +145,8 @@ static const std::string ZIP_ROOT_DIR = "FS"; // Must be hardcoded because dumpstate HAL implementation need SELinux access to it static const std::string kDumpstateBoardPath = "/bugreports/"; +static const std::string kProtoPath = "proto/"; +static const std::string kProtoExt = ".proto"; static const std::string kDumpstateBoardFiles[] = { "dumpstate_board.txt", "dumpstate_board.bin" @@ -221,7 +241,7 @@ static bool AddDumps(const std::vector::const_iterator start, } if (ds.IsZipping() && add_to_zip) { - if (!ds.AddZipEntryFromFd(ZIP_ROOT_DIR + name, fd)) { + if (ds.AddZipEntryFromFd(ZIP_ROOT_DIR + name, fd, /* timeout = */ 0ms) != OK) { MYLOGE("Unable to add %s to zip file, addZipEntryFromFd failed\n", name.c_str()); } } else { @@ -708,11 +728,12 @@ static const std::set PROBLEMATIC_FILE_EXTENSIONS = { ".shb", ".sys", ".vb", ".vbe", ".vbs", ".vxd", ".wsc", ".wsf", ".wsh" }; -bool Dumpstate::AddZipEntryFromFd(const std::string& entry_name, int fd) { +status_t Dumpstate::AddZipEntryFromFd(const std::string& entry_name, int fd, + std::chrono::milliseconds timeout = 0ms) { if (!IsZipping()) { MYLOGD("Not adding zip entry %s from fd because it's not a zipped bugreport\n", entry_name.c_str()); - return false; + return INVALID_OPERATION; } std::string valid_name = entry_name; @@ -734,32 +755,55 @@ bool Dumpstate::AddZipEntryFromFd(const std::string& entry_name, int fd) { if (err != 0) { MYLOGE("zip_writer_->StartEntryWithTime(%s): %s\n", valid_name.c_str(), ZipWriter::ErrorCodeString(err)); - return false; + return UNKNOWN_ERROR; } + auto start = std::chrono::steady_clock::now(); + auto end = start + timeout; + struct pollfd pfd = {fd, POLLIN}; std::vector buffer(65536); while (1) { + if (timeout.count() > 0) { + // lambda to recalculate the timeout. + auto time_left_ms = [end]() { + auto now = std::chrono::steady_clock::now(); + auto diff = std::chrono::duration_cast(end - now); + return std::max(diff.count(), 0LL); + }; + + int rc = TEMP_FAILURE_RETRY(poll(&pfd, 1, time_left_ms())); + if (rc < 0) { + MYLOGE("Error in poll while adding from fd to zip entry %s:%s", entry_name.c_str(), + strerror(errno)); + return -errno; + } else if (rc == 0) { + MYLOGE("Timed out adding from fd to zip entry %s:%s Timeout:%lldms", + entry_name.c_str(), strerror(errno), timeout.count()); + return TIMED_OUT; + } + } + ssize_t bytes_read = TEMP_FAILURE_RETRY(read(fd, buffer.data(), buffer.size())); if (bytes_read == 0) { break; } else if (bytes_read == -1) { MYLOGE("read(%s): %s\n", entry_name.c_str(), strerror(errno)); - return false; + return -errno; } err = zip_writer_->WriteBytes(buffer.data(), bytes_read); if (err) { MYLOGE("zip_writer_->WriteBytes(): %s\n", ZipWriter::ErrorCodeString(err)); - return false; + return UNKNOWN_ERROR; } } err = zip_writer_->FinishEntry(); if (err != 0) { MYLOGE("zip_writer_->FinishEntry(): %s\n", ZipWriter::ErrorCodeString(err)); - return false; + return UNKNOWN_ERROR; } - return true; + return OK; } bool Dumpstate::AddZipEntry(const std::string& entry_name, const std::string& entry_path) { @@ -770,12 +814,12 @@ bool Dumpstate::AddZipEntry(const std::string& entry_name, const std::string& en return false; } - return AddZipEntryFromFd(entry_name, fd.get()); + return (AddZipEntryFromFd(entry_name, fd.get()) == OK); } /* adds a file to the existing zipped bugreport */ static int _add_file_from_fd(const char* title __attribute__((unused)), const char* path, int fd) { - return ds.AddZipEntryFromFd(ZIP_ROOT_DIR + path, fd) ? 0 : 1; + return (ds.AddZipEntryFromFd(ZIP_ROOT_DIR + path, fd) == OK) ? 0 : 1; } void Dumpstate::AddDir(const std::string& dir, bool recursive) { @@ -1069,11 +1113,97 @@ static void DumpIpAddrAndRules() { RunCommand("IP RULES v6", {"ip", "-6", "rule", "show"}); } +void RunDumpsysText(const std::string& title, int priority, std::chrono::milliseconds timeout, + std::chrono::milliseconds service_timeout) { + sp sm = defaultServiceManager(); + Dumpsys dumpsys(sm.get()); + DurationReporter duration_reporter(title); + Vector args; + Dumpsys::setServiceArgs(args, /* asProto = */ false, priority); + + if (!title.empty()) { + dprintf(STDOUT_FILENO, "------ %s (%s) ------\n", title.c_str(), "/system/bin/dumpsys"); + fsync(STDOUT_FILENO); + } + + auto start = std::chrono::steady_clock::now(); + Vector services = dumpsys.listServices(priority, /* supports_proto = */ false); + for (const String16& service : services) { + std::string path(title); + path.append(" - ").append(String8(service).c_str()); + DumpstateSectionReporter section_reporter(path, ds.listener_, ds.report_section_); + size_t bytes_written = 0; + status_t status = dumpsys.startDumpThread(service, args); + if (status == OK) { + dumpsys.writeDumpHeader(STDOUT_FILENO, service, priority); + std::chrono::duration elapsed_seconds; + status = dumpsys.writeDump(STDOUT_FILENO, service, service_timeout, + /* as_proto = */ false, elapsed_seconds, bytes_written); + section_reporter.setSize(bytes_written); + dumpsys.writeDumpFooter(STDOUT_FILENO, service, elapsed_seconds); + bool dump_complete = (status == OK); + dumpsys.stopDumpThread(dump_complete); + } + section_reporter.setStatus(status); + + auto elapsed_duration = std::chrono::duration_cast( + std::chrono::steady_clock::now() - start); + if (elapsed_duration > timeout) { + MYLOGE("*** command '%s' timed out after %llums\n", title.c_str(), + elapsed_duration.count()); + break; + } + } +} + +void RunDumpsysProto(const std::string& title, int priority, std::chrono::milliseconds timeout, + std::chrono::milliseconds service_timeout) { + sp sm = defaultServiceManager(); + Dumpsys dumpsys(sm.get()); + Vector args; + Dumpsys::setServiceArgs(args, /* asProto = */ true, priority); + DurationReporter duration_reporter(title); + + auto start = std::chrono::steady_clock::now(); + Vector services = dumpsys.listServices(priority, /* supports_proto = */ true); + for (const String16& service : services) { + std::string path(kProtoPath); + path.append(String8(service).c_str()); + if (priority == IServiceManager::DUMP_FLAG_PRIORITY_CRITICAL) { + path.append("_CRITICAL"); + } else if (priority == IServiceManager::DUMP_FLAG_PRIORITY_HIGH) { + path.append("_HIGH"); + } + path.append(kProtoExt); + DumpstateSectionReporter section_reporter(path, ds.listener_, ds.report_section_); + status_t status = dumpsys.startDumpThread(service, args); + if (status == OK) { + status = ds.AddZipEntryFromFd(path, dumpsys.getDumpFd(), service_timeout); + bool dumpTerminated = (status == OK); + dumpsys.stopDumpThread(dumpTerminated); + } + ZipWriter::FileEntry file_entry; + ds.zip_writer_->GetLastEntry(&file_entry); + section_reporter.setSize(file_entry.compressed_size); + section_reporter.setStatus(status); + + auto elapsed_duration = std::chrono::duration_cast( + std::chrono::steady_clock::now() - start); + if (elapsed_duration > timeout) { + MYLOGE("*** command '%s' timed out after %llums\n", title.c_str(), + elapsed_duration.count()); + break; + } + } +} + // Runs dumpsys on services that must dump first and and will take less than 100ms to dump. static void RunDumpsysCritical() { if (ds.CurrentVersionSupportsPriorityDumps()) { - RunDumpsys("DUMPSYS CRITICAL", {"--priority", "CRITICAL"}, - CommandOptions::WithTimeout(5).DropRoot().Build()); + RunDumpsysText("DUMPSYS CRITICAL", IServiceManager::DUMP_FLAG_PRIORITY_CRITICAL, + /* timeout= */ 5s, /* service_timeout= */ 500ms); + RunDumpsysProto("DUMPSYS CRITICAL PROTO", IServiceManager::DUMP_FLAG_PRIORITY_CRITICAL, + /* timeout= */ 5s, /* service_timeout= */ 500ms); } else { RunDumpsys("DUMPSYS MEMINFO", {"meminfo", "-a"}, CommandOptions::WithTimeout(90).DropRoot().Build()); @@ -1085,8 +1215,13 @@ static void RunDumpsysCritical() { // Runs dumpsys on services that must dump first but can take up to 250ms to dump. static void RunDumpsysHigh() { if (ds.CurrentVersionSupportsPriorityDumps()) { - RunDumpsys("DUMPSYS HIGH", {"--priority", "HIGH"}, - CommandOptions::WithTimeout(20).DropRoot().Build()); + // TODO meminfo takes ~10s, connectivity takes ~5sec to dump. They are both + // high priority. Reduce timeout once they are able to dump in a shorter time or + // moved to a parallel task. + RunDumpsysText("DUMPSYS HIGH", IServiceManager::DUMP_FLAG_PRIORITY_HIGH, + /* timeout= */ 90s, /* service_timeout= */ 30s); + RunDumpsysProto("DUMPSYS HIGH PROTO", IServiceManager::DUMP_FLAG_PRIORITY_HIGH, + /* timeout= */ 5s, /* service_timeout= */ 1s); } else { RunDumpsys("NETWORK DIAGNOSTICS", {"connectivity", "--diag"}); } @@ -1095,8 +1230,10 @@ static void RunDumpsysHigh() { // Runs dumpsys on services that must dump but can take up to 10s to dump. static void RunDumpsysNormal() { if (ds.CurrentVersionSupportsPriorityDumps()) { - RunDumpsys("DUMPSYS NORMAL", {"--priority", "NORMAL"}, - CommandOptions::WithTimeout(90).DropRoot().Build()); + RunDumpsysText("DUMPSYS", IServiceManager::DUMP_FLAG_PRIORITY_NORMAL, + /* timeout= */ 90s, /* service_timeout= */ 10s); + RunDumpsysProto("DUMPSYS PROTO", IServiceManager::DUMP_FLAG_PRIORITY_NORMAL, + /* timeout= */ 90s, /* service_timeout= */ 10s); } else { RunDumpsys("DUMPSYS", {"--skip", "meminfo", "cpuinfo"}, CommandOptions::WithTimeout(90).Build(), SEC_TO_MSEC(10)); diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h index 843c545e50..2554b6374b 100644 --- a/cmds/dumpstate/dumpstate.h +++ b/cmds/dumpstate/dumpstate.h @@ -226,8 +226,14 @@ class Dumpstate { /* * Adds a new entry to the existing zip file. + * + * |entry_name| destination path of the new entry. + * |fd| file descriptor to read from. + * |timeout| timeout to terminate the read if not completed. Set + * value of 0s (default) to disable timeout. */ - bool AddZipEntryFromFd(const std::string& entry_name, int fd); + android::status_t AddZipEntryFromFd(const std::string& entry_name, int fd, + std::chrono::milliseconds timeout); /* * Adds a text entry entry to the existing zip file. diff --git a/cmds/dumpstate/tests/dumpstate_smoke_test.cpp b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp new file mode 100644 index 0000000000..61a5ef5b7d --- /dev/null +++ b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp @@ -0,0 +1,286 @@ +/* + * Copyright (C) 2018 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 +#include + +#include +#include + +#include +#include +#include + +#include "dumpstate.h" + +#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0])) + +namespace android { +namespace os { +namespace dumpstate { + +using ::testing::Test; +using ::std::literals::chrono_literals::operator""s; + +struct SectionInfo { + std::string name; + status_t status; + int32_t size_bytes; + int32_t duration_ms; +}; + +/** + * Listens to bugreport progress and updates the user by writing the progress to STDOUT. All the + * section details generated by dumpstate are added to a vector to be used by Tests later. + */ +class DumpstateListener : public IDumpstateListener { + public: + int outFd_, max_progress_; + std::shared_ptr> sections_; + DumpstateListener(int fd, std::shared_ptr> sections) + : outFd_(fd), max_progress_(5000), sections_(sections) { + } + binder::Status onProgressUpdated(int32_t progress) override { + dprintf(outFd_, "\rIn progress %d/%d", progress, max_progress_); + return binder::Status::ok(); + } + binder::Status onMaxProgressUpdated(int32_t max_progress) override { + max_progress_ = max_progress; + return binder::Status::ok(); + } + binder::Status onSectionComplete(const ::std::string& name, int32_t status, int32_t size_bytes, + int32_t duration_ms) override { + sections_->push_back({name, status, size_bytes, duration_ms}); + return binder::Status::ok(); + } + IBinder* onAsBinder() override { + return nullptr; + } +}; + +/** + * Generates bug report and provide access to the bug report file and other info for other tests. + * Since bug report generation is slow, the bugreport is only generated once. + */ +class ZippedBugreportGenerationTest : public Test { + public: + static std::shared_ptr> sections; + static Dumpstate& ds; + static std::chrono::milliseconds duration; + static void SetUpTestCase() { + property_set("dumpstate.options", "bugreportplus"); + // clang-format off + char* argv[] = { + (char*)"dumpstate", + (char*)"-d", + (char*)"-z", + (char*)"-B", + (char*)"-o", + (char*)dirname(android::base::GetExecutablePath().c_str()) + }; + // clang-format on + sp listener(new DumpstateListener(dup(fileno(stdout)), sections)); + ds.listener_ = listener; + ds.listener_name_ = "Smokey"; + ds.report_section_ = true; + auto start = std::chrono::steady_clock::now(); + run_main(ARRAY_SIZE(argv), argv); + auto end = std::chrono::steady_clock::now(); + duration = std::chrono::duration_cast(end - start); + } + + static const char* getZipFilePath() { + return ds.GetPath(".zip").c_str(); + } +}; +std::shared_ptr> ZippedBugreportGenerationTest::sections = + std::make_shared>(); +Dumpstate& ZippedBugreportGenerationTest::ds = Dumpstate::GetInstance(); +std::chrono::milliseconds ZippedBugreportGenerationTest::duration = 0s; + +TEST_F(ZippedBugreportGenerationTest, IsGeneratedWithoutErrors) { + EXPECT_EQ(access(getZipFilePath(), F_OK), 0); +} + +TEST_F(ZippedBugreportGenerationTest, Is3MBto30MBinSize) { + struct stat st; + EXPECT_EQ(stat(getZipFilePath(), &st), 0); + EXPECT_GE(st.st_size, 3000000 /* 3MB */); + EXPECT_LE(st.st_size, 30000000 /* 30MB */); +} + +TEST_F(ZippedBugreportGenerationTest, TakesBetween30And150Seconds) { + EXPECT_GE(duration, 30s) << "Expected completion in more than 30s. Actual time " + << duration.count() << " s."; + EXPECT_LE(duration, 150s) << "Expected completion in less than 150s. Actual time " + << duration.count() << " s."; +} + +/** + * Run tests on contents of zipped bug report. + */ +class ZippedBugReportContentsTest : public Test { + public: + ZipArchiveHandle handle; + void SetUp() { + ASSERT_EQ(OpenArchive(ZippedBugreportGenerationTest::getZipFilePath(), &handle), 0); + } + void TearDown() { + CloseArchive(handle); + } + + void FileExists(const char* filename, uint32_t minsize, uint32_t maxsize) { + ZipEntry entry; + EXPECT_EQ(FindEntry(handle, ZipString(filename), &entry), 0); + EXPECT_GT(entry.uncompressed_length, minsize); + EXPECT_LT(entry.uncompressed_length, maxsize); + } +}; + +TEST_F(ZippedBugReportContentsTest, ContainsMainEntry) { + ZipEntry mainEntryLoc; + // contains main entry name file + EXPECT_EQ(FindEntry(handle, ZipString("main_entry.txt"), &mainEntryLoc), 0); + + char* buf = new char[mainEntryLoc.uncompressed_length]; + ExtractToMemory(handle, &mainEntryLoc, (uint8_t*)buf, mainEntryLoc.uncompressed_length); + delete[] buf; + + // contains main entry file + FileExists(buf, 1000000U, 50000000U); +} + +TEST_F(ZippedBugReportContentsTest, ContainsVersion) { + ZipEntry entry; + // contains main entry name file + EXPECT_EQ(FindEntry(handle, ZipString("version.txt"), &entry), 0); + + char* buf = new char[entry.uncompressed_length + 1]; + ExtractToMemory(handle, &entry, (uint8_t*)buf, entry.uncompressed_length); + buf[entry.uncompressed_length] = 0; + EXPECT_STREQ(buf, ZippedBugreportGenerationTest::ds.version_.c_str()); + delete[] buf; +} + +TEST_F(ZippedBugReportContentsTest, ContainsBoardSpecificFiles) { + FileExists("dumpstate_board.bin", 1000000U, 80000000U); + FileExists("dumpstate_board.txt", 100000U, 1000000U); +} + +// Spot check on some files pulled from the file system +TEST_F(ZippedBugReportContentsTest, ContainsSomeFileSystemFiles) { + // FS/proc/*/mountinfo size > 0 + FileExists("FS/proc/1/mountinfo", 0U, 100000U); + + // FS/data/misc/profiles/cur/0/*/primary.prof size > 0 + FileExists("FS/data/misc/profiles/cur/0/com.android.phone/primary.prof", 0U, 100000U); +} + +/** + * Runs tests on section data generated by dumpstate and captured by DumpstateListener. + */ +class BugreportSectionTest : public Test { + public: + int numMatches(const std::string& substring) { + int matches = 0; + for (auto const& section : *ZippedBugreportGenerationTest::sections) { + if (section.name.find(substring) != std::string::npos) { + matches++; + } + } + return matches; + } + void SectionExists(const std::string& sectionName, int minsize) { + for (auto const& section : *ZippedBugreportGenerationTest::sections) { + if (sectionName == section.name) { + EXPECT_GE(section.size_bytes, minsize); + return; + } + } + FAIL() << sectionName << " not found."; + } +}; + +// Test all sections are generated without timeouts or errors +TEST_F(BugreportSectionTest, GeneratedWithoutErrors) { + for (auto const& section : *ZippedBugreportGenerationTest::sections) { + EXPECT_EQ(section.status, 0) << section.name << " failed with status " << section.status; + } +} + +TEST_F(BugreportSectionTest, Atleast3CriticalDumpsysSectionsGenerated) { + int numSections = numMatches("DUMPSYS CRITICAL"); + EXPECT_GE(numSections, 3); +} + +TEST_F(BugreportSectionTest, Atleast2HighDumpsysSectionsGenerated) { + int numSections = numMatches("DUMPSYS HIGH"); + EXPECT_GE(numSections, 2); +} + +TEST_F(BugreportSectionTest, Atleast50NormalDumpsysSectionsGenerated) { + int allSections = numMatches("DUMPSYS"); + int criticalSections = numMatches("DUMPSYS CRITICAL"); + int highSections = numMatches("DUMPSYS HIGH"); + int normalSections = allSections - criticalSections - highSections; + + EXPECT_GE(normalSections, 50) << "Total sections less than 50 (Critical:" << criticalSections + << "High:" << highSections << "Normal:" << normalSections << ")"; +} + +TEST_F(BugreportSectionTest, Atleast1ProtoDumpsysSectionGenerated) { + int numSections = numMatches("proto/"); + EXPECT_GE(numSections, 1); +} + +// Test if some critical sections are being generated. +TEST_F(BugreportSectionTest, CriticalSurfaceFlingerSectionGenerated) { + SectionExists("DUMPSYS CRITICAL - SurfaceFlinger", /* bytes= */ 10000); +} + +TEST_F(BugreportSectionTest, ActivitySectionsGenerated) { + SectionExists("DUMPSYS CRITICAL - activity", /* bytes= */ 5000); + SectionExists("DUMPSYS - activity", /* bytes= */ 10000); +} + +TEST_F(BugreportSectionTest, CpuinfoSectionGenerated) { + SectionExists("DUMPSYS CRITICAL - cpuinfo", /* bytes= */ 1000); +} + +TEST_F(BugreportSectionTest, WindowSectionGenerated) { + SectionExists("DUMPSYS CRITICAL - window", /* bytes= */ 20000); +} + +TEST_F(BugreportSectionTest, ConnectivitySectionsGenerated) { + SectionExists("DUMPSYS HIGH - connectivity", /* bytes= */ 5000); + SectionExists("DUMPSYS - connectivity", /* bytes= */ 5000); +} + +TEST_F(BugreportSectionTest, MeminfoSectionGenerated) { + SectionExists("DUMPSYS HIGH - meminfo", /* bytes= */ 100000); +} + +TEST_F(BugreportSectionTest, BatteryStatsSectionGenerated) { + SectionExists("DUMPSYS - batterystats", /* bytes= */ 1000); +} + +TEST_F(BugreportSectionTest, WifiSectionGenerated) { + SectionExists("DUMPSYS - wifi", /* bytes= */ 100000); +} + +} // namespace dumpstate +} // namespace os +} // namespace android diff --git a/cmds/dumpsys/dumpsys.cpp b/cmds/dumpsys/dumpsys.cpp index ae0cc01ee4..ca7d95e152 100644 --- a/cmds/dumpsys/dumpsys.cpp +++ b/cmds/dumpsys/dumpsys.cpp @@ -283,7 +283,7 @@ Vector Dumpsys::listServices(int priorityFilterFlags, bool filterByPro return services; } -void Dumpsys::setServiceArgs(Vector& args, bool asProto, int priorityFlags) const { +void Dumpsys::setServiceArgs(Vector& args, bool asProto, int priorityFlags) { if ((priorityFlags == IServiceManager::DUMP_FLAG_PRIORITY_ALL) || (priorityFlags == IServiceManager::DUMP_FLAG_PRIORITY_NORMAL)) { args.add(String16("-a")); diff --git a/cmds/dumpsys/dumpsys.h b/cmds/dumpsys/dumpsys.h index 1d78aa426b..84f3b0236e 100644 --- a/cmds/dumpsys/dumpsys.h +++ b/cmds/dumpsys/dumpsys.h @@ -49,7 +49,7 @@ class Dumpsys { * @param priorityFlags indicates priority of dump by passing additional priority args * to the service */ - void setServiceArgs(Vector& args, bool asProto, int priorityFlags) const; + static void setServiceArgs(Vector& args, bool asProto, int priorityFlags); /** * Starts a thread to connect to a service and get its dump output. The thread redirects -- cgit v1.2.3-59-g8ed1b From 253dad4af7ed9df588d5b1403f7c95f9610c72ef Mon Sep 17 00:00:00 2001 From: mukesh agrawal Date: Tue, 23 Jan 2018 21:59:59 -0800 Subject: dumpstate: add support for wifi-specific bug report Under certain (limited) conditions, we would like to automatically trigger a bug report to help diagnose Wifi problems. The regular bugreport can't be used for this purpose, as it takes a long time to generate. To support lightweight auto-bug generation, add a Wifi-specific bugreport. Bug: 69934148 Test: manual Manual test ----------- - boot walleye - enable wifi - connect to googleguest $ adb root $ adb shell pkill -f 'wifi@' - notice "Bug report #1 captured" notification $ adb pull '/data/user_de/0/com.android.shell/files/bugreports $ unzip bugreports/*.zip $ grep "was the duration of 'DUMPSTATE'" bugreport*.txt -> ------ 1.548s was the duration of 'DUMPSTATE' ------ Change-Id: I9320a9ef0ca841508538f58dbbcb02eeca290001 --- cmds/dumpstate/dumpstate.cpp | 41 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 4 deletions(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index ef5064cba6..cce0579865 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1463,10 +1463,8 @@ static void dumpstate() { printf("========================================================\n"); } -// This method collects dumpsys for telephony debugging only -static void DumpstateTelephonyOnly() { - DurationReporter duration_reporter("DUMPSTATE"); - +// This method collects common dumpsys for telephony and wifi +static void DumpstateRadioCommon() { DumpIpTablesAsRoot(); if (!DropRootUser()) { @@ -1482,6 +1480,13 @@ static void DumpstateTelephonyOnly() { RunDumpsys("NETWORK DIAGNOSTICS", {"connectivity", "--diag"}, CommandOptions::WithTimeout(10).Build()); +} + +// This method collects dumpsys for telephony debugging only +static void DumpstateTelephonyOnly() { + DurationReporter duration_reporter("DUMPSTATE"); + + DumpstateRadioCommon(); RunCommand("SYSTEM PROPERTIES", {"getprop"}); @@ -1505,6 +1510,26 @@ static void DumpstateTelephonyOnly() { printf("========================================================\n"); } +// This method collects dumpsys for wifi debugging only +static void DumpstateWifiOnly() { + DurationReporter duration_reporter("DUMPSTATE"); + + DumpstateRadioCommon(); + + printf("========================================================\n"); + printf("== Android Framework Services\n"); + printf("========================================================\n"); + + RunDumpsys("DUMPSYS", {"connectivity"}, CommandOptions::WithTimeout(90).Build(), + SEC_TO_MSEC(10)); + RunDumpsys("DUMPSYS", {"wifi"}, CommandOptions::WithTimeout(90).Build(), + SEC_TO_MSEC(10)); + + printf("========================================================\n"); + printf("== dumpstate: done (id %d)\n", ds.id_); + printf("========================================================\n"); +} + void Dumpstate::DumpstateBoard() { DurationReporter duration_reporter("dumpstate_board()"); printf("========================================================\n"); @@ -1751,6 +1776,7 @@ int run_main(int argc, char* argv[]) { bool show_header_only = false; bool do_start_service = false; bool telephony_only = false; + bool wifi_only = false; int dup_stdout_fd; int dup_stderr_fd; @@ -1823,6 +1849,9 @@ int run_main(int argc, char* argv[]) { do_zip_file = 1; } else if (ds.extra_options_ == "bugreporttelephony") { telephony_only = true; + } else if (ds.extra_options_ == "bugreportwifi") { + wifi_only = true; + do_zip_file = 1; } else { MYLOGE("Unknown extra option: %s\n", ds.extra_options_.c_str()); } @@ -1943,6 +1972,8 @@ int run_main(int argc, char* argv[]) { if (telephony_only) { ds.base_name_ += "-telephony"; + } else if (wifi_only) { + ds.base_name_ += "-wifi"; } if (do_fb) { @@ -2052,6 +2083,8 @@ int run_main(int argc, char* argv[]) { if (telephony_only) { DumpstateTelephonyOnly(); ds.DumpstateBoard(); + } else if (wifi_only) { + DumpstateWifiOnly(); } else { // Dumps systrace right away, otherwise it will be filled with unnecessary events. // First try to dump anrd trace if the daemon is running. Otherwise, dump -- cgit v1.2.3-59-g8ed1b From 64afc024d760e31f3f41e0c5cb8fc543c9392ef1 Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Thu, 1 Feb 2018 15:29:34 -0800 Subject: Flip bugreport version to 2.0 - Adds support for proto dumps - Adds support for priority service dumps - Change order of arguments when dumping Normal priority services - Adds a new DEFAULT priority for services which is treated the same as NORMAL priority but dumpsys does not send "--dump-priority" arguments to the service. Bug: 67716082, 27429130 Test: Manually generate bugreport (default version) and check for any issues Test: Load bugreport on ABT Test: mmm -j56 frameworks/native/cmds/dumpstate && \ adb sync data && adb shell /data/nativetest/dumpstate_test/dumpstate_test && \ adb shell /data/nativetest64/dumpstate_test/dumpstate_test && \ adb shell /data/nativetest64/dumpstate_smoke_test/dumpstate_smoke_test && \ adb shell /data/nativetest/dumpsys_test/dumpsys_test && \ adb shell /data/nativetest64/dumpsys_test/dumpsys_test && \ printf "\n\n#### ALL TESTS PASSED ####\n" Change-Id: Ie8761a2dd0425574b0d905752e1562196a1f7426 --- cmds/dumpstate/bugreport-format.md | 35 +++++++--- cmds/dumpstate/dumpstate.cpp | 96 ++++++++++++++-------------- cmds/dumpstate/dumpstate.h | 12 +--- cmds/dumpstate/utils.cpp | 4 -- cmds/dumpsys/dumpsys.cpp | 22 +++++-- cmds/dumpsys/tests/dumpsys_test.cpp | 59 +++++++++++++++++ libs/binder/include/binder/BinderService.h | 4 +- libs/binder/include/binder/IServiceManager.h | 17 +++-- 8 files changed, 165 insertions(+), 84 deletions(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/bugreport-format.md b/cmds/dumpstate/bugreport-format.md index dee89cf174..39e70d131b 100644 --- a/cmds/dumpstate/bugreport-format.md +++ b/cmds/dumpstate/bugreport-format.md @@ -59,18 +59,35 @@ files upon the end user’s request: On _Android O (Oreo)_, the following changes were made: - The ANR traces are added to the `FS` folder, typically under `FS/data/anr` (version `2.0-dev-split-anr`). -## Android P versions -On _Android P (PleaseMightyAndroidWhatsYourNextReleaseName?)_, the following changes were made: -- Dumpsys sections are dumped by priority (version `2.0-dev-priority-dumps`). - Supported priorities can be specified when registering framework services. Section headers are - changed to contain priority info. - `DUMPSYS` -> `DUMPSYS CRITICAL/HIGH/NORMAL` - `DUMP OF SERVICE ` -> `DUMP OF SERVICE CRITICAL/HIGH/NORMAL ` - Supported Priorities: +## Version 2.0 (Android P) +On _Android P_, the following changes were made: +- Framework services are dumped by priority. Supported priorities can be specified + when registering the service. If a service does not specify its priority, its + assumed to be NORMAL. + Supported priorities: - CRITICAL - services that must dump first, and fast (under 100ms). Ex: cpuinfo. - - HIGH - services that also must dump first, but can take longer (under 250ms) to dump. Ex: meminfo. + - HIGH - services that also must dump first, but can take longer (under 250ms) + to dump. Ex: meminfo. - NORMAL - services that have no rush to dump and can take a long time (under 10s). + Format changes: + - Two additional dumpsys sections are generated. The two new sections can be + identified by their HEADER `DUMPSYS CRITICAL` and `DUMPSYS HIGH`. + - Services in the new sections will have a new header containing the + priority. + `DUMP OF SERVICE CRITICAL ` and + `DUMP OF SERVICE HIGH `. + For example, cpuinfo will now move to `DUMPSYS CRITICAL` and will have a + header `DUMP OF SERVICE CRITICAL CPUINFO`. + +- Bug report will contain proto dumps from all supporting services. Support can be + specified when registering framework services. + Format changes: + - All protos will be generated into separate files per service, per priority. The files + will be stored in `proto/(_CRITICAL|_HIGH|).proto` + +- ANR trace feature has been pushed to version `3.0-dev-split-anr` + ## Intermediate versions During development, the versions will be suffixed with _-devX_ or _-devX-EXPERIMENTAL_FEATURE_, where _X_ is a number that increases as the diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 40566e0cfa..f17ac5b98d 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1110,20 +1110,14 @@ static void DumpIpAddrAndRules() { RunCommand("IP RULES v6", {"ip", "-6", "rule", "show"}); } -void RunDumpsysText(const std::string& title, int priority, std::chrono::milliseconds timeout, - std::chrono::milliseconds service_timeout) { +static void RunDumpsysTextByPriority(const std::string& title, int priority, + std::chrono::milliseconds timeout, + std::chrono::milliseconds service_timeout) { + auto start = std::chrono::steady_clock::now(); sp sm = defaultServiceManager(); Dumpsys dumpsys(sm.get()); - DurationReporter duration_reporter(title); Vector args; Dumpsys::setServiceArgs(args, /* asProto = */ false, priority); - - if (!title.empty()) { - dprintf(STDOUT_FILENO, "------ %s (%s) ------\n", title.c_str(), "/system/bin/dumpsys"); - fsync(STDOUT_FILENO); - } - - auto start = std::chrono::steady_clock::now(); Vector services = dumpsys.listServices(priority, /* supports_proto = */ false); for (const String16& service : services) { std::string path(title); @@ -1153,8 +1147,31 @@ void RunDumpsysText(const std::string& title, int priority, std::chrono::millise } } -void RunDumpsysProto(const std::string& title, int priority, std::chrono::milliseconds timeout, - std::chrono::milliseconds service_timeout) { +static void RunDumpsysText(const std::string& title, int priority, + std::chrono::milliseconds timeout, + std::chrono::milliseconds service_timeout) { + DurationReporter duration_reporter(title); + dprintf(STDOUT_FILENO, "------ %s (/system/bin/dumpsys) ------\n", title.c_str()); + fsync(STDOUT_FILENO); + RunDumpsysTextByPriority(title, priority, timeout, service_timeout); +} + +/* Dump all services registered with Normal or Default priority. */ +static void RunDumpsysTextNormalPriority(const std::string& title, + std::chrono::milliseconds timeout, + std::chrono::milliseconds service_timeout) { + DurationReporter duration_reporter(title); + dprintf(STDOUT_FILENO, "------ %s (/system/bin/dumpsys) ------\n", title.c_str()); + fsync(STDOUT_FILENO); + RunDumpsysTextByPriority(title, IServiceManager::DUMP_FLAG_PRIORITY_NORMAL, timeout, + service_timeout); + RunDumpsysTextByPriority(title, IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT, timeout, + service_timeout); +} + +static void RunDumpsysProto(const std::string& title, int priority, + std::chrono::milliseconds timeout, + std::chrono::milliseconds service_timeout) { sp sm = defaultServiceManager(); Dumpsys dumpsys(sm.get()); Vector args; @@ -1196,45 +1213,28 @@ void RunDumpsysProto(const std::string& title, int priority, std::chrono::millis // Runs dumpsys on services that must dump first and and will take less than 100ms to dump. static void RunDumpsysCritical() { - if (ds.CurrentVersionSupportsPriorityDumps()) { - RunDumpsysText("DUMPSYS CRITICAL", IServiceManager::DUMP_FLAG_PRIORITY_CRITICAL, - /* timeout= */ 5s, /* service_timeout= */ 500ms); - RunDumpsysProto("DUMPSYS CRITICAL PROTO", IServiceManager::DUMP_FLAG_PRIORITY_CRITICAL, - /* timeout= */ 5s, /* service_timeout= */ 500ms); - } else { - RunDumpsys("DUMPSYS MEMINFO", {"meminfo", "-a"}, - CommandOptions::WithTimeout(90).DropRoot().Build()); - RunDumpsys("DUMPSYS CPUINFO", {"cpuinfo", "-a"}, - CommandOptions::WithTimeout(10).DropRoot().Build()); - } + RunDumpsysText("DUMPSYS CRITICAL", IServiceManager::DUMP_FLAG_PRIORITY_CRITICAL, + /* timeout= */ 5s, /* service_timeout= */ 500ms); + RunDumpsysProto("DUMPSYS CRITICAL PROTO", IServiceManager::DUMP_FLAG_PRIORITY_CRITICAL, + /* timeout= */ 5s, /* service_timeout= */ 500ms); } // Runs dumpsys on services that must dump first but can take up to 250ms to dump. static void RunDumpsysHigh() { - if (ds.CurrentVersionSupportsPriorityDumps()) { - // TODO meminfo takes ~10s, connectivity takes ~5sec to dump. They are both - // high priority. Reduce timeout once they are able to dump in a shorter time or - // moved to a parallel task. - RunDumpsysText("DUMPSYS HIGH", IServiceManager::DUMP_FLAG_PRIORITY_HIGH, - /* timeout= */ 90s, /* service_timeout= */ 30s); - RunDumpsysProto("DUMPSYS HIGH PROTO", IServiceManager::DUMP_FLAG_PRIORITY_HIGH, - /* timeout= */ 5s, /* service_timeout= */ 1s); - } else { - RunDumpsys("NETWORK DIAGNOSTICS", {"connectivity", "--diag"}); - } + // TODO meminfo takes ~10s, connectivity takes ~5sec to dump. They are both + // high priority. Reduce timeout once they are able to dump in a shorter time or + // moved to a parallel task. + RunDumpsysText("DUMPSYS HIGH", IServiceManager::DUMP_FLAG_PRIORITY_HIGH, + /* timeout= */ 90s, /* service_timeout= */ 30s); + RunDumpsysProto("DUMPSYS HIGH PROTO", IServiceManager::DUMP_FLAG_PRIORITY_HIGH, + /* timeout= */ 5s, /* service_timeout= */ 1s); } // Runs dumpsys on services that must dump but can take up to 10s to dump. static void RunDumpsysNormal() { - if (ds.CurrentVersionSupportsPriorityDumps()) { - RunDumpsysText("DUMPSYS", IServiceManager::DUMP_FLAG_PRIORITY_NORMAL, - /* timeout= */ 90s, /* service_timeout= */ 10s); - RunDumpsysProto("DUMPSYS PROTO", IServiceManager::DUMP_FLAG_PRIORITY_NORMAL, - /* timeout= */ 90s, /* service_timeout= */ 10s); - } else { - RunDumpsys("DUMPSYS", {"--skip", "meminfo", "cpuinfo"}, - CommandOptions::WithTimeout(90).Build(), SEC_TO_MSEC(10)); - } + RunDumpsysTextNormalPriority("DUMPSYS", /* timeout= */ 90s, /* service_timeout= */ 10s); + RunDumpsysProto("DUMPSYS PROTO", IServiceManager::DUMP_FLAG_PRIORITY_NORMAL, + /* timeout= */ 90s, /* service_timeout= */ 10s); } static void DumpHals() { @@ -1893,12 +1893,10 @@ int run_main(int argc, char* argv[]) { ds.version_ = VERSION_CURRENT; } - if (ds.version_ != VERSION_CURRENT && ds.version_ != VERSION_SPLIT_ANR && - ds.version_ != VERSION_PRIORITY_DUMPS) { - MYLOGE( - "invalid version requested ('%s'); suppported values are: ('%s', '%s', '%s', '%s')\n", - ds.version_.c_str(), VERSION_DEFAULT.c_str(), VERSION_CURRENT.c_str(), - VERSION_SPLIT_ANR.c_str(), VERSION_PRIORITY_DUMPS.c_str()); + if (ds.version_ != VERSION_CURRENT && ds.version_ != VERSION_SPLIT_ANR) { + MYLOGE("invalid version requested ('%s'); suppported values are: ('%s', '%s', '%s')\n", + ds.version_.c_str(), VERSION_DEFAULT.c_str(), VERSION_CURRENT.c_str(), + VERSION_SPLIT_ANR.c_str()); exit(1); } diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h index 2554b6374b..ea4fccd815 100644 --- a/cmds/dumpstate/dumpstate.h +++ b/cmds/dumpstate/dumpstate.h @@ -145,19 +145,13 @@ class Progress { * * See bugreport-format.md for more info. */ -static std::string VERSION_CURRENT = "1.0"; +static std::string VERSION_CURRENT = "2.0"; /* * Temporary version that adds a anr-traces.txt entry. Once tools support it, the current version - * will be bumped to 2.0. + * will be bumped to 3.0. */ -static std::string VERSION_SPLIT_ANR = "2.0-dev-split-anr"; - -/* - * Temporary version that adds priority based dumps. Once tools support it, the current version - * will be bumped to 2.0. - */ -static std::string VERSION_PRIORITY_DUMPS = "2.0-dev-priority-dumps"; +static std::string VERSION_SPLIT_ANR = "3.0-dev-split-anr"; /* * "Alias" for the current version. diff --git a/cmds/dumpstate/utils.cpp b/cmds/dumpstate/utils.cpp index 2224b3d8cb..a89925f8e6 100644 --- a/cmds/dumpstate/utils.cpp +++ b/cmds/dumpstate/utils.cpp @@ -259,10 +259,6 @@ std::string Dumpstate::GetPath(const std::string& suffix) const { name_.c_str(), suffix.c_str()); } -bool Dumpstate::CurrentVersionSupportsPriorityDumps() const { - return (version_ == VERSION_PRIORITY_DUMPS); -} - void Dumpstate::SetProgress(std::unique_ptr progress) { progress_ = std::move(progress); } diff --git a/cmds/dumpsys/dumpsys.cpp b/cmds/dumpsys/dumpsys.cpp index ca7d95e152..5412d4df7b 100644 --- a/cmds/dumpsys/dumpsys.cpp +++ b/cmds/dumpsys/dumpsys.cpp @@ -284,14 +284,23 @@ Vector Dumpsys::listServices(int priorityFilterFlags, bool filterByPro } void Dumpsys::setServiceArgs(Vector& args, bool asProto, int priorityFlags) { - if ((priorityFlags == IServiceManager::DUMP_FLAG_PRIORITY_ALL) || - (priorityFlags == IServiceManager::DUMP_FLAG_PRIORITY_NORMAL)) { - args.add(String16("-a")); - } + // Add proto flag if dumping service as proto. if (asProto) { args.insertAt(String16(PriorityDumper::PROTO_ARG), 0); } - if (priorityFlags != IServiceManager::DUMP_FLAG_PRIORITY_ALL) { + + // Add -a (dump all) flag if dumping all services, dumping normal services or + // services not explicitly registered to a priority bucket (default services). + if ((priorityFlags == IServiceManager::DUMP_FLAG_PRIORITY_ALL) || + (priorityFlags == IServiceManager::DUMP_FLAG_PRIORITY_NORMAL) || + (priorityFlags == IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT)) { + args.insertAt(String16("-a"), 0); + } + + // Add priority flags when dumping services registered to a specific priority bucket. + if ((priorityFlags == IServiceManager::DUMP_FLAG_PRIORITY_CRITICAL) || + (priorityFlags == IServiceManager::DUMP_FLAG_PRIORITY_HIGH) || + (priorityFlags == IServiceManager::DUMP_FLAG_PRIORITY_NORMAL)) { String16 priorityType = ConvertBitmaskToPriorityType(priorityFlags); args.insertAt(String16(PriorityDumper::PRIORITY_ARG), 0); args.insertAt(priorityType, 1); @@ -349,7 +358,8 @@ void Dumpsys::writeDumpHeader(int fd, const String16& serviceName, int priorityF "----------------------------------------" "---------------------------------------\n"); if (priorityFlags == IServiceManager::DUMP_FLAG_PRIORITY_ALL || - priorityFlags == IServiceManager::DUMP_FLAG_PRIORITY_NORMAL) { + priorityFlags == IServiceManager::DUMP_FLAG_PRIORITY_NORMAL || + priorityFlags == IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT) { StringAppendF(&msg, "DUMP OF SERVICE %s:\n", String8(serviceName).c_str()); } else { String16 priorityType = ConvertBitmaskToPriorityType(priorityFlags); diff --git a/cmds/dumpsys/tests/dumpsys_test.cpp b/cmds/dumpsys/tests/dumpsys_test.cpp index b13f59d344..502935259a 100644 --- a/cmds/dumpsys/tests/dumpsys_test.cpp +++ b/cmds/dumpsys/tests/dumpsys_test.cpp @@ -351,6 +351,65 @@ TEST_F(DumpsysTest, DumpWithArgsRunningService) { AssertOutput("I DO!"); } +// Tests dumpsys passes the -a flag when called on all services +TEST_F(DumpsysTest, PassAllFlagsToServices) { + ExpectListServices({"Locksmith", "Valet"}); + ExpectCheckService("Locksmith"); + ExpectCheckService("Valet"); + ExpectDumpWithArgs("Locksmith", {"-a"}, "dumped1"); + ExpectDumpWithArgs("Valet", {"-a"}, "dumped2"); + + CallMain({"-T", "500"}); + + AssertDumped("Locksmith", "dumped1"); + AssertDumped("Valet", "dumped2"); +} + +// Tests dumpsys passes the -a flag when called on NORMAL priority services +TEST_F(DumpsysTest, PassAllFlagsToNormalServices) { + ExpectListServicesWithPriority({"Locksmith", "Valet"}, + IServiceManager::DUMP_FLAG_PRIORITY_NORMAL); + ExpectCheckService("Locksmith"); + ExpectCheckService("Valet"); + ExpectDumpWithArgs("Locksmith", {"-a", "--dump-priority", "NORMAL"}, "dump1"); + ExpectDumpWithArgs("Valet", {"-a", "--dump-priority", "NORMAL"}, "dump2"); + + CallMain({"--priority", "NORMAL"}); + + AssertDumped("Locksmith", "dump1"); + AssertDumped("Valet", "dump2"); +} + +// Tests dumpsys passes only priority flags when called on CRITICAL priority services +TEST_F(DumpsysTest, PassPriorityFlagsToCriticalServices) { + ExpectListServicesWithPriority({"Locksmith", "Valet"}, + IServiceManager::DUMP_FLAG_PRIORITY_CRITICAL); + ExpectCheckService("Locksmith"); + ExpectCheckService("Valet"); + ExpectDumpWithArgs("Locksmith", {"--dump-priority", "CRITICAL"}, "dump1"); + ExpectDumpWithArgs("Valet", {"--dump-priority", "CRITICAL"}, "dump2"); + + CallMain({"--priority", "CRITICAL"}); + + AssertDumpedWithPriority("Locksmith", "dump1", PriorityDumper::PRIORITY_ARG_CRITICAL); + AssertDumpedWithPriority("Valet", "dump2", PriorityDumper::PRIORITY_ARG_CRITICAL); +} + +// Tests dumpsys passes only priority flags when called on HIGH priority services +TEST_F(DumpsysTest, PassPriorityFlagsToHighServices) { + ExpectListServicesWithPriority({"Locksmith", "Valet"}, + IServiceManager::DUMP_FLAG_PRIORITY_HIGH); + ExpectCheckService("Locksmith"); + ExpectCheckService("Valet"); + ExpectDumpWithArgs("Locksmith", {"--dump-priority", "HIGH"}, "dump1"); + ExpectDumpWithArgs("Valet", {"--dump-priority", "HIGH"}, "dump2"); + + CallMain({"--priority", "HIGH"}); + + AssertDumpedWithPriority("Locksmith", "dump1", PriorityDumper::PRIORITY_ARG_HIGH); + AssertDumpedWithPriority("Valet", "dump2", PriorityDumper::PRIORITY_ARG_HIGH); +} + // Tests 'dumpsys' with no arguments TEST_F(DumpsysTest, DumpMultipleServices) { ExpectListServices({"running1", "stopped2", "running3"}); diff --git a/libs/binder/include/binder/BinderService.h b/libs/binder/include/binder/BinderService.h index 4ce82a122c..9230e89cdf 100644 --- a/libs/binder/include/binder/BinderService.h +++ b/libs/binder/include/binder/BinderService.h @@ -35,7 +35,7 @@ class BinderService { public: static status_t publish(bool allowIsolated = false, - int dumpFlags = IServiceManager::DUMP_FLAG_PRIORITY_NORMAL) { + int dumpFlags = IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT) { sp sm(defaultServiceManager()); return sm->addService(String16(SERVICE::getServiceName()), new SERVICE(), allowIsolated, dumpFlags); @@ -43,7 +43,7 @@ public: static void publishAndJoinThreadPool( bool allowIsolated = false, - int dumpFlags = IServiceManager::DUMP_FLAG_PRIORITY_NORMAL) { + int dumpFlags = IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT) { publish(allowIsolated, dumpFlags); joinThreadPool(); } diff --git a/libs/binder/include/binder/IServiceManager.h b/libs/binder/include/binder/IServiceManager.h index 19e841a347..cf4c08a46a 100644 --- a/libs/binder/include/binder/IServiceManager.h +++ b/libs/binder/include/binder/IServiceManager.h @@ -31,15 +31,22 @@ class IServiceManager : public IInterface { public: DECLARE_META_INTERFACE(ServiceManager) - /* + /** * Must match values in IServiceManager.java */ + /* Allows services to dump sections according to priorities. */ static const int DUMP_FLAG_PRIORITY_CRITICAL = 1 << 0; static const int DUMP_FLAG_PRIORITY_HIGH = 1 << 1; static const int DUMP_FLAG_PRIORITY_NORMAL = 1 << 2; - static const int DUMP_FLAG_PRIORITY_ALL = - DUMP_FLAG_PRIORITY_CRITICAL | DUMP_FLAG_PRIORITY_HIGH | DUMP_FLAG_PRIORITY_NORMAL; - static const int DUMP_FLAG_PROTO = 1 << 3; + /** + * Services are by default registered with a DEFAULT dump priority. DEFAULT priority has the + * same priority as NORMAL priority but the services are not called with dump priority + * arguments. + */ + static const int DUMP_FLAG_PRIORITY_DEFAULT = 1 << 3; + static const int DUMP_FLAG_PRIORITY_ALL = DUMP_FLAG_PRIORITY_CRITICAL | + DUMP_FLAG_PRIORITY_HIGH | DUMP_FLAG_PRIORITY_NORMAL | DUMP_FLAG_PRIORITY_DEFAULT; + static const int DUMP_FLAG_PROTO = 1 << 4; /** * Retrieve an existing service, blocking for a few seconds @@ -57,7 +64,7 @@ public: */ virtual status_t addService(const String16& name, const sp& service, bool allowIsolated = false, - int dumpsysFlags = DUMP_FLAG_PRIORITY_NORMAL) = 0; + int dumpsysFlags = DUMP_FLAG_PRIORITY_DEFAULT) = 0; /** * Return list of all existing services. -- cgit v1.2.3-59-g8ed1b From 963b04fa5c8f30e01e648bb1f1b37c0a8ac1a964 Mon Sep 17 00:00:00 2001 From: Sooraj Sasindran Date: Fri, 9 Mar 2018 13:26:39 -0800 Subject: Add battery stats to telephony bugreport Add battery stats to telephony bugreport Test: verified by triggering telephony bugreport Bug:74211680 Change-Id: I2d2dfc144fd6f0faa81583808d5f7cc3bf9841e8 --- cmds/dumpstate/dumpstate.cpp | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index d8bbdbb5f6..371533c4db 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1505,6 +1505,12 @@ static void DumpstateTelephonyOnly() { RunDumpsys("TELEPHONY SERVICES", {"activity", "service", "TelephonyDebugService"}); + printf("========================================================\n"); + printf("== Checkins\n"); + printf("========================================================\n"); + + RunDumpsys("CHECKIN BATTERYSTATS", {"batterystats", "-c"}); + printf("========================================================\n"); printf("== dumpstate: done (id %d)\n", ds.id_); printf("========================================================\n"); -- cgit v1.2.3-59-g8ed1b From d05127172ce1f113230b1774119b6d54475dc158 Mon Sep 17 00:00:00 2001 From: Luis Hector Chavez Date: Wed, 14 Mar 2018 12:15:56 -0700 Subject: Avoid crashing when a directory does not exist This change adds a null-check for the DIR* object to avoid a crash when the directory cannot be opened for any reason. Bug: 74568776 Test: calling dumpstate early in the boot sequence no longer crashes Change-Id: I6c80f38dcf7890e607ce73e25a8ec0e16ba92586 --- cmds/dumpstate/dumpstate.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 371533c4db..10f252fb7e 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -176,6 +176,11 @@ static std::vector* GetDumpFds(const std::string& dir_path, std::unique_ptr> dump_data(new std::vector()); std::unique_ptr dump_dir(opendir(dir_path.c_str()), closedir); + if (dump_dir == nullptr) { + MYLOGW("Unable to open directory %s: %s\n", dir_path.c_str(), strerror(errno)); + return dump_data.release(); + } + struct dirent* entry = nullptr; while ((entry = readdir(dump_dir.get()))) { if (entry->d_type != DT_REG) { @@ -191,13 +196,13 @@ static std::vector* GetDumpFds(const std::string& dir_path, android::base::unique_fd fd( TEMP_FAILURE_RETRY(open(abs_path.c_str(), O_RDONLY | O_CLOEXEC | O_NOFOLLOW | O_NONBLOCK))); if (fd == -1) { - MYLOGW("Unable to open dump file: %s %s\n", abs_path.c_str(), strerror(errno)); + MYLOGW("Unable to open dump file %s: %s\n", abs_path.c_str(), strerror(errno)); break; } struct stat st = {}; if (fstat(fd, &st) == -1) { - MYLOGW("Unable to stat dump file: %s %s\n", abs_path.c_str(), strerror(errno)); + MYLOGW("Unable to stat dump file %s: %s\n", abs_path.c_str(), strerror(errno)); continue; } -- cgit v1.2.3-59-g8ed1b From 5f6ee4a11de1de262cea6465c2f991834b2e4792 Mon Sep 17 00:00:00 2001 From: Luis Hector Chavez Date: Wed, 14 Mar 2018 15:12:46 -0700 Subject: Clean up DumpData-related functions This change cleans a few DumpData-related functions: * Makes GetDumpFds return the std::vector by value instead of by pointer. * Makes Dumpstate own the list of open tombstone and ANR files. * Removes two more globals. * Prevents a potential FD leak. Bug: http://b/73140330 Test: `adb bugreport` succeeded (cherry picked from commit 91c2ae50d57149366d5ab8ee9c763b4c4c6c9e0b) Change-Id: I5ceb9e4fc1e123dfd6823a7626492ce0b63cf822 --- cmds/dumpstate/dumpstate.cpp | 62 ++++++++++++++++---------------------------- cmds/dumpstate/dumpstate.h | 21 +++++++++++++++ 2 files changed, 43 insertions(+), 40 deletions(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 10f252fb7e..fd2fccba4e 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -112,19 +112,6 @@ static const std::string TOMBSTONE_FILE_PREFIX = "tombstone_"; static const std::string ANR_DIR = "/data/anr/"; static const std::string ANR_FILE_PREFIX = "anr_"; -struct DumpData { - std::string name; - int fd; - time_t mtime; -}; - -static bool operator<(const DumpData& d1, const DumpData& d2) { - return d1.mtime > d2.mtime; -} - -static std::unique_ptr> tombstone_data; -static std::unique_ptr> anr_data; - // TODO: temporary variables and functions used during C++ refactoring static Dumpstate& ds = Dumpstate::GetInstance(); static int RunCommand(const std::string& title, const std::vector& fullCommand, @@ -167,20 +154,20 @@ static const CommandOptions AS_ROOT_20 = CommandOptions::WithTimeout(20).AsRoot( * is set, the vector only contains files that were written in the last 30 minutes. * If |limit_by_count| is set, the vector only contains the ten latest files. */ -static std::vector* GetDumpFds(const std::string& dir_path, - const std::string& file_prefix, - bool limit_by_mtime, - bool limit_by_count = true) { +static std::vector GetDumpFds(const std::string& dir_path, + const std::string& file_prefix, + bool limit_by_mtime, + bool limit_by_count = true) { const time_t thirty_minutes_ago = ds.now_ - 60 * 30; - std::unique_ptr> dump_data(new std::vector()); std::unique_ptr dump_dir(opendir(dir_path.c_str()), closedir); if (dump_dir == nullptr) { MYLOGW("Unable to open directory %s: %s\n", dir_path.c_str(), strerror(errno)); - return dump_data.release(); + return std::vector(); } + std::vector dump_data; struct dirent* entry = nullptr; while ((entry = readdir(dump_dir.get()))) { if (entry->d_type != DT_REG) { @@ -211,18 +198,19 @@ static std::vector* GetDumpFds(const std::string& dir_path, continue; } - DumpData data = {.name = abs_path, .fd = fd.release(), .mtime = st.st_mtime}; - - dump_data->push_back(data); + dump_data.emplace_back(DumpData{abs_path, std::move(fd), st.st_mtime}); } - std::sort(dump_data->begin(), dump_data->end()); + // Sort in descending modification time so that we only keep the newest + // reports if |limit_by_count| is true. + std::sort(dump_data.begin(), dump_data.end(), + [](const DumpData& d1, const DumpData& d2) { return d1.mtime > d2.mtime; }); - if (limit_by_count && dump_data->size() > 10) { - dump_data->erase(dump_data->begin() + 10, dump_data->end()); + if (limit_by_count && dump_data.size() > 10) { + dump_data.erase(dump_data.begin() + 10, dump_data.end()); } - return dump_data.release(); + return dump_data; } static bool AddDumps(const std::vector::const_iterator start, @@ -257,12 +245,6 @@ static bool AddDumps(const std::vector::const_iterator start, return dumped; } -static void CloseDumpFds(const std::vector* dumps) { - for (auto it = dumps->begin(); it != dumps->end(); ++it) { - close(it->fd); - } -} - // for_each_pid() callback to get mount info about a process. void do_mountinfo(int pid, const char* name __attribute__((unused))) { char path[PATH_MAX]; @@ -1007,15 +989,15 @@ static void AddAnrTraceDir(const bool add_to_zip, const std::string& anr_traces_ } // Add a specific message for the first ANR Dump. - if (anr_data->size() > 0) { - AddDumps(anr_data->begin(), anr_data->begin() + 1, + if (ds.anr_data_.size() > 0) { + AddDumps(ds.anr_data_.begin(), ds.anr_data_.begin() + 1, "VM TRACES AT LAST ANR", add_to_zip); // The "last" ANR will always be included as separate entry in the zip file. In addition, // it will be present in the body of the main entry if |add_to_zip| == false. // // Historical ANRs are always included as separate entries in the bugreport zip file. - AddDumps(anr_data->begin() + ((add_to_zip) ? 1 : 0), anr_data->end(), + AddDumps(ds.anr_data_.begin() + ((add_to_zip) ? 1 : 0), ds.anr_data_.end(), "HISTORICAL ANR", true /* add_to_zip */); } else { printf("*** NO ANRs to dump in %s\n\n", ANR_DIR.c_str()); @@ -1362,7 +1344,7 @@ static void dumpstate() { // NOTE: tombstones are always added as separate entries in the zip archive // and are not interspersed with the main report. - const bool tombstones_dumped = AddDumps(tombstone_data->begin(), tombstone_data->end(), + const bool tombstones_dumped = AddDumps(ds.tombstone_data_.begin(), ds.tombstone_data_.end(), "TOMBSTONE", true /* add_to_zip */); if (!tombstones_dumped) { printf("*** NO TOMBSTONES to dump in %s\n\n", TOMBSTONE_DIR.c_str()); @@ -2114,8 +2096,8 @@ int run_main(int argc, char* argv[]) { dump_traces_path = dump_traces(); /* Run some operations that require root. */ - tombstone_data.reset(GetDumpFds(TOMBSTONE_DIR, TOMBSTONE_FILE_PREFIX, !ds.IsZipping())); - anr_data.reset(GetDumpFds(ANR_DIR, ANR_FILE_PREFIX, !ds.IsZipping())); + ds.tombstone_data_ = GetDumpFds(TOMBSTONE_DIR, TOMBSTONE_FILE_PREFIX, !ds.IsZipping()); + ds.anr_data_ = GetDumpFds(ANR_DIR, ANR_FILE_PREFIX, !ds.IsZipping()); ds.AddDir(RECOVERY_DIR, true); ds.AddDir(RECOVERY_DATA_DIR, true); @@ -2288,8 +2270,8 @@ int run_main(int argc, char* argv[]) { close(ds.control_socket_fd_); } - CloseDumpFds(tombstone_data.get()); - CloseDumpFds(anr_data.get()); + ds.tombstone_data_.clear(); + ds.anr_data_.clear(); return 0; } diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h index ea4fccd815..b220013f17 100644 --- a/cmds/dumpstate/dumpstate.h +++ b/cmds/dumpstate/dumpstate.h @@ -26,6 +26,7 @@ #include #include +#include #include #include #include @@ -158,6 +159,20 @@ static std::string VERSION_SPLIT_ANR = "3.0-dev-split-anr"; */ static std::string VERSION_DEFAULT = "default"; +/* + * Structure that contains the information of an open dump file. + */ +struct DumpData { + // Path of the file. + std::string name; + + // Open file descriptor for the file. + android::base::unique_fd fd; + + // Modification time of the file. + time_t mtime; +}; + /* * Main class driving a bugreport generation. * @@ -350,6 +365,12 @@ class Dumpstate { std::string notification_title; std::string notification_description; + // List of open tombstone dump files. + std::vector tombstone_data_; + + // List of open ANR dump files. + std::vector anr_data_; + private: // Used by GetInstance() only. Dumpstate(const std::string& version = VERSION_CURRENT); -- cgit v1.2.3-59-g8ed1b From 1e27b08c1c96926f88c782b047214e09f29d57d2 Mon Sep 17 00:00:00 2001 From: Luis Hector Chavez Date: Thu, 22 Mar 2018 15:36:42 -0700 Subject: Avoid crashing in RunDumpsysProto when not zipping When a .zip file is not being generated, RunDumpsysProto will segfault while trying to get the last ZipWriter::FileEntry because ds.zip_writer_ is nullptr. This change avoids running the method altogether if not zipping. Bug: 76120715 Test: dumpsys Change-Id: I184dd7b72564408e313f32f3045e2fe0e4521b5b Merged-In: I184dd7b72564408e313f32f3045e2fe0e4521b5b --- cmds/dumpstate/dumpstate.cpp | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index fd2fccba4e..bfd0f16b3e 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1159,6 +1159,10 @@ static void RunDumpsysTextNormalPriority(const std::string& title, static void RunDumpsysProto(const std::string& title, int priority, std::chrono::milliseconds timeout, std::chrono::milliseconds service_timeout) { + if (!ds.IsZipping()) { + MYLOGD("Not dumping %s because it's not a zipped bugreport\n", title.c_str()); + return; + } sp sm = defaultServiceManager(); Dumpsys dumpsys(sm.get()); Vector args; -- cgit v1.2.3-59-g8ed1b From 558e1ef07fb6169a1501a9b8637387abef611f34 Mon Sep 17 00:00:00 2001 From: Luis Hector Chavez Date: Thu, 22 Mar 2018 15:39:17 -0700 Subject: Let dumpstate die a normal death when receiving a singal Having dumpstate not generate a crash report / tombstone makes diagnosing crashes a bit harder. This change avoids calling _exit(3) when receiving a signal, except for SIGPIPE which we just SIG_IGN. Bug: 76120715 Test: dumpstate Change-Id: I7bd986251f7174de77bdcfd4ff3ee26a1ef4afc7 --- cmds/dumpstate/dumpstate.cpp | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index bfd0f16b3e..9648edefe9 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1634,20 +1634,8 @@ static void ExitOnInvalidArgs() { ShowUsageAndExit(); } -static void sig_handler(int) { - _exit(EXIT_FAILURE); -} - static void register_sig_handler() { - struct sigaction sa; - sigemptyset(&sa.sa_mask); - sa.sa_flags = 0; - sa.sa_handler = sig_handler; - sigaction(SIGPIPE, &sa, NULL); // broken pipe - sigaction(SIGSEGV, &sa, NULL); // segment fault - sigaction(SIGINT, &sa, NULL); // ctrl-c - sigaction(SIGTERM, &sa, NULL); // killed - sigaction(SIGQUIT, &sa, NULL); // quit + signal(SIGPIPE, SIG_IGN); } bool Dumpstate::FinishZipFile() { -- cgit v1.2.3-59-g8ed1b From 69fe5eca7d2352edd90bd701e546d2c2abe44bcd Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Fri, 23 Mar 2018 11:04:25 -0700 Subject: Remove non-tombstoned ANR path. Bug: http://b/73140330 Test: `adb bugreport` includes /data/anr/anr_* files Change-Id: I3bd7fd932fd24cb798ef1819845d2812e5c577f1 (cherry picked from commit da56ab30720515d524008f01810050f97258ab79) --- cmds/dumpstate/dumpstate.cpp | 99 +++--------------------- cmds/dumpstate/utils.cpp | 174 +------------------------------------------ 2 files changed, 15 insertions(+), 258 deletions(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 9648edefe9..6e0578c5dc 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -918,53 +918,6 @@ static void DumpIpTablesAsRoot() { RunCommand("IP6TABLES RAW", {"ip6tables", "-t", "raw", "-L", "-nvx"}); } -static void AddGlobalAnrTraceFile(const bool add_to_zip, const std::string& anr_traces_file, - const std::string& anr_traces_dir) { - std::string dump_traces_dir; - - if (dump_traces_path != nullptr) { - if (add_to_zip) { - dump_traces_dir = dirname(dump_traces_path); - MYLOGD("Adding ANR traces (directory %s) to the zip file\n", dump_traces_dir.c_str()); - ds.AddDir(dump_traces_dir, true); - } else { - MYLOGD("Dumping current ANR traces (%s) to the main bugreport entry\n", - dump_traces_path); - ds.DumpFile("VM TRACES JUST NOW", dump_traces_path); - } - } - - - // Make sure directory is not added twice. - // TODO: this is an overzealous check because it's relying on dump_traces_path - which is - // generated by dump_traces() - and anr_traces_path - which is retrieved from a system - // property - but in reality they're the same path (although the former could be nullptr). - // Anyways, once dump_traces() is refactored as a private Dumpstate function, this logic should - // be revisited. - bool already_dumped = anr_traces_dir == dump_traces_dir; - - MYLOGD("AddGlobalAnrTraceFile(): dump_traces_dir=%s, anr_traces_dir=%s, already_dumped=%d\n", - dump_traces_dir.c_str(), anr_traces_dir.c_str(), already_dumped); - - android::base::unique_fd fd(TEMP_FAILURE_RETRY( - open(anr_traces_file.c_str(), O_RDONLY | O_CLOEXEC | O_NOFOLLOW | O_NONBLOCK))); - if (fd.get() < 0) { - printf("*** NO ANR VM TRACES FILE (%s): %s\n\n", anr_traces_file.c_str(), strerror(errno)); - } else { - if (add_to_zip) { - if (!already_dumped) { - MYLOGD("Adding dalvik ANR traces (directory %s) to the zip file\n", - anr_traces_dir.c_str()); - ds.AddDir(anr_traces_dir, true); - } - } else { - MYLOGD("Dumping last ANR traces (%s) to the main bugreport entry\n", - anr_traces_file.c_str()); - dump_file_from_fd("VM TRACES AT LAST ANR", anr_traces_file.c_str(), fd.get()); - } - } -} - static void AddAnrTraceDir(const bool add_to_zip, const std::string& anr_traces_dir) { MYLOGD("AddAnrTraceDir(): dump_traces_file=%s, anr_traces_dir=%s\n", dump_traces_path, anr_traces_dir.c_str()); @@ -1007,50 +960,22 @@ static void AddAnrTraceDir(const bool add_to_zip, const std::string& anr_traces_ static void AddAnrTraceFiles() { const bool add_to_zip = ds.IsZipping() && ds.version_ == VERSION_SPLIT_ANR; - std::string anr_traces_file; - std::string anr_traces_dir; - bool is_global_trace_file = true; - - // First check whether the stack-trace-dir property is set. When it's set, - // each ANR trace will be written to a separate file and not to a global - // stack trace file. - anr_traces_dir = android::base::GetProperty("dalvik.vm.stack-trace-dir", ""); - if (anr_traces_dir.empty()) { - anr_traces_file = android::base::GetProperty("dalvik.vm.stack-trace-file", ""); - if (!anr_traces_file.empty()) { - anr_traces_dir = dirname(anr_traces_file.c_str()); - } - } else { - is_global_trace_file = false; - } - - // We have neither configured a global trace file nor a trace directory, - // there will be nothing to dump. - if (anr_traces_file.empty() && anr_traces_dir.empty()) { - printf("*** NO VM TRACES FILE DEFINED (dalvik.vm.stack-trace-file)\n\n"); - return; - } + std::string anr_traces_dir = "/data/anr"; - if (is_global_trace_file) { - AddGlobalAnrTraceFile(add_to_zip, anr_traces_file, anr_traces_dir); - } else { - AddAnrTraceDir(add_to_zip, anr_traces_dir); - } + AddAnrTraceDir(add_to_zip, anr_traces_dir); - /* slow traces for slow operations */ + // Slow traces for slow operations. struct stat st; - if (!anr_traces_dir.empty()) { - int i = 0; - while (true) { - const std::string slow_trace_path = - anr_traces_dir + android::base::StringPrintf("slow%02d.txt", i); - if (stat(slow_trace_path.c_str(), &st)) { - // No traces file at this index, done with the files. - break; - } - ds.DumpFile("VM TRACES WHEN SLOW", slow_trace_path.c_str()); - i++; + int i = 0; + while (true) { + const std::string slow_trace_path = + anr_traces_dir + android::base::StringPrintf("slow%02d.txt", i); + if (stat(slow_trace_path.c_str(), &st)) { + // No traces file at this index, done with the files. + break; } + ds.DumpFile("VM TRACES WHEN SLOW", slow_trace_path.c_str()); + i++; } } diff --git a/cmds/dumpstate/utils.cpp b/cmds/dumpstate/utils.cpp index 022f4fc98e..9beff989df 100644 --- a/cmds/dumpstate/utils.cpp +++ b/cmds/dumpstate/utils.cpp @@ -779,29 +779,11 @@ void redirect_to_existing_file(FILE *redirect, char *path) { _redirect_to_file(redirect, path, O_APPEND); } -const char* DumpTraces(const std::string& traces_path); -const char* DumpTracesTombstoned(const std::string& traces_dir); - -/* dump Dalvik and native stack traces, return the trace file location (NULL if none) */ -const char *dump_traces() { +// Dump Dalvik and native stack traces, return the trace file location (nullptr if none). +const char* dump_traces() { DurationReporter duration_reporter("DUMP TRACES"); - const std::string traces_dir = android::base::GetProperty("dalvik.vm.stack-trace-dir", ""); - if (!traces_dir.empty()) { - return DumpTracesTombstoned(traces_dir); - } - - const std::string traces_file = android::base::GetProperty("dalvik.vm.stack-trace-file", ""); - if (!traces_file.empty()) { - return DumpTraces(traces_file); - } - - return nullptr; -} - -const char* DumpTracesTombstoned(const std::string& traces_dir) { - const std::string temp_file_pattern = traces_dir + "/dumptrace_XXXXXX"; - + const std::string temp_file_pattern = "/data/anr/dumptrace_XXXXXX"; const size_t buf_size = temp_file_pattern.length() + 1; std::unique_ptr file_name_buf(new char[buf_size]); memcpy(file_name_buf.get(), temp_file_pattern.c_str(), buf_size); @@ -902,156 +884,6 @@ const char* DumpTracesTombstoned(const std::string& traces_dir) { return file_name_buf.release(); } -const char* DumpTraces(const std::string& traces_path) { - const char* result = NULL; - /* move the old traces.txt (if any) out of the way temporarily */ - std::string anrtraces_path = traces_path + ".anr"; - if (rename(traces_path.c_str(), anrtraces_path.c_str()) && errno != ENOENT) { - MYLOGE("rename(%s, %s): %s\n", traces_path.c_str(), anrtraces_path.c_str(), strerror(errno)); - return nullptr; // Can't rename old traces.txt -- no permission? -- leave it alone instead - } - - /* create a new, empty traces.txt file to receive stack dumps */ - int fd = TEMP_FAILURE_RETRY( - open(traces_path.c_str(), O_CREAT | O_WRONLY | O_APPEND | O_TRUNC | O_NOFOLLOW | O_CLOEXEC, - 0666)); /* -rw-rw-rw- */ - if (fd < 0) { - MYLOGE("%s: %s\n", traces_path.c_str(), strerror(errno)); - return nullptr; - } - int chmod_ret = fchmod(fd, 0666); - if (chmod_ret < 0) { - MYLOGE("fchmod on %s failed: %s\n", traces_path.c_str(), strerror(errno)); - close(fd); - return nullptr; - } - - /* Variables below must be initialized before 'goto' statements */ - int dalvik_found = 0; - int ifd, wfd = -1; - std::set hal_pids = get_interesting_hal_pids(); - - /* walk /proc and kill -QUIT all Dalvik processes */ - DIR *proc = opendir("/proc"); - if (proc == NULL) { - MYLOGE("/proc: %s\n", strerror(errno)); - goto error_close_fd; - } - - /* use inotify to find when processes are done dumping */ - ifd = inotify_init(); - if (ifd < 0) { - MYLOGE("inotify_init: %s\n", strerror(errno)); - goto error_close_fd; - } - - wfd = inotify_add_watch(ifd, traces_path.c_str(), IN_CLOSE_WRITE); - if (wfd < 0) { - MYLOGE("inotify_add_watch(%s): %s\n", traces_path.c_str(), strerror(errno)); - goto error_close_ifd; - } - - struct dirent *d; - while ((d = readdir(proc))) { - int pid = atoi(d->d_name); - if (pid <= 0) continue; - - char path[PATH_MAX]; - char data[PATH_MAX]; - snprintf(path, sizeof(path), "/proc/%d/exe", pid); - ssize_t len = readlink(path, data, sizeof(data) - 1); - if (len <= 0) { - continue; - } - data[len] = '\0'; - - if (!strncmp(data, "/system/bin/app_process", strlen("/system/bin/app_process"))) { - /* skip zygote -- it won't dump its stack anyway */ - snprintf(path, sizeof(path), "/proc/%d/cmdline", pid); - int cfd = TEMP_FAILURE_RETRY(open(path, O_RDONLY | O_CLOEXEC)); - len = read(cfd, data, sizeof(data) - 1); - close(cfd); - if (len <= 0) { - continue; - } - data[len] = '\0'; - if (!strncmp(data, "zygote", strlen("zygote"))) { - continue; - } - - ++dalvik_found; - uint64_t start = Nanotime(); - if (kill(pid, SIGQUIT)) { - MYLOGE("kill(%d, SIGQUIT): %s\n", pid, strerror(errno)); - continue; - } - - /* wait for the writable-close notification from inotify */ - struct pollfd pfd = { ifd, POLLIN, 0 }; - int ret = poll(&pfd, 1, TRACE_DUMP_TIMEOUT_MS); - if (ret < 0) { - MYLOGE("poll: %s\n", strerror(errno)); - } else if (ret == 0) { - MYLOGE("warning: timed out dumping pid %d\n", pid); - } else { - struct inotify_event ie; - read(ifd, &ie, sizeof(ie)); - } - - if (lseek(fd, 0, SEEK_END) < 0) { - MYLOGE("lseek: %s\n", strerror(errno)); - } else { - dprintf(fd, "[dump dalvik stack %d: %.3fs elapsed]\n", pid, - (float)(Nanotime() - start) / NANOS_PER_SEC); - } - } else if (should_dump_native_traces(data) || - hal_pids.find(pid) != hal_pids.end()) { - /* dump native process if appropriate */ - if (lseek(fd, 0, SEEK_END) < 0) { - MYLOGE("lseek: %s\n", strerror(errno)); - } else { - static uint16_t timeout_failures = 0; - uint64_t start = Nanotime(); - - /* If 3 backtrace dumps fail in a row, consider debuggerd dead. */ - if (timeout_failures == 3) { - dprintf(fd, "too many stack dump failures, skipping...\n"); - } else if (dump_backtrace_to_file_timeout( - pid, kDebuggerdNativeBacktrace, 20, fd) == -1) { - dprintf(fd, "dumping failed, likely due to a timeout\n"); - timeout_failures++; - } else { - timeout_failures = 0; - } - dprintf(fd, "[dump native stack %d: %.3fs elapsed]\n", pid, - (float)(Nanotime() - start) / NANOS_PER_SEC); - } - } - } - - if (dalvik_found == 0) { - MYLOGE("Warning: no Dalvik processes found to dump stacks\n"); - } - - static std::string dumptraces_path = android::base::StringPrintf( - "%s/bugreport-%s", dirname(traces_path.c_str()), basename(traces_path.c_str())); - if (rename(traces_path.c_str(), dumptraces_path.c_str())) { - MYLOGE("rename(%s, %s): %s\n", traces_path.c_str(), dumptraces_path.c_str(), - strerror(errno)); - goto error_close_ifd; - } - result = dumptraces_path.c_str(); - - /* replace the saved [ANR] traces.txt file */ - rename(anrtraces_path.c_str(), traces_path.c_str()); - -error_close_ifd: - close(ifd); -error_close_fd: - close(fd); - return result; -} - void dump_route_tables() { DurationReporter duration_reporter("DUMP ROUTE TABLES"); if (PropertiesHelper::IsDryRun()) return; -- cgit v1.2.3-59-g8ed1b From 7aecd381d74c4acfecdcd8fa03fda4debf5adfff Mon Sep 17 00:00:00 2001 From: Luis Hector Chavez Date: Mon, 19 Mar 2018 11:16:59 -0700 Subject: Add a 10s timeout to DumpstateBoard This change adds a timeout to DumpstateBoard so that it does not hang. Bug: 34764308 Test: adb shell bugreportz # With a 0s timeout, didn't crash. Test: adb shell bugreportz # Without an IDumpstateDevice impl, didn't crash. Change-Id: Ib35fe3b3a1185afd15b0f228cb0cf868cc1eed65 --- cmds/dumpstate/dumpstate.cpp | 139 ++++++++++++++++++++++++++----------------- 1 file changed, 85 insertions(+), 54 deletions(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 9648edefe9..80ef788187 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -32,14 +32,20 @@ #include #include #include + +#include +#include +#include #include #include #include #include +#include #include #include #include +#include #include #include #include @@ -53,6 +59,7 @@ #include #include #include +#include #include "DumpstateInternal.h" #include "DumpstateSectionReporter.h" #include "DumpstateService.h" @@ -1533,77 +1540,101 @@ void Dumpstate::DumpstateBoard() { printf("== Board\n"); printf("========================================================\n"); - ::android::sp dumpstate_device(IDumpstateDevice::getService()); - if (dumpstate_device == nullptr) { - MYLOGE("No IDumpstateDevice implementation\n"); - return; - } - if (!IsZipping()) { MYLOGD("Not dumping board info because it's not a zipped bugreport\n"); return; } - std::string path[NUM_OF_DUMPS]; - android::base::unique_fd fd[NUM_OF_DUMPS]; - int numFds = 0; - + std::vector paths; + std::vector>> remover; for (int i = 0; i < NUM_OF_DUMPS; i++) { - path[i] = kDumpstateBoardPath + kDumpstateBoardFiles[i]; - MYLOGI("Calling IDumpstateDevice implementation using path %s\n", path[i].c_str()); - - fd[i] = android::base::unique_fd( - TEMP_FAILURE_RETRY(open(path[i].c_str(), - O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW, - S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH))); - if (fd[i] < 0) { - MYLOGE("Could not open file %s: %s\n", path[i].c_str(), strerror(errno)); - return; - } else { - numFds++; - } - } + paths.emplace_back(kDumpstateBoardPath + kDumpstateBoardFiles[i]); + remover.emplace_back(android::base::make_scope_guard(std::bind( + [](std::string path) { + if (remove(path.c_str()) != 0 && errno != ENOENT) { + MYLOGE("Could not remove(%s): %s\n", path.c_str(), strerror(errno)); + } + }, + paths[i]))); + } + + // Given that bugreport is required to diagnose failures, it's better to + // drop the result of IDumpstateDevice than to block the rest of bugreport + // for an arbitrary amount of time. + std::packaged_task()> + dumpstate_task([paths]() -> std::unique_ptr { + ::android::sp dumpstate_device(IDumpstateDevice::getService()); + if (dumpstate_device == nullptr) { + MYLOGE("No IDumpstateDevice implementation\n"); + return nullptr; + } - native_handle_t *handle = native_handle_create(numFds, 0); - if (handle == nullptr) { - MYLOGE("Could not create native_handle\n"); - return; - } + using ScopedNativeHandle = + std::unique_ptr>; + ScopedNativeHandle handle(native_handle_create(static_cast(paths.size()), 0), + [](native_handle_t* handle) { + native_handle_close(handle); + native_handle_delete(handle); + }); + if (handle == nullptr) { + MYLOGE("Could not create native_handle\n"); + return nullptr; + } - for (int i = 0; i < numFds; i++) { - handle->data[i] = fd[i].release(); - } + for (size_t i = 0; i < paths.size(); i++) { + MYLOGI("Calling IDumpstateDevice implementation using path %s\n", paths[i].c_str()); + + android::base::unique_fd fd(TEMP_FAILURE_RETRY( + open(paths[i].c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW, + S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH))); + if (fd < 0) { + MYLOGE("Could not open file %s: %s\n", paths[i].c_str(), strerror(errno)); + return nullptr; + } + handle.get()->data[i] = fd.release(); + } - // TODO: need a timeout mechanism so dumpstate does not hang on device implementation call. - android::hardware::Return status = dumpstate_device->dumpstateBoard(handle); - if (!status.isOk()) { - MYLOGE("dumpstateBoard failed: %s\n", status.description().c_str()); - native_handle_close(handle); - native_handle_delete(handle); + android::hardware::Return status = dumpstate_device->dumpstateBoard(handle.get()); + if (!status.isOk()) { + MYLOGE("dumpstateBoard failed: %s\n", status.description().c_str()); + return nullptr; + } + auto file_sizes = std::make_unique(paths.size()); + for (size_t i = 0; i < paths.size(); i++) { + struct stat s; + if (fstat(handle.get()->data[i], &s) == -1) { + MYLOGE("Failed to fstat %s: %s\n", kDumpstateBoardFiles[i].c_str(), + strerror(errno)); + file_sizes[i] = -1; + continue; + } + file_sizes[i] = s.st_size; + } + return file_sizes; + }); + auto result = dumpstate_task.get_future(); + std::thread(std::move(dumpstate_task)).detach(); + if (result.wait_for(10s) != std::future_status::ready) { + MYLOGE("dumpstateBoard timed out after 10s\n"); + return; + } + std::unique_ptr file_sizes = result.get(); + if (file_sizes == nullptr) { return; } - for (int i = 0; i < numFds; i++) { - struct stat s; - if (fstat(handle->data[i], &s) == -1) { - MYLOGE("Failed to fstat %s: %d\n", kDumpstateBoardFiles[i].c_str(), errno); - } else if (s.st_size > 0) { - AddZipEntry(kDumpstateBoardFiles[i], path[i]); - } else { + for (size_t i = 0; i < paths.size(); i++) { + if (file_sizes[i] == -1) { + continue; + } + if (file_sizes[i] == 0) { MYLOGE("Ignoring empty %s\n", kDumpstateBoardFiles[i].c_str()); + continue; } + AddZipEntry(kDumpstateBoardFiles[i], paths[i]); } printf("*** See dumpstate-board.txt entry ***\n"); - - native_handle_close(handle); - native_handle_delete(handle); - - for (int i = 0; i < numFds; i++) { - if (remove(path[i].c_str()) != 0) { - MYLOGE("Could not remove(%s): %s\n", path[i].c_str(), strerror(errno)); - } - } } static void ShowUsageAndExit(int exitCode = 1) { -- cgit v1.2.3-59-g8ed1b From d25a914cd3d54dfa8760a928cf2b19d53bee27f4 Mon Sep 17 00:00:00 2001 From: Amruth Ramachandran Date: Mon, 2 Apr 2018 16:16:09 -0700 Subject: Add Wifi stats to telephony bugreport Test: Verified through bugreports Bug: 77489797 Change-Id: I868fe004b6b605b7c10e24c64ab400d5aaf45e27 --- cmds/dumpstate/dumpstate.cpp | 2 ++ 1 file changed, 2 insertions(+) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 80ef788187..1b2dc706d7 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1496,6 +1496,8 @@ static void DumpstateTelephonyOnly() { SEC_TO_MSEC(10)); RunDumpsys("DUMPSYS", {"carrier_config"}, CommandOptions::WithTimeout(90).Build(), SEC_TO_MSEC(10)); + RunDumpsys("DUMPSYS", {"wifi"}, CommandOptions::WithTimeout(90).Build(), + SEC_TO_MSEC(10)); printf("========================================================\n"); printf("== Running Application Services\n"); -- cgit v1.2.3-59-g8ed1b From 1b584d062ea998cb17f424a47bbebd27f0258529 Mon Sep 17 00:00:00 2001 From: Jie Song Date: Mon, 2 Apr 2018 20:46:02 -0700 Subject: Increase DumpstateBoard timeout to 30s Bug: 77494218 Test: adb bugreport # dumpstate_board.* generated with no error Change-Id: Iabaa46ea1f74e9c275d0394702bfe46ece3ef28a --- cmds/dumpstate/dumpstate.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 80ef788187..9d0c30b8b5 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1614,8 +1614,8 @@ void Dumpstate::DumpstateBoard() { }); auto result = dumpstate_task.get_future(); std::thread(std::move(dumpstate_task)).detach(); - if (result.wait_for(10s) != std::future_status::ready) { - MYLOGE("dumpstateBoard timed out after 10s\n"); + if (result.wait_for(30s) != std::future_status::ready) { + MYLOGE("dumpstateBoard timed out after 30s\n"); return; } std::unique_ptr file_sizes = result.get(); -- cgit v1.2.3-59-g8ed1b From 587eac9815d8994f0d9f0b7584402aae353bcb2c Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Thu, 5 Apr 2018 12:17:20 -0700 Subject: dumpstate: Kill vendor dumpstate HAL and grab partial dump in timeout dumpstate_board should not block forever but it is also a valuable source for debugging system problems. In timeout case, we should kill dumpstate HAL in case it hangs (e.g. cat from a pipe or so), so the following calls can be served by HAL service. Bug: 77489941 Test: simulate delay in dumpstate HAL and get BR, see partial content from dumpstate_board.tx and below log from dumpstate_log.txt dumpstateBoard timed out after 10s, killing dumpstate vendor HAL dumpstateBoard failed: Status(EX_TRANSACTION_FAILED): 'DEAD_OBJECT: ' Change-Id: Iadfd0c234c26714b6a2f1aa2941a85d4e01d2cbc --- cmds/dumpstate/dumpstate.cpp | 114 +++++++++++++++++++++++-------------------- 1 file changed, 62 insertions(+), 52 deletions(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 19bf216a57..5ee543cb6f 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1236,7 +1236,6 @@ static void RunDumpsysNormal() { } static void DumpHals() { - using android::sp; using android::hidl::manager::V1_0::IServiceManager; using android::hardware::defaultServiceManager; @@ -1560,69 +1559,80 @@ void Dumpstate::DumpstateBoard() { paths[i]))); } - // Given that bugreport is required to diagnose failures, it's better to - // drop the result of IDumpstateDevice than to block the rest of bugreport - // for an arbitrary amount of time. - std::packaged_task()> - dumpstate_task([paths]() -> std::unique_ptr { - ::android::sp dumpstate_device(IDumpstateDevice::getService()); - if (dumpstate_device == nullptr) { - MYLOGE("No IDumpstateDevice implementation\n"); - return nullptr; - } + sp dumpstate_device(IDumpstateDevice::getService()); + if (dumpstate_device == nullptr) { + MYLOGE("No IDumpstateDevice implementation\n"); + return; + } - using ScopedNativeHandle = - std::unique_ptr>; - ScopedNativeHandle handle(native_handle_create(static_cast(paths.size()), 0), - [](native_handle_t* handle) { - native_handle_close(handle); - native_handle_delete(handle); - }); - if (handle == nullptr) { - MYLOGE("Could not create native_handle\n"); - return nullptr; - } + using ScopedNativeHandle = + std::unique_ptr>; + ScopedNativeHandle handle(native_handle_create(static_cast(paths.size()), 0), + [](native_handle_t* handle) { + native_handle_close(handle); + native_handle_delete(handle); + }); + if (handle == nullptr) { + MYLOGE("Could not create native_handle\n"); + return; + } - for (size_t i = 0; i < paths.size(); i++) { - MYLOGI("Calling IDumpstateDevice implementation using path %s\n", paths[i].c_str()); + for (size_t i = 0; i < paths.size(); i++) { + MYLOGI("Calling IDumpstateDevice implementation using path %s\n", paths[i].c_str()); - android::base::unique_fd fd(TEMP_FAILURE_RETRY( - open(paths[i].c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW, - S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH))); - if (fd < 0) { - MYLOGE("Could not open file %s: %s\n", paths[i].c_str(), strerror(errno)); - return nullptr; - } - handle.get()->data[i] = fd.release(); - } + android::base::unique_fd fd(TEMP_FAILURE_RETRY( + open(paths[i].c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW, + S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH))); + if (fd < 0) { + MYLOGE("Could not open file %s: %s\n", paths[i].c_str(), strerror(errno)); + return; + } + handle.get()->data[i] = fd.release(); + } + // Given that bugreport is required to diagnose failures, it's better to + // set an arbitrary amount of timeout for IDumpstateDevice than to block the + // rest of bugreport. In the timeout case, we will kill dumpstate board HAL + // and grab whatever dumped + std::packaged_task + dumpstate_task([paths, dumpstate_device, &handle]() -> bool { android::hardware::Return status = dumpstate_device->dumpstateBoard(handle.get()); if (!status.isOk()) { MYLOGE("dumpstateBoard failed: %s\n", status.description().c_str()); - return nullptr; + return false; } - auto file_sizes = std::make_unique(paths.size()); - for (size_t i = 0; i < paths.size(); i++) { - struct stat s; - if (fstat(handle.get()->data[i], &s) == -1) { - MYLOGE("Failed to fstat %s: %s\n", kDumpstateBoardFiles[i].c_str(), - strerror(errno)); - file_sizes[i] = -1; - continue; - } - file_sizes[i] = s.st_size; - } - return file_sizes; + return true; }); + auto result = dumpstate_task.get_future(); std::thread(std::move(dumpstate_task)).detach(); - if (result.wait_for(30s) != std::future_status::ready) { - MYLOGE("dumpstateBoard timed out after 30s\n"); - return; + + constexpr size_t timeout_sec = 30; + if (result.wait_for(std::chrono::seconds(timeout_sec)) != std::future_status::ready) { + MYLOGE("dumpstateBoard timed out after %zus, killing dumpstate vendor HAL\n", timeout_sec); + if (!android::base::SetProperty("ctl.interface_restart", + android::base::StringPrintf("%s/default", + IDumpstateDevice::descriptor))) { + MYLOGE("Couldn't restart dumpstate HAL\n"); + } } - std::unique_ptr file_sizes = result.get(); - if (file_sizes == nullptr) { - return; + // Wait some time for init to kill dumpstate vendor HAL + constexpr size_t killing_timeout_sec = 10; + if (result.wait_for(std::chrono::seconds(killing_timeout_sec)) != std::future_status::ready) { + MYLOGE("killing dumpstateBoard timed out after %zus, continue and " + "there might be racing in content\n", killing_timeout_sec); + } + + auto file_sizes = std::make_unique(paths.size()); + for (size_t i = 0; i < paths.size(); i++) { + struct stat s; + if (fstat(handle.get()->data[i], &s) == -1) { + MYLOGE("Failed to fstat %s: %s\n", kDumpstateBoardFiles[i].c_str(), + strerror(errno)); + file_sizes[i] = -1; + continue; + } + file_sizes[i] = s.st_size; } for (size_t i = 0; i < paths.size(); i++) { -- cgit v1.2.3-59-g8ed1b From 6078098adadbdacd21f247fef0f88d9c02e26bac Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Mon, 16 Apr 2018 15:34:00 -0700 Subject: Split provider / service dumpsys into platform and non-platform Also extend the timeout to 60 seconds. - Because each provider / service dump may time out, the total time should relatively be large. Bug: 78017892 Fix: 78017892 Test: Manual test with the following dumpsys commands: dumpsys activity provider all dumpsys activity provider all-platform dumpsys activity provider all-nonplatform dumpsys activity provider com.android.providers.contacts/com.android.providers.contacts.VoicemailContentProvider dumpsys activity provider com.android.providers.contacts/.VoicemailContentProvider dumpsys activity provider contacts dumpsys activity provider voicemail dumpsys activity provider 4d45a78 dumpsys activity service all dumpsys activity service all-platform dumpsys activity service all-nonplatform dumpsys activity service bluetooth Test: atest /android/pi-dev/frameworks/base/core/tests/coretests/src/com/android/internal/util/DumpTest.java Test: atest /android/pi-dev/frameworks/base/core/tests/coretests/src/com/android/internal/util/ParseUtilsTest.java Test: Manual test with "adb bugreport" with adding sleep(10s) to ProviderMap.dumpProvider() Change-Id: If1639e6961e49265f7f9e59b1e370acdff584989 --- cmds/dumpstate/dumpstate.cpp | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 19bf216a57..c929e3596f 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1432,19 +1432,40 @@ static void dumpstate() { printf("== Running Application Activities\n"); printf("========================================================\n"); - RunDumpsys("APP ACTIVITIES", {"activity", "-v", "all"}); + // The following dumpsys internally collects output from running apps, so it can take a long + // time. So let's extend the timeout. + + const CommandOptions DUMPSYS_COMPONENTS_OPTIONS = CommandOptions::WithTimeout(60).Build(); + + RunDumpsys("APP ACTIVITIES", {"activity", "-v", "all"}, DUMPSYS_COMPONENTS_OPTIONS); printf("========================================================\n"); - printf("== Running Application Services\n"); + printf("== Running Application Services (platform)\n"); + printf("========================================================\n"); + + RunDumpsys("APP SERVICES PLATFORM", {"activity", "service", "all-platform"}, + DUMPSYS_COMPONENTS_OPTIONS); + + printf("========================================================\n"); + printf("== Running Application Services (non-platform)\n"); + printf("========================================================\n"); + + RunDumpsys("APP SERVICES NON-PLATFORM", {"activity", "service", "all-non-platform"}, + DUMPSYS_COMPONENTS_OPTIONS); + + printf("========================================================\n"); + printf("== Running Application Providers (platform)\n"); printf("========================================================\n"); - RunDumpsys("APP SERVICES", {"activity", "service", "all"}); + RunDumpsys("APP PROVIDERS PLATFORM", {"activity", "provider", "all-platform"}, + DUMPSYS_COMPONENTS_OPTIONS); printf("========================================================\n"); - printf("== Running Application Providers\n"); + printf("== Running Application Providers (non-platform)\n"); printf("========================================================\n"); - RunDumpsys("APP PROVIDERS", {"activity", "provider", "all"}); + RunDumpsys("APP PROVIDERS NON-PLATFORM", {"activity", "provider", "all-non-platform"}, + DUMPSYS_COMPONENTS_OPTIONS); printf("========================================================\n"); printf("== Dropbox crashes\n"); -- cgit v1.2.3-59-g8ed1b From 1b34605e92179f2b1c37cc9f101d985750838d9d Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Wed, 11 Apr 2018 16:42:28 -0700 Subject: Collect update_engine logs in dumpstate The update_engine logs before reboot maybe useful to debug the OTA failures. Bug: 78201703 Test: Capture a bugreport and check the entries. Change-Id: I3a7a48e524adf338c4a9f9525c98f9703e220100 (cherry picked from commit 75d53363bd1a3826d9673bc6617cd17b247e0f50) --- cmds/dumpstate/dumpstate.cpp | 2 ++ 1 file changed, 2 insertions(+) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 19bf216a57..8330fe56cb 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -104,6 +104,7 @@ void add_mountinfo(); #define RAFT_DIR "/data/misc/raft" #define RECOVERY_DIR "/cache/recovery" #define RECOVERY_DATA_DIR "/data/misc/recovery" +#define UPDATE_ENGINE_LOG_DIR "/data/misc/update_engine_log" #define LOGPERSIST_DATA_DIR "/data/misc/logd" #define PROFILE_DATA_DIR_CUR "/data/misc/profiles/cur" #define PROFILE_DATA_DIR_REF "/data/misc/profiles/ref" @@ -2126,6 +2127,7 @@ int run_main(int argc, char* argv[]) { ds.AddDir(RECOVERY_DIR, true); ds.AddDir(RECOVERY_DATA_DIR, true); + ds.AddDir(UPDATE_ENGINE_LOG_DIR, true); ds.AddDir(LOGPERSIST_DATA_DIR, false); if (!PropertiesHelper::IsUserBuild()) { ds.AddDir(PROFILE_DATA_DIR_CUR, true); -- cgit v1.2.3-59-g8ed1b From e8d9891b85c5e65888157d60b1d166d124e14b31 Mon Sep 17 00:00:00 2001 From: Sooraj Sasindran Date: Fri, 20 Apr 2018 11:31:55 -0700 Subject: Make batterystats logging readable Make batterystats logging readable Bug: 74146897 Test: verified by running telephony bugreport Change-Id: Ic638af786d7674067af804cf4105b99380bd4edb --- cmds/dumpstate/dumpstate.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 3f7f6e9b06..3e7d2a07bc 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1519,6 +1519,8 @@ static void DumpstateTelephonyOnly() { SEC_TO_MSEC(10)); RunDumpsys("DUMPSYS", {"wifi"}, CommandOptions::WithTimeout(90).Build(), SEC_TO_MSEC(10)); + RunDumpsys("BATTERYSTATS", {"batterystats"}, CommandOptions::WithTimeout(90).Build(), + SEC_TO_MSEC(10)); printf("========================================================\n"); printf("== Running Application Services\n"); @@ -1526,12 +1528,6 @@ static void DumpstateTelephonyOnly() { RunDumpsys("TELEPHONY SERVICES", {"activity", "service", "TelephonyDebugService"}); - printf("========================================================\n"); - printf("== Checkins\n"); - printf("========================================================\n"); - - RunDumpsys("CHECKIN BATTERYSTATS", {"batterystats", "-c"}); - printf("========================================================\n"); printf("== dumpstate: done (id %d)\n", ds.id_); printf("========================================================\n"); -- cgit v1.2.3-59-g8ed1b From a0829e7e1427d101046b33f8fb2ac752b94927f3 Mon Sep 17 00:00:00 2001 From: Sooraj Sasindran Date: Sat, 19 May 2018 15:52:17 -0700 Subject: Add non platform services to telephony bugreport Add non platform services to telephony bugreport Bug: 79400204 Test: Tested telephony bugreport and confirmed existence of Location service Tested the size difference. Here is the final results Without the change: 3MB With the change: 5 MB Full bugreport:10 MB Change-Id: If128a33106d70e827c7c26f0195c84e5722f6f3d --- cmds/dumpstate/dumpstate.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'cmds/dumpstate/dumpstate.cpp') diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 3e7d2a07bc..9d897f523d 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1504,6 +1504,7 @@ static void DumpstateRadioCommon() { // This method collects dumpsys for telephony debugging only static void DumpstateTelephonyOnly() { DurationReporter duration_reporter("DUMPSTATE"); + const CommandOptions DUMPSYS_COMPONENTS_OPTIONS = CommandOptions::WithTimeout(60).Build(); DumpstateRadioCommon(); @@ -1528,6 +1529,13 @@ static void DumpstateTelephonyOnly() { RunDumpsys("TELEPHONY SERVICES", {"activity", "service", "TelephonyDebugService"}); + printf("========================================================\n"); + printf("== Running Application Services (non-platform)\n"); + printf("========================================================\n"); + + RunDumpsys("APP SERVICES NON-PLATFORM", {"activity", "service", "all-non-platform"}, + DUMPSYS_COMPONENTS_OPTIONS); + printf("========================================================\n"); printf("== dumpstate: done (id %d)\n", ds.id_); printf("========================================================\n"); -- cgit v1.2.3-59-g8ed1b