On Thu, 29 Jul 2021 08:09:00 +0300 "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@xxxxxxxxx> wrote: > Extended the BUFFER trace option in trace file version 7 with CPU > flyrecord trace metadata. In the version 6 of the trace file, this metadata > is located at several places in the file. Part of the metadata is only for > the top trace instance, thus limiting per-instance configuration. Moving > all CPU trace related metadata in the BUFFER option simplifies the parsing > and makes per-instance configuration more flexible. In the new file > structure, the top instance is treated as any other instances. > The format of the extended BUFFER option is: > - offset of the buffer in the trace file > - name of the buffer > - trace clock, used in this buffer for events timestamps > - count of CPUs with trace data > - array, describing each CPU with trace data: > - CPU id > - offset of CPU trace data in the trace file > - size of the recorded CPU trace data > > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@xxxxxxxxx> > --- > lib/trace-cmd/trace-output.c | 171 ++++++++++++++++++++++++++--------- > 1 file changed, 127 insertions(+), 44 deletions(-) > > diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c > index d60bd456..21555d6d 100644 > --- a/lib/trace-cmd/trace-output.c > +++ b/lib/trace-cmd/trace-output.c > @@ -1826,7 +1826,7 @@ int tracecmd_append_options(struct tracecmd_output *handle) > } > > static struct tracecmd_option * > -add_buffer_option(struct tracecmd_output *handle, const char *name, int cpus) > +add_buffer_option_v6(struct tracecmd_output *handle, const char *name, int cpus) > { > struct tracecmd_option *option; > char *buf; > @@ -1876,8 +1876,11 @@ int tracecmd_write_buffer_info(struct tracecmd_output *handle) > struct tracecmd_option *option; > struct tracecmd_buffer *buf; > > + if (handle->file_version >= 7) > + return 0; > + > list_for_each_entry(buf, &handle->buffers, list) { > - option = add_buffer_option(handle, buf->name, buf->cpus); > + option = add_buffer_option_v6(handle, buf->name, buf->cpus); > if (!option) > return -1; > buf->option = option; > @@ -1938,6 +1941,98 @@ int tracecmd_write_cmdlines(struct tracecmd_output *handle) > return 0; > } > > +static char *get_clock(struct tracecmd_output *handle) > +{ > + struct tracefs_instance *inst; > + > + if (handle->trace_clock) > + return handle->trace_clock; > + > + /* > + * If no clock is set on this handle, get the trace clock of > + * the top instance in the handle's tracing dir > + */ > + inst = tracefs_instance_alloc(handle->tracing_dir, NULL); > + if (!inst) > + return NULL; > + handle->trace_clock = tracefs_get_clock(inst); > + tracefs_instance_free(inst); > + return handle->trace_clock; > +} > + > +__hidden struct tracecmd_option * > +out_add_buffer_option_v7(struct tracecmd_output *handle, const char *name, > + unsigned short id, unsigned long long data_offset, > + int cpus, struct data_file_write *cpu_data) Why is this hidden, and not static? I noticed that you are using "out_" as a normal prefix. Is that really needed? > +{ > + struct tracecmd_option *option; > + int i, j = 0, k = 0; > + int *cpu_ids = NULL; > + struct iovec *vect; > + char *clock; > + > + if (handle->file_version < 7) > + return NULL; > + > + clock = get_clock(handle); > + > + /* Buffer flyrecord option, v7: Nit, for consistency, use: /* * Buffer flyrecord option, v7: > + * - trace data offset in the file Is the above the "meta data". If so, perhaps we should call it that. > + * - buffer name > + * - buffer clock > + * - CPU count > + * - for each CPU: > + * - CPU id > + * - CPU trace data offset in the file > + * - CPU trace data size > + */ > + > + /* Buffer latency option, v7: > + * - trace data offset in the file Is this the actual latency? > + * - buffer name > + * - buffer clock > + */ > + > + vect = calloc(5 + (cpus * 3), sizeof(struct iovec)); Comment what these magic numbers are: 5 and 3. -- Steve > + if (!vect) > + return NULL; > + if (cpus) { > + cpu_ids = calloc(cpus, sizeof(int)); > + if (!cpu_ids) { > + free(vect); > + return NULL; > + } > + } > + vect[j].iov_base = (void *) &data_offset; > + vect[j++].iov_len = 8; > + vect[j].iov_base = (void *) name; > + vect[j++].iov_len = strlen(name) + 1; > + vect[j].iov_base = (void *) clock; > + vect[j++].iov_len = strlen(clock) + 1; > + if (id == TRACECMD_OPTION_BUFFER) { > + vect[j].iov_base = (void *) &k; > + vect[j++].iov_len = 4; > + for (i = 0; i < cpus; i++) { > + if (!cpu_data[i].file_size) > + continue; > + cpu_ids[i] = i; > + vect[j].iov_base = &cpu_ids[i]; > + vect[j++].iov_len = 4; > + vect[j].iov_base = &cpu_data[i].data_offset; > + vect[j++].iov_len = 8; > + vect[j].iov_base = &cpu_data[i].write_size; > + vect[j++].iov_len = 8; > + k++; > + } > + } > + > + option = tracecmd_add_option_v(handle, id, vect, j); > + free(vect); > + free(cpu_ids); > + > + return option; > +} > +