On Thu, 17 Apr 2025 10:18:04 -0400 Steven Rostedt <[email protected]> wrote:
> On Thu, 17 Apr 2025 14:17:29 +0900 > "Masami Hiramatsu (Google)" <[email protected]> wrote: > > > --- a/kernel/trace/trace.c > > +++ b/kernel/trace/trace.c > > @@ -6004,6 +6004,7 @@ struct trace_mod_entry { > > }; > > > > struct trace_scratch { > > + char clock[TRACE_CLOCK_NAME_MAX]; > > unsigned long text_addr; > > unsigned long nr_entries; > > struct trace_mod_entry entries[]; > > @@ -6114,6 +6115,8 @@ static void update_last_data(struct trace_array *tr) > > if (tr->scratch) { > > struct trace_scratch *tscratch = tr->scratch; > > > > + strscpy(tscratch->clock, trace_clocks[tr->clock_id].name, > > Why copy the name and not the clock_id? The name is more robust if we rebooted in the different kernel. > > The clock ids should not change between kernels. Hmm, I thought it could be changed because we didn't define it. ---- static struct { u64 (*func)(void); const char *name; int in_ns; /* is this clock in nanoseconds? */ } trace_clocks[] = { { trace_clock_local, "local", 1 }, { trace_clock_global, "global", 1 }, { trace_clock_counter, "counter", 0 }, { trace_clock_jiffies, "uptime", 0 }, { trace_clock, "perf", 1 }, { ktime_get_mono_fast_ns, "mono", 1 }, { ktime_get_raw_fast_ns, "mono_raw", 1 }, { ktime_get_boot_fast_ns, "boot", 1 }, { ktime_get_tai_fast_ns, "tai", 1 }, ARCH_TRACE_CLOCKS }; ---- So the clock id is just an index of this array. That can be changed easily between the kernel version. If we discard the previous boot data in that case, I agree with using clock ids. (Another reason for the implementation is that tracing_set_clock() only accepts a name, but this is just due to laziness on my part. :-( ) Thank you, > > -- Steve > > > + TRACE_CLOCK_NAME_MAX); > > memset(tscratch->entries, 0, > > flex_array_size(tscratch, entries, > > tscratch->nr_entries)); > > tscratch->nr_entries = 0; -- Masami Hiramatsu (Google) <[email protected]>
