logd: single std::mutex for locking log buffers and tracking readers
There are only three places where the log buffer lock is not already
held when the reader lock is taken:
1) In LogReader, when a new reader connects
2) In LogReader, when a misbehaving reader disconnects
3) LogReaderThread::ThreadFunction()
1) and 2) happen sufficiently rarely that there's no impact if they
additionally held a global lock.
3) is refactored in this CL. Previously, it would do the below in a loop
1) Lock the reader lock then wait on a condition variable
2) Unlock the reader lock
3) Lock the log buffer lock in LogBuffer::FlushTo()
4) In each iteration in the LogBuffer::FlushTo() loop
1) Lock then unlock the reader lock in FilterSecondPass()
2) Unlock the log buffer lock to send the message, then re-lock it
5) Unlock the log buffer lock when leaving LogBuffer::FlushTo()
If these locks are collapsed into a single lock, then this simplifies to:
1) Lock the single lock then wait on a condition variable
2) In each iteration in the LogBuffer::FlushTo() loop
1) Unlock the single lock to send the message, then re-lock it
Collapsing both these locks into a single lock simplifes the code and
removes the overhead of acquiring the second lock, in the majority of
use cases where the first lock is already held.
Secondly, this lock will be a plain std::mutex instead of a RwLock.
RwLock's are appropriate when there is a substantial imbalance between
readers and writers and high contention, neither are true for logd.
Bug: 169736426
Test: logging unit tests
Change-Id: Ia511506f2d0935a5321c1b2f65569066f91ecb06
diff --git a/logd/SimpleLogBuffer.h b/logd/SimpleLogBuffer.h
index 8e5b50e..51779ab 100644
--- a/logd/SimpleLogBuffer.h
+++ b/logd/SimpleLogBuffer.h
@@ -25,7 +25,7 @@
#include "LogReaderList.h"
#include "LogStatistics.h"
#include "LogTags.h"
-#include "rwlock.h"
+#include "LogdLock.h"
class SimpleLogBuffer : public LogBuffer {
public:
@@ -35,10 +35,12 @@
int Log(log_id_t log_id, log_time realtime, uid_t uid, pid_t pid, pid_t tid, const char* msg,
uint16_t len) override;
- std::unique_ptr<FlushToState> CreateFlushToState(uint64_t start, LogMask log_mask) override;
+ std::unique_ptr<FlushToState> CreateFlushToState(uint64_t start, LogMask log_mask)
+ REQUIRES(logd_lock) override;
bool FlushTo(LogWriter* writer, FlushToState& state,
const std::function<FilterResult(log_id_t log_id, pid_t pid, uint64_t sequence,
- log_time realtime)>& filter) override;
+ log_time realtime)>& filter)
+ REQUIRES(logd_lock) override;
bool Clear(log_id_t id, uid_t uid) override;
size_t GetSize(log_id_t id) override;
@@ -47,27 +49,25 @@
uint64_t sequence() const override { return sequence_.load(std::memory_order_relaxed); }
protected:
- virtual bool Prune(log_id_t id, unsigned long prune_rows, uid_t uid) REQUIRES(lock_);
- virtual void LogInternal(LogBufferElement&& elem) REQUIRES(lock_);
+ virtual bool Prune(log_id_t id, unsigned long prune_rows, uid_t uid) REQUIRES(logd_lock);
+ virtual void LogInternal(LogBufferElement&& elem) REQUIRES(logd_lock);
// Returns an iterator to the oldest element for a given log type, or logs_.end() if
- // there are no logs for the given log type. Requires logs_lock_ to be held.
- std::list<LogBufferElement>::iterator GetOldest(log_id_t log_id) REQUIRES(lock_);
+ // there are no logs for the given log type. Requires logs_logd_lock to be held.
+ std::list<LogBufferElement>::iterator GetOldest(log_id_t log_id) REQUIRES(logd_lock);
std::list<LogBufferElement>::iterator Erase(std::list<LogBufferElement>::iterator it)
- REQUIRES(lock_);
+ REQUIRES(logd_lock);
void KickReader(LogReaderThread* reader, log_id_t id, unsigned long prune_rows)
- REQUIRES_SHARED(lock_);
+ REQUIRES(logd_lock);
LogStatistics* stats() { return stats_; }
LogReaderList* reader_list() { return reader_list_; }
- size_t max_size(log_id_t id) REQUIRES_SHARED(lock_) { return max_size_[id]; }
+ size_t max_size(log_id_t id) REQUIRES_SHARED(logd_lock) { return max_size_[id]; }
std::list<LogBufferElement>& logs() { return logs_; }
- RwLock lock_;
-
private:
bool ShouldLog(log_id_t log_id, const char* msg, uint16_t len);
- void MaybePrune(log_id_t id) REQUIRES(lock_);
+ void MaybePrune(log_id_t id) REQUIRES(logd_lock);
LogReaderList* reader_list_;
LogTags* tags_;
@@ -75,9 +75,9 @@
std::atomic<uint64_t> sequence_ = 1;
- size_t max_size_[LOG_ID_MAX] GUARDED_BY(lock_);
- std::list<LogBufferElement> logs_ GUARDED_BY(lock_);
+ size_t max_size_[LOG_ID_MAX] GUARDED_BY(logd_lock);
+ std::list<LogBufferElement> logs_ GUARDED_BY(logd_lock);
// Keeps track of the iterator to the oldest log message of a given log type, as an
// optimization when pruning logs. Use GetOldest() to retrieve.
- std::optional<std::list<LogBufferElement>::iterator> oldest_[LOG_ID_MAX] GUARDED_BY(lock_);
+ std::optional<std::list<LogBufferElement>::iterator> oldest_[LOG_ID_MAX] GUARDED_BY(logd_lock);
};