On Wed, Feb 17, 2021 at 6:00 PM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
>
> > /* --- Opening and Reading the trace.dat file --- */
> >
> > +enum {
> > + TRACECMD_FILE_INIT = (1 << 0),
> > + TRACECMD_FILE_HEADERS = (1 << 1),
> > + TRACECMD_FILE_FTRACE_EVENTS = (1 << 2),
> > + TRACECMD_FILE_ALL_EVENTS = (1 << 3),
> > + TRACECMD_FILE_KALLSYMS = (1 << 4),
> > + TRACECMD_FILE_PRINTK = (1 << 5),
> > + TRACECMD_FILE_CMD_LINES = (1 << 6),
> > + TRACECMD_FILE_CPU_COUNT = (1 << 7),
> > + TRACECMD_FILE_OPTIONS = (1 << 8),
> > + TRACECMD_FILE_CPU_LATENCY = (1 << 9),
> > + TRACECMD_FILE_CPU_FLYRECORD = (1 << 10),
>
> Why did you use bits, and not just a number?
>
> It's not that this is quantum physics and we are in multiple states at once
> ;-)
I use these flags to track what is in the file, not just a state. The
state is the most significant bit which is set,
but that way there is information for all passed states. I use the
same logic when reading the file, to verify if
all required information is in the file.
>
> Order matters, so we want to make sure that when we enter one state, we are
> coming in from another state.
Yes, I added checks before entering any of these states to verify its
dependencies.
>
>
> > +};
> > +#define TRACECMD_FILE_TRACE_DATA (TRACECMD_FILE_CPU_LATENCY | TRACECMD_FILE_CPU_FLYRECORD)
> > +
> > enum {
> > TRACECMD_OPTION_DONE,
> > TRACECMD_OPTION_DATE,
> > @@ -115,9 +130,7 @@ enum {
> > enum {
> > TRACECMD_FL_IGNORE_DATE = (1 << 0),
> > TRACECMD_FL_BUFFER_INSTANCE = (1 << 1),
> > - TRACECMD_FL_LATENCY = (1 << 2),
> > - TRACECMD_FL_IN_USECS = (1 << 3),
> > - TRACECMD_FL_FLYRECORD = (1 << 4),
> > + TRACECMD_FL_IN_USECS = (1 << 2),
> > };
> >
> > struct tracecmd_ftrace {
> > @@ -150,6 +163,7 @@ int tracecmd_copy_headers(struct tracecmd_input *handle, int fd);
> > void tracecmd_set_flag(struct tracecmd_input *handle, int flag);
> > void tracecmd_clear_flag(struct tracecmd_input *handle, int flag);
> > unsigned long tracecmd_get_flags(struct tracecmd_input *handle);
> > +unsigned long tracecmd_get_file_state(struct tracecmd_input *handle);
> > unsigned long long tracecmd_get_tsync_peer(struct tracecmd_input *handle);
> > int tracecmd_enable_tsync(struct tracecmd_input *handle, bool enable);
> >
> > @@ -273,6 +287,7 @@ struct tracecmd_option *tracecmd_add_buffer_option(struct tracecmd_output *handl
> > const char *name, int cpus);
> >
> > int tracecmd_write_cpus(struct tracecmd_output *handle, int cpus);
> > +int tracecmd_write_cmdlines(struct tracecmd_output *handle);
> > int tracecmd_write_options(struct tracecmd_output *handle);
> > int tracecmd_append_options(struct tracecmd_output *handle);
> > int tracecmd_update_option(struct tracecmd_output *handle,
> > @@ -500,7 +515,4 @@ void *tracecmd_record_page(struct tracecmd_input *handle,
> > void *tracecmd_record_offset(struct tracecmd_input *handle,
> > struct tep_record *record);
> >
> > -int save_tracing_file_data(struct tracecmd_output *handle,
> > - const char *filename);
> > -
> > #endif /* _TRACE_CMD_PRIVATE_H */
> > diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
> > index 76bcb215..c171e93c 100644
> > --- a/lib/trace-cmd/trace-input.c
> > +++ b/lib/trace-cmd/trace-input.c
> > @@ -102,6 +102,7 @@ struct host_trace_info {
> >
> > struct tracecmd_input {
> > struct tep_handle *pevent;
> > + unsigned long file_state;
> > struct tep_plugin_list *plugin_list;
> > struct tracecmd_input *parent;
> > unsigned long flags;
> > @@ -161,6 +162,11 @@ unsigned long tracecmd_get_flags(struct tracecmd_input *handle)
> > return handle->flags;
> > }
> >
> > +unsigned long tracecmd_get_file_state(struct tracecmd_input *handle)
> > +{
> > + return handle->file_state;
> > +}
> > +
> > #if DEBUG_RECORD
> > static void remove_record(struct page *page, struct tep_record *record)
> > {
> > @@ -782,34 +788,40 @@ int tracecmd_read_headers(struct tracecmd_input *handle)
> > ret = read_header_files(handle);
> > if (ret < 0)
> > return -1;
> > + handle->file_state |= TRACECMD_FILE_HEADERS;
> > + tep_set_long_size(handle->pevent, handle->long_size);
> >
> > ret = read_ftrace_files(handle, NULL);
> > if (ret < 0)
> > return -1;
> > + handle->file_state |= TRACECMD_FILE_FTRACE_EVENTS;
> >
> > ret = read_event_files(handle, NULL);
> > if (ret < 0)
> > return -1;
> > + handle->file_state |= TRACECMD_FILE_ALL_EVENTS;
> >
> > ret = read_proc_kallsyms(handle);
> > if (ret < 0)
> > return -1;
> > + handle->file_state |= TRACECMD_FILE_KALLSYMS;
> >
> > ret = read_ftrace_printk(handle);
> > if (ret < 0)
> > return -1;
> > + handle->file_state |= TRACECMD_FILE_PRINTK;
> >
> > if (read_and_parse_cmdlines(handle) < 0)
> > return -1;
> > + handle->file_state |= TRACECMD_FILE_CMD_LINES;
> >
> > if (read_cpus(handle) < 0)
> > return -1;
> > + handle->file_state |= TRACECMD_FILE_CPU_COUNT;
> >
> > if (read_options_type(handle) < 0)
> > return -1;
> >
> > - tep_set_long_size(handle->pevent, handle->long_size);
> > -
> > return 0;
> > }
> >
> > @@ -2657,6 +2669,7 @@ static int read_options_type(struct tracecmd_input *handle)
> > if (strncmp(buf, "options", 7) == 0) {
> > if (handle_options(handle) < 0)
> > return -1;
> > + handle->file_state |= TRACECMD_FILE_OPTIONS;
> > if (do_read_check(handle, buf, 10))
> > return -1;
> > }
> > @@ -2665,9 +2678,9 @@ static int read_options_type(struct tracecmd_input *handle)
> > * Check if this is a latency report or flyrecord.
> > */
> > if (strncmp(buf, "latency", 7) == 0)
> > - handle->flags |= TRACECMD_FL_LATENCY;
> > + handle->file_state |= TRACECMD_FILE_CPU_LATENCY;
> > else if (strncmp(buf, "flyrecord", 9) == 0)
> > - handle->flags |= TRACECMD_FL_FLYRECORD;
> > + handle->file_state |= TRACECMD_FILE_CPU_FLYRECORD;
>
> I think we should keep the LATENCY and FLYRECORD flags, as they have
> different meanings and define the type of handle, not just the state they
> are in. We can have the logic flow through different states depending on if
> it is a latency or flyrecord file.
As I use the new flags to track what is written / read from the file,
it overlaps with the
old TRACECMD_FL_FLYRECORD and TRACECMD_FL_LATENCY flags.
>
> -- Steve
--
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center
|