linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/6] perf tools: Warning fixes
@ 2021-07-29 15:57 James Clark
  2021-07-29 15:58 ` [RFC PATCH 1/6] perf tools: Add WARN_ONCE equivalent for UI warnings James Clark
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: James Clark @ 2021-07-29 15:57 UTC (permalink / raw)
  To: acme, mathieu.poirier, coresight, linux-perf-users
  Cc: leo.yan, suzuki.poulose, mike.leach, James Clark, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, John Garry,
	Will Deacon, linux-kernel, linux-arm-kernel

I'm submitting this as an RFC because there are a few changes I'd
like to get feedback on. The two changes I wanted to make were the
last two for Coresight warnings, but I ended up making some perf-wide
changes along the way.

For #1 (perf tools: Add WARN_ONCE equivalent for UI warnings)
 * Does it make sense to add warn once equivalents at all, or
   should the once be re-done for each usage?
 * Or should there be some kind of generic 'once' wrapper?

For #3 (perf tools: Add disassembly warnings for annotate --stdio)
 * If the output is interpreted by any other tools, then adding
   these warnings could be an issue, so maybe this change could
   be dropped, but no error output at all isn't ideal.

For #4 (perf tools: Add flag for tracking warnings of missing DSOs)
 * In theory I could re-use 'annotate_warned', but it might make sense
   to rename it in that case, or just leave the new auxtrace_warned and
   remove any confusion.

This set applies to perf/core e73f0f0ee7541

Thanks
James

James Clark (6):
  perf tools: Add WARN_ONCE equivalent for UI warnings
  perf tools: Re-add annotate_warned functionality
  perf tools: Add disassembly warnings for annotate --stdio
  perf tools: Add flag for tracking warnings of missing DSOs
  perf cs-etm: Improve Coresight zero timestamp warning
  perf cs-etm: Add warnings for missing DSOs

 tools/perf/ui/browsers/annotate.c             |  1 +
 tools/perf/ui/gtk/annotate.c                  |  1 +
 tools/perf/util/annotate.c                    | 20 +++++++++++++++++--
 .../perf/util/cs-etm-decoder/cs-etm-decoder.c |  7 +++++--
 tools/perf/util/cs-etm.c                      | 10 +++++++++-
 tools/perf/util/debug.h                       | 14 +++++++++++++
 tools/perf/util/dso.h                         |  1 +
 7 files changed, 49 insertions(+), 5 deletions(-)

-- 
2.28.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [RFC PATCH 1/6] perf tools: Add WARN_ONCE equivalent for UI warnings
  2021-07-29 15:57 [RFC PATCH 0/6] perf tools: Warning fixes James Clark
@ 2021-07-29 15:58 ` James Clark
  2021-08-02 14:44   ` Arnaldo Carvalho de Melo
  2021-07-29 15:58 ` [RFC PATCH 2/6] perf tools: Re-add annotate_warned functionality James Clark
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: James Clark @ 2021-07-29 15:58 UTC (permalink / raw)
  To: acme, mathieu.poirier, coresight, linux-perf-users
  Cc: leo.yan, suzuki.poulose, mike.leach, James Clark, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, John Garry,
	Will Deacon, linux-kernel, linux-arm-kernel

Currently WARN_ONCE prints to stderr and corrupts the TUI. Add
equivalent methods for UI warnings.

Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/perf/util/debug.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tools/perf/util/debug.h b/tools/perf/util/debug.h
index 48f631966067..f99468a7f681 100644
--- a/tools/perf/util/debug.h
+++ b/tools/perf/util/debug.h
@@ -22,6 +22,13 @@ extern int debug_data_convert;
 	eprintf(0, verbose, pr_fmt(fmt), ##__VA_ARGS__)
 #define pr_warning(fmt, ...) \
 	eprintf(0, verbose, pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_warning_once(fmt, ...) ({		\
+	static int __warned;			\
+	if (unlikely(!__warned)) {		\
+		pr_warning(fmt, ##__VA_ARGS__); \
+		__warned = 1;			\
+	}					\
+})
 #define pr_info(fmt, ...) \
 	eprintf(0, verbose, pr_fmt(fmt), ##__VA_ARGS__)
 #define pr_debug(fmt, ...) \
@@ -55,6 +62,13 @@ void trace_event(union perf_event *event);
 
 int ui__error(const char *format, ...) __printf(1, 2);
 int ui__warning(const char *format, ...) __printf(1, 2);
+#define ui__warning_once(format, ...) ({		\
+	static int __warned;				\
+	if (unlikely(!__warned)) {			\
+		ui__warning(format, ##__VA_ARGS__);	\
+		__warned = 1;				\
+	}						\
+})
 
 void pr_stat(const char *fmt, ...);
 
-- 
2.28.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [RFC PATCH 2/6] perf tools: Re-add annotate_warned functionality
  2021-07-29 15:57 [RFC PATCH 0/6] perf tools: Warning fixes James Clark
  2021-07-29 15:58 ` [RFC PATCH 1/6] perf tools: Add WARN_ONCE equivalent for UI warnings James Clark
@ 2021-07-29 15:58 ` James Clark
  2021-07-29 15:58 ` [RFC PATCH 3/6] perf tools: Add disassembly warnings for annotate --stdio James Clark
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: James Clark @ 2021-07-29 15:58 UTC (permalink / raw)
  To: acme, mathieu.poirier, coresight, linux-perf-users
  Cc: leo.yan, suzuki.poulose, mike.leach, James Clark, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, John Garry,
	Will Deacon, linux-kernel, linux-arm-kernel

Setting annotate_warned to true on errors was removed in
commit ee51d851392e ("perf annotate: Introduce strerror for handling
symbol__disassemble() errors") which means when 'perf annotate
--skip-missing' is used warnings are shown multiple times for the same
DSO.

Setting this again restores the original behavior of only one warning
each.

Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/perf/ui/browsers/annotate.c | 1 +
 tools/perf/ui/gtk/annotate.c      | 1 +
 2 files changed, 2 insertions(+)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 701130ad43a2..ef4da4295bf7 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -966,6 +966,7 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
 	err = symbol__annotate2(ms, evsel, opts, &browser.arch);
 	if (err) {
 		char msg[BUFSIZ];
+		ms->map->dso->annotate_warned = true;
 		symbol__strerror_disassemble(ms, err, msg, sizeof(msg));
 		ui__error("Couldn't annotate %s:\n%s", sym->name, msg);
 		goto out_free_offsets;
diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c
index 94167bfed722..0a50e962f9a3 100644
--- a/tools/perf/ui/gtk/annotate.c
+++ b/tools/perf/ui/gtk/annotate.c
@@ -177,6 +177,7 @@ static int symbol__gtk_annotate(struct map_symbol *ms, struct evsel *evsel,
 	err = symbol__annotate(ms, evsel, &annotation__default_options, NULL);
 	if (err) {
 		char msg[BUFSIZ];
+		ms->map->dso->annotate_warned = true;
 		symbol__strerror_disassemble(ms, err, msg, sizeof(msg));
 		ui__error("Couldn't annotate %s: %s\n", sym->name, msg);
 		return -1;
-- 
2.28.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [RFC PATCH 3/6] perf tools: Add disassembly warnings for annotate --stdio
  2021-07-29 15:57 [RFC PATCH 0/6] perf tools: Warning fixes James Clark
  2021-07-29 15:58 ` [RFC PATCH 1/6] perf tools: Add WARN_ONCE equivalent for UI warnings James Clark
  2021-07-29 15:58 ` [RFC PATCH 2/6] perf tools: Re-add annotate_warned functionality James Clark
@ 2021-07-29 15:58 ` James Clark
  2021-07-29 15:58 ` [RFC PATCH 4/6] perf tools: Add flag for tracking warnings of missing DSOs James Clark
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: James Clark @ 2021-07-29 15:58 UTC (permalink / raw)
  To: acme, mathieu.poirier, coresight, linux-perf-users
  Cc: leo.yan, suzuki.poulose, mike.leach, James Clark, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, John Garry,
	Will Deacon, linux-kernel, linux-arm-kernel

Currently 'perf annotate --stdio' (and --stdio2) will exit without
printing anything if there are disassembly errors. Apply the same
error handler that's used for TUI and GTK modes. This makes comparing
disassembly across the different modes more consistent.

Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/perf/util/annotate.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index aa04a3655236..1ed097bcb78a 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -2787,9 +2787,17 @@ int symbol__tty_annotate2(struct map_symbol *ms, struct evsel *evsel,
 	struct rb_root source_line = RB_ROOT;
 	struct hists *hists = evsel__hists(evsel);
 	char buf[1024];
+	int err;
+
+	err = symbol__annotate2(ms, evsel, opts, NULL);
+	if (err) {
+		char msg[BUFSIZ];
 
-	if (symbol__annotate2(ms, evsel, opts, NULL) < 0)
+		dso->annotate_warned = true;
+		symbol__strerror_disassemble(ms, err, msg, sizeof(msg));
+		ui__error("Couldn't annotate %s:\n%s", sym->name, msg);
 		return -1;
+	}
 
 	if (opts->print_lines) {
 		srcline_full_filename = opts->full_path;
@@ -2813,9 +2821,17 @@ int symbol__tty_annotate(struct map_symbol *ms, struct evsel *evsel,
 	struct dso *dso = ms->map->dso;
 	struct symbol *sym = ms->sym;
 	struct rb_root source_line = RB_ROOT;
+	int err;
+
+	err = symbol__annotate(ms, evsel, opts, NULL);
+	if (err) {
+		char msg[BUFSIZ];
 
-	if (symbol__annotate(ms, evsel, opts, NULL) < 0)
+		dso->annotate_warned = true;
+		symbol__strerror_disassemble(ms, err, msg, sizeof(msg));
+		ui__error("Couldn't annotate %s:\n%s", sym->name, msg);
 		return -1;
+	}
 
 	symbol__calc_percent(sym, evsel);
 
-- 
2.28.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [RFC PATCH 4/6] perf tools: Add flag for tracking warnings of missing DSOs
  2021-07-29 15:57 [RFC PATCH 0/6] perf tools: Warning fixes James Clark
                   ` (2 preceding siblings ...)
  2021-07-29 15:58 ` [RFC PATCH 3/6] perf tools: Add disassembly warnings for annotate --stdio James Clark
@ 2021-07-29 15:58 ` James Clark
  2021-07-29 15:58 ` [RFC PATCH 5/6] perf cs-etm: Improve Coresight zero timestamp warning James Clark
  2021-07-29 15:58 ` [RFC PATCH 6/6] perf cs-etm: Add warnings for missing DSOs James Clark
  5 siblings, 0 replies; 18+ messages in thread
From: James Clark @ 2021-07-29 15:58 UTC (permalink / raw)
  To: acme, mathieu.poirier, coresight, linux-perf-users
  Cc: leo.yan, suzuki.poulose, mike.leach, James Clark, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, John Garry,
	Will Deacon, linux-kernel, linux-arm-kernel

Auxtrace support may need DSOs for decoding (for example Arm
Coresight). If one of these is missing it would make sense to
warn once for each one that's missing, but not flood the output
with every address as there could be thousands of lookups.

This flag will allow tracking whether a warning was shown for
each DSO.

Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/perf/util/dso.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 52e7101c5609..83723ba11dc8 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -170,6 +170,7 @@ struct dso {
 	u8		 has_srcline:1;
 	u8		 hit:1;
 	u8		 annotate_warned:1;
+	u8		 auxtrace_warned:1;
 	u8		 short_name_allocated:1;
 	u8		 long_name_allocated:1;
 	u8		 is_64_bit:1;
-- 
2.28.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [RFC PATCH 5/6] perf cs-etm: Improve Coresight zero timestamp warning
  2021-07-29 15:57 [RFC PATCH 0/6] perf tools: Warning fixes James Clark
                   ` (3 preceding siblings ...)
  2021-07-29 15:58 ` [RFC PATCH 4/6] perf tools: Add flag for tracking warnings of missing DSOs James Clark
@ 2021-07-29 15:58 ` James Clark
  2021-08-02 15:17   ` Leo Yan
  2021-07-29 15:58 ` [RFC PATCH 6/6] perf cs-etm: Add warnings for missing DSOs James Clark
  5 siblings, 1 reply; 18+ messages in thread
From: James Clark @ 2021-07-29 15:58 UTC (permalink / raw)
  To: acme, mathieu.poirier, coresight, linux-perf-users
  Cc: leo.yan, suzuki.poulose, mike.leach, James Clark, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, John Garry,
	Will Deacon, linux-kernel, linux-arm-kernel

Only show the warning if the user hasn't already set timeless mode and
improve the text because there was ambiguity around the meaning of '...'

Change the warning to a UI warning instead of printing straight to
stderr because this corrupts the UI when perf report TUI is used. The UI
warning function also handles printing to stderr when in perf script
mode.

Suggested-by: Leo Yan <leo.yan@linaro.org>
Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
index 3e1a05bc82cc..5084bd2ca6eb 100644
--- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
+++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
@@ -324,8 +324,11 @@ cs_etm_decoder__do_hard_timestamp(struct cs_etm_queue *etmq,
 		 * underflow.
 		 */
 		packet_queue->cs_timestamp = 0;
-		WARN_ONCE(true, "Zero Coresight timestamp found at Idx:%" OCSD_TRC_IDX_STR
-				". Decoding may be improved with --itrace=Z...\n", indx);
+		if (!cs_etm__etmq_is_timeless(etmq))
+			pr_warning_once("Zero Coresight timestamp found at Idx:%" OCSD_TRC_IDX_STR
+					". Decoding may be improved by prepending 'Z' to your current --itrace arguments.\n",
+					indx);
+
 	} else if (packet_queue->instr_count > elem->timestamp) {
 		/*
 		 * Sanity check that the elem->timestamp - packet_queue->instr_count would not
-- 
2.28.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [RFC PATCH 6/6] perf cs-etm: Add warnings for missing DSOs
  2021-07-29 15:57 [RFC PATCH 0/6] perf tools: Warning fixes James Clark
                   ` (4 preceding siblings ...)
  2021-07-29 15:58 ` [RFC PATCH 5/6] perf cs-etm: Improve Coresight zero timestamp warning James Clark
@ 2021-07-29 15:58 ` James Clark
  2021-08-02 14:51   ` Arnaldo Carvalho de Melo
  2021-08-02 15:41   ` Leo Yan
  5 siblings, 2 replies; 18+ messages in thread
From: James Clark @ 2021-07-29 15:58 UTC (permalink / raw)
  To: acme, mathieu.poirier, coresight, linux-perf-users
  Cc: leo.yan, suzuki.poulose, mike.leach, James Clark, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, John Garry,
	Will Deacon, linux-kernel, linux-arm-kernel

Currently decode will silently fail if no binary data is available for
the decode. This is made worse if only partial data is available because
the decode will appear to work, but any trace from that missing DSO will
silently not be generated.

Add a UI popup once if there is any data missing, and then warn in the
bottom left for each individual DSO that's missing.

Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/perf/util/cs-etm.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 32ad92d3e454..e6851260d059 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -746,8 +746,16 @@ static u32 cs_etm__mem_access(struct cs_etm_queue *etmq, u8 trace_chan_id,
 
 	len = dso__data_read_offset(al.map->dso, machine, offset, buffer, size);
 
-	if (len <= 0)
+	if (len <= 0) {
+		ui__warning_once("CS ETM Trace: Missing DSO. Use 'perf archive' to export data from the traced system.\n");
+		if (!al.map->dso->auxtrace_warned) {
+			pr_err("CS ETM Trace: Debug data not found for address %#"PRIx64" in %s\n",
+				    address,
+				    al.map->dso->long_name ? al.map->dso->long_name : "Unknown");
+			al.map->dso->auxtrace_warned = true;
+		}
 		return 0;
+	}
 
 	return len;
 }
-- 
2.28.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 1/6] perf tools: Add WARN_ONCE equivalent for UI warnings
  2021-07-29 15:58 ` [RFC PATCH 1/6] perf tools: Add WARN_ONCE equivalent for UI warnings James Clark
@ 2021-08-02 14:44   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-08-02 14:44 UTC (permalink / raw)
  To: James Clark
  Cc: mathieu.poirier, coresight, linux-perf-users, leo.yan,
	suzuki.poulose, mike.leach, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, John Garry, Will Deacon, linux-kernel,
	linux-arm-kernel

Em Thu, Jul 29, 2021 at 04:58:00PM +0100, James Clark escreveu:
> Currently WARN_ONCE prints to stderr and corrupts the TUI. Add
> equivalent methods for UI warnings.

This one and the the next 3 seem clean and useful, applying and then
waiting for people to comment on the coresight specific ones.

- Arnaldo
 
> Signed-off-by: James Clark <james.clark@arm.com>
> ---
>  tools/perf/util/debug.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/tools/perf/util/debug.h b/tools/perf/util/debug.h
> index 48f631966067..f99468a7f681 100644
> --- a/tools/perf/util/debug.h
> +++ b/tools/perf/util/debug.h
> @@ -22,6 +22,13 @@ extern int debug_data_convert;
>  	eprintf(0, verbose, pr_fmt(fmt), ##__VA_ARGS__)
>  #define pr_warning(fmt, ...) \
>  	eprintf(0, verbose, pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_warning_once(fmt, ...) ({		\
> +	static int __warned;			\
> +	if (unlikely(!__warned)) {		\
> +		pr_warning(fmt, ##__VA_ARGS__); \
> +		__warned = 1;			\
> +	}					\
> +})
>  #define pr_info(fmt, ...) \
>  	eprintf(0, verbose, pr_fmt(fmt), ##__VA_ARGS__)
>  #define pr_debug(fmt, ...) \
> @@ -55,6 +62,13 @@ void trace_event(union perf_event *event);
>  
>  int ui__error(const char *format, ...) __printf(1, 2);
>  int ui__warning(const char *format, ...) __printf(1, 2);
> +#define ui__warning_once(format, ...) ({		\
> +	static int __warned;				\
> +	if (unlikely(!__warned)) {			\
> +		ui__warning(format, ##__VA_ARGS__);	\
> +		__warned = 1;				\
> +	}						\
> +})
>  
>  void pr_stat(const char *fmt, ...);
>  
> -- 
> 2.28.0
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 6/6] perf cs-etm: Add warnings for missing DSOs
  2021-07-29 15:58 ` [RFC PATCH 6/6] perf cs-etm: Add warnings for missing DSOs James Clark
@ 2021-08-02 14:51   ` Arnaldo Carvalho de Melo
  2021-08-02 17:03     ` Mathieu Poirier
  2021-08-02 15:41   ` Leo Yan
  1 sibling, 1 reply; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-08-02 14:51 UTC (permalink / raw)
  To: James Clark
  Cc: mathieu.poirier, coresight, linux-perf-users, leo.yan,
	suzuki.poulose, mike.leach, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, John Garry, Will Deacon, linux-kernel,
	linux-arm-kernel

Em Thu, Jul 29, 2021 at 04:58:05PM +0100, James Clark escreveu:
> Currently decode will silently fail if no binary data is available for
> the decode. This is made worse if only partial data is available because
> the decode will appear to work, but any trace from that missing DSO will
> silently not be generated.
> 
> Add a UI popup once if there is any data missing, and then warn in the
> bottom left for each individual DSO that's missing.

Looks ok to me (the last 3 patches in this series, the rest I applied
already), can I get some Acked-by/Reviewed-by from the CoreSight people?

Thanks,

- Arnaldo
 
> Signed-off-by: James Clark <james.clark@arm.com>
> ---
>  tools/perf/util/cs-etm.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 32ad92d3e454..e6851260d059 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -746,8 +746,16 @@ static u32 cs_etm__mem_access(struct cs_etm_queue *etmq, u8 trace_chan_id,
>  
>  	len = dso__data_read_offset(al.map->dso, machine, offset, buffer, size);
>  
> -	if (len <= 0)
> +	if (len <= 0) {
> +		ui__warning_once("CS ETM Trace: Missing DSO. Use 'perf archive' to export data from the traced system.\n");
> +		if (!al.map->dso->auxtrace_warned) {
> +			pr_err("CS ETM Trace: Debug data not found for address %#"PRIx64" in %s\n",
> +				    address,
> +				    al.map->dso->long_name ? al.map->dso->long_name : "Unknown");
> +			al.map->dso->auxtrace_warned = true;
> +		}
>  		return 0;
> +	}
>  
>  	return len;
>  }
> -- 
> 2.28.0
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 5/6] perf cs-etm: Improve Coresight zero timestamp warning
  2021-07-29 15:58 ` [RFC PATCH 5/6] perf cs-etm: Improve Coresight zero timestamp warning James Clark
@ 2021-08-02 15:17   ` Leo Yan
  2021-08-03 13:25     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 18+ messages in thread
From: Leo Yan @ 2021-08-02 15:17 UTC (permalink / raw)
  To: James Clark
  Cc: acme, mathieu.poirier, coresight, linux-perf-users,
	suzuki.poulose, mike.leach, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, John Garry, Will Deacon, linux-kernel,
	linux-arm-kernel

On Thu, Jul 29, 2021 at 04:58:04PM +0100, James Clark wrote:
> Only show the warning if the user hasn't already set timeless mode and
> improve the text because there was ambiguity around the meaning of '...'
> 
> Change the warning to a UI warning instead of printing straight to
> stderr because this corrupts the UI when perf report TUI is used. The UI
> warning function also handles printing to stderr when in perf script
> mode.
> 
> Suggested-by: Leo Yan <leo.yan@linaro.org>
> Signed-off-by: James Clark <james.clark@arm.com>

Reviewed-by: Leo Yan <leo.yan@linaro.org>

> ---
>  tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> index 3e1a05bc82cc..5084bd2ca6eb 100644
> --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> @@ -324,8 +324,11 @@ cs_etm_decoder__do_hard_timestamp(struct cs_etm_queue *etmq,
>  		 * underflow.
>  		 */
>  		packet_queue->cs_timestamp = 0;
> -		WARN_ONCE(true, "Zero Coresight timestamp found at Idx:%" OCSD_TRC_IDX_STR
> -				". Decoding may be improved with --itrace=Z...\n", indx);
> +		if (!cs_etm__etmq_is_timeless(etmq))
> +			pr_warning_once("Zero Coresight timestamp found at Idx:%" OCSD_TRC_IDX_STR
> +					". Decoding may be improved by prepending 'Z' to your current --itrace arguments.\n",
> +					indx);
> +
>  	} else if (packet_queue->instr_count > elem->timestamp) {
>  		/*
>  		 * Sanity check that the elem->timestamp - packet_queue->instr_count would not
> -- 
> 2.28.0
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 6/6] perf cs-etm: Add warnings for missing DSOs
  2021-07-29 15:58 ` [RFC PATCH 6/6] perf cs-etm: Add warnings for missing DSOs James Clark
  2021-08-02 14:51   ` Arnaldo Carvalho de Melo
@ 2021-08-02 15:41   ` Leo Yan
  2021-08-03 13:24     ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 18+ messages in thread
From: Leo Yan @ 2021-08-02 15:41 UTC (permalink / raw)
  To: James Clark
  Cc: acme, mathieu.poirier, coresight, linux-perf-users,
	suzuki.poulose, mike.leach, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, John Garry, Will Deacon, linux-kernel,
	linux-arm-kernel

On Thu, Jul 29, 2021 at 04:58:05PM +0100, James Clark wrote:
> Currently decode will silently fail if no binary data is available for
> the decode. This is made worse if only partial data is available because
> the decode will appear to work, but any trace from that missing DSO will
> silently not be generated.
> 
> Add a UI popup once if there is any data missing, and then warn in the
> bottom left for each individual DSO that's missing.
> 
> Signed-off-by: James Clark <james.clark@arm.com>
> ---
>  tools/perf/util/cs-etm.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 32ad92d3e454..e6851260d059 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -746,8 +746,16 @@ static u32 cs_etm__mem_access(struct cs_etm_queue *etmq, u8 trace_chan_id,
>  
>  	len = dso__data_read_offset(al.map->dso, machine, offset, buffer, size);
>  
> -	if (len <= 0)
> +	if (len <= 0) {
> +		ui__warning_once("CS ETM Trace: Missing DSO. Use 'perf archive' to export data from the traced system.\n");
> +		if (!al.map->dso->auxtrace_warned) {
> +			pr_err("CS ETM Trace: Debug data not found for address %#"PRIx64" in %s\n",
> +				    address,
> +				    al.map->dso->long_name ? al.map->dso->long_name : "Unknown");
> +			al.map->dso->auxtrace_warned = true;
> +		}

This is very useful.

Just one comment: in particularly if the perf fails to find the kernel
symbols, the user needs to enable config "CONFIG_PROC_KCORE=y" or
specify option "-k /path/to/vmlinux".  In this case, using 'perf
archive' is not helpful.  So I think the UI warning can be imporved
like:

        ui__warning_once("CS ETM Trace: Missing DSO. Use 'perf archive' to export data from the traced system.\n"
                         " Enable CONFIG_PROC_KCORE or use option '-k /path/to/vmlinux' for kernel symbols\n");

With this improvement, the patch looks good to me:

Reviewed-by: Leo Yan <leo.yan@linaro.org>

>  		return 0;
> +	}
>  
>  	return len;
>  }
> -- 
> 2.28.0
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 6/6] perf cs-etm: Add warnings for missing DSOs
  2021-08-02 14:51   ` Arnaldo Carvalho de Melo
@ 2021-08-02 17:03     ` Mathieu Poirier
  2021-08-03  8:06       ` Mike Leach
  0 siblings, 1 reply; 18+ messages in thread
From: Mathieu Poirier @ 2021-08-02 17:03 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: James Clark, coresight, linux-perf-users, leo.yan,
	suzuki.poulose, mike.leach, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, John Garry, Will Deacon, linux-kernel,
	linux-arm-kernel

On Mon, Aug 02, 2021 at 11:51:58AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jul 29, 2021 at 04:58:05PM +0100, James Clark escreveu:
> > Currently decode will silently fail if no binary data is available for
> > the decode. This is made worse if only partial data is available because
> > the decode will appear to work, but any trace from that missing DSO will
> > silently not be generated.
> > 
> > Add a UI popup once if there is any data missing, and then warn in the
> > bottom left for each individual DSO that's missing.
> 
> Looks ok to me (the last 3 patches in this series, the rest I applied
> already), can I get some Acked-by/Reviewed-by from the CoreSight people?
>

I have a substantial backlog of patches to go through and I will be away for the
next two weeks.  As such it will be some time before I get to review this set.

Regards,
Mathieu

> Thanks,
> 
> - Arnaldo
>  
> > Signed-off-by: James Clark <james.clark@arm.com>
> > ---
> >  tools/perf/util/cs-etm.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > index 32ad92d3e454..e6851260d059 100644
> > --- a/tools/perf/util/cs-etm.c
> > +++ b/tools/perf/util/cs-etm.c
> > @@ -746,8 +746,16 @@ static u32 cs_etm__mem_access(struct cs_etm_queue *etmq, u8 trace_chan_id,
> >  
> >  	len = dso__data_read_offset(al.map->dso, machine, offset, buffer, size);
> >  
> > -	if (len <= 0)
> > +	if (len <= 0) {
> > +		ui__warning_once("CS ETM Trace: Missing DSO. Use 'perf archive' to export data from the traced system.\n");
> > +		if (!al.map->dso->auxtrace_warned) {
> > +			pr_err("CS ETM Trace: Debug data not found for address %#"PRIx64" in %s\n",
> > +				    address,
> > +				    al.map->dso->long_name ? al.map->dso->long_name : "Unknown");
> > +			al.map->dso->auxtrace_warned = true;
> > +		}
> >  		return 0;
> > +	}
> >  
> >  	return len;
> >  }
> > -- 
> > 2.28.0
> > 
> 
> -- 
> 
> - Arnaldo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 6/6] perf cs-etm: Add warnings for missing DSOs
  2021-08-02 17:03     ` Mathieu Poirier
@ 2021-08-03  8:06       ` Mike Leach
  0 siblings, 0 replies; 18+ messages in thread
From: Mike Leach @ 2021-08-03  8:06 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: Arnaldo Carvalho de Melo, James Clark, Coresight ML,
	linux-perf-users, Leo Yan, Suzuki K. Poulose, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, John Garry,
	Will Deacon, Linux Kernel Mailing List, linux-arm-kernel

HI James,

On Mon, 2 Aug 2021 at 18:03, Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
>
> On Mon, Aug 02, 2021 at 11:51:58AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Jul 29, 2021 at 04:58:05PM +0100, James Clark escreveu:
> > > Currently decode will silently fail if no binary data is available for
> > > the decode. This is made worse if only partial data is available because
> > > the decode will appear to work, but any trace from that missing DSO will
> > > silently not be generated.
> > >

The decoder actually outputs a OCSD_GEN_TRC_ELEM_ADDR_NACC packet if
binary memory data is missing.
These packets are currently ignored by perf / cs-etm-decoder.c.

I think this per DSO warning is fine, but perhaps at some point add in
handling for OCSD_GEN_TRC_ELEM_ADDR_NACC - which perhaps is only
active when dumping trace packets.

Regards

Mike


> > > Add a UI popup once if there is any data missing, and then warn in the
> > > bottom left for each individual DSO that's missing.
> >
> > Looks ok to me (the last 3 patches in this series, the rest I applied
> > already), can I get some Acked-by/Reviewed-by from the CoreSight people?
> >
>
> I have a substantial backlog of patches to go through and I will be away for the
> next two weeks.  As such it will be some time before I get to review this set.
>
> Regards,
> Mathieu
>
> > Thanks,
> >
> > - Arnaldo
> >
> > > Signed-off-by: James Clark <james.clark@arm.com>
> > > ---
> > >  tools/perf/util/cs-etm.c | 10 +++++++++-
> > >  1 file changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > > index 32ad92d3e454..e6851260d059 100644
> > > --- a/tools/perf/util/cs-etm.c
> > > +++ b/tools/perf/util/cs-etm.c
> > > @@ -746,8 +746,16 @@ static u32 cs_etm__mem_access(struct cs_etm_queue *etmq, u8 trace_chan_id,
> > >
> > >     len = dso__data_read_offset(al.map->dso, machine, offset, buffer, size);
> > >
> > > -   if (len <= 0)
> > > +   if (len <= 0) {
> > > +           ui__warning_once("CS ETM Trace: Missing DSO. Use 'perf archive' to export data from the traced system.\n");
> > > +           if (!al.map->dso->auxtrace_warned) {
> > > +                   pr_err("CS ETM Trace: Debug data not found for address %#"PRIx64" in %s\n",
> > > +                               address,
> > > +                               al.map->dso->long_name ? al.map->dso->long_name : "Unknown");
> > > +                   al.map->dso->auxtrace_warned = true;
> > > +           }
> > >             return 0;
> > > +   }
> > >
> > >     return len;
> > >  }
> > > --
> > > 2.28.0
> > >
> >
> > --
> >
> > - Arnaldo



-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 6/6] perf cs-etm: Add warnings for missing DSOs
  2021-08-02 15:41   ` Leo Yan
@ 2021-08-03 13:24     ` Arnaldo Carvalho de Melo
  2021-08-03 14:01       ` Leo Yan
  0 siblings, 1 reply; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-08-03 13:24 UTC (permalink / raw)
  To: Leo Yan
  Cc: James Clark, mathieu.poirier, coresight, linux-perf-users,
	suzuki.poulose, mike.leach, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, John Garry, Will Deacon, linux-kernel,
	linux-arm-kernel

Em Mon, Aug 02, 2021 at 11:41:45PM +0800, Leo Yan escreveu:
> On Thu, Jul 29, 2021 at 04:58:05PM +0100, James Clark wrote:
> > Currently decode will silently fail if no binary data is available for
> > the decode. This is made worse if only partial data is available because
> > the decode will appear to work, but any trace from that missing DSO will
> > silently not be generated.
> > 
> > Add a UI popup once if there is any data missing, and then warn in the
> > bottom left for each individual DSO that's missing.
> > 
> > Signed-off-by: James Clark <james.clark@arm.com>
> > ---
> >  tools/perf/util/cs-etm.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > index 32ad92d3e454..e6851260d059 100644
> > --- a/tools/perf/util/cs-etm.c
> > +++ b/tools/perf/util/cs-etm.c
> > @@ -746,8 +746,16 @@ static u32 cs_etm__mem_access(struct cs_etm_queue *etmq, u8 trace_chan_id,
> >  
> >  	len = dso__data_read_offset(al.map->dso, machine, offset, buffer, size);
> >  
> > -	if (len <= 0)
> > +	if (len <= 0) {
> > +		ui__warning_once("CS ETM Trace: Missing DSO. Use 'perf archive' to export data from the traced system.\n");
> > +		if (!al.map->dso->auxtrace_warned) {
> > +			pr_err("CS ETM Trace: Debug data not found for address %#"PRIx64" in %s\n",
> > +				    address,
> > +				    al.map->dso->long_name ? al.map->dso->long_name : "Unknown");
> > +			al.map->dso->auxtrace_warned = true;
> > +		}
> 
> This is very useful.
> 
> Just one comment: in particularly if the perf fails to find the kernel
> symbols, the user needs to enable config "CONFIG_PROC_KCORE=y" or
> specify option "-k /path/to/vmlinux".  In this case, using 'perf
> archive' is not helpful.  So I think the UI warning can be imporved
> like:
> 
>         ui__warning_once("CS ETM Trace: Missing DSO. Use 'perf archive' to export data from the traced system.\n"
>                          " Enable CONFIG_PROC_KCORE or use option '-k /path/to/vmlinux' for kernel symbols\n");

one can also use debuginfod-client, which, as time passes, probably will
be the main way of finding DSOs now that we have build-ids in
PERF_RECORD_MMAP2 and debuginfod servers such as:

export DEBUGINFOD_URLS=https://debuginfod.fedoraproject.org/

https://fedoraproject.org/wiki/Debuginfod
 
> With this improvement, the patch looks good to me:
> 
> Reviewed-by: Leo Yan <leo.yan@linaro.org>

Does this apply to the other 5 patches in this series?

- Arnaldo
 
> >  		return 0;
> > +	}
> >  
> >  	return len;
> >  }
> > -- 
> > 2.28.0
> > 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 5/6] perf cs-etm: Improve Coresight zero timestamp warning
  2021-08-02 15:17   ` Leo Yan
@ 2021-08-03 13:25     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-08-03 13:25 UTC (permalink / raw)
  To: Leo Yan
  Cc: James Clark, mathieu.poirier, coresight, linux-perf-users,
	suzuki.poulose, mike.leach, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, John Garry, Will Deacon, linux-kernel,
	linux-arm-kernel

Em Mon, Aug 02, 2021 at 11:17:10PM +0800, Leo Yan escreveu:
> On Thu, Jul 29, 2021 at 04:58:04PM +0100, James Clark wrote:
> > Only show the warning if the user hasn't already set timeless mode and
> > improve the text because there was ambiguity around the meaning of '...'
> > 
> > Change the warning to a UI warning instead of printing straight to
> > stderr because this corrupts the UI when perf report TUI is used. The UI
> > warning function also handles printing to stderr when in perf script
> > mode.
> > 
> > Suggested-by: Leo Yan <leo.yan@linaro.org>
> > Signed-off-by: James Clark <james.clark@arm.com>
> 
> Reviewed-by: Leo Yan <leo.yan@linaro.org>

Thanks, applied.

- Arnaldo

 
> > ---
> >  tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> > index 3e1a05bc82cc..5084bd2ca6eb 100644
> > --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> > +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c
> > @@ -324,8 +324,11 @@ cs_etm_decoder__do_hard_timestamp(struct cs_etm_queue *etmq,
> >  		 * underflow.
> >  		 */
> >  		packet_queue->cs_timestamp = 0;
> > -		WARN_ONCE(true, "Zero Coresight timestamp found at Idx:%" OCSD_TRC_IDX_STR
> > -				". Decoding may be improved with --itrace=Z...\n", indx);
> > +		if (!cs_etm__etmq_is_timeless(etmq))
> > +			pr_warning_once("Zero Coresight timestamp found at Idx:%" OCSD_TRC_IDX_STR
> > +					". Decoding may be improved by prepending 'Z' to your current --itrace arguments.\n",
> > +					indx);
> > +
> >  	} else if (packet_queue->instr_count > elem->timestamp) {
> >  		/*
> >  		 * Sanity check that the elem->timestamp - packet_queue->instr_count would not
> > -- 
> > 2.28.0
> > 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 6/6] perf cs-etm: Add warnings for missing DSOs
  2021-08-03 13:24     ` Arnaldo Carvalho de Melo
@ 2021-08-03 14:01       ` Leo Yan
  2021-08-05 12:59         ` James Clark
  0 siblings, 1 reply; 18+ messages in thread
From: Leo Yan @ 2021-08-03 14:01 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: James Clark, mathieu.poirier, coresight, linux-perf-users,
	suzuki.poulose, mike.leach, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, John Garry, Will Deacon, linux-kernel,
	linux-arm-kernel

On Tue, Aug 03, 2021 at 10:24:09AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Aug 02, 2021 at 11:41:45PM +0800, Leo Yan escreveu:
> > On Thu, Jul 29, 2021 at 04:58:05PM +0100, James Clark wrote:
> > > Currently decode will silently fail if no binary data is available for
> > > the decode. This is made worse if only partial data is available because
> > > the decode will appear to work, but any trace from that missing DSO will
> > > silently not be generated.
> > > 
> > > Add a UI popup once if there is any data missing, and then warn in the
> > > bottom left for each individual DSO that's missing.
> > > 
> > > Signed-off-by: James Clark <james.clark@arm.com>
> > > ---
> > >  tools/perf/util/cs-etm.c | 10 +++++++++-
> > >  1 file changed, 9 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > > index 32ad92d3e454..e6851260d059 100644
> > > --- a/tools/perf/util/cs-etm.c
> > > +++ b/tools/perf/util/cs-etm.c
> > > @@ -746,8 +746,16 @@ static u32 cs_etm__mem_access(struct cs_etm_queue *etmq, u8 trace_chan_id,
> > >  
> > >  	len = dso__data_read_offset(al.map->dso, machine, offset, buffer, size);
> > >  
> > > -	if (len <= 0)
> > > +	if (len <= 0) {
> > > +		ui__warning_once("CS ETM Trace: Missing DSO. Use 'perf archive' to export data from the traced system.\n");
> > > +		if (!al.map->dso->auxtrace_warned) {
> > > +			pr_err("CS ETM Trace: Debug data not found for address %#"PRIx64" in %s\n",
> > > +				    address,
> > > +				    al.map->dso->long_name ? al.map->dso->long_name : "Unknown");
> > > +			al.map->dso->auxtrace_warned = true;
> > > +		}
> > 
> > This is very useful.
> > 
> > Just one comment: in particularly if the perf fails to find the kernel
> > symbols, the user needs to enable config "CONFIG_PROC_KCORE=y" or
> > specify option "-k /path/to/vmlinux".  In this case, using 'perf
> > archive' is not helpful.  So I think the UI warning can be imporved
> > like:
> > 
> >         ui__warning_once("CS ETM Trace: Missing DSO. Use 'perf archive' to export data from the traced system.\n"
> >                          " Enable CONFIG_PROC_KCORE or use option '-k /path/to/vmlinux' for kernel symbols\n");
> 
> one can also use debuginfod-client, which, as time passes, probably will
> be the main way of finding DSOs now that we have build-ids in
> PERF_RECORD_MMAP2 and debuginfod servers such as:
> 
> export DEBUGINFOD_URLS=https://debuginfod.fedoraproject.org/

If so, maybe should use more general description for missing DSO.

> https://fedoraproject.org/wiki/Debuginfod
>  
> > With this improvement, the patch looks good to me:
> > 
> > Reviewed-by: Leo Yan <leo.yan@linaro.org>
> 
> Does this apply to the other 5 patches in this series?

I finished to reivew patches 01, 04, 05, 06/06, so my review tag can
apply on these patches.  Current patch 06/06 needs James to improve for
the comments.

Thanks,
Leo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 6/6] perf cs-etm: Add warnings for missing DSOs
  2021-08-03 14:01       ` Leo Yan
@ 2021-08-05 12:59         ` James Clark
  2021-08-06 18:36           ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 18+ messages in thread
From: James Clark @ 2021-08-05 12:59 UTC (permalink / raw)
  To: Leo Yan, Arnaldo Carvalho de Melo
  Cc: mathieu.poirier, coresight, linux-perf-users, suzuki.poulose,
	mike.leach, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, John Garry, Will Deacon, linux-kernel,
	linux-arm-kernel



On 03/08/2021 15:01, Leo Yan wrote:
>> one can also use debuginfod-client, which, as time passes, probably will
>> be the main way of finding DSOs now that we have build-ids in
>> PERF_RECORD_MMAP2 and debuginfod servers such as:
>>
>> export DEBUGINFOD_URLS=https://debuginfod.fedoraproject.org/
> If so, maybe should use more general description for missing DSO.
> 
>> https://fedoraproject.org/wiki/Debuginfod
>>  
>>> With this improvement, the patch looks good to me:
>>>
>>> Reviewed-by: Leo Yan <leo.yan@linaro.org>
>> Does this apply to the other 5 patches in this series?
> I finished to reivew patches 01, 04, 05, 06/06, so my review tag can
> apply on these patches.  Current patch 06/06 needs James to improve for
> the comments.

Thanks for the reviews, I've submitted v2.

I also had a play around with debuginfod, it looks promising.
Especially if it's integrated with perf report, which I assume is the plan?

Thanks
James

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 6/6] perf cs-etm: Add warnings for missing DSOs
  2021-08-05 12:59         ` James Clark
@ 2021-08-06 18:36           ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-08-06 18:36 UTC (permalink / raw)
  To: James Clark
  Cc: Leo Yan, mathieu.poirier, coresight, linux-perf-users,
	suzuki.poulose, mike.leach, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, John Garry, Will Deacon, linux-kernel,
	linux-arm-kernel

Em Thu, Aug 05, 2021 at 01:59:49PM +0100, James Clark escreveu:
> 
> 
> On 03/08/2021 15:01, Leo Yan wrote:
> >> one can also use debuginfod-client, which, as time passes, probably will
> >> be the main way of finding DSOs now that we have build-ids in
> >> PERF_RECORD_MMAP2 and debuginfod servers such as:
> >>
> >> export DEBUGINFOD_URLS=https://debuginfod.fedoraproject.org/
> > If so, maybe should use more general description for missing DSO.
> > 
> >> https://fedoraproject.org/wiki/Debuginfod
> >>  
> >>> With this improvement, the patch looks good to me:
> >>>
> >>> Reviewed-by: Leo Yan <leo.yan@linaro.org>
> >> Does this apply to the other 5 patches in this series?
> > I finished to reivew patches 01, 04, 05, 06/06, so my review tag can
> > apply on these patches.  Current patch 06/06 needs James to improve for
> > the comments.
> 
> Thanks for the reviews, I've submitted v2.
> 
> I also had a play around with debuginfod, it looks promising.
> Especially if it's integrated with perf report, which I assume is the plan?

Yeah, code is needed for that in the underlying symbols library so that
we can tune its usage, i.e. the threshold of samples to trigger asking
for the auto-download of debuginfo files.

- Arnaldo

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-08-06 18:36 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-29 15:57 [RFC PATCH 0/6] perf tools: Warning fixes James Clark
2021-07-29 15:58 ` [RFC PATCH 1/6] perf tools: Add WARN_ONCE equivalent for UI warnings James Clark
2021-08-02 14:44   ` Arnaldo Carvalho de Melo
2021-07-29 15:58 ` [RFC PATCH 2/6] perf tools: Re-add annotate_warned functionality James Clark
2021-07-29 15:58 ` [RFC PATCH 3/6] perf tools: Add disassembly warnings for annotate --stdio James Clark
2021-07-29 15:58 ` [RFC PATCH 4/6] perf tools: Add flag for tracking warnings of missing DSOs James Clark
2021-07-29 15:58 ` [RFC PATCH 5/6] perf cs-etm: Improve Coresight zero timestamp warning James Clark
2021-08-02 15:17   ` Leo Yan
2021-08-03 13:25     ` Arnaldo Carvalho de Melo
2021-07-29 15:58 ` [RFC PATCH 6/6] perf cs-etm: Add warnings for missing DSOs James Clark
2021-08-02 14:51   ` Arnaldo Carvalho de Melo
2021-08-02 17:03     ` Mathieu Poirier
2021-08-03  8:06       ` Mike Leach
2021-08-02 15:41   ` Leo Yan
2021-08-03 13:24     ` Arnaldo Carvalho de Melo
2021-08-03 14:01       ` Leo Yan
2021-08-05 12:59         ` James Clark
2021-08-06 18:36           ` Arnaldo Carvalho de Melo

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).