| Message ID | 20201009140338.25260-4-tz.stoyanov@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Timestamp synchronization of host - guest tracing session | expand |
On Fri, 9 Oct 2020 17:03:31 +0300 "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > Added enum with ftrace clock IDs and APIs to convert ftrace name to ID > and vice versa, as part of libtracecmd. > > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> > --- > include/trace-cmd/trace-cmd.h | 16 +++++++++++++ > lib/trace-cmd/trace-util.c | 45 +++++++++++++++++++++++++++++++++++ > 2 files changed, 61 insertions(+) > > diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h > index f9c1f843..393a2e7b 100644 > --- a/include/trace-cmd/trace-cmd.h > +++ b/include/trace-cmd/trace-cmd.h > @@ -415,6 +415,22 @@ int tracecmd_msg_recv_time_sync(struct tracecmd_msg_handle *msg_handle, > unsigned int *sync_msg_id, > unsigned int *payload_size, char **payload); > > +enum tracecmd_clocks { > + TRACECMD_CLOCK_UNKNOWN = 0, > + TRACECMD_CLOCK_LOCAL = 1, > + TRACECMD_CLOCK_GLOBAL = 1 << 1, > + TRACECMD_CLOCK_COUNTER = 1 << 2, > + TRACECMD_CLOCK_UPTIME = 1 << 3, > + TRACECMD_CLOCK_PERF = 1 << 4, > + TRACECMD_CLOCK_MONO = 1 << 5, > + TRACECMD_CLOCK_MONO_RAW = 1 << 6, > + TRACECMD_CLOCK_BOOT = 1 << 7, > + TRACECMD_CLOCK_X86_TSC = 1 << 8 I'm curious to why you have this as a bitmask. We can only have on clock at a time, right? Also, if we make it a simple counter, we can create a string as well: #define TRACECMD_CLOCKS \ C(UNKNOWN, unknown), \ C(LOCAL, local), \ C(GLOBAL, global),\ C(COUNTER, counter),\ C(UPTIME, uptime),\ C(PERF, perf), \ C(MONO, mono), \ C(MONO_RAW, mono_raw),\ C(BOOT, boot, \ C(X86_TSC, x86-tsc) #undef C #define C(a, b) TRACECMD_CLOCK_##a enum tracecmd_clocks { TRACECMD_CLOCKS }; #undef C > +}; > + > +enum tracecmd_clocks tracecmd_clock_str2id(const char *clock); > +char *tracecmd_clock_id2str(enum tracecmd_clocks clock); > + > /* --- Timestamp synchronization --- */ > > enum{ > diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c > index 0ead96ea..e20362e3 100644 > --- a/lib/trace-cmd/trace-util.c > +++ b/lib/trace-cmd/trace-util.c > @@ -33,6 +33,51 @@ static bool debug; > > static FILE *logfp; > > +const static struct { > + char *clock_str; > + enum tracecmd_clocks clock_id; > +} trace_clocks[] = { > + {"local", TRACECMD_CLOCK_LOCAL}, > + {"global", TRACECMD_CLOCK_GLOBAL}, > + {"counter", TRACECMD_CLOCK_COUNTER}, > + {"uptime", TRACECMD_CLOCK_UPTIME}, > + {"perf", TRACECMD_CLOCK_PERF}, > + {"mono", TRACECMD_CLOCK_MONO}, > + {"mono_raw", TRACECMD_CLOCK_MONO_RAW}, > + {"boot", TRACECMD_CLOCK_BOOT}, > + {"x86-tsc", TRACECMD_CLOCK_X86_TSC}, > + {NULL, -1} > +}; He we would have: #define C(a, b) #b const char * trace_clocks[] = { TRACECMD_CLOCKS } > + > +/** > + * tracecmd_clock_str2id - Convert ftrace clock name to clock ID > + * @clock: Ftrace clock name > + * Returns ID of the ftrace clock > + */ > +enum tracecmd_clocks tracecmd_clock_str2id(const char *clock) > +{ int i; btw, in normal C, it's not recommended to declare a counter in a loop. for (i = 0; i < ARRAY_SIZE(trace_clocks); i++) { if (strcmp(trace_clocks[i], clock) == 0) return i; return 0; } > + for (int i = 0; trace_clocks[i].clock_str; i++) { > + if (!strncmp(clock, trace_clocks[i].clock_str, > + strlen(trace_clocks[i].clock_str))) > + return trace_clocks[i].clock_id; > + } > + return TRACECMD_CLOCK_UNKNOWN; > +} > + > +/** > + * tracecmd_clock_id2str - Convert clock ID to ftare clock name > + * @clock: Clock ID > + * Returns name of a ftrace clock > + */ > +char *tracecmd_clock_id2str(enum tracecmd_clocks clock) Should probably have this return const char *. Such that callers wont modify it. > +{ And here we would have: if (clock < ARRAY_SIZE(trace_clocks)) return trace_clocks[i]; return NULL; -- Steve > + for (int i = 0; trace_clocks[i].clock_str; i++) { > + if (trace_clocks[i].clock_id == clock) > + return trace_clocks[i].clock_str; > + } > + return NULL; > +} > + > /** > * tracecmd_set_debug - Set debug mode of the tracecmd library > * @set_debug: The new "debug" mode. If true, the tracecmd library is
On Wed, 21 Oct 2020 21:26:26 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > On Fri, 9 Oct 2020 17:03:31 +0300 > "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > > > Added enum with ftrace clock IDs and APIs to convert ftrace name to ID > > and vice versa, as part of libtracecmd. > > > > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> > > --- > > include/trace-cmd/trace-cmd.h | 16 +++++++++++++ > > lib/trace-cmd/trace-util.c | 45 +++++++++++++++++++++++++++++++++++ > > 2 files changed, 61 insertions(+) > > > > diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h > > index f9c1f843..393a2e7b 100644 > > --- a/include/trace-cmd/trace-cmd.h > > +++ b/include/trace-cmd/trace-cmd.h > > @@ -415,6 +415,22 @@ int tracecmd_msg_recv_time_sync(struct tracecmd_msg_handle *msg_handle, > > unsigned int *sync_msg_id, > > unsigned int *payload_size, char **payload); > > > > +enum tracecmd_clocks { > > + TRACECMD_CLOCK_UNKNOWN = 0, > > + TRACECMD_CLOCK_LOCAL = 1, > > + TRACECMD_CLOCK_GLOBAL = 1 << 1, > > + TRACECMD_CLOCK_COUNTER = 1 << 2, > > + TRACECMD_CLOCK_UPTIME = 1 << 3, > > + TRACECMD_CLOCK_PERF = 1 << 4, > > + TRACECMD_CLOCK_MONO = 1 << 5, > > + TRACECMD_CLOCK_MONO_RAW = 1 << 6, > > + TRACECMD_CLOCK_BOOT = 1 << 7, > > + TRACECMD_CLOCK_X86_TSC = 1 << 8 > > I'm curious to why you have this as a bitmask. We can only have on > clock at a time, right? I got to patch 5 and see that you do need this to be a bitmask. When this is the case, the change log should state that. That is, the change log should have something like: The clock enum will be used in a bitmask such that the synchronization protocol can pass a bitmask of supported clocks. Remember, all patches should be "stand alone". That is, do not assume that someone will have access to other patches when they are looking at the current patch. You may disregard the rest of this email. -- Steve
diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h index f9c1f843..393a2e7b 100644 --- a/include/trace-cmd/trace-cmd.h +++ b/include/trace-cmd/trace-cmd.h @@ -415,6 +415,22 @@ int tracecmd_msg_recv_time_sync(struct tracecmd_msg_handle *msg_handle, unsigned int *sync_msg_id, unsigned int *payload_size, char **payload); +enum tracecmd_clocks { + TRACECMD_CLOCK_UNKNOWN = 0, + TRACECMD_CLOCK_LOCAL = 1, + TRACECMD_CLOCK_GLOBAL = 1 << 1, + TRACECMD_CLOCK_COUNTER = 1 << 2, + TRACECMD_CLOCK_UPTIME = 1 << 3, + TRACECMD_CLOCK_PERF = 1 << 4, + TRACECMD_CLOCK_MONO = 1 << 5, + TRACECMD_CLOCK_MONO_RAW = 1 << 6, + TRACECMD_CLOCK_BOOT = 1 << 7, + TRACECMD_CLOCK_X86_TSC = 1 << 8 +}; + +enum tracecmd_clocks tracecmd_clock_str2id(const char *clock); +char *tracecmd_clock_id2str(enum tracecmd_clocks clock); + /* --- Timestamp synchronization --- */ enum{ diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c index 0ead96ea..e20362e3 100644 --- a/lib/trace-cmd/trace-util.c +++ b/lib/trace-cmd/trace-util.c @@ -33,6 +33,51 @@ static bool debug; static FILE *logfp; +const static struct { + char *clock_str; + enum tracecmd_clocks clock_id; +} trace_clocks[] = { + {"local", TRACECMD_CLOCK_LOCAL}, + {"global", TRACECMD_CLOCK_GLOBAL}, + {"counter", TRACECMD_CLOCK_COUNTER}, + {"uptime", TRACECMD_CLOCK_UPTIME}, + {"perf", TRACECMD_CLOCK_PERF}, + {"mono", TRACECMD_CLOCK_MONO}, + {"mono_raw", TRACECMD_CLOCK_MONO_RAW}, + {"boot", TRACECMD_CLOCK_BOOT}, + {"x86-tsc", TRACECMD_CLOCK_X86_TSC}, + {NULL, -1} +}; + +/** + * tracecmd_clock_str2id - Convert ftrace clock name to clock ID + * @clock: Ftrace clock name + * Returns ID of the ftrace clock + */ +enum tracecmd_clocks tracecmd_clock_str2id(const char *clock) +{ + for (int i = 0; trace_clocks[i].clock_str; i++) { + if (!strncmp(clock, trace_clocks[i].clock_str, + strlen(trace_clocks[i].clock_str))) + return trace_clocks[i].clock_id; + } + return TRACECMD_CLOCK_UNKNOWN; +} + +/** + * tracecmd_clock_id2str - Convert clock ID to ftare clock name + * @clock: Clock ID + * Returns name of a ftrace clock + */ +char *tracecmd_clock_id2str(enum tracecmd_clocks clock) +{ + for (int i = 0; trace_clocks[i].clock_str; i++) { + if (trace_clocks[i].clock_id == clock) + return trace_clocks[i].clock_str; + } + return NULL; +} + /** * tracecmd_set_debug - Set debug mode of the tracecmd library * @set_debug: The new "debug" mode. If true, the tracecmd library is
Added enum with ftrace clock IDs and APIs to convert ftrace name to ID and vice versa, as part of libtracecmd. Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- include/trace-cmd/trace-cmd.h | 16 +++++++++++++ lib/trace-cmd/trace-util.c | 45 +++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+)