Messages in this thread Patch in this message | | | Date | Wed, 13 Jan 2021 10:27:32 +0800 | From | Leo Yan <> | Subject | Re: [PATCH v1 3/7] perf cs-etm: Calculate per CPU metadata array size |
| |
Hi Mike,
On Wed, Jan 13, 2021 at 12:00:10AM +0000, Mike Leach wrote:
[...]
> > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c > > index a2a369e2fbb6..edaec57362f0 100644 > > --- a/tools/perf/util/cs-etm.c > > +++ b/tools/perf/util/cs-etm.c > > @@ -2558,12 +2558,19 @@ int cs_etm__process_auxtrace_info(union perf_event *event, > > err = -ENOMEM; > > goto err_free_metadata; > > } > > - for (k = 0; k < CS_ETM_PRIV_MAX; k++) > > + for (k = 0; k < CS_ETM_PRIV_MAX; k++) { > > metadata[j][k] = ptr[i + k]; > > > > + if (ptr[i + k + 1] == __perf_cs_etmv3_magic || > > + ptr[i + k + 1] == __perf_cs_etmv4_magic) { > > + k++; > > + break; > > + } > > + } > > + > > /* The traceID is our handle */ > > idx = metadata[j][CS_ETM_ETMTRACEIDR]; > > - i += CS_ETM_PRIV_MAX; > > + i += k; > > } else if (ptr[i] == __perf_cs_etmv4_magic) { > > metadata[j] = zalloc(sizeof(*metadata[j]) * > > CS_ETMV4_PRIV_MAX); > > @@ -2571,12 +2578,19 @@ int cs_etm__process_auxtrace_info(union perf_event *event, > > err = -ENOMEM; > > goto err_free_metadata; > > } > > - for (k = 0; k < CS_ETMV4_PRIV_MAX; k++) > > + for (k = 0; k < CS_ETMV4_PRIV_MAX; k++) { > > metadata[j][k] = ptr[i + k]; > > > > + if (ptr[i + k + 1] == __perf_cs_etmv3_magic || > > + ptr[i + k + 1] == __perf_cs_etmv4_magic) { > > + k++; > > + break; > > + } > > + } > > + > > /* The traceID is our handle */ > > idx = metadata[j][CS_ETMV4_TRCTRACEIDR]; > > - i += CS_ETMV4_PRIV_MAX; > > + i += k; > > } > > > > /* Get an RB node for this CPU */ > > That would be a spot fix for the read /copy case, but will not fix the > print routine which will still bail out on older versions of the > format. (when using perf report --dump). > > The "self describing" format I have been looking at will add an > NR_PARAMS value to the common block in the CPU metadata parameter > list, increment the header version to '1' and update the format writer > to use the version 1 format while having the reader understand both v0 > and v1 formats. > > i..e in cs-etm.h perf I add: > /* > * Update the version for new format. > * > * New version 1 format adds a param count to the per cpu metadata. > * This allows easy adding of new metadata parameters. > * Requires that new params always added after current ones. > * Also allows client reader to handle file versions that are different by > * checking the number of params in the file vs the number expected. > */ > #define CS_HEADER_CURRENT_VERSION 1 > > /* Beginning of header common to both ETMv3 and V4 */ > enum { > CS_ETM_MAGIC, > CS_ETM_CPU, > CS_ETM_NR_PARAMS, /* number of parameters to follow in this block */ > }; > > where in verison 1, NR_PARAMS indicates the total number of params > that follow - so adding new parameters can be added to the metadata > enums and the tool will automatically adjust, and will handle v0 > files, plus older and newer files that have differing numbers of > parameters, as long as the parameters are only ever added to the end > of the list. > > I have been working on a patch for this today, which took a little > longer than expected as it was a little more complex than expected > (the printing routines in for the --dump command!). > > I will post this tomorrow when tested - and if we agree it works it > could be rolled into your set - it would make adding the PID parameter > easier, and ensure that this new format is available for the upcoming > developments.
Thanks for the info. I will look at your patch and see how to fit with it.
Thanks, Leo
| |