From d2743ef5c904f76dd7c388c28a8cd5d4fb7a5c94 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Mon, 27 Jul 2020 16:53:29 -0700 Subject: [PATCH] liblog: don't cache property size values and move to own file Don't cache the property size values since they're only queried at the start of logd and only once during dumpstate. Initializing SerializedLogBuffer, which includes all of the logd queries, takes under 100us without the cache, certainly fast enough that this cache is unneeded. Move these functions to their own file in preparation for removing them from liblog. Test: log sizes set appropriately Change-Id: I15a2fd687dcffb4eab2f22ee0825ca86e40cdba3 --- liblog/Android.bp | 1 + liblog/log_size.cpp | 83 ++++++++++++++++ liblog/properties.cpp | 147 ---------------------------- logd/LogBufferTest.cpp | 10 -- logd/ReplayMessages.cpp | 11 --- logd/fuzz/log_buffer_log_fuzzer.cpp | 10 -- 6 files changed, 84 insertions(+), 178 deletions(-) create mode 100644 liblog/log_size.cpp diff --git a/liblog/Android.bp b/liblog/Android.bp index 6051ac7fa..3a9196977 100644 --- a/liblog/Android.bp +++ b/liblog/Android.bp @@ -17,6 +17,7 @@ liblog_sources = [ "log_event_list.cpp", "log_event_write.cpp", + "log_size.cpp", "logger_name.cpp", "logger_read.cpp", "logger_write.cpp", diff --git a/liblog/log_size.cpp b/liblog/log_size.cpp new file mode 100644 index 000000000..7f13c8c2b --- /dev/null +++ b/liblog/log_size.cpp @@ -0,0 +1,83 @@ +/* + * Copyright 2020 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 + +#ifdef __ANDROID__ +#include +#endif + +bool __android_logger_valid_buffer_size(unsigned long value) { + return LOG_BUFFER_MIN_SIZE <= value && value <= LOG_BUFFER_MAX_SIZE; +} + +#ifdef __ANDROID__ + +static std::optional GetBufferSizeProperty(const std::string& key) { + char value[PROP_VALUE_MAX] = {}; + if (__system_property_get(key.c_str(), value) <= 0) { + return {}; + } + + uint32_t size; + if (!android::base::ParseByteCount(value, &size)) { + return {}; + } + + if (!__android_logger_valid_buffer_size(size)) { + return {}; + } + + return size; +} + +unsigned long __android_logger_get_buffer_size(log_id_t log_id) { + std::string buffer_name = android_log_id_to_name(log_id); + std::array properties = { + "persist.logd.size." + buffer_name, + "ro.logd.size." + buffer_name, + "persist.logd.size", + "ro.logd.size", + }; + + for (const auto& property : properties) { + if (auto size = GetBufferSizeProperty(property)) { + return *size; + } + } + + char value[PROP_VALUE_MAX] = {}; + if (__system_property_get("ro.config.low_ram", value) > 0 && !strcmp(value, "true")) { + return LOG_BUFFER_MIN_SIZE; + } + + return LOG_BUFFER_SIZE; +} + +#else + +// Default to 1MB for host. +unsigned long __android_logger_get_buffer_size(log_id_t) { + return 1024 * 1024; +} + +#endif \ No newline at end of file diff --git a/liblog/properties.cpp b/liblog/properties.cpp index 9990137e6..88f0bf1ee 100644 --- a/liblog/properties.cpp +++ b/liblog/properties.cpp @@ -364,153 +364,6 @@ int __android_log_security() { return do_cache2_char(&security); } -/* - * Interface that represents the logd buffer size determination so that others - * need not guess our intentions. - */ - -/* cache structure */ -struct cache_property { - struct cache cache; - char property[PROP_VALUE_MAX]; -}; - -static void refresh_cache_property(struct cache_property* cache, const char* key) { - if (!cache->cache.pinfo) { - cache->cache.pinfo = __system_property_find(key); - if (!cache->cache.pinfo) { - return; - } - } - cache->cache.serial = __system_property_serial(cache->cache.pinfo); - __system_property_read(cache->cache.pinfo, 0, cache->property); -} - -bool __android_logger_valid_buffer_size(unsigned long value) { - return LOG_BUFFER_MIN_SIZE <= value && value <= LOG_BUFFER_MAX_SIZE; -} - -struct cache2_property_size { - pthread_mutex_t lock; - uint32_t serial; - const char* key_persist; - struct cache_property cache_persist; - const char* key_ro; - struct cache_property cache_ro; - unsigned long (*const evaluate)(const struct cache2_property_size* self); -}; - -static inline unsigned long do_cache2_property_size(struct cache2_property_size* self) { - uint32_t current_serial; - int change_detected; - unsigned long v; - - if (pthread_mutex_trylock(&self->lock)) { - /* We are willing to accept some race in this context */ - return self->evaluate(self); - } - - change_detected = check_cache(&self->cache_persist.cache) || check_cache(&self->cache_ro.cache); - current_serial = __system_property_area_serial(); - if (current_serial != self->serial) { - change_detected = 1; - } - if (change_detected) { - refresh_cache_property(&self->cache_persist, self->key_persist); - refresh_cache_property(&self->cache_ro, self->key_ro); - self->serial = current_serial; - } - v = self->evaluate(self); - - pthread_mutex_unlock(&self->lock); - - return v; -} - -static unsigned long property_get_size_from_cache(const struct cache_property* cache) { - char* cp; - unsigned long value = strtoul(cache->property, &cp, 10); - - switch (*cp) { - case 'm': - case 'M': - value *= 1024; - [[fallthrough]]; - case 'k': - case 'K': - value *= 1024; - [[fallthrough]]; - case '\0': - break; - - default: - value = 0; - } - - if (!__android_logger_valid_buffer_size(value)) { - value = 0; - } - - return value; -} - -static unsigned long evaluate_property_get_size(const struct cache2_property_size* self) { - unsigned long size = property_get_size_from_cache(&self->cache_persist); - if (size) { - return size; - } - return property_get_size_from_cache(&self->cache_ro); -} - -unsigned long __android_logger_get_buffer_size(log_id_t logId) { - static const char global_tunable[] = "persist.logd.size"; /* Settings App */ - static const char global_default[] = "ro.logd.size"; /* BoardConfig.mk */ - static struct cache2_property_size global = { - /* clang-format off */ - PTHREAD_MUTEX_INITIALIZER, 0, - global_tunable, { { NULL, 0xFFFFFFFF }, {} }, - global_default, { { NULL, 0xFFFFFFFF }, {} }, - evaluate_property_get_size - /* clang-format on */ - }; - char key_persist[strlen(global_tunable) + strlen(".security") + 1]; - char key_ro[strlen(global_default) + strlen(".security") + 1]; - struct cache2_property_size local = { - /* clang-format off */ - PTHREAD_MUTEX_INITIALIZER, 0, - key_persist, { { NULL, 0xFFFFFFFF }, {} }, - key_ro, { { NULL, 0xFFFFFFFF }, {} }, - evaluate_property_get_size - /* clang-format on */ - }; - unsigned long property_size, default_size; - - default_size = do_cache2_property_size(&global); - if (!default_size) { - char value[PROP_VALUE_MAX] = {}; - if (__system_property_get("ro.config.low_ram", value) == 0 || strcmp(value, "true") != 0) { - default_size = LOG_BUFFER_SIZE; - } else { - default_size = LOG_BUFFER_MIN_SIZE; - } - } - - snprintf(key_persist, sizeof(key_persist), "%s.%s", global_tunable, - android_log_id_to_name(logId)); - snprintf(key_ro, sizeof(key_ro), "%s.%s", global_default, android_log_id_to_name(logId)); - property_size = do_cache2_property_size(&local); - - if (!property_size) { - property_size = default_size; - } - - if (!property_size) { - property_size = LOG_BUFFER_SIZE; - } - - return property_size; -} - #else int __android_log_is_loggable(int prio, const char*, int) { diff --git a/logd/LogBufferTest.cpp b/logd/LogBufferTest.cpp index 47d2a2f92..191110522 100644 --- a/logd/LogBufferTest.cpp +++ b/logd/LogBufferTest.cpp @@ -34,16 +34,6 @@ using android::base::Join; using android::base::Split; using android::base::StringPrintf; -#ifndef __ANDROID__ -unsigned long __android_logger_get_buffer_size(log_id_t) { - return 1024 * 1024; -} - -bool __android_logger_valid_buffer_size(unsigned long) { - return true; -} -#endif - char* android::uidToName(uid_t) { return nullptr; } diff --git a/logd/ReplayMessages.cpp b/logd/ReplayMessages.cpp index 73b0bd068..5347ba9a4 100644 --- a/logd/ReplayMessages.cpp +++ b/logd/ReplayMessages.cpp @@ -40,17 +40,6 @@ using android::base::ParseInt; using android::base::ParseUint; using android::base::Split; -#ifndef __ANDROID__ -// This is hard coded to 1MB on host. On device use persist.logd.size to configure. -unsigned long __android_logger_get_buffer_size(log_id_t) { - return 1 * 1024 * 1024; -} - -bool __android_logger_valid_buffer_size(unsigned long) { - return true; -} -#endif - char* android::uidToName(uid_t) { return nullptr; } diff --git a/logd/fuzz/log_buffer_log_fuzzer.cpp b/logd/fuzz/log_buffer_log_fuzzer.cpp index 8309f9514..d71a2f91f 100644 --- a/logd/fuzz/log_buffer_log_fuzzer.cpp +++ b/logd/fuzz/log_buffer_log_fuzzer.cpp @@ -30,16 +30,6 @@ #define MIN_TAG_ID 1000 #define TAG_MOD 10 -#ifndef __ANDROID__ -unsigned long __android_logger_get_buffer_size(log_id_t) { - return 1024 * 1024; -} - -bool __android_logger_valid_buffer_size(unsigned long) { - return true; -} -#endif - char* android::uidToName(uid_t) { return strdup("fake"); }