Merge "logd: simplify Clear() + Prune() logic" am: 7a372d168a am: b4fc12cc6c
Original change: https://android-review.googlesource.com/c/platform/system/core/+/1329340
Change-Id: I6925a72bb87f51085dfd085c9ff0a88d5e98b180
diff --git a/logd/ChattyLogBuffer.cpp b/logd/ChattyLogBuffer.cpp
index f92fe65..c213448 100644
--- a/logd/ChattyLogBuffer.cpp
+++ b/logd/ChattyLogBuffer.cpp
@@ -327,7 +327,6 @@
//
bool ChattyLogBuffer::Prune(log_id_t id, unsigned long pruneRows, uid_t caller_uid) {
LogReaderThread* oldest = nullptr;
- bool busy = false;
bool clearAll = pruneRows == ULONG_MAX;
auto reader_threads_lock = std::lock_guard{reader_list()->reader_threads_lock()};
@@ -359,17 +358,16 @@
}
if (oldest && oldest->start() <= element.sequence()) {
- busy = true;
KickReader(oldest, id, pruneRows);
- break;
+ return false;
}
it = Erase(it);
if (--pruneRows == 0) {
- break;
+ return true;
}
}
- return busy;
+ return true;
}
// prune by worst offenders; by blacklist, UID, and by PID of system UID
@@ -440,7 +438,6 @@
LogBufferElement& element = *it;
if (oldest && oldest->start() <= element.sequence()) {
- busy = true;
// Do not let chatty eliding trigger any reader mitigation
break;
}
@@ -577,7 +574,6 @@
}
if (oldest && oldest->start() <= element.sequence()) {
- busy = true;
if (!whitelist) KickReader(oldest, id, pruneRows);
break;
}
@@ -605,7 +601,6 @@
}
if (oldest && oldest->start() <= element.sequence()) {
- busy = true;
KickReader(oldest, id, pruneRows);
break;
}
@@ -615,5 +610,5 @@
}
}
- return (pruneRows > 0) && busy;
+ return pruneRows == 0 || it == logs().end();
}
diff --git a/logd/CommandListener.cpp b/logd/CommandListener.cpp
index 9764c44..39c5490 100644
--- a/logd/CommandListener.cpp
+++ b/logd/CommandListener.cpp
@@ -82,7 +82,7 @@
return 0;
}
- cli->sendMsg(buf()->Clear((log_id_t)id, uid) ? "busy" : "success");
+ cli->sendMsg(buf()->Clear((log_id_t)id, uid) ? "success" : "busy");
return 0;
}
diff --git a/logd/SimpleLogBuffer.cpp b/logd/SimpleLogBuffer.cpp
index 292a7e4..ec08d54 100644
--- a/logd/SimpleLogBuffer.cpp
+++ b/logd/SimpleLogBuffer.cpp
@@ -212,46 +212,38 @@
return true;
}
-// clear all rows of type "id" from the buffer.
bool SimpleLogBuffer::Clear(log_id_t id, uid_t uid) {
- bool busy = true;
- // If it takes more than 4 tries (seconds) to clear, then kill reader(s)
- for (int retry = 4;;) {
- if (retry == 1) { // last pass
- // Check if it is still busy after the sleep, we say prune
- // one entry, not another clear run, so we are looking for
- // the quick side effect of the return value to tell us if
- // we have a _blocked_ reader.
- {
- auto lock = std::lock_guard{lock_};
- busy = Prune(id, 1, uid);
- }
- // It is still busy, blocked reader(s), lets kill them all!
- // otherwise, lets be a good citizen and preserve the slow
- // readers and let the clear run (below) deal with determining
- // if we are still blocked and return an error code to caller.
- if (busy) {
- auto reader_threads_lock = std::lock_guard{reader_list_->reader_threads_lock()};
- for (const auto& reader_thread : reader_list_->reader_threads()) {
- if (reader_thread->IsWatching(id)) {
- LOG(WARNING) << "Kicking blocked reader, " << reader_thread->name()
- << ", from LogBuffer::clear()";
- reader_thread->release_Locked();
- }
- }
- }
- }
+ // Try three times to clear, then disconnect the readers and try one final time.
+ for (int retry = 0; retry < 3; ++retry) {
{
auto lock = std::lock_guard{lock_};
- busy = Prune(id, ULONG_MAX, uid);
+ if (Prune(id, ULONG_MAX, uid)) {
+ return true;
+ }
}
-
- if (!busy || !--retry) {
- break;
- }
- sleep(1); // Let reader(s) catch up after notification
+ sleep(1);
}
- return busy;
+ // Check if it is still busy after the sleep, we try to prune one entry, not another clear run,
+ // so we are looking for the quick side effect of the return value to tell us if we have a
+ // _blocked_ reader.
+ bool busy = false;
+ {
+ auto lock = std::lock_guard{lock_};
+ busy = !Prune(id, 1, uid);
+ }
+ // It is still busy, disconnect all readers.
+ if (busy) {
+ auto reader_threads_lock = std::lock_guard{reader_list_->reader_threads_lock()};
+ for (const auto& reader_thread : reader_list_->reader_threads()) {
+ if (reader_thread->IsWatching(id)) {
+ LOG(WARNING) << "Kicking blocked reader, " << reader_thread->name()
+ << ", from LogBuffer::clear()";
+ reader_thread->release_Locked();
+ }
+ }
+ }
+ auto lock = std::lock_guard{lock_};
+ return Prune(id, ULONG_MAX, uid);
}
// get the total space allocated to "id"
@@ -313,16 +305,16 @@
if (oldest && oldest->start() <= element.sequence()) {
KickReader(oldest, id, prune_rows);
- return true;
+ return false;
}
stats_->Subtract(element.ToLogStatisticsElement());
it = Erase(it);
if (--prune_rows == 0) {
- return false;
+ return true;
}
}
- return false;
+ return true;
}
std::list<LogBufferElement>::iterator SimpleLogBuffer::Erase(