| Message ID | 20210729050959.12263-49-tz.stoyanov@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Trace file version 7 | expand |
On Thu, 29 Jul 2021 08:09:20 +0300 "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > The BUFFER option is extended in trace file version 7. It holds CPU > metadata related to the recorded CPU traces. Also, there is a new > BUFFER_LAT option for describing the latency trace data. A new logic > is implemented for these new options. > In trace file version 7, the top buffer is saved as other buffers in the > file, no special treatment. But saving the top buffer in the list of > buffers in the input handler causes problems. It breaks the legacy logic > of trace-cmd library users, which have special logic for trace buffers > processing. That's why "top_buffer" member is added in the input handler > structure, to hold the top buffer. > > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> > --- > lib/trace-cmd/trace-input.c | 116 ++++++++++++++++++++++++++++++------ > 1 file changed, 98 insertions(+), 18 deletions(-) > > diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c > index e7f97561..57ae535a 100644 > --- a/lib/trace-cmd/trace-input.c > +++ b/lib/trace-cmd/trace-input.c > @@ -74,9 +74,19 @@ struct cpu_data { > int pipe_fd; > }; > > +struct cpu_file_data { > + int cpu; > + unsigned long long offset; > + unsigned long long size; > +}; > + > struct input_buffer_instance { > char *name; > size_t offset; > + char *clock; > + bool latency; > + int cpus; > + struct cpu_file_data *cpu_data; > }; > > struct ts_offset_sample { > @@ -155,6 +165,7 @@ struct tracecmd_input { > char * uname; > char * version; > char * trace_clock; > + struct input_buffer_instance top_buffer; > struct input_buffer_instance *buffers; > int parsing_failures; > struct guest_trace_info *guest; > @@ -2904,6 +2915,80 @@ tracecmd_search_task_map(struct tracecmd_input *handle, > return lib; > } > > +#define save_read_number(R, C) \ > + do { \ > + if ((C) > size) \ > + return -1; \ > + (R) = tep_read_number(handle->pevent, (data + rsize), (C)); \ > + rsize += (C); \ > + size -= (C); \ > + } while (0) > + > +#define save_read_string(R) \ > + do { \ > + if (size < 1) \ > + return -1; \ > + (R) = strdup(data + rsize); \ > + if (!(R)) \ > + return -1; \ > + rsize += (strlen((R)) + 1); \ > + size -= (strlen((R)) + 1); \ > + if (size < 0) \ > + return -1; \ > + } while (0) The above should be inline functions, not macros. Just pass the address of the values you want to modify. > + > +static int handle_buffer_option(struct tracecmd_input *handle, > + unsigned short id, char *data, int size) > +{ > + struct input_buffer_instance *buff; > + unsigned long long offset; > + int rsize = 0; > + char *name; > + int i; > + > + save_read_number(offset, 8); > + save_read_string(name); > + > + if (*name == '\0') { > + /* top buffer */ > + buff = &handle->top_buffer; > + } else { > + buff = realloc(handle->buffers, sizeof(*handle->buffers) * (handle->nr_buffers + 1)); > + if (!buff) { > + free(name); > + return -1; > + } > + handle->buffers = buff; > + handle->nr_buffers++; > + > + buff = &handle->buffers[handle->nr_buffers - 1]; > + } > + memset(buff, 0, sizeof(struct input_buffer_instance)); > + buff->name = name; > + buff->offset = offset; > + if (handle->file_version < 7) > + return 0; > + > + /* file version >= 7 specific data */ > + save_read_string(buff->clock); > + if (id == TRACECMD_OPTION_BUFFER) { > + save_read_number(buff->cpus, 4); > + if (!buff->cpus) > + return 0; > + buff->cpu_data = calloc(buff->cpus, sizeof(struct cpu_file_data)); > + if (!buff->cpu_data) > + return -1; > + for (i = 0; i < buff->cpus; i++) { > + save_read_number(buff->cpu_data[i].cpu, 4); > + save_read_number(buff->cpu_data[i].offset, 8); > + save_read_number(buff->cpu_data[i].size, 8); > + } > + } else { > + buff->latency = true; > + } > + return 0; > +} > + > static int handle_options(struct tracecmd_input *handle) > { > long long offset; > @@ -2911,7 +2996,6 @@ static int handle_options(struct tracecmd_input *handle) > unsigned int size; > unsigned short id, flags; > char *cpustats = NULL; > - struct input_buffer_instance *buffer; > struct hook_list *hook; > bool comperss = false; > char *buf; > @@ -3016,21 +3100,10 @@ static int handle_options(struct tracecmd_input *handle) > handle->cpustats = cpustats; > break; > case TRACECMD_OPTION_BUFFER: > - /* A buffer instance is saved at the end of the file */ > - handle->nr_buffers++; > - handle->buffers = realloc(handle->buffers, > - sizeof(*handle->buffers) * handle->nr_buffers); > - if (!handle->buffers) > - return -ENOMEM; > - buffer = &handle->buffers[handle->nr_buffers - 1]; > - buffer->name = strdup(buf + 8); > - if (!buffer->name) { > - free(handle->buffers); > - handle->buffers = NULL; > - return -ENOMEM; > - } > - offset = *(unsigned long long *)buf; > - buffer->offset = tep_read_number(handle->pevent, &offset, 8); > + case TRACECMD_OPTION_BUFFER_LAT: > + ret = handle_buffer_option(handle, option, buf, size); > + if (ret < 0) > + goto out; > break; > case TRACECMD_OPTION_TRACECLOCK: > if (!handle->ts2secs) > @@ -3825,9 +3898,15 @@ void tracecmd_close(struct tracecmd_input *handle) > handle->sections = handle->sections->next; > free(del_sec); > } > - for (i = 0; i < handle->nr_buffers; i++) > - free(handle->buffers[i].name); > > + free(handle->top_buffer.name); > + free(handle->top_buffer.clock); > + free(handle->top_buffer.cpu_data); > + for (i = 0; i < handle->nr_buffers; i++) { > + free(handle->buffers[i].name); > + free(handle->buffers[i].clock); > + free(handle->buffers[i].cpu_data); > + } The freeing of the buffer should be a function that encapsulates this, where all you need to do is: free_buffer(&handle->top_buffer); for (i = 0; i < handle->nr_buffers; i++) free_buffer(&handle->buffers[i]); -- Steve > free(handle->buffers); > > tracecmd_free_hooks(handle->hooks); > @@ -4272,6 +4351,7 @@ tracecmd_buffer_instance_handle(struct tracecmd_input *handle, int indx) > return NULL; > > *new_handle = *handle; > + memset(&new_handle->top_buffer, 0, sizeof(new_handle->top_buffer)); > new_handle->cpu_data = NULL; > new_handle->nr_buffers = 0; > new_handle->buffers = NULL;
diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c index e7f97561..57ae535a 100644 --- a/lib/trace-cmd/trace-input.c +++ b/lib/trace-cmd/trace-input.c @@ -74,9 +74,19 @@ struct cpu_data { int pipe_fd; }; +struct cpu_file_data { + int cpu; + unsigned long long offset; + unsigned long long size; +}; + struct input_buffer_instance { char *name; size_t offset; + char *clock; + bool latency; + int cpus; + struct cpu_file_data *cpu_data; }; struct ts_offset_sample { @@ -155,6 +165,7 @@ struct tracecmd_input { char * uname; char * version; char * trace_clock; + struct input_buffer_instance top_buffer; struct input_buffer_instance *buffers; int parsing_failures; struct guest_trace_info *guest; @@ -2904,6 +2915,80 @@ tracecmd_search_task_map(struct tracecmd_input *handle, return lib; } +#define save_read_number(R, C) \ + do { \ + if ((C) > size) \ + return -1; \ + (R) = tep_read_number(handle->pevent, (data + rsize), (C)); \ + rsize += (C); \ + size -= (C); \ + } while (0) + +#define save_read_string(R) \ + do { \ + if (size < 1) \ + return -1; \ + (R) = strdup(data + rsize); \ + if (!(R)) \ + return -1; \ + rsize += (strlen((R)) + 1); \ + size -= (strlen((R)) + 1); \ + if (size < 0) \ + return -1; \ + } while (0) + +static int handle_buffer_option(struct tracecmd_input *handle, + unsigned short id, char *data, int size) +{ + struct input_buffer_instance *buff; + unsigned long long offset; + int rsize = 0; + char *name; + int i; + + save_read_number(offset, 8); + save_read_string(name); + + if (*name == '\0') { + /* top buffer */ + buff = &handle->top_buffer; + } else { + buff = realloc(handle->buffers, sizeof(*handle->buffers) * (handle->nr_buffers + 1)); + if (!buff) { + free(name); + return -1; + } + handle->buffers = buff; + handle->nr_buffers++; + + buff = &handle->buffers[handle->nr_buffers - 1]; + } + memset(buff, 0, sizeof(struct input_buffer_instance)); + buff->name = name; + buff->offset = offset; + if (handle->file_version < 7) + return 0; + + /* file version >= 7 specific data */ + save_read_string(buff->clock); + if (id == TRACECMD_OPTION_BUFFER) { + save_read_number(buff->cpus, 4); + if (!buff->cpus) + return 0; + buff->cpu_data = calloc(buff->cpus, sizeof(struct cpu_file_data)); + if (!buff->cpu_data) + return -1; + for (i = 0; i < buff->cpus; i++) { + save_read_number(buff->cpu_data[i].cpu, 4); + save_read_number(buff->cpu_data[i].offset, 8); + save_read_number(buff->cpu_data[i].size, 8); + } + } else { + buff->latency = true; + } + return 0; +} + static int handle_options(struct tracecmd_input *handle) { long long offset; @@ -2911,7 +2996,6 @@ static int handle_options(struct tracecmd_input *handle) unsigned int size; unsigned short id, flags; char *cpustats = NULL; - struct input_buffer_instance *buffer; struct hook_list *hook; bool comperss = false; char *buf; @@ -3016,21 +3100,10 @@ static int handle_options(struct tracecmd_input *handle) handle->cpustats = cpustats; break; case TRACECMD_OPTION_BUFFER: - /* A buffer instance is saved at the end of the file */ - handle->nr_buffers++; - handle->buffers = realloc(handle->buffers, - sizeof(*handle->buffers) * handle->nr_buffers); - if (!handle->buffers) - return -ENOMEM; - buffer = &handle->buffers[handle->nr_buffers - 1]; - buffer->name = strdup(buf + 8); - if (!buffer->name) { - free(handle->buffers); - handle->buffers = NULL; - return -ENOMEM; - } - offset = *(unsigned long long *)buf; - buffer->offset = tep_read_number(handle->pevent, &offset, 8); + case TRACECMD_OPTION_BUFFER_LAT: + ret = handle_buffer_option(handle, option, buf, size); + if (ret < 0) + goto out; break; case TRACECMD_OPTION_TRACECLOCK: if (!handle->ts2secs) @@ -3825,9 +3898,15 @@ void tracecmd_close(struct tracecmd_input *handle) handle->sections = handle->sections->next; free(del_sec); } - for (i = 0; i < handle->nr_buffers; i++) - free(handle->buffers[i].name); + free(handle->top_buffer.name); + free(handle->top_buffer.clock); + free(handle->top_buffer.cpu_data); + for (i = 0; i < handle->nr_buffers; i++) { + free(handle->buffers[i].name); + free(handle->buffers[i].clock); + free(handle->buffers[i].cpu_data); + } free(handle->buffers); tracecmd_free_hooks(handle->hooks); @@ -4272,6 +4351,7 @@ tracecmd_buffer_instance_handle(struct tracecmd_input *handle, int indx) return NULL; *new_handle = *handle; + memset(&new_handle->top_buffer, 0, sizeof(new_handle->top_buffer)); new_handle->cpu_data = NULL; new_handle->nr_buffers = 0; new_handle->buffers = NULL;
The BUFFER option is extended in trace file version 7. It holds CPU metadata related to the recorded CPU traces. Also, there is a new BUFFER_LAT option for describing the latency trace data. A new logic is implemented for these new options. In trace file version 7, the top buffer is saved as other buffers in the file, no special treatment. But saving the top buffer in the list of buffers in the input handler causes problems. It breaks the legacy logic of trace-cmd library users, which have special logic for trace buffers processing. That's why "top_buffer" member is added in the input handler structure, to hold the top buffer. Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- lib/trace-cmd/trace-input.c | 116 ++++++++++++++++++++++++++++++------ 1 file changed, 98 insertions(+), 18 deletions(-)