From: Leo Yan <leo.yan@linaro.org>
To: Mike Leach <mike.leach@linaro.org>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Mathieu Poirier <mathieu.poirier@linaro.org>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
John Garry <john.garry@huawei.com>, Will Deacon <will@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Mark Rutland <mark.rutland@arm.com>, Jiri Olsa <jolsa@redhat.com>,
Namhyung Kim <namhyung@kernel.org>,
Daniel Kiss <Daniel.Kiss@arm.com>,
Denis Nikitin <denik@chromium.org>,
Coresight ML <coresight@lists.linaro.org>,
linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v1 3/7] perf cs-etm: Calculate per CPU metadata array size
Date: Wed, 13 Jan 2021 10:27:32 +0800 [thread overview]
Message-ID: <20210113022721.GG18965@leoy-ThinkPad-X240s> (raw)
In-Reply-To: <CAJ9a7VjhMOq=r=W2xtjgiHhmvwk1xDzMnWF4hKPogX8PX+ZADg@mail.gmail.com>
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
next prev parent reply other threads:[~2021-01-13 2:28 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-09 7:44 [PATCH v1 0/7] coresight: etm-perf: Fix pid tracing with VHE Leo Yan
2021-01-09 7:44 ` [PATCH v1 1/7] coresight: etm-perf: Add support for PID tracing for kernel at EL2 Leo Yan
2021-01-11 16:22 ` Mike Leach
2021-01-12 7:22 ` Leo Yan
2021-01-12 8:58 ` Leo Yan
2021-01-12 11:03 ` Suzuki K Poulose
2021-01-12 11:23 ` Mike Leach
2021-01-12 14:14 ` Leo Yan
2021-01-12 23:43 ` Mike Leach
2021-01-15 22:30 ` Mathieu Poirier
2021-01-19 7:05 ` Suzuki K Poulose
2021-01-09 7:44 ` [PATCH v1 2/7] perf cs_etm: Use pid tracing explicitly instead of contextid Leo Yan
2021-01-15 22:44 ` Mathieu Poirier
2021-01-19 2:32 ` Leo Yan
2021-01-09 7:44 ` [PATCH v1 3/7] perf cs-etm: Calculate per CPU metadata array size Leo Yan
2021-01-11 7:28 ` Suzuki K Poulose
2021-01-11 12:09 ` Mike Leach
2021-01-11 15:06 ` Leo Yan
2021-01-13 0:00 ` Mike Leach
2021-01-13 2:27 ` Leo Yan [this message]
2021-01-15 22:46 ` Mathieu Poirier
2021-01-16 0:50 ` Leo Yan
2021-01-09 7:44 ` [PATCH v1 4/7] perf cs-etm: Add PID format into metadata Leo Yan
2021-01-11 9:45 ` Suzuki K Poulose
2021-01-11 13:12 ` Leo Yan
2021-01-09 7:44 ` [PATCH v1 5/7] perf cs-etm: Fixup PID_FMT when it is zero Leo Yan
2021-01-11 9:47 ` Suzuki K Poulose
2021-01-09 7:44 ` [PATCH v1 6/7] perf cs-etm: Add helper cs_etm__get_pid_fmt() Leo Yan
2021-01-11 9:55 ` Suzuki K Poulose
2021-01-09 7:44 ` [PATCH v1 7/7] perf cs-etm: Detect pid in VMID for kernel running at EL2 Leo Yan
2021-01-11 10:07 ` Suzuki K Poulose
2021-01-11 13:10 ` Leo Yan
2021-01-11 18:16 ` [PATCH v1 0/7] coresight: etm-perf: Fix pid tracing with VHE Mathieu Poirier
2021-01-12 7:23 ` Leo Yan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210113022721.GG18965@leoy-ThinkPad-X240s \
--to=leo.yan@linaro.org \
--cc=Daniel.Kiss@arm.com \
--cc=acme@kernel.org \
--cc=alexander.shishkin@linux.intel.com \
--cc=coresight@lists.linaro.org \
--cc=denik@chromium.org \
--cc=john.garry@huawei.com \
--cc=jolsa@redhat.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mathieu.poirier@linaro.org \
--cc=mike.leach@linaro.org \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=suzuki.poulose@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).