When THREAD_TRACE_BUFFER enables, it uses by default a list 10240 trace string as a circular buffer. There is a concern that how much extra memory of all threads may use.
This patch makes THREAD_TRACE_BUFFER variable represent the number of trace string in buffer, and each trace string has at most 256 character length. This patch also includes a fix of incorrect increment of @index_ in the method LogTraceBuffer::WriteToBuffer. --- src/amf/amfd/amfd.conf | 6 +++++- src/base/logtrace.cc | 16 +++++++++------- src/base/logtrace_buffer.cc | 41 ++++++++++++++++++++++++----------------- src/base/logtrace_buffer.h | 3 +++ 4 files changed, 41 insertions(+), 25 deletions(-) diff --git a/src/amf/amfd/amfd.conf b/src/amf/amfd/amfd.conf index 8b4fe75..d7827fe 100644 --- a/src/amf/amfd/amfd.conf +++ b/src/amf/amfd/amfd.conf @@ -28,4 +28,8 @@ export AVSV_HB_PERIOD=10000000000 # THREAD_TRACE_BUFFER variable enables the tracing, writes the trace # to thread based buffer in circular fashion. The trace buffers will # be flushed to file if an abnormal end hits, i.e. LOG_ER is called -# export THREAD_TRACE_BUFFER=1 +# The value of THREAD_TRACE_BUFFER indicates the number of trace string +# in a buffer. The length of a trace string is at most 256 characters. +# It can be disabled if set THREAD_TRACE_BUFFER as 0, the maximum value +# can be set as 65535. +# export THREAD_TRACE_BUFFER=10240 diff --git a/src/base/logtrace.cc b/src/base/logtrace.cc index 1dc1ae0..f755d71 100644 --- a/src/base/logtrace.cc +++ b/src/base/logtrace.cc @@ -42,7 +42,7 @@ char *msg_id; int logmask; const char* osaf_log_file = "osaf.log"; bool enable_osaf_log = false; -bool enable_thread_trace_buffer = false; +size_t thread_trace_buffer_size = 0; } // namespace global @@ -56,7 +56,7 @@ LogTraceClient* gl_local_thread_trace = nullptr; std::once_flag init_flag; thread_local LogTraceBuffer gl_thread_buffer{gl_local_thread_trace, - LogTraceBuffer::kBufferSize_10K}; + global::thread_trace_buffer_size}; static pid_t gettid() { return syscall(SYS_gettid); } @@ -107,7 +107,7 @@ void trace_output(const char *file, unsigned line, unsigned priority, static_cast<base::LogMessage::Severity>(priority), preamble, ap); } // thread trace - if (global::enable_thread_trace_buffer == true && + if (global::thread_trace_buffer_size > 0 && (category == CAT_TRACE_ENTER || category == CAT_TRACE_LEAVE)) { // reuse @entry if legacy trace is enabled if (!entry) { @@ -131,7 +131,7 @@ void log_output(const char *file, unsigned line, unsigned priority, LogTraceClient::Log(gl_remote_osaflog, static_cast<base::LogMessage::Severity>(priority), preamble, ap); // Flush the thread buffer for logging error or lower - if (global::enable_thread_trace_buffer == true && gl_local_thread_trace && + if (global::thread_trace_buffer_size > 0 && gl_local_thread_trace && static_cast<base::LogMessage::Severity>(priority) <= base::LogMessage::Severity::kErr) { gl_thread_buffer.RequestFlush(); @@ -187,7 +187,7 @@ void logtrace_trace(const char *file, unsigned line, unsigned category, va_list ap; if (is_logtrace_enabled(category) == false && - global::enable_thread_trace_buffer == false) return; + global::thread_trace_buffer_size == 0) return; va_start(ap, format); trace_output(file, line, LOG_DEBUG, category, format, ap); @@ -198,6 +198,7 @@ static void logtrace_init_interal(const char *pathname, unsigned mask, int* result_init) { bool result = false; char *tmp = nullptr; + uint16_t th_buffer_size; if (global::msg_id != nullptr) goto done; tmp = strdup(pathname); if (tmp != nullptr) { @@ -218,8 +219,9 @@ static void logtrace_init_interal(const char *pathname, unsigned mask, LogTraceClient::kRemoteBlocking); } } - if (base::GetEnv("THREAD_TRACE_BUFFER", uint32_t{0}) == 1) { - global::enable_thread_trace_buffer = true; + th_buffer_size = base::GetEnv("THREAD_TRACE_BUFFER", uint16_t{0}); + if (th_buffer_size > 0) { + global::thread_trace_buffer_size = th_buffer_size; if (!gl_local_thread_trace) { gl_local_thread_trace = new LogTraceClient(global::msg_id, LogTraceClient::kLocalBuffer); diff --git a/src/base/logtrace_buffer.cc b/src/base/logtrace_buffer.cc index e945dab..9a48bc3 100644 --- a/src/base/logtrace_buffer.cc +++ b/src/base/logtrace_buffer.cc @@ -29,33 +29,40 @@ LogTraceBuffer::LogTraceBuffer(LogTraceClient* owner, size_t buffer_size) : buffer_size_(buffer_size), index_(0), flush_required_(false) { - std::string log_file_name; - tid_ = syscall(SYS_gettid); - vector_.resize(buffer_size_); - if (owner_) { - owner_->AddExternalBuffer(tid_, this); - log_file_name = std::string(owner_->app_name()) + "_" + owner_->proc_id() - + "_" + std::to_string(tid_); - } else { - log_file_name = std::string(getpid() + "_" + std::to_string(tid_)); + // ensure we don't allocate any memory when THREAD_TRACE_BUFFER is disabled + if (buffer_size_ > 0) { + std::string log_file_name; + tid_ = syscall(SYS_gettid); + vector_.resize(buffer_size_); + if (owner_) { + owner_->AddExternalBuffer(tid_, this); + log_file_name = std::string(owner_->app_name()) + "_" + owner_->proc_id() + + "_" + std::to_string(tid_); + } else { + log_file_name = std::string(getpid() + "_" + std::to_string(tid_)); + } + log_writer_ = new LogWriter{log_file_name, 10, + LogWriter::kMaxFileSize_10MB}; } - log_writer_ = new LogWriter{log_file_name, 10, LogWriter::kMaxFileSize_10MB}; } LogTraceBuffer::~LogTraceBuffer() { if (owner_) { owner_->RemoveExternalBuffer(tid_); } - delete log_writer_; + if (log_writer_) delete log_writer_; } void LogTraceBuffer::WriteToBuffer(std::string trace) { - vector_[index_] = kLogTraceString + trace; - // add break line char - if (trace.at(trace.length() - 1) != '\n') - vector_[index_] += "\n"; - - if (index_++ == buffer_size_) index_ = 0; + size_t length = trace.length(); + if (length > kMaxTraceString) length = kMaxTraceString; + vector_[index_] = std::string(kLogTraceString) + + std::string(trace.c_str(), length); + length = vector_[index_].length(); + if (trace.at(trace.length() - 1) != '\n') { + vector_[index_].replace(length - 1, 1, "\n"); + } + if (++index_ == buffer_size_) index_ = 0; if (flush_required_) FlushBuffer(); } void LogTraceBuffer::RequestFlush() { diff --git a/src/base/logtrace_buffer.h b/src/base/logtrace_buffer.h index 3ba8c63..18b5bad 100644 --- a/src/base/logtrace_buffer.h +++ b/src/base/logtrace_buffer.h @@ -37,12 +37,15 @@ class LogTraceBuffer { // Identified string, added at beginning of logtrace. This string // can be used as searching key for the log trace records in coredump file constexpr static const char* kLogTraceString = "1qaz2wsx"; + // Maximum characters per trace string + static const uint32_t kMaxTraceString = 256 - strlen(kLogTraceString); LogTraceBuffer(LogTraceClient* owner, size_t buffer_size); ~LogTraceBuffer(); void WriteToBuffer(std::string trace); bool FlushBuffer(); void RequestFlush(); void SetFlush(const bool flush) { flush_required_ = flush; } + private: LogTraceClient* owner_; const size_t buffer_size_; -- 2.7.4 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel