From 3b4bddc6386f40d57181f0e93cf8f2f37695f230 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Tue, 15 Jan 2019 15:19:43 -0800 Subject: [PATCH] Remove liblog's rate limiting After a few years of being available, there only ended up being one user of this, so it is clear that logd's duplicate message mechanism is the favored solution. The one user of this rate limiting is questionable as is, since due to the nature of storaged, the mainloop only runs once per minute by default as is, so there is essentially nothing to be gained by rate limiting any further. Test: build Change-Id: I0610d11efda1ce8b581b939bad11ff295cb2daa6 --- liblog/Android.bp | 1 - liblog/include/log/log.h | 27 ----------- liblog/log_ratelimit.cpp | 86 ------------------------------------ liblog/tests/liblog_test.cpp | 35 --------------- storaged/storaged.cpp | 10 +---- 5 files changed, 2 insertions(+), 157 deletions(-) delete mode 100644 liblog/log_ratelimit.cpp diff --git a/liblog/Android.bp b/liblog/Android.bp index e213e9026..5c6ee5922 100644 --- a/liblog/Android.bp +++ b/liblog/Android.bp @@ -19,7 +19,6 @@ liblog_sources = [ "config_write.cpp", "log_event_list.cpp", "log_event_write.cpp", - "log_ratelimit.cpp", "logger_lock.cpp", "logger_name.cpp", "logger_read.cpp", diff --git a/liblog/include/log/log.h b/liblog/include/log/log.h index 544b3a640..dc1e5cf2a 100644 --- a/liblog/include/log/log.h +++ b/liblog/include/log/log.h @@ -166,33 +166,6 @@ clockid_t android_log_clockid(void); */ void __android_log_close(void); -/* - * if last is NULL, caller _must_ provide a consistent value for seconds. - * - * Return -1 if we can not acquire a lock, which below will permit the logging, - * error on allowing a log message through. - */ -int __android_log_ratelimit(time_t seconds, time_t* last); - -/* - * Usage: - * - * // Global default and state - * IF_ALOG_RATELIMIT() { - * ALOG*(...); - * } - * - * // local state, 10 seconds ratelimit - * static time_t local_state; - * IF_ALOG_RATELIMIT_LOCAL(10, &local_state) { - * ALOG*(...); - * } - */ - -#define IF_ALOG_RATELIMIT() if (__android_log_ratelimit(0, NULL) > 0) -#define IF_ALOG_RATELIMIT_LOCAL(seconds, state) \ - if (__android_log_ratelimit(seconds, state) > 0) - #if defined(__clang__) #pragma clang diagnostic pop #endif diff --git a/liblog/log_ratelimit.cpp b/liblog/log_ratelimit.cpp deleted file mode 100644 index 33770dda4..000000000 --- a/liblog/log_ratelimit.cpp +++ /dev/null @@ -1,86 +0,0 @@ -/* -** Copyright 2016, 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 "log_portability.h" - -// Global default if 'last' argument in __android_log_ratelimit is NULL -static time_t g_last_clock; -// Global above can not deal well with callers playing games with the -// seconds argument, so we will also hold on to the maximum value -// ever provided and use that to gain consistency. If the caller -// provides their own 'last' argument, then they can play such games -// of varying the 'seconds' argument to their pleasure. -static time_t g_last_seconds; -static const time_t last_seconds_default = 10; -static const time_t last_seconds_max = 24 * 60 * 60; // maximum of a day -static const time_t last_seconds_min = 2; // granularity -// Lock to protect last_clock and last_seconds, but also 'last' -// argument (not NULL) as supplied to __android_log_ratelimit. -static pthread_mutex_t lock_ratelimit = PTHREAD_MUTEX_INITIALIZER; - -// if last is NULL, caller _must_ provide a consistent value for -// seconds, otherwise we will take the maximum ever issued and hold -// on to that. Preserves value of non-zero errno. Return -1 if we -// can not acquire a lock, 0 if we are not to log a message, and 1 -// if we are ok to log a message. Caller should check > 0 for true. -LIBLOG_ABI_PUBLIC int __android_log_ratelimit(time_t seconds, time_t* last) { - int save_errno = errno; - - // Two reasons for trylock failure: - // 1. In a signal handler. Must prevent deadlock - // 2. Too many threads calling __android_log_ratelimit. - // Bonus to not print if they race here because that - // dovetails the goal of ratelimiting. One may print - // and the others will wait their turn ... - if (pthread_mutex_trylock(&lock_ratelimit)) { - if (save_errno) errno = save_errno; - return -1; - } - - if (seconds == 0) { - seconds = last_seconds_default; - } else if (seconds < last_seconds_min) { - seconds = last_seconds_min; - } else if (seconds > last_seconds_max) { - seconds = last_seconds_max; - } - - if (!last) { - if (g_last_seconds > seconds) { - seconds = g_last_seconds; - } else if (g_last_seconds < seconds) { - g_last_seconds = seconds; - } - last = &g_last_clock; - } - - time_t now = time(NULL); - if ((now == (time_t)-1) || ((*last + seconds) > now)) { - pthread_mutex_unlock(&lock_ratelimit); - if (save_errno) errno = save_errno; - return 0; - } - *last = now; - pthread_mutex_unlock(&lock_ratelimit); - if (save_errno) errno = save_errno; - return 1; -} diff --git a/liblog/tests/liblog_test.cpp b/liblog/tests/liblog_test.cpp index 2d0fc9bc8..83f0fa91f 100644 --- a/liblog/tests/liblog_test.cpp +++ b/liblog/tests/liblog_test.cpp @@ -3160,41 +3160,6 @@ TEST(liblog, __android_log_pmsg_file_read) { } #endif // USING_LOGGER_DEFAULT -#ifdef USING_LOGGER_DEFAULT // Do not retest ratelimit -TEST(liblog, __android_log_ratelimit) { - time_t state = 0; - - errno = 42; - // Prime - __android_log_ratelimit(3, &state); - EXPECT_EQ(errno, 42); - // Check - EXPECT_FALSE(__android_log_ratelimit(3, &state)); - sleep(1); - EXPECT_FALSE(__android_log_ratelimit(3, &state)); - sleep(4); - EXPECT_TRUE(__android_log_ratelimit(3, &state)); - sleep(5); - EXPECT_TRUE(__android_log_ratelimit(3, &state)); - - // API checks - IF_ALOG_RATELIMIT_LOCAL(3, &state) { - EXPECT_FALSE(0 != "IF_ALOG_RATELIMIT_LOCAL(3, &state)"); - } - - IF_ALOG_RATELIMIT() { - ; - } - else { - EXPECT_TRUE(0 == "IF_ALOG_RATELIMIT()"); - } - IF_ALOG_RATELIMIT() { - EXPECT_FALSE(0 != "IF_ALOG_RATELIMIT()"); - } - // Do not test default seconds, to allow liblog to tune freely -} -#endif // USING_LOGGER_DEFAULT - #ifdef USING_LOGGER_DEFAULT // Do not retest event mapping functionality TEST(liblog, android_lookupEventTagNum) { #ifdef __ANDROID__ diff --git a/storaged/storaged.cpp b/storaged/storaged.cpp index 77c6167de..689766382 100644 --- a/storaged/storaged.cpp +++ b/storaged/storaged.cpp @@ -341,20 +341,14 @@ void storaged_t::event_checked(void) { if (mConfig.event_time_check_usec && clock_gettime(CLOCK_PROCESS_CPUTIME_ID, &start_ts) < 0) { check_time = false; - static time_t state_a; - IF_ALOG_RATELIMIT_LOCAL(300, &state_a) { - PLOG_TO(SYSTEM, ERROR) << "clock_gettime() failed"; - } + PLOG_TO(SYSTEM, ERROR) << "clock_gettime() failed"; } event(); if (mConfig.event_time_check_usec && check_time) { if (clock_gettime(CLOCK_PROCESS_CPUTIME_ID, &end_ts) < 0) { - static time_t state_b; - IF_ALOG_RATELIMIT_LOCAL(300, &state_b) { - PLOG_TO(SYSTEM, ERROR) << "clock_gettime() failed"; - } + PLOG_TO(SYSTEM, ERROR) << "clock_gettime() failed"; return; } int64_t cost = (end_ts.tv_sec - start_ts.tv_sec) * SEC_TO_USEC +