From 3fb75e9059dad927aaf1549331ffe21325d37f25 Mon Sep 17 00:00:00 2001 From: Lalit Maganti Date: Thu, 5 Sep 2024 13:48:38 +0100 Subject: [PATCH] tp: always try to convert ftrace timestamps We were assuming the default trace clock was always BOOTTIME for ftrace which is not a valid assumption if the primary_trace_clock field is set. Make sure to always go into clock tracker conversion so that the correct conversion always happens. Change-Id: I840ca6b0ff161bf740a01d773f183905b3cb4a49 --- .../importers/common/clock_tracker.h | 9 ++-- .../importers/ftrace/ftrace_tokenizer.cc | 48 +++++++++---------- 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/src/trace_processor/importers/common/clock_tracker.h b/src/trace_processor/importers/common/clock_tracker.h index 36a4a9a737..4a5a28e76c 100644 --- a/src/trace_processor/importers/common/clock_tracker.h +++ b/src/trace_processor/importers/common/clock_tracker.h @@ -176,7 +176,7 @@ class ClockTracker { } // Apply the clock offset to convert remote trace times to host trace time. - int64_t ToHostTraceTime(int64_t timestamp) { + PERFETTO_ALWAYS_INLINE int64_t ToHostTraceTime(int64_t timestamp) { if (PERFETTO_LIKELY(!context_->machine_id())) { // No need to convert host timestamps. return timestamp; @@ -188,7 +188,9 @@ class ClockTracker { return timestamp - clock_offset; } - base::StatusOr ToTraceTime(ClockId clock_id, int64_t timestamp) { + PERFETTO_ALWAYS_INLINE base::StatusOr ToTraceTime( + ClockId clock_id, + int64_t timestamp) { if (PERFETTO_UNLIKELY(!trace_time_clock_id_used_for_conversion_)) { context_->metadata_tracker->SetMetadata( metadata::trace_time_clock_id, @@ -197,8 +199,9 @@ class ClockTracker { } trace_time_clock_id_used_for_conversion_ = true; - if (clock_id == trace_time_clock_id_) + if (clock_id == trace_time_clock_id_) { return ToHostTraceTime(timestamp); + } ASSIGN_OR_RETURN(int64_t ts, Convert(clock_id, timestamp, trace_time_clock_id_)); diff --git a/src/trace_processor/importers/ftrace/ftrace_tokenizer.cc b/src/trace_processor/importers/ftrace/ftrace_tokenizer.cc index 14f25c8ef9..6eb477de65 100644 --- a/src/trace_processor/importers/ftrace/ftrace_tokenizer.cc +++ b/src/trace_processor/importers/ftrace/ftrace_tokenizer.cc @@ -15,14 +15,26 @@ */ #include "src/trace_processor/importers/ftrace/ftrace_tokenizer.h" +#include +#include +#include +#include +#include #include "perfetto/base/logging.h" #include "perfetto/base/status.h" +#include "perfetto/protozero/field.h" #include "perfetto/protozero/proto_decoder.h" #include "perfetto/protozero/proto_utils.h" +#include "perfetto/public/compiler.h" #include "perfetto/trace_processor/basic_types.h" +#include "perfetto/trace_processor/ref_counted.h" +#include "perfetto/trace_processor/trace_blob_view.h" +#include "src/trace_processor/importers/common/clock_tracker.h" #include "src/trace_processor/importers/common/machine_tracker.h" #include "src/trace_processor/importers/common/metadata_tracker.h" +#include "src/trace_processor/importers/common/parser_types.h" +#include "src/trace_processor/importers/proto/packet_sequence_state_generation.h" #include "src/trace_processor/sorter/trace_sorter.h" #include "src/trace_processor/storage/metadata.h" #include "src/trace_processor/storage/stats.h" @@ -51,17 +63,6 @@ namespace { static constexpr uint32_t kFtraceGlobalClockIdForOldKernels = 64; -PERFETTO_ALWAYS_INLINE base::StatusOr ResolveTraceTime( - TraceProcessorContext* context, - ClockTracker::ClockId clock_id, - int64_t ts) { - // On most traces (i.e. P+), the clock should be BOOTTIME. - if (PERFETTO_LIKELY(clock_id == BuiltinClock::BUILTIN_CLOCK_BOOTTIME && - !context->machine_id())) - return ts; - return context->clock_tracker->ToTraceTime(clock_id, ts); -} - // Fast path for parsing the event id of an ftrace event. // Speculate on the fact that, if the timestamp was found, the common pid // will appear immediately after and the event id immediately after that. @@ -189,9 +190,8 @@ base::Status FtraceTokenizer::TokenizeFtraceBundle( ? decoder.previous_bundle_end_timestamp() : decoder.last_read_event_timestamp(); int64_t timestamp = 0; - ASSIGN_OR_RETURN( - timestamp, - ResolveTraceTime(context_, clock_id, static_cast(raw_ts))); + ASSIGN_OR_RETURN(timestamp, context_->clock_tracker->ToTraceTime( + clock_id, static_cast(raw_ts))); std::optional curr_latest_timestamp = context_->metadata_tracker->GetMetadata( @@ -281,10 +281,8 @@ void FtraceTokenizer::TokenizeFtraceEvent( return; } - int64_t int64_timestamp = static_cast(raw_timestamp); - base::StatusOr timestamp = - ResolveTraceTime(context_, clock_id, int64_timestamp); - + auto timestamp = context_->clock_tracker->ToTraceTime( + clock_id, static_cast(raw_timestamp)); // ClockTracker will increment some error stats if it failed to convert the // timestamp so just return. if (!timestamp.ok()) { @@ -347,8 +345,8 @@ void FtraceTokenizer::TokenizeFtraceCompactSchedSwitch( event.next_pid = *npid_it; event.next_prio = *nprio_it; - base::StatusOr timestamp = - ResolveTraceTime(context_, clock_id, event_timestamp); + auto timestamp = + context_->clock_tracker->ToTraceTime(clock_id, event_timestamp); if (!timestamp.ok()) { DlogWithLimit(timestamp.status()); return; @@ -404,8 +402,8 @@ void FtraceTokenizer::TokenizeFtraceCompactSchedWaking( common_flags_it++; } - base::StatusOr timestamp = - ResolveTraceTime(context_, clock_id, event_timestamp); + auto timestamp = + context_->clock_tracker->ToTraceTime(clock_id, event_timestamp); if (!timestamp.ok()) { DlogWithLimit(timestamp.status()); return; @@ -465,9 +463,9 @@ void FtraceTokenizer::TokenizeFtraceGpuWorkPeriod( // Enforce clock type for the event data to be CLOCK_MONOTONIC_RAW // as specified, to calculate the timestamp correctly. - int64_t int64_timestamp = static_cast(raw_timestamp); - base::StatusOr timestamp = ResolveTraceTime( - context_, BuiltinClock::BUILTIN_CLOCK_MONOTONIC_RAW, int64_timestamp); + auto timestamp = context_->clock_tracker->ToTraceTime( + BuiltinClock::BUILTIN_CLOCK_MONOTONIC_RAW, + static_cast(raw_timestamp)); // ClockTracker will increment some error stats if it failed to convert the // timestamp so just return.