summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMark Salyzyn2017-03-03 12:21:23 -0600
committerMark Salyzyn2017-03-16 10:13:43 -0500
commit1d84f0b2afd36c4a6a367761c3d518789a424419 (patch)
tree81c42a66e825109a108c3d5da0e7d32c4ced69b2
parentb3fde950f22d62c5849f85dd9faad68ca47f6962 (diff)
downloadplatform-system-core-1d84f0b2afd36c4a6a367761c3d518789a424419.tar.gz
platform-system-core-1d84f0b2afd36c4a6a367761c3d518789a424419.tar.xz
platform-system-core-1d84f0b2afd36c4a6a367761c3d518789a424419.zip
logd: ensure LogBufferElement mSequence is monotonic
- Improves accuracy of -t/-T '<timestamp>' behavior when out of order arrival of entries messes with mSequence as the list will now have monotonic sequence numbers enforced. - Out of order time entries still remain because of reader requiring the ability to receive newly arrived old entries. - -t/-T '<timestamp>' can still quit backward search prematurely because an old entry lands later in the list. - Adjust insert in place algorithm from two loops of scan placement and then limit against watermark, into one that does all of that plus iteratively swap update the sequence numbers to set monotonicity. Side effect will be that the read lock (which is actually the LogTimes lock) will be held longer while we search for a placement above the youngest LogTimes watermark. We need to hold the read (LogTimes) lock because we may be altering the sequence numbers affecting -t/-T '<timestamp>' search. Test: gTest logd-unit-tests liblog-unit-tests logcat-unit-tests Bug: 35373582 Change-Id: I79a385fc149bac2179128b53d4c8f71e429181ae
-rw-r--r--logd/LogBuffer.cpp29
-rw-r--r--logd/LogBufferElement.h2
2 files changed, 19 insertions, 12 deletions
diff --git a/logd/LogBuffer.cpp b/logd/LogBuffer.cpp
index 2b6c27688..fbd04ef8c 100644
--- a/logd/LogBuffer.cpp
+++ b/logd/LogBuffer.cpp
@@ -27,6 +27,7 @@
27#include <unistd.h> 27#include <unistd.h>
28 28
29#include <unordered_map> 29#include <unordered_map>
30#include <utility>
30 31
31#include <cutils/properties.h> 32#include <cutils/properties.h>
32#include <private/android_logger.h> 33#include <private/android_logger.h>
@@ -374,15 +375,9 @@ void LogBuffer::log(LogBufferElement* elem) {
374 // NB: if end is region locked, place element at end of list 375 // NB: if end is region locked, place element at end of list
375 LogBufferElementCollection::iterator it = mLogElements.end(); 376 LogBufferElementCollection::iterator it = mLogElements.end();
376 LogBufferElementCollection::iterator last = it; 377 LogBufferElementCollection::iterator last = it;
377 while (last != mLogElements.begin()) { 378 if (__predict_true(it != mLogElements.begin())) --it;
378 --it; 379 if (__predict_false(it == mLogElements.begin()) ||
379 if ((*it)->getRealTime() <= elem->getRealTime()) { 380 __predict_true((*it)->getRealTime() <= elem->getRealTime())) {
380 break;
381 }
382 last = it;
383 }
384
385 if (last == mLogElements.end()) {
386 mLogElements.push_back(elem); 381 mLogElements.push_back(elem);
387 } else { 382 } else {
388 uint64_t end = 1; 383 uint64_t end = 1;
@@ -399,6 +394,7 @@ void LogBuffer::log(LogBufferElement* elem) {
399 end_always = true; 394 end_always = true;
400 break; 395 break;
401 } 396 }
397 // it passing mEnd is blocked by the following checks.
402 if (!end_set || (end <= entry->mEnd)) { 398 if (!end_set || (end <= entry->mEnd)) {
403 end = entry->mEnd; 399 end = entry->mEnd;
404 end_set = true; 400 end_set = true;
@@ -407,12 +403,23 @@ void LogBuffer::log(LogBufferElement* elem) {
407 times++; 403 times++;
408 } 404 }
409 405
410 if (end_always || (end_set && (end >= (*last)->getSequence()))) { 406 if (end_always || (end_set && (end > (*it)->getSequence()))) {
411 mLogElements.push_back(elem); 407 mLogElements.push_back(elem);
412 } else { 408 } else {
409 // should be short as timestamps are localized near end()
410 do {
411 last = it;
412 if (__predict_false(it == mLogElements.begin())) {
413 break;
414 }
415
416 std::swap((*it)->mSequence, elem->mSequence);
417
418 --it;
419 } while (((*it)->getRealTime() > elem->getRealTime()) &&
420 (!end_set || (end <= (*it)->getSequence())));
413 mLogElements.insert(last, elem); 421 mLogElements.insert(last, elem);
414 } 422 }
415
416 LogTimeEntry::unlock(); 423 LogTimeEntry::unlock();
417 } 424 }
418 425
diff --git a/logd/LogBufferElement.h b/logd/LogBufferElement.h
index 43990e835..90756dd4b 100644
--- a/logd/LogBufferElement.h
+++ b/logd/LogBufferElement.h
@@ -40,7 +40,7 @@ class LogBufferElement {
40 const uint32_t mUid; 40 const uint32_t mUid;
41 const uint32_t mPid; 41 const uint32_t mPid;
42 const uint32_t mTid; 42 const uint32_t mTid;
43 const uint64_t mSequence; 43 uint64_t mSequence;
44 log_time mRealTime; 44 log_time mRealTime;
45 char* mMsg; 45 char* mMsg;
46 union { 46 union {