On Wed, Jul 14, 2021 at 09:40:15AM +0100, Russell King (Oracle) wrote:
> On Tue, Jul 13, 2021 at 07:13:02PM +0100, Catalin Marinas wrote:
> > We could try to clarify E2.2.1 to simply state that naturally aligned
> > LDRD/STRD are single-copy atomic without any subsequent statement on the
> > translation table.
>
> I think that clarification would be most helpful. Thanks.
Thanks for the suggestion and confirmation, Russell & Catalin.
If so, I will implement the weak functions for
compat_auxtrace_mmap__{read_head|write_tail}; and write the arm/arm64
specific functions with using LDRD/STRD instructions.
For better patches organization, I will use a separate patch set for
enabling the compat functions (in particular patches 10, 11/11) in
the next spin.
Thanks,
Leo
Em Tue, Jul 13, 2021 at 05:31:03PM +0000, Hunter, Adrian escreveu:
> > On Mon, Jul 12, 2021 at 03:14:35PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Sun, Jul 11, 2021 at 06:41:04PM +0800, Leo Yan escreveu:
> > > > +++ b/tools/perf/util/env.c
> > > > @@ -11,6 +11,7 @@
> > > > #include <stdlib.h>
> > > > #include <string.h>
> > > > +int kernel_is_64_bit;
> > > > struct perf_env perf_env;
> > > Why can't this be in 'struct perf_env'?
> > Good question. I considered to add it in struct perf_env but finally I used this
> > way; the reason is this variable "kernel_is_64_bit" is only used during
> > recording phase for AUX ring buffer, and don't use it for report. So seems to
> > me it's over complexity to add a new field and just wander if it's necessary to
> > save this field as new feature in the perf header.
> I think we store the arch, so if the "kernel_is_64_bit" calculation depends only on arch
> then I guess we don't need a new feature at the moment.
So, I wasn't suggesting to add this info to the perf.data file header,
just to the in-memory 'struct perf_env'.
And also we should avoid unconditionally initializing things that we may
never need, please structure it as:
static void perf_env__init_kernel_mode(struct perf_env *env)
{
const char *arch = perf_env__raw_arch(env);
if (!strncmp(arch, "x86_64", 6) || !strncmp(arch, "aarch64", 7) ||
!strncmp(arch, "arm64", 5) || !strncmp(arch, "mips64", 6) ||
!strncmp(arch, "parisc64", 8) || !strncmp(arch, "riscv64", 7) ||
!strncmp(arch, "s390x", 5) || !strncmp(arch, "sparc64", 7))
kernel_is_64_bit = 1;
else
kernel_is_64_bit = 0;
}
void perf_env__init(struct perf_env *env)
{
...
env->kernel_is_64_bit = -1;
...
}
bool perf_env__kernel_is_64_bit(struct perf_env *env)
{
if (env->kernel_is_64_bit == -1)
perf_env__init_kernel_mode(env);
return env->kernel_is_64_bit;
}
One thing in my TODO is to crack down on the tons of initializations
perf does unconditionally, last time I looked there are lots :-\
- Arnaldo
> > Combining the comment from Adrian in another email, I think it's good to add
> > a new field "compat_mode" in the struct perf_env, and this field will be
> > initialized in build-record.c. Currently we don't need to save this value into
> > the perf file, if later we need to use this value for decoding phase, then we
> > can add a new feature item to save "compat_mode"
> > into the perf file's header.
> > If you have any different idea, please let me know. Thanks!
This patchset consists of refactoring to allow the decoder to be
created in advance when the AUX records are iterated over. The
AUX record flags are used to communicate whether the data is
formatted or not which is the reason this refactoring is required.
These changes result in some simplifications, removal of early exit
conditions etc.
A change was also made to --dump-raw-trace code to allow the
formatted/unformatted status to persist and for the decoder to
not be continually deleted and recreated.
The changes apply on top of the previous patchset "[PATCH v7 0/2] perf
cs-etm: Split Coresight decode by aux records".
Changes since v1:
* Change 'decoders_per_cpu' variable name to 'decoders' and add a comment
* Add a warning that piped mode is best effort, suggested by Suzuki
James Clark (6):
perf cs-etm: Refactor initialisation of kernel start address
perf cs-etm: Split setup and timestamp search functions
perf cs-etm: Only setup queues when they are modified
perf cs-etm: Suppress printing when resetting decoder
perf cs-etm: Use existing decoder instead of resetting it
perf cs-etm: Pass unformatted flag to decoder
.../perf/util/cs-etm-decoder/cs-etm-decoder.c | 14 +-
tools/perf/util/cs-etm.c | 185 +++++++++---------
2 files changed, 97 insertions(+), 102 deletions(-)
--
2.28.0
Patch release for OpenCSD v.1.1.1 made to address C-API include file
issues for the ETE decoder, raised by work on perf tools
Regards
Mike
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
Add -lstdc++ to perf when linking libopencsd as it is a dependency. It
does not hurt to add it when dynamic linking.
Filter out -static flag when building plugins as they are always built
as dynamic libraries and -static and -dynamic don't work well together
on arm and arm64.
Signed-off-by: Tamas Zsoldos <tamas.zsoldos(a)arm.com>
Signed-off-by: Branislav Rankov <branislav.rankov(a)arm.com>
---
tools/perf/Makefile.config | 2 +-
tools/perf/Makefile.perf | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 73df23dd664c..b014a9bdd0db 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -143,7 +143,7 @@ FEATURE_CHECK_LDFLAGS-libcrypto = -lcrypto
ifdef CSINCLUDES
LIBOPENCSD_CFLAGS := -I$(CSINCLUDES)
endif
-OPENCSDLIBS := -lopencsd_c_api -lopencsd
+OPENCSDLIBS := -lopencsd_c_api -lopencsd -lstdc++
ifdef CSLIBS
LIBOPENCSD_LDFLAGS := -L$(CSLIBS)
endif
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index e47f04e5b51e..cd3cf910fa8a 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -792,7 +792,7 @@ endif
$(patsubst perf-%,%.o,$(PROGRAMS)): $(wildcard */*.h)
-LIBTRACEEVENT_FLAGS += plugin_dir=$(plugindir_SQ) 'EXTRA_CFLAGS=$(EXTRA_CFLAGS)' 'LDFLAGS=$(LDFLAGS)'
+LIBTRACEEVENT_FLAGS += plugin_dir=$(plugindir_SQ) 'EXTRA_CFLAGS=$(EXTRA_CFLAGS)' 'LDFLAGS=$(filter-out -static,$(LDFLAGS))'
$(LIBTRACEEVENT): FORCE
$(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)libtraceevent.a
--
2.17.1
This patchset consists of refactoring to allow the decoder to be
created in advance when the AUX records are iterated over. The
AUX record flags are used to communicate whether the data is
formatted or not which is the reason this refactoring is required.
These changes result in some simplifications, removal of early exit
conditions etc.
A change was also made to --dump-raw-trace code to allow the
formatted/unformatted status to persist and for the decoder to
not be continually deleted and recreated.
The changes apply on top of the previous patchset "[PATCH v7 0/2] perf
cs-etm: Split Coresight decode by aux records".
James Clark (6):
perf cs-etm: Refactor initialisation of kernel start address
perf cs-etm: Split setup and timestamp search functions
perf cs-etm: Only setup queues when they are modified
perf cs-etm: Suppress printing when resetting decoder
perf cs-etm: Use existing decoder instead of resetting it
perf cs-etm: Pass unformatted flag to decoder
.../perf/util/cs-etm-decoder/cs-etm-decoder.c | 10 +-
tools/perf/util/cs-etm.c | 174 ++++++++----------
2 files changed, 84 insertions(+), 100 deletions(-)
--
2.28.0
Change applies to perf/core (45237f9898fc)
Changes since v6:
* Fix for snapshot mode where buffers are wrapped. This fix was done by clamping the aux record
size to the size of the buffer (see comment).
* Added an extra debugging printout.
* Typo/formatting fixes.
* Add the change for --dump-raw-trace as a second commit. I planned to do this later, but have now
finished it so I'll submit it at the same time.
* Did some more thorough testing around the different snapshot scenarios.
Decoding snapshot files with duplicate data is improved by this patchset because of the reason
mentioned at the end of the testing section. Coincidentally, the same issue is also fixed in
"[PATCH v1 0/3] coresight: Fix for snapshot mode" but by not saving duplicates, rather than not
decoding them.
James Clark (2):
perf cs-etm: Split Coresight decode by aux records
perf cs-etm: Split --dump-raw-trace by AUX records
tools/perf/util/cs-etm.c | 188 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 185 insertions(+), 3 deletions(-)
--
2.28.0
This is a series of fixes addressing the issues in the way we handle
- Self-Hosted trace filter control register for ETM/ETE
- AUX buffer and event handling of TRBE at overflow.
The use of TRUNCATED flag on an IRQ for the TRBE driver is
something that needs to be rexamined. Please see Patch 3 for
more details.
Suzuki K Poulose (5):
coresight: etm4x: Save restore TRFCR_EL1
coresight: etm4x: Use Trace Filtering controls dynamically
coresight: trbe: Keep TRBE disabled on overflow irq
coresight: trbe: Move irq_work queue processing
coresight: trbe: Prohibit tracing while handling an IRQ
.../coresight/coresight-etm4x-core.c | 109 ++++++++++++++----
drivers/hwtracing/coresight/coresight-etm4x.h | 7 +-
drivers/hwtracing/coresight/coresight-trbe.c | 91 ++++++++++-----
3 files changed, 149 insertions(+), 58 deletions(-)
--
2.24.1
HI James,
Agreed the header is missing from the install. Looking in more detail
the header should also be part of ocsd_c_api_types.h
so that it gets pulled in by any program including the main api header.
I'll look at an update to fix both these issues.
Normally these should come through the CS mailing list, or as an issue
on the git hub project.
Thanks
Mike
On Fri, 16 Jul 2021 at 14:56, James Clark <james.clark(a)arm.com> wrote:
>
> To be able to instantiate the OCSD_BUILTIN_DCD_ETE decoder the header
> trc_pkt_types_ete.h is required, so install it alongside the headers for
> the other versions.
>
> Signed-off-by: James Clark <james.clark(a)arm.com>
> ---
> decoder/build/linux/rctdl_c_api_lib/makefile | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/decoder/build/linux/rctdl_c_api_lib/makefile b/decoder/build/linux/rctdl_c_api_lib/makefile
> index a0bd5a3..7b4055d 100644
> --- a/decoder/build/linux/rctdl_c_api_lib/makefile
> +++ b/decoder/build/linux/rctdl_c_api_lib/makefile
> @@ -113,6 +113,8 @@ install_inc:
> $(INSTALL) --mode=0644 $(INST_INC_SRC)/etmv3/trc_pkt_types_etmv3.h $(INST_INC_DST)/etmv3/
> $(INSTALL) -d --mode=0755 $(INST_INC_DST)/etmv4
> $(INSTALL) --mode=0644 $(INST_INC_SRC)/etmv4/trc_pkt_types_etmv4.h $(INST_INC_DST)/etmv4/
> + $(INSTALL) -d --mode=0755 $(INST_INC_DST)/ete
> + $(INSTALL) --mode=0644 $(INST_INC_SRC)/ete/trc_pkt_types_ete.h $(INST_INC_DST)/ete/
> $(INSTALL) -d --mode=0755 $(INST_INC_DST)/c_api
> $(INSTALL) --mode=0644 $(INST_INC_SRC)/c_api/ocsd_c_api_types.h $(INST_INC_DST)/c_api/
> $(INSTALL) --mode=0644 $(INST_INC_SRC)/c_api/opencsd_c_api.h $(INST_INC_DST)/c_api/
> --
> 2.28.0
>
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK