From b06766cbbf9b94d8aa36501839134714fe4c57d6 Mon Sep 17 00:00:00 2001 From: Florian Mayer Date: Tue, 10 Dec 2019 12:20:03 +0000 Subject: [PATCH] Use sequence number to check if to reload atrace tags. This is to deprecate the sysprop change notification in atrace. After this change, processes will only update their enabled tags at the first atrace event. Previously we reloaded the tags as a result of the sysprop changed Binder notification, which woke up every process in the system. Test: adb shell su root atrace -t 10 ss Test: #define ATRACE_SHMEM 0; adb shell su root atrace -t 10 ss Bug: 137366208 Change-Id: Idffba5fd4ba23fba2f6b9f594365df68ac0c1626 --- libcutils/include/cutils/trace.h | 27 +++++++++++--- libcutils/trace-container.cpp | 5 +++ libcutils/trace-dev.cpp | 27 ++++++++++++++ libcutils/trace-dev.inc | 64 ++++++++++++++++++++++++++++---- libcutils/trace-host.cpp | 7 ++++ 5 files changed, 117 insertions(+), 13 deletions(-) diff --git a/libcutils/include/cutils/trace.h b/libcutils/include/cutils/trace.h index 79b4b355b..e12c32cff 100644 --- a/libcutils/include/cutils/trace.h +++ b/libcutils/include/cutils/trace.h @@ -25,7 +25,6 @@ #include #include #include - #include __BEGIN_DECLS @@ -89,6 +88,12 @@ __BEGIN_DECLS #error ATRACE_TAG must be defined to be one of the tags defined in cutils/trace.h #endif +// Set this to 0 to revert to the old Binder-based atrace implementation. +// This is only here in case rollbacks do not apply cleanly. +// TODO(fmayer): Remove this once we are confident this won't need to be +// rolled back, no later than 2020-03-01. +#define ATRACE_SHMEM 1 + /** * Opens the trace file for writing and reads the property for initial tags. * The atrace.tags.enableflags property sets the tags to trace. @@ -116,11 +121,15 @@ void atrace_set_debuggable(bool debuggable); * prevent tracing within the Zygote process. */ void atrace_set_tracing_enabled(bool enabled); - /** - * Flag indicating whether setup has been completed, initialized to 0. - * Nonzero indicates setup has completed. - * Note: This does NOT indicate whether or not setup was successful. + * If !ATRACE_SHMEM: + * Flag indicating whether setup has been completed, initialized to 0. + * Nonzero indicates setup has completed. + * Note: This does NOT indicate whether or not setup was successful. + * If ATRACE_SHMEM: + * This is always set to false. This forces code that uses an old version + * of this header to always call into atrace_setup, in which we call + * atrace_init unconditionally. */ extern atomic_bool atrace_is_ready; @@ -143,6 +152,12 @@ extern int atrace_marker_fd; * This can be explicitly run to avoid setup delay on first trace function. */ #define ATRACE_INIT() atrace_init() +#define ATRACE_GET_ENABLED_TAGS() atrace_get_enabled_tags() + +#if ATRACE_SHMEM +void atrace_init(); +uint64_t atrace_get_enabled_tags(); +#else static inline void atrace_init() { if (CC_UNLIKELY(!atomic_load_explicit(&atrace_is_ready, memory_order_acquire))) { @@ -155,12 +170,12 @@ static inline void atrace_init() * It can be used as a guard condition around more expensive trace calculations. * Every trace function calls this, which ensures atrace_init is run. */ -#define ATRACE_GET_ENABLED_TAGS() atrace_get_enabled_tags() static inline uint64_t atrace_get_enabled_tags() { atrace_init(); return atrace_enabled_tags; } +#endif /** * Test if a given tag is currently enabled. diff --git a/libcutils/trace-container.cpp b/libcutils/trace-container.cpp index d981f8fa5..c23d5e2bd 100644 --- a/libcutils/trace-container.cpp +++ b/libcutils/trace-container.cpp @@ -39,6 +39,11 @@ static int atrace_container_sock_fd = -1; static pthread_mutex_t atrace_enabling_mutex = PTHREAD_MUTEX_INITIALIZER; static pthread_rwlock_t atrace_container_sock_rwlock = PTHREAD_RWLOCK_INITIALIZER; +static void atrace_seq_number_changed(uint32_t, uint32_t seq_no) { + pthread_once(&atrace_once_control, atrace_init_once); + atomic_store_explicit(&last_sequence_number, seq_no, memory_order_relaxed); +} + static bool atrace_init_container_sock() { pthread_rwlock_wrlock(&atrace_container_sock_rwlock); diff --git a/libcutils/trace-dev.cpp b/libcutils/trace-dev.cpp index bff16c174..2ee39d328 100644 --- a/libcutils/trace-dev.cpp +++ b/libcutils/trace-dev.cpp @@ -37,12 +37,39 @@ static void atrace_init_once() } else { atrace_enabled_tags = atrace_get_property(); } +#if !ATRACE_SHMEM atomic_store_explicit(&atrace_is_ready, true, memory_order_release); +#endif +} + +static void atrace_seq_number_changed(uint32_t prev_seq_no, uint32_t seq_no) { + if (!atomic_load_explicit(&atrace_is_enabled, memory_order_acquire)) { + return; + } + + // Someone raced us. + if (!atomic_compare_exchange_strong(&last_sequence_number, &prev_seq_no, seq_no)) { + return; + } + + if (CC_UNLIKELY(prev_seq_no == kSeqNoNotInit)) { +#if defined(__BIONIC__) + const prop_info* new_pi = __system_property_find("debug.atrace.tags.enableflags"); + if (new_pi) atrace_property_info = new_pi; +#endif + pthread_once(&atrace_once_control, atrace_init_once); + } + + atrace_update_tags(); } void atrace_setup() { +#if ATRACE_SHMEM + atrace_init(); +#else pthread_once(&atrace_once_control, atrace_init_once); +#endif } void atrace_begin_body(const char* name) diff --git a/libcutils/trace-dev.inc b/libcutils/trace-dev.inc index e3da77bec..a57a4c59e 100644 --- a/libcutils/trace-dev.inc +++ b/libcutils/trace-dev.inc @@ -34,6 +34,11 @@ #include #include +#if defined(__BIONIC__) +#define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_ +#include +#endif + /** * Maximum size of a message that can be logged to the trace buffer. * Note this message includes a tag, the pid, and the string given as the name. @@ -41,12 +46,57 @@ */ #define ATRACE_MESSAGE_LENGTH 1024 -atomic_bool atrace_is_ready = ATOMIC_VAR_INIT(false); -int atrace_marker_fd = -1; -uint64_t atrace_enabled_tags = ATRACE_TAG_NOT_READY; -static bool atrace_is_debuggable = false; -static atomic_bool atrace_is_enabled = ATOMIC_VAR_INIT(true); -static pthread_mutex_t atrace_tags_mutex = PTHREAD_MUTEX_INITIALIZER; +constexpr uint32_t kSeqNoNotInit = static_cast(-1); + +atomic_bool atrace_is_ready = ATOMIC_VAR_INIT(false); +int atrace_marker_fd = -1; +uint64_t atrace_enabled_tags = ATRACE_TAG_NOT_READY; +static bool atrace_is_debuggable = false; +static atomic_bool atrace_is_enabled = ATOMIC_VAR_INIT(true); +static pthread_mutex_t atrace_tags_mutex = PTHREAD_MUTEX_INITIALIZER; + +/** + * Sequence number of debug.atrace.tags.enableflags the last time the enabled + * tags were reloaded. + **/ +static _Atomic(uint32_t) last_sequence_number = ATOMIC_VAR_INIT(kSeqNoNotInit); + +#if defined(__BIONIC__) +// All zero prop_info that has a sequence number of 0. This is easier than +// depending on implementation details of the property implementation. +// +// prop_info is static_assert-ed to be 96 bytes, which cannot change due to +// ABI compatibility. +alignas(uint64_t) static char empty_pi[96]; +static const prop_info* atrace_property_info = reinterpret_cast(empty_pi); +#endif + +#if ATRACE_SHMEM + +/** + * This is called when the sequence number of debug.atrace.tags.enableflags + * changes and we need to reload the enabled tags. + **/ +static void atrace_seq_number_changed(uint32_t prev_seq_no, uint32_t seq_no); + +void atrace_init() { +#if defined(__BIONIC__) + uint32_t seq_no = __system_property_serial(atrace_property_info); // Acquire semantics. +#else + uint32_t seq_no = 0; +#endif + uint32_t prev_seq_no = atomic_load_explicit(&last_sequence_number, memory_order_relaxed); + if (CC_UNLIKELY(seq_no != prev_seq_no)) { + atrace_seq_number_changed(prev_seq_no, seq_no); + } +} + +uint64_t atrace_get_enabled_tags() +{ + atrace_init(); + return atrace_enabled_tags; +} +#endif // Set whether this process is debuggable, which determines whether // application-level tracing is allowed when the ro.debuggable system property @@ -136,7 +186,7 @@ static uint64_t atrace_get_property() void atrace_update_tags() { uint64_t tags; - if (CC_UNLIKELY(atomic_load_explicit(&atrace_is_ready, memory_order_acquire))) { + if (ATRACE_SHMEM || CC_UNLIKELY(atomic_load_explicit(&atrace_is_ready, memory_order_acquire))) { if (atomic_load_explicit(&atrace_is_enabled, memory_order_acquire)) { tags = atrace_get_property(); pthread_mutex_lock(&atrace_tags_mutex); diff --git a/libcutils/trace-host.cpp b/libcutils/trace-host.cpp index d47cc1861..c21d0ee5e 100644 --- a/libcutils/trace-host.cpp +++ b/libcutils/trace-host.cpp @@ -30,3 +30,10 @@ void atrace_async_begin_body(const char* /*name*/, int32_t /*cookie*/) {} void atrace_async_end_body(const char* /*name*/, int32_t /*cookie*/) {} void atrace_int_body(const char* /*name*/, int32_t /*value*/) {} void atrace_int64_body(const char* /*name*/, int64_t /*value*/) {} +#if ATRACE_SHMEM +void atrace_init() {} +uint64_t atrace_get_enabled_tags() +{ + return ATRACE_TAG_NOT_READY; +} +#endif