From bc72197b33bb97cdbb4227d66b2b38d889179c08 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Wed, 22 Jul 2020 15:30:02 -0700 Subject: logd: use the compressed (serialized) log buffer by default The serialized log buffer along with compression results in: * ~3.5x more logs than chatty * Less CPU usage * Less memory usage * Equivalent log range Also, delete tests that assume that the device logd implementation is chatty. There are actual unit tests for this same behavior that don't rely on the device logd. Test: serialized log buffer is used Change-Id: Ie12898617429a75b6caff92725aa7145650f8fc6 --- logd/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'logd/main.cpp') diff --git a/logd/main.cpp b/logd/main.cpp index 897e11e3a..e7a69ebef 100644 --- a/logd/main.cpp +++ b/logd/main.cpp @@ -258,7 +258,7 @@ int main(int argc, char* argv[]) { // Pruning configuration. PruneList prune_list; - std::string buffer_type = GetProperty("logd.buffer_type", "chatty"); + std::string buffer_type = GetProperty("logd.buffer_type", "serialized"); // Partial (required for chatty) or full logging statistics. bool enable_full_log_statistics = __android_logger_property_get_bool( -- cgit v1.2.3 From 68261eec2442f25a223faac2b3601a87021ea72a Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Tue, 28 Jul 2020 09:51:54 -0700 Subject: logd: remove users of __android_logger_property_get_bool() __android_logger_property_get_bool() has a clunky API and doesn't belong in liblog, since a vast majority of liblog users will never query this property. Specifically 1) Replace with GetBoolProperty() when completely equivalent. 2) Remove checking if property values are 'eng' or 'svelte', since there's no evidence that those values were ever used. 3) Remove checking 'persist.logd.statistics' and 'ro.logd.statistics', since there's no evidence that those values were ever used. 4) Set ro.logd.kernel explicitly, so other processes don't need to replicate the defaults that logd uses. Test: build Change-Id: I7c37af64ba7754e839185f46da66bf077f09d9c3 --- logd/main.cpp | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) (limited to 'logd/main.cpp') diff --git a/logd/main.cpp b/logd/main.cpp index e7a69ebef..93dadf597 100644 --- a/logd/main.cpp +++ b/logd/main.cpp @@ -62,7 +62,9 @@ #include "SerializedLogBuffer.h" #include "SimpleLogBuffer.h" +using android::base::GetBoolProperty; using android::base::GetProperty; +using android::base::SetProperty; #define KMSG_PRIORITY(PRI) \ '<', '0' + LOG_MAKEPRI(LOG_DAEMON, LOG_PRI(PRI)) / 10, \ @@ -82,9 +84,10 @@ static void DropPrivs(bool klogd, bool auditd) { PLOG(FATAL) << "failed to set batch scheduler"; } - if (!__android_logger_property_get_bool("ro.debuggable", BOOL_DEFAULT_FALSE) && - prctl(PR_SET_DUMPABLE, 0) == -1) { - PLOG(FATAL) << "failed to clear PR_SET_DUMPABLE"; + if (!GetBoolProperty("ro.debuggable", false)) { + if (prctl(PR_SET_DUMPABLE, 0) == -1) { + PLOG(FATAL) << "failed to clear PR_SET_DUMPABLE"; + } } std::unique_ptr caps(cap_init(), cap_free); @@ -110,6 +113,14 @@ static void DropPrivs(bool klogd, bool auditd) { } } +// GetBoolProperty that defaults to true if `ro.debuggable == true && ro.config.low_rawm == false`. +static bool GetBoolPropertyEngSvelteDefault(const std::string& name) { + bool default_value = + GetBoolProperty("ro.debuggable", false) && !GetBoolProperty("ro.config.low_ram", false); + + return GetBoolProperty(name, default_value); +} + char* android::uidToName(uid_t u) { struct Userdata { uid_t uid; @@ -236,10 +247,9 @@ int main(int argc, char* argv[]) { } int fdPmesg = -1; - bool klogd = __android_logger_property_get_bool( - "ro.logd.kernel", - BOOL_DEFAULT_TRUE | BOOL_DEFAULT_FLAG_ENG | BOOL_DEFAULT_FLAG_SVELTE); + bool klogd = GetBoolPropertyEngSvelteDefault("ro.logd.kernel"); if (klogd) { + SetProperty("ro.logd.kernel", "true"); static const char proc_kmsg[] = "/proc/kmsg"; fdPmesg = android_get_control_file(proc_kmsg); if (fdPmesg < 0) { @@ -249,7 +259,7 @@ int main(int argc, char* argv[]) { if (fdPmesg < 0) PLOG(ERROR) << "Failed to open " << proc_kmsg; } - bool auditd = __android_logger_property_get_bool("ro.logd.auditd", BOOL_DEFAULT_TRUE); + bool auditd = GetBoolProperty("ro.logd.auditd", true); DropPrivs(klogd, auditd); // A cache of event log tags @@ -261,10 +271,8 @@ int main(int argc, char* argv[]) { std::string buffer_type = GetProperty("logd.buffer_type", "serialized"); // Partial (required for chatty) or full logging statistics. - bool enable_full_log_statistics = __android_logger_property_get_bool( - "logd.statistics", BOOL_DEFAULT_TRUE | BOOL_DEFAULT_FLAG_PERSIST | - BOOL_DEFAULT_FLAG_ENG | BOOL_DEFAULT_FLAG_SVELTE); - LogStatistics log_statistics(enable_full_log_statistics, buffer_type == "serialized"); + LogStatistics log_statistics(GetBoolPropertyEngSvelteDefault("logd.statistics"), + buffer_type == "serialized"); // Serves the purpose of managing the last logs times read on a socket connection, and as a // reader lock on a range of log entries. @@ -309,9 +317,7 @@ int main(int argc, char* argv[]) { // and LogReader is notified to send updates to connected clients. LogAudit* al = nullptr; if (auditd) { - int dmesg_fd = __android_logger_property_get_bool("ro.logd.auditd.dmesg", BOOL_DEFAULT_TRUE) - ? fdDmesg - : -1; + int dmesg_fd = GetBoolProperty("ro.logd.auditd.dmesg", true) ? fdDmesg : -1; al = new LogAudit(log_buffer, dmesg_fd, &log_statistics); } -- cgit v1.2.3 From 2b3ebaf5c03acaffa1607d6385a2508f4bdd320e Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Fri, 31 Jul 2020 15:21:54 -0700 Subject: SocketClient: don't ignore SIGPIPE 1) All current users are better off ignoring SIGPIPE at the beginning of their process instead of ignoring it just for SocketClient 2) This isn't thread safe if users did want it, since sigaction() ignores SIGPIPE for the entire process 3) This costs 5-10% of logd CPU time when logcat is reading logs Also clean up the error handling in SocketClient::sendDataLockedv(). Test: kill logcat and see that logd doesn't crash Test: run simpleperf and see that no cycles are going to sigaction Change-Id: I6532c8a0d71338e534411707b9a9bd785145c730 --- logd/main.cpp | 2 ++ 1 file changed, 2 insertions(+) (limited to 'logd/main.cpp') diff --git a/logd/main.cpp b/logd/main.cpp index 93dadf597..c92c5b719 100644 --- a/logd/main.cpp +++ b/logd/main.cpp @@ -218,6 +218,8 @@ static int issueReinit() { // logging plugins like auditd and restart control. Additional // transitory per-client threads are created for each reader. int main(int argc, char* argv[]) { + // We want EPIPE when a reader disconnects, not to terminate logd. + signal(SIGPIPE, SIG_IGN); // logd is written under the assumption that the timezone is UTC. // If TZ is not set, persist.sys.timezone is looked up in some time utility // libc functions, including mktime. It confuses the logd time handling, -- cgit v1.2.3