diff options
author | Mark Salyzyn | 2017-03-03 12:21:23 -0600 |
---|---|---|
committer | Mark Salyzyn | 2017-03-16 10:13:43 -0500 |
commit | 1d84f0b2afd36c4a6a367761c3d518789a424419 (patch) | |
tree | 81c42a66e825109a108c3d5da0e7d32c4ced69b2 | |
parent | b3fde950f22d62c5849f85dd9faad68ca47f6962 (diff) | |
download | platform-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.cpp | 29 | ||||
-rw-r--r-- | logd/LogBufferElement.h | 2 |
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 { |