From: Mathieu Poirier <mathieu.poirier@linaro.org>
To: Mike Leach <mike.leach@linaro.org>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>,
Leo Yan <leo.yan@linaro.org>,
Arnaldo Carvalho de Melo <acme@kernel.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: Fri, 15 Jan 2021 15:46:58 -0700 [thread overview]
Message-ID: <20210115224658.GC375055@xps15> (raw)
In-Reply-To: <CAJ9a7VjtUuRRYBBu63kSXKwrGdB8ZoWJz-bE1g9tMLSbkFVDGg@mail.gmail.com>
On Mon, Jan 11, 2021 at 12:09:12PM +0000, Mike Leach wrote:
> Hi Leo,
>
> I think there is an issue here in that your modification assumes that
> all cpus in the system are of the same ETM type. The original routine
> allowed for differing ETM types, thus differing cpu ETM field lengths
> between ETMv4 / ETMv3, the field size was used after the relevant
> magic number for the cpu ETM was read.
>
> You have replaced two different sizes - with a single calculated size.
I usually go through an entire patchset before looking at the comments people
have made. In this case Mike and I are coming to the exact same conclusion.
I will look at Mike's patch on Monday.
>
> Moving forwards we are seeing the newer FEAT_ETE protocol drivers
> appearing on the list, which will ultimately need a new metadata
> structure.
>
> We have had discussions within ARM regarding the changing of the
> format to be more self describing - which should probably be opened
> out to the CS mailing list.
>
> Regards
>
> Mike
>
>
> On Mon, 11 Jan 2021 at 07:29, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
> >
> > On 1/9/21 7:44 AM, Leo Yan wrote:
> > > The metadata array can be extended over time and the tool, if using the
> > > predefined macro (like CS_ETMV4_PRIV_MAX for ETMv4) as metadata array
> > > size to copy data, it can cause compatible issue within different
> > > versions of perf tool.
> > >
> > > E.g. we recorded a data file with an old version tool, afterwards if
> > > use the new version perf tool to parse the file, since the metadata
> > > array has been extended and the macro CS_ETMV4_PRIV_MAX has been
> > > altered, if use it to parse the perf data with old format, this will
> > > lead to mismatch.
> > >
> > > To maintain backward compatibility, this patch calculates per CPU
> > > metadata array size on the runtime, the calculation is based on the
> > > info stored in the data file so that it's reliable.
> > >
> > > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> >
> > Looks good to me.
> >
> > Acked-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> >
>
>
> --
> Mike Leach
> Principal Engineer, ARM Ltd.
> Manchester Design Centre. UK
WARNING: multiple messages have this Message-ID (diff)
From: Mathieu Poirier <mathieu.poirier@linaro.org>
To: Mike Leach <mike.leach@linaro.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
Denis Nikitin <denik@chromium.org>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@redhat.com>,
Coresight ML <coresight@lists.linaro.org>,
John Garry <john.garry@huawei.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>, Leo Yan <leo.yan@linaro.org>,
Namhyung Kim <namhyung@kernel.org>, Will Deacon <will@kernel.org>,
linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
Daniel Kiss <Daniel.Kiss@arm.com>
Subject: Re: [PATCH v1 3/7] perf cs-etm: Calculate per CPU metadata array size
Date: Fri, 15 Jan 2021 15:46:58 -0700 [thread overview]
Message-ID: <20210115224658.GC375055@xps15> (raw)
In-Reply-To: <CAJ9a7VjtUuRRYBBu63kSXKwrGdB8ZoWJz-bE1g9tMLSbkFVDGg@mail.gmail.com>
On Mon, Jan 11, 2021 at 12:09:12PM +0000, Mike Leach wrote:
> Hi Leo,
>
> I think there is an issue here in that your modification assumes that
> all cpus in the system are of the same ETM type. The original routine
> allowed for differing ETM types, thus differing cpu ETM field lengths
> between ETMv4 / ETMv3, the field size was used after the relevant
> magic number for the cpu ETM was read.
>
> You have replaced two different sizes - with a single calculated size.
I usually go through an entire patchset before looking at the comments people
have made. In this case Mike and I are coming to the exact same conclusion.
I will look at Mike's patch on Monday.
>
> Moving forwards we are seeing the newer FEAT_ETE protocol drivers
> appearing on the list, which will ultimately need a new metadata
> structure.
>
> We have had discussions within ARM regarding the changing of the
> format to be more self describing - which should probably be opened
> out to the CS mailing list.
>
> Regards
>
> Mike
>
>
> On Mon, 11 Jan 2021 at 07:29, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
> >
> > On 1/9/21 7:44 AM, Leo Yan wrote:
> > > The metadata array can be extended over time and the tool, if using the
> > > predefined macro (like CS_ETMV4_PRIV_MAX for ETMv4) as metadata array
> > > size to copy data, it can cause compatible issue within different
> > > versions of perf tool.
> > >
> > > E.g. we recorded a data file with an old version tool, afterwards if
> > > use the new version perf tool to parse the file, since the metadata
> > > array has been extended and the macro CS_ETMV4_PRIV_MAX has been
> > > altered, if use it to parse the perf data with old format, this will
> > > lead to mismatch.
> > >
> > > To maintain backward compatibility, this patch calculates per CPU
> > > metadata array size on the runtime, the calculation is based on the
> > > info stored in the data file so that it's reliable.
> > >
> > > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> >
> > Looks good to me.
> >
> > Acked-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> >
>
>
> --
> Mike Leach
> Principal Engineer, ARM Ltd.
> Manchester Design Centre. UK
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-01-15 22:47 UTC|newest]
Thread overview: 74+ 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 ` 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-09 7:44 ` Leo Yan
2021-01-09 10:05 ` kernel test robot
2021-01-10 1:24 ` [PATCH] " Suzuki K Poulose
2021-01-10 1:41 ` Leo Yan
2021-01-10 22:34 ` Suzuki K Poulose
2021-01-11 0:05 ` Leo Yan
2021-01-09 11:24 ` [PATCH v1 1/7] " kernel test robot
2021-01-11 16:22 ` Mike Leach
2021-01-11 16:22 ` Mike Leach
2021-01-12 7:22 ` Leo Yan
2021-01-12 7:22 ` Leo Yan
2021-01-12 8:58 ` Leo Yan
2021-01-12 8:58 ` Leo Yan
2021-01-12 11:03 ` Suzuki K Poulose
2021-01-12 11:03 ` Suzuki K Poulose
2021-01-12 11:23 ` Mike Leach
2021-01-12 11:23 ` Mike Leach
2021-01-12 14:14 ` Leo Yan
2021-01-12 14:14 ` Leo Yan
2021-01-12 23:43 ` Mike Leach
2021-01-12 23:43 ` Mike Leach
2021-01-15 22:30 ` Mathieu Poirier
2021-01-15 22:30 ` Mathieu Poirier
2021-01-19 7:05 ` Suzuki K Poulose
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-09 7:44 ` Leo Yan
2021-01-15 22:44 ` Mathieu Poirier
2021-01-15 22:44 ` Mathieu Poirier
2021-01-19 2:32 ` Leo Yan
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-09 7:44 ` Leo Yan
2021-01-11 7:28 ` Suzuki K Poulose
2021-01-11 7:28 ` Suzuki K Poulose
2021-01-11 12:09 ` Mike Leach
2021-01-11 12:09 ` Mike Leach
2021-01-11 15:06 ` Leo Yan
2021-01-11 15:06 ` Leo Yan
2021-01-13 0:00 ` Mike Leach
2021-01-13 0:00 ` Mike Leach
2021-01-13 2:27 ` Leo Yan
2021-01-13 2:27 ` Leo Yan
2021-01-15 22:46 ` Mathieu Poirier [this message]
2021-01-15 22:46 ` Mathieu Poirier
2021-01-16 0:50 ` Leo Yan
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-09 7:44 ` Leo Yan
2021-01-11 9:45 ` Suzuki K Poulose
2021-01-11 9:45 ` Suzuki K Poulose
2021-01-11 13:12 ` Leo Yan
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-09 7:44 ` Leo Yan
2021-01-11 9:47 ` Suzuki K Poulose
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-09 7:44 ` Leo Yan
2021-01-11 9:55 ` Suzuki K Poulose
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-09 7:44 ` Leo Yan
2021-01-11 10:07 ` Suzuki K Poulose
2021-01-11 10:07 ` Suzuki K Poulose
2021-01-11 13:10 ` Leo Yan
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-11 18:16 ` Mathieu Poirier
2021-01-12 7:23 ` Leo Yan
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=20210115224658.GC375055@xps15 \
--to=mathieu.poirier@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=leo.yan@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.