linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] perf record: encode -k clockid frequency into Perf trace
@ 2018-10-03 16:57 Alexey Budankov
  2018-10-04 12:36 ` Jiri Olsa
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Alexey Budankov @ 2018-10-03 16:57 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: Alexander Shishkin, Jiri Olsa, Namhyung Kim, Andi Kleen, linux-kernel


Store -k clockid frequency into Perf trace to enable timestamps 
derived metrics conversion into wall clock time on reporting stage.

Below is the example of perf report output:

tools/perf/perf record -k raw -- ../../matrix/linux/matrix.gcc
...
[ perf record: Captured and wrote 31.222 MB perf.data (818054 samples) ]

tools/perf/perf report --header
# ========
...
# event : name = cycles:ppp, , size = 112, { sample_period, sample_freq } = 4000, sample_type = IP|TID|TIME|PERIOD, disabled = 1, inherit = 1, mmap = 1, comm = 1, freq = 1, enable_on_exec = 1, task = 1, precise_ip = 3, sample_id_all = 1, exclude_guest = 1, mmap2 = 1, comm_exec = 1, use_clockid = 1, clockid = 4
...
# clockid frequency: 1000 MHz
...
# ========

Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
Changes in v3:
 - moved header's clockid_res_ns initialization out of record__init_features()
 - added explicit warning for case of failed clock_getres() call
Changes in v2:
 - renamed clockid_freq to clockid_res_ns and get_clockid_freq() to get_clockid_res()
 - avoided redundant define of NSEC_IN_SEC, reused linux/time64.h:NSEC_PER_SEC
 - moved MHz conversion into print_clockid() and shortened write_clockid()
---
 tools/perf/builtin-record.c | 24 ++++++++++++++++++++++--
 tools/perf/perf.h           |  1 +
 tools/perf/util/env.h       |  1 +
 tools/perf/util/header.c    | 27 +++++++++++++++++++++++++++
 tools/perf/util/header.h    |  1 +
 5 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 0980dfe3396b..d803d3264465 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -592,6 +592,9 @@ static void record__init_features(struct record *rec)
 	if (!rec->opts.full_auxtrace)
 		perf_header__clear_feat(&session->header, HEADER_AUXTRACE);
 
+	if (!(rec->opts.use_clockid && rec->opts.clockid_res_ns))
+		perf_header__clear_feat(&session->header, HEADER_CLOCKID);
+
 	perf_header__clear_feat(&session->header, HEADER_STAT);
 }
 
@@ -897,6 +900,9 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 
 	record__init_features(rec);
 
+	if (rec->opts.use_clockid && rec->opts.clockid_res_ns)
+		session->header.env.clockid_res_ns = rec->opts.clockid_res_ns;
+
 	if (forks) {
 		err = perf_evlist__prepare_workload(rec->evlist, &opts->target,
 						    argv, data->is_pipe,
@@ -1337,6 +1343,19 @@ static const struct clockid_map clockids[] = {
 	CLOCKID_END,
 };
 
+static int get_clockid_res(clockid_t clk_id, size_t *res_ns)
+{
+	struct timespec res;
+
+	*res_ns = 0;
+	if (!clock_getres(clk_id, &res))
+		*res_ns = res.tv_nsec + res.tv_sec * NSEC_PER_SEC;
+	else
+		pr_warning("WARNING: Failed to determine specified clock resolution.\n");
+
+	return 0;
+}
+
 static int parse_clockid(const struct option *opt, const char *str, int unset)
 {
 	struct record_opts *opts = (struct record_opts *)opt->value;
@@ -1360,7 +1379,7 @@ static int parse_clockid(const struct option *opt, const char *str, int unset)
 
 	/* if its a number, we're done */
 	if (sscanf(str, "%d", &opts->clockid) == 1)
-		return 0;
+		return get_clockid_res(opts->clockid, &opts->clockid_res_ns);
 
 	/* allow a "CLOCK_" prefix to the name */
 	if (!strncasecmp(str, "CLOCK_", 6))
@@ -1369,7 +1388,8 @@ static int parse_clockid(const struct option *opt, const char *str, int unset)
 	for (cm = clockids; cm->name; cm++) {
 		if (!strcasecmp(str, cm->name)) {
 			opts->clockid = cm->clockid;
-			return 0;
+			return get_clockid_res(opts->clockid,
+					       &opts->clockid_res_ns);
 		}
 	}
 
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 21bf7f5a3cf5..981db3c2ed60 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -81,6 +81,7 @@ struct record_opts {
 	unsigned     initial_delay;
 	bool         use_clockid;
 	clockid_t    clockid;
+	size_t       clockid_res_ns;
 	unsigned int proc_map_timeout;
 };
 
diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
index 1f3ccc368530..b167a54d635f 100644
--- a/tools/perf/util/env.h
+++ b/tools/perf/util/env.h
@@ -63,6 +63,7 @@ struct perf_env {
 	struct numa_node	*numa_nodes;
 	struct memory_node	*memory_nodes;
 	unsigned long long	 memory_bsize;
+	size_t                   clockid_res_ns;
 };
 
 extern struct perf_env perf_env;
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 1ec1d9bc2d63..4ce5339158f7 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1034,6 +1034,13 @@ static int write_auxtrace(struct feat_fd *ff,
 	return err;
 }
 
+static int write_clockid(struct feat_fd *ff,
+			 struct perf_evlist *evlist __maybe_unused)
+{
+	return do_write(ff, &ff->ph->env.clockid_res_ns,
+			sizeof(ff->ph->env.clockid_res_ns));
+}
+
 static int cpu_cache_level__sort(const void *a, const void *b)
 {
 	struct cpu_cache_level *cache_a = (struct cpu_cache_level *)a;
@@ -1508,6 +1515,12 @@ static void print_cpu_topology(struct feat_fd *ff, FILE *fp)
 		fprintf(fp, "# Core ID and Socket ID information is not available\n");
 }
 
+static void print_clockid(struct feat_fd *ff, FILE *fp)
+{
+	fprintf(fp, "# clockid frequency: %ld MHz\n",
+		ff->ph->env.clockid_res_ns * 1000);
+}
+
 static void free_event_desc(struct perf_evsel *events)
 {
 	struct perf_evsel *evsel;
@@ -2531,6 +2544,19 @@ static int process_mem_topology(struct feat_fd *ff,
 	return ret;
 }
 
+static int process_clockid(struct feat_fd *ff,
+			   void *data __maybe_unused)
+{
+	size_t clockid_res_ns;
+
+	if (do_read_u64(ff, &clockid_res_ns))
+		return -1;
+
+	ff->ph->env.clockid_res_ns = clockid_res_ns;
+
+	return 0;
+}
+
 struct feature_ops {
 	int (*write)(struct feat_fd *ff, struct perf_evlist *evlist);
 	void (*print)(struct feat_fd *ff, FILE *fp);
@@ -2590,6 +2616,7 @@ static const struct feature_ops feat_ops[HEADER_LAST_FEATURE] = {
 	FEAT_OPN(CACHE,		cache,		true),
 	FEAT_OPR(SAMPLE_TIME,	sample_time,	false),
 	FEAT_OPR(MEM_TOPOLOGY,	mem_topology,	true),
+	FEAT_OPR(CLOCKID,       clockid,        false)
 };
 
 struct header_print_data {
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index e17903caa71d..0d553ddca0a3 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -38,6 +38,7 @@ enum {
 	HEADER_CACHE,
 	HEADER_SAMPLE_TIME,
 	HEADER_MEM_TOPOLOGY,
+	HEADER_CLOCKID,
 	HEADER_LAST_FEATURE,
 	HEADER_FEAT_BITS	= 256,
 };

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

* Re: [PATCH v3] perf record: encode -k clockid frequency into Perf trace
  2018-10-03 16:57 [PATCH v3] perf record: encode -k clockid frequency into Perf trace Alexey Budankov
@ 2018-10-04 12:36 ` Jiri Olsa
  2018-10-06 11:51   ` Arnaldo Carvalho de Melo
  2018-10-04 15:27 ` Arnaldo Carvalho de Melo
  2018-10-08 17:42 ` Arnaldo Carvalho de Melo
  2 siblings, 1 reply; 9+ messages in thread
From: Jiri Olsa @ 2018-10-04 12:36 UTC (permalink / raw)
  To: Alexey Budankov
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Namhyung Kim, Andi Kleen, linux-kernel

On Wed, Oct 03, 2018 at 07:57:12PM +0300, Alexey Budankov wrote:
> 
> Store -k clockid frequency into Perf trace to enable timestamps 
> derived metrics conversion into wall clock time on reporting stage.
> 
> Below is the example of perf report output:
> 
> tools/perf/perf record -k raw -- ../../matrix/linux/matrix.gcc
> ...
> [ perf record: Captured and wrote 31.222 MB perf.data (818054 samples) ]
> 
> tools/perf/perf report --header
> # ========
> ...
> # event : name = cycles:ppp, , size = 112, { sample_period, sample_freq } = 4000, sample_type = IP|TID|TIME|PERIOD, disabled = 1, inherit = 1, mmap = 1, comm = 1, freq = 1, enable_on_exec = 1, task = 1, precise_ip = 3, sample_id_all = 1, exclude_guest = 1, mmap2 = 1, comm_exec = 1, use_clockid = 1, clockid = 4
> ...
> # clockid frequency: 1000 MHz
> ...
> # ========
> 
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>

Reviewed-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

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

* Re: [PATCH v3] perf record: encode -k clockid frequency into Perf trace
  2018-10-03 16:57 [PATCH v3] perf record: encode -k clockid frequency into Perf trace Alexey Budankov
  2018-10-04 12:36 ` Jiri Olsa
@ 2018-10-04 15:27 ` Arnaldo Carvalho de Melo
  2018-10-04 16:13   ` Alexey Budankov
  2018-10-08 17:42 ` Arnaldo Carvalho de Melo
  2 siblings, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-04 15:27 UTC (permalink / raw)
  To: Alexey Budankov
  Cc: Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Andi Kleen, linux-kernel

Em Wed, Oct 03, 2018 at 07:57:12PM +0300, Alexey Budankov escreveu:
> 
> Store -k clockid frequency into Perf trace to enable timestamps 
> derived metrics conversion into wall clock time on reporting stage.

Humm, looking at where this is documented, we already have this in the
tools/perf/Documentation/perf-record.txt file, but it doesn't specify
how to select the option, just lists the CONFIG_MONOTONIC_FOO clocks,
leaving the user to figure that they have to use "-k raw" :-\

This is not a problem introduced by this patch, which works as
advertised:

  [acme@jouet perf]$ perf record -k raw sleep 0.1
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.001 MB perf.data (8 samples) ]
  [acme@jouet perf]$ perf evlist -v
  cycles:uppp: size: 112, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|PERIOD, disabled: 1, inherit: 1, exclude_kernel: 1, mmap: 1, comm: 1, freq: 1, enable_on_exec: 1, task: 1, precise_ip: 3, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1, use_clockid: 1, clockid: 4
  [acme@jouet perf]$ 
  
Also that "clockid: 4" would be best as "clockid: raw" :-\

> Below is the example of perf report output:
> 
> tools/perf/perf record -k raw -- ../../matrix/linux/matrix.gcc
> ...
> [ perf record: Captured and wrote 31.222 MB perf.data (818054 samples) ]
> 
> tools/perf/perf report --header
> # ========
> ...
> # event : name = cycles:ppp, , size = 112, { sample_period, sample_freq } = 4000, sample_type = IP|TID|TIME|PERIOD, disabled = 1, inherit = 1, mmap = 1, comm = 1, freq = 1, enable_on_exec = 1, task = 1, precise_ip = 3, sample_id_all = 1, exclude_guest = 1, mmap2 = 1, comm_exec = 1, use_clockid = 1, clockid = 4
> ...
> # clockid frequency: 1000 MHz
> ...
> # ========

Please add two spaces before the output of programs, specially those
that start with a #, otherwise when applying they will vanish.

Fixed that up, applied!

- Arnaldo

> 
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> ---
> Changes in v3:
>  - moved header's clockid_res_ns initialization out of record__init_features()
>  - added explicit warning for case of failed clock_getres() call
> Changes in v2:
>  - renamed clockid_freq to clockid_res_ns and get_clockid_freq() to get_clockid_res()
>  - avoided redundant define of NSEC_IN_SEC, reused linux/time64.h:NSEC_PER_SEC
>  - moved MHz conversion into print_clockid() and shortened write_clockid()
> ---
>  tools/perf/builtin-record.c | 24 ++++++++++++++++++++++--
>  tools/perf/perf.h           |  1 +
>  tools/perf/util/env.h       |  1 +
>  tools/perf/util/header.c    | 27 +++++++++++++++++++++++++++
>  tools/perf/util/header.h    |  1 +
>  5 files changed, 52 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 0980dfe3396b..d803d3264465 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -592,6 +592,9 @@ static void record__init_features(struct record *rec)
>  	if (!rec->opts.full_auxtrace)
>  		perf_header__clear_feat(&session->header, HEADER_AUXTRACE);
>  
> +	if (!(rec->opts.use_clockid && rec->opts.clockid_res_ns))
> +		perf_header__clear_feat(&session->header, HEADER_CLOCKID);
> +
>  	perf_header__clear_feat(&session->header, HEADER_STAT);
>  }
>  
> @@ -897,6 +900,9 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>  
>  	record__init_features(rec);
>  
> +	if (rec->opts.use_clockid && rec->opts.clockid_res_ns)
> +		session->header.env.clockid_res_ns = rec->opts.clockid_res_ns;
> +
>  	if (forks) {
>  		err = perf_evlist__prepare_workload(rec->evlist, &opts->target,
>  						    argv, data->is_pipe,
> @@ -1337,6 +1343,19 @@ static const struct clockid_map clockids[] = {
>  	CLOCKID_END,
>  };
>  
> +static int get_clockid_res(clockid_t clk_id, size_t *res_ns)
> +{
> +	struct timespec res;
> +
> +	*res_ns = 0;
> +	if (!clock_getres(clk_id, &res))
> +		*res_ns = res.tv_nsec + res.tv_sec * NSEC_PER_SEC;
> +	else
> +		pr_warning("WARNING: Failed to determine specified clock resolution.\n");
> +
> +	return 0;
> +}
> +
>  static int parse_clockid(const struct option *opt, const char *str, int unset)
>  {
>  	struct record_opts *opts = (struct record_opts *)opt->value;
> @@ -1360,7 +1379,7 @@ static int parse_clockid(const struct option *opt, const char *str, int unset)
>  
>  	/* if its a number, we're done */
>  	if (sscanf(str, "%d", &opts->clockid) == 1)
> -		return 0;
> +		return get_clockid_res(opts->clockid, &opts->clockid_res_ns);
>  
>  	/* allow a "CLOCK_" prefix to the name */
>  	if (!strncasecmp(str, "CLOCK_", 6))
> @@ -1369,7 +1388,8 @@ static int parse_clockid(const struct option *opt, const char *str, int unset)
>  	for (cm = clockids; cm->name; cm++) {
>  		if (!strcasecmp(str, cm->name)) {
>  			opts->clockid = cm->clockid;
> -			return 0;
> +			return get_clockid_res(opts->clockid,
> +					       &opts->clockid_res_ns);
>  		}
>  	}
>  
> diff --git a/tools/perf/perf.h b/tools/perf/perf.h
> index 21bf7f5a3cf5..981db3c2ed60 100644
> --- a/tools/perf/perf.h
> +++ b/tools/perf/perf.h
> @@ -81,6 +81,7 @@ struct record_opts {
>  	unsigned     initial_delay;
>  	bool         use_clockid;
>  	clockid_t    clockid;
> +	size_t       clockid_res_ns;
>  	unsigned int proc_map_timeout;
>  };
>  
> diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
> index 1f3ccc368530..b167a54d635f 100644
> --- a/tools/perf/util/env.h
> +++ b/tools/perf/util/env.h
> @@ -63,6 +63,7 @@ struct perf_env {
>  	struct numa_node	*numa_nodes;
>  	struct memory_node	*memory_nodes;
>  	unsigned long long	 memory_bsize;
> +	size_t                   clockid_res_ns;
>  };
>  
>  extern struct perf_env perf_env;
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 1ec1d9bc2d63..4ce5339158f7 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -1034,6 +1034,13 @@ static int write_auxtrace(struct feat_fd *ff,
>  	return err;
>  }
>  
> +static int write_clockid(struct feat_fd *ff,
> +			 struct perf_evlist *evlist __maybe_unused)
> +{
> +	return do_write(ff, &ff->ph->env.clockid_res_ns,
> +			sizeof(ff->ph->env.clockid_res_ns));
> +}
> +
>  static int cpu_cache_level__sort(const void *a, const void *b)
>  {
>  	struct cpu_cache_level *cache_a = (struct cpu_cache_level *)a;
> @@ -1508,6 +1515,12 @@ static void print_cpu_topology(struct feat_fd *ff, FILE *fp)
>  		fprintf(fp, "# Core ID and Socket ID information is not available\n");
>  }
>  
> +static void print_clockid(struct feat_fd *ff, FILE *fp)
> +{
> +	fprintf(fp, "# clockid frequency: %ld MHz\n",
> +		ff->ph->env.clockid_res_ns * 1000);
> +}
> +
>  static void free_event_desc(struct perf_evsel *events)
>  {
>  	struct perf_evsel *evsel;
> @@ -2531,6 +2544,19 @@ static int process_mem_topology(struct feat_fd *ff,
>  	return ret;
>  }
>  
> +static int process_clockid(struct feat_fd *ff,
> +			   void *data __maybe_unused)
> +{
> +	size_t clockid_res_ns;
> +
> +	if (do_read_u64(ff, &clockid_res_ns))
> +		return -1;
> +
> +	ff->ph->env.clockid_res_ns = clockid_res_ns;
> +
> +	return 0;
> +}
> +
>  struct feature_ops {
>  	int (*write)(struct feat_fd *ff, struct perf_evlist *evlist);
>  	void (*print)(struct feat_fd *ff, FILE *fp);
> @@ -2590,6 +2616,7 @@ static const struct feature_ops feat_ops[HEADER_LAST_FEATURE] = {
>  	FEAT_OPN(CACHE,		cache,		true),
>  	FEAT_OPR(SAMPLE_TIME,	sample_time,	false),
>  	FEAT_OPR(MEM_TOPOLOGY,	mem_topology,	true),
> +	FEAT_OPR(CLOCKID,       clockid,        false)
>  };
>  
>  struct header_print_data {
> diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
> index e17903caa71d..0d553ddca0a3 100644
> --- a/tools/perf/util/header.h
> +++ b/tools/perf/util/header.h
> @@ -38,6 +38,7 @@ enum {
>  	HEADER_CACHE,
>  	HEADER_SAMPLE_TIME,
>  	HEADER_MEM_TOPOLOGY,
> +	HEADER_CLOCKID,
>  	HEADER_LAST_FEATURE,
>  	HEADER_FEAT_BITS	= 256,
>  };

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

* Re: [PATCH v3] perf record: encode -k clockid frequency into Perf trace
  2018-10-04 15:27 ` Arnaldo Carvalho de Melo
@ 2018-10-04 16:13   ` Alexey Budankov
  0 siblings, 0 replies; 9+ messages in thread
From: Alexey Budankov @ 2018-10-04 16:13 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Andi Kleen, linux-kernel

On 04.10.2018 18:27, Arnaldo Carvalho de Melo wrote:

<SNIP>

> Please add two spaces before the output of programs, specially those
> that start with a #, otherwise when applying they will vanish.

Good to know. will do.

> 
> Fixed that up, applied!

Thanks!

- Alexey

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

* Re: [PATCH v3] perf record: encode -k clockid frequency into Perf trace
  2018-10-04 12:36 ` Jiri Olsa
@ 2018-10-06 11:51   ` Arnaldo Carvalho de Melo
  2018-10-06 13:14     ` Alexey Budankov
  0 siblings, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-06 11:51 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexey Budankov, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
	Namhyung Kim, Andi Kleen, linux-kernel

Em Thu, Oct 04, 2018 at 02:36:57PM +0200, Jiri Olsa escreveu:
> On Wed, Oct 03, 2018 at 07:57:12PM +0300, Alexey Budankov wrote:
> > 
> > Store -k clockid frequency into Perf trace to enable timestamps 
> > derived metrics conversion into wall clock time on reporting stage.
> > 
> > Below is the example of perf report output:
> > 
> > tools/perf/perf record -k raw -- ../../matrix/linux/matrix.gcc
> > ...
> > [ perf record: Captured and wrote 31.222 MB perf.data (818054 samples) ]
> > 
> > tools/perf/perf report --header
> > # ========
> > ...
> > # event : name = cycles:ppp, , size = 112, { sample_period, sample_freq } = 4000, sample_type = IP|TID|TIME|PERIOD, disabled = 1, inherit = 1, mmap = 1, comm = 1, freq = 1, enable_on_exec = 1, task = 1, precise_ip = 3, sample_id_all = 1, exclude_guest = 1, mmap2 = 1, comm_exec = 1, use_clockid = 1, clockid = 4
> > ...
> > # clockid frequency: 1000 MHz
> > ...
> > # ========
> > 
> > Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> Reviewed-by: Jiri Olsa <jolsa@kernel.org>



  CC       /tmp/build/perf/builtin-script.o
  CC       /tmp/build/perf/util/header.o
util/header.c: In function 'print_clockid':
util/header.c:1520:38: error: format '%ld' expects argument of type 'long int', but argument 3 has type 'size_t' {aka 'unsigned int'} [-Werror=format=]
  fprintf(fp, "# clockid frequency: %ld MHz\n",
                                    ~~^
                                    %d
   ff->ph->env.clockid_res_ns * 1000);
   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~   
util/header.c: In function 'process_clockid':
util/header.c:2552:22: error: passing argument 2 of 'do_read_u64' from incompatible pointer type [-Werror=incompatible-pointer-types]
  if (do_read_u64(ff, &clockid_res_ns))
                      ^~~~~~~~~~~~~~~
util/header.c:229:49: note: expected 'u64 *' {aka 'long long unsigned int *'} but argument is of type 'size_t *' {aka 'unsigned int *'}
 static int do_read_u64(struct feat_fd *ff, u64 *addr)
                                            ~~~~~^~~~
In file included from /usr/mips-linux-gnu/include/string.h:494,
                 from util/string2.h:7,
                 from util/header.c:5:
In function 'memcpy',
    inlined from '__do_read_buf' at util/header.c:202:2,
    inlined from '__do_read' at util/header.c:213:9,
    inlined from 'do_read_u64' at util/header.c:233:8,
    inlined from 'process_clockid' at util/header.c:2552:6:
/usr/mips-linux-gnu/include/bits/string_fortified.h:34:10: error: '__builtin___memcpy_chk' forming offset [5, 8] is out of the bounds [0, 4] of object 'clockid_res_ns' with type 'size_t' {aka 'unsigned int'} [-Werror=array-bounds]
   return __builtin___memcpy_chk (__dest, __src, __len, __bos0 (__dest));
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
util/header.c: In function 'process_clockid':
util/header.c:2550:9: note: 'clockid_res_ns' declared here
  size_t clockid_res_ns;
         ^~~~~~~~~~~~~~
  MKDIR    /tmp/build/perf/scripts/
  LD       /tmp/build/perf/scripts/libperf-in.o
  CC       /tmp/build/perf/builtin-kmem.o
  CC       /tmp/build/perf/builtin-lock.o
  CC       /tmp/build/perf/ui/browsers/map.o
  CC       /tmp/build/perf/builtin-kvm.o
cc1: all warnings being treated as errors
mv: cannot stat '/tmp/build/perf/util/.header.o.tmp': No such file or directory
make[4]: *** [/git/linux/tools/build/Makefile.build:97: /tmp/build/perf/util/header.o] Error 1
make[3]: *** [/git/linux/tools/build/Makefile.build:139: util] Error 2
make[3]: *** Waiting for unfinished jobs....

In a series of cross compiling environments:

  20    22.19 debian:experimental-x-mips    : FAIL mips-linux-gnu-gcc (Debian 8.1.0-12) 8.1.0
  21    47.76 debian:experimental-x-mips64  : Ok   mips64-linux-gnuabi64-gcc (Debian 8.1.0-12) 8.1.0
  22    19.27 debian:experimental-x-mipsel  : FAIL mipsel-linux-gnu-gcc (Debian 8.1.0-12) 8.1.0
  23    51.27 fedora:20                     : Ok   gcc (GCC) 4.8.3 20140911 (Red Hat 4.8.3-7)
  24    44.45 fedora:21                     : Ok   gcc (GCC) 4.9.2 20150212 (Red Hat 4.9.2-6)
  25    43.60 fedora:22                     : Ok   gcc (GCC) 5.3.1 20160406 (Red Hat 5.3.1-6)
  26    51.07 fedora:23                     : Ok   gcc (GCC) 5.3.1 20160406 (Red Hat 5.3.1-6)
  27    49.89 fedora:24                     : Ok   gcc (GCC) 6.3.1 20161221 (Red Hat 6.3.1-1)
  28    19.38 fedora:24-x-ARC-uClibc        : FAIL arc-linux-gcc (ARCompact ISA Linux uClibc toolchain 2017.09-rc2) 7.1.1 20170710
  29   138.25 fedora:25                     : Ok   gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1)
  30   145.14 fedora:26                     : Ok   gcc (GCC) 7.3.1 20180130 (Red Hat 7.3.1-2)
  31   163.94 fedora:27                     : Ok   gcc (GCC) 7.3.1 20180712 (Red Hat 7.3.1-6)
  32   166.77 fedora:28                     : Ok   gcc (GCC) 8.1.1 20180712 (Red Hat 8.1.1-5)
  33   157.20 fedora:rawhide                : Ok   gcc (GCC) 8.2.1 20180905 (Red Hat 8.2.1-3)
  34    40.37 gentoo-stage3-amd64:latest    : Ok   gcc (Gentoo 7.3.0-r3 p1.4) 7.3.0
  35    46.41 mageia:5                      : Ok   gcc (GCC) 4.9.2
  36    44.33 mageia:6                      : Ok   gcc (Mageia 5.5.0-1.mga6) 5.5.0
  37    41.12 opensuse:13.2                 : Ok   gcc (SUSE Linux) 4.8.3 20140627 [gcc-4_8-branch revision 212064]
  38    43.96 opensuse:42.1                 : Ok   gcc (SUSE Linux) 4.8.5
  39    43.83 opensuse:42.2                 : Ok   gcc (SUSE Linux) 4.8.5
  40    44.45 opensuse:42.3                 : Ok   gcc (SUSE Linux) 4.8.5
  41   141.53 opensuse:tumbleweed           : Ok   gcc (SUSE Linux) 7.3.1 20180323 [gcc-7-branch revision 258812]
  42    35.29 oraclelinux:6                 : Ok   gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-23.0.1)
  43    41.76 oraclelinux:7                 : Ok   gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-28.0.1)
  44    32.06 ubuntu:12.04.5                : Ok   gcc (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3
  45    39.26 ubuntu:14.04.4                : Ok   gcc (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4
  46    35.33 ubuntu:14.04.4-x-linaro-arm64 : Ok   aarch64-linux-gnu-gcc (Linaro GCC 5.5-2017.10) 5.5.0
  47    95.94 ubuntu:16.04                  : Ok   gcc (Ubuntu 5.4.0-6ubuntu1~16.04.10) 5.4.0 20160609
  48    13.76 ubuntu:16.04-x-arm            : FAIL arm-linux-gnueabihf-gcc (Ubuntu/Linaro 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
  49    34.59 ubuntu:16.04-x-arm64          : Ok   aarch64-linux-gnu-gcc (Ubuntu/Linaro 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
  50    13.59 ubuntu:16.04-x-powerpc        : FAIL powerpc-linux-gnu-gcc (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
  51    34.39 ubuntu:16.04-x-powerpc64      : Ok   powerpc64-linux-gnu-gcc (Ubuntu/IBM 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
  52    35.60 ubuntu:16.04-x-powerpc64el    : Ok   powerpc64le-linux-gnu-gcc (Ubuntu/IBM 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
  53    32.96 ubuntu:16.04-x-s390           : Ok   s390x-linux-gnu-gcc (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
  54   100.88 ubuntu:16.10                  : Ok   gcc (Ubuntu 6.2.0-5ubuntu12) 6.2.0 20161005
  55   111.23 ubuntu:17.10                  : Ok   gcc (Ubuntu 7.2.0-8ubuntu3.2) 7.2.0
  56   114.81 ubuntu:18.04                  : Ok   gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0
  57    15.12 ubuntu:18.04-x-arm            : FAIL arm-linux-gnueabihf-gcc (Ubuntu/Linaro 7.3.0-16ubuntu3) 7.3.0
  58    36.84 ubuntu:18.04-x-arm64          : Ok   aarch64-linux-gnu-gcc (Ubuntu/Linaro 7.3.0-16ubuntu3) 7.3.0
  59    11.27 ubuntu:18.04-x-m68k           : FAIL m68k-linux-gnu-gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0
  60    14.18 ubuntu:18.04-x-powerpc        : FAIL powerpc-linux-gnu-gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0
  61    39.74 ubuntu:18.04-x-powerpc64      : Ok   powerpc64-linux-gnu-gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0
  62    40.19 ubuntu:18.04-x-powerpc64el    : Ok   powerpc64le-linux-gnu-gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0
  63    70.67 ubuntu:18.04-x-riscv64        : Ok   riscv64-linux-gnu-gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0
  64    32.72 ubuntu:18.04-x-s390           : Ok   s390x-linux-gnu-gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0
  65    14.43 ubuntu:18.04-x-sh4            : FAIL sh4-linux-gnu-gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0


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

* Re: [PATCH v3] perf record: encode -k clockid frequency into Perf trace
  2018-10-06 11:51   ` Arnaldo Carvalho de Melo
@ 2018-10-06 13:14     ` Alexey Budankov
  2018-10-08 17:37       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 9+ messages in thread
From: Alexey Budankov @ 2018-10-06 13:14 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Namhyung Kim,
	Andi Kleen, linux-kernel

On 06.10.2018 14:51, Arnaldo Carvalho de Melo wrote:
> Em Thu, Oct 04, 2018 at 02:36:57PM +0200, Jiri Olsa escreveu:
>> On Wed, Oct 03, 2018 at 07:57:12PM +0300, Alexey Budankov wrote:
>>>
>>> Store -k clockid frequency into Perf trace to enable timestamps 
>>> derived metrics conversion into wall clock time on reporting stage.
>>>
>>> Below is the example of perf report output:
>>>
>>> tools/perf/perf record -k raw -- ../../matrix/linux/matrix.gcc
>>> ...
>>> [ perf record: Captured and wrote 31.222 MB perf.data (818054 samples) ]
>>>
>>> tools/perf/perf report --header
>>> # ========
>>> ...
>>> # event : name = cycles:ppp, , size = 112, { sample_period, sample_freq } = 4000, sample_type = IP|TID|TIME|PERIOD, disabled = 1, inherit = 1, mmap = 1, comm = 1, freq = 1, enable_on_exec = 1, task = 1, precise_ip = 3, sample_id_all = 1, exclude_guest = 1, mmap2 = 1, comm_exec = 1, use_clockid = 1, clockid = 4
>>> ...
>>> # clockid frequency: 1000 MHz
>>> ...
>>> # ========
>>>
>>> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
>> Reviewed-by: Jiri Olsa <jolsa@kernel.org>
> 
> 
> 
>   CC       /tmp/build/perf/builtin-script.o
>   CC       /tmp/build/perf/util/header.o
> util/header.c: In function 'print_clockid':
> util/header.c:1520:38: error: format '%ld' expects argument of type 'long int', but argument 3 has type 'size_t' {aka 'unsigned int'} [-Werror=format=]
>   fprintf(fp, "# clockid frequency: %ld MHz\n",
>                                     ~~^
>                                     %d
>    ff->ph->env.clockid_res_ns * 1000);
>    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~   
> util/header.c: In function 'process_clockid':
> util/header.c:2552:22: error: passing argument 2 of 'do_read_u64' from incompatible pointer type [-Werror=incompatible-pointer-types]
>   if (do_read_u64(ff, &clockid_res_ns))
>                       ^~~~~~~~~~~~~~~
> util/header.c:229:49: note: expected 'u64 *' {aka 'long long unsigned int *'} but argument is of type 'size_t *' {aka 'unsigned int *'}
>  static int do_read_u64(struct feat_fd *ff, u64 *addr)
>                                             ~~~~~^~~~
> In file included from /usr/mips-linux-gnu/include/string.h:494,
>                  from util/string2.h:7,
>                  from util/header.c:5:
> In function 'memcpy',
>     inlined from '__do_read_buf' at util/header.c:202:2,
>     inlined from '__do_read' at util/header.c:213:9,
>     inlined from 'do_read_u64' at util/header.c:233:8,
>     inlined from 'process_clockid' at util/header.c:2552:6:
> /usr/mips-linux-gnu/include/bits/string_fortified.h:34:10: error: '__builtin___memcpy_chk' forming offset [5, 8] is out of the bounds [0, 4] of object 'clockid_res_ns' with type 'size_t' {aka 'unsigned int'} [-Werror=array-bounds]
>    return __builtin___memcpy_chk (__dest, __src, __len, __bos0 (__dest));
>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> util/header.c: In function 'process_clockid':
> util/header.c:2550:9: note: 'clockid_res_ns' declared here
>   size_t clockid_res_ns;
>          ^~~~~~~~~~~~~~
>   MKDIR    /tmp/build/perf/scripts/
>   LD       /tmp/build/perf/scripts/libperf-in.o
>   CC       /tmp/build/perf/builtin-kmem.o
>   CC       /tmp/build/perf/builtin-lock.o
>   CC       /tmp/build/perf/ui/browsers/map.o
>   CC       /tmp/build/perf/builtin-kvm.o
> cc1: all warnings being treated as errors
> mv: cannot stat '/tmp/build/perf/util/.header.o.tmp': No such file or directory
> make[4]: *** [/git/linux/tools/build/Makefile.build:97: /tmp/build/perf/util/header.o] Error 1
> make[3]: *** [/git/linux/tools/build/Makefile.build:139: util] Error 2
> make[3]: *** Waiting for unfinished jobs....
> 

Hope this helps.

---
tools/perf/util/header.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 4ce5339158f7..afaebbb53035 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1517,7 +1517,7 @@ static void print_cpu_topology(struct feat_fd *ff, FILE *fp)
 
 static void print_clockid(struct feat_fd *ff, FILE *fp)
 {
-       fprintf(fp, "# clockid frequency: %ld MHz\n",
+       fprintf(fp, "# clockid frequency: %zd MHz\n",
                ff->ph->env.clockid_res_ns * 1000);
 }
 
@@ -2549,7 +2549,7 @@ static int process_clockid(struct feat_fd *ff,
 {
        size_t clockid_res_ns;
 
-       if (do_read_u64(ff, &clockid_res_ns))
+       if (__do_read(ff, &clockid_res_ns, sizeof(clockid_res_ns)))
                return -1;
 
        ff->ph->env.clockid_res_ns = clockid_res_ns;
---

Thanks,
Alexey

> In a series of cross compiling environments:
> 
>   20    22.19 debian:experimental-x-mips    : FAIL mips-linux-gnu-gcc (Debian 8.1.0-12) 8.1.0
>   21    47.76 debian:experimental-x-mips64  : Ok   mips64-linux-gnuabi64-gcc (Debian 8.1.0-12) 8.1.0
>   22    19.27 debian:experimental-x-mipsel  : FAIL mipsel-linux-gnu-gcc (Debian 8.1.0-12) 8.1.0
>   23    51.27 fedora:20                     : Ok   gcc (GCC) 4.8.3 20140911 (Red Hat 4.8.3-7)
>   24    44.45 fedora:21                     : Ok   gcc (GCC) 4.9.2 20150212 (Red Hat 4.9.2-6)
>   25    43.60 fedora:22                     : Ok   gcc (GCC) 5.3.1 20160406 (Red Hat 5.3.1-6)
>   26    51.07 fedora:23                     : Ok   gcc (GCC) 5.3.1 20160406 (Red Hat 5.3.1-6)
>   27    49.89 fedora:24                     : Ok   gcc (GCC) 6.3.1 20161221 (Red Hat 6.3.1-1)
>   28    19.38 fedora:24-x-ARC-uClibc        : FAIL arc-linux-gcc (ARCompact ISA Linux uClibc toolchain 2017.09-rc2) 7.1.1 20170710
>   29   138.25 fedora:25                     : Ok   gcc (GCC) 6.4.1 20170727 (Red Hat 6.4.1-1)
>   30   145.14 fedora:26                     : Ok   gcc (GCC) 7.3.1 20180130 (Red Hat 7.3.1-2)
>   31   163.94 fedora:27                     : Ok   gcc (GCC) 7.3.1 20180712 (Red Hat 7.3.1-6)
>   32   166.77 fedora:28                     : Ok   gcc (GCC) 8.1.1 20180712 (Red Hat 8.1.1-5)
>   33   157.20 fedora:rawhide                : Ok   gcc (GCC) 8.2.1 20180905 (Red Hat 8.2.1-3)
>   34    40.37 gentoo-stage3-amd64:latest    : Ok   gcc (Gentoo 7.3.0-r3 p1.4) 7.3.0
>   35    46.41 mageia:5                      : Ok   gcc (GCC) 4.9.2
>   36    44.33 mageia:6                      : Ok   gcc (Mageia 5.5.0-1.mga6) 5.5.0
>   37    41.12 opensuse:13.2                 : Ok   gcc (SUSE Linux) 4.8.3 20140627 [gcc-4_8-branch revision 212064]
>   38    43.96 opensuse:42.1                 : Ok   gcc (SUSE Linux) 4.8.5
>   39    43.83 opensuse:42.2                 : Ok   gcc (SUSE Linux) 4.8.5
>   40    44.45 opensuse:42.3                 : Ok   gcc (SUSE Linux) 4.8.5
>   41   141.53 opensuse:tumbleweed           : Ok   gcc (SUSE Linux) 7.3.1 20180323 [gcc-7-branch revision 258812]
>   42    35.29 oraclelinux:6                 : Ok   gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-23.0.1)
>   43    41.76 oraclelinux:7                 : Ok   gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-28.0.1)
>   44    32.06 ubuntu:12.04.5                : Ok   gcc (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3
>   45    39.26 ubuntu:14.04.4                : Ok   gcc (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4
>   46    35.33 ubuntu:14.04.4-x-linaro-arm64 : Ok   aarch64-linux-gnu-gcc (Linaro GCC 5.5-2017.10) 5.5.0
>   47    95.94 ubuntu:16.04                  : Ok   gcc (Ubuntu 5.4.0-6ubuntu1~16.04.10) 5.4.0 20160609
>   48    13.76 ubuntu:16.04-x-arm            : FAIL arm-linux-gnueabihf-gcc (Ubuntu/Linaro 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
>   49    34.59 ubuntu:16.04-x-arm64          : Ok   aarch64-linux-gnu-gcc (Ubuntu/Linaro 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
>   50    13.59 ubuntu:16.04-x-powerpc        : FAIL powerpc-linux-gnu-gcc (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
>   51    34.39 ubuntu:16.04-x-powerpc64      : Ok   powerpc64-linux-gnu-gcc (Ubuntu/IBM 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
>   52    35.60 ubuntu:16.04-x-powerpc64el    : Ok   powerpc64le-linux-gnu-gcc (Ubuntu/IBM 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
>   53    32.96 ubuntu:16.04-x-s390           : Ok   s390x-linux-gnu-gcc (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
>   54   100.88 ubuntu:16.10                  : Ok   gcc (Ubuntu 6.2.0-5ubuntu12) 6.2.0 20161005
>   55   111.23 ubuntu:17.10                  : Ok   gcc (Ubuntu 7.2.0-8ubuntu3.2) 7.2.0
>   56   114.81 ubuntu:18.04                  : Ok   gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0
>   57    15.12 ubuntu:18.04-x-arm            : FAIL arm-linux-gnueabihf-gcc (Ubuntu/Linaro 7.3.0-16ubuntu3) 7.3.0
>   58    36.84 ubuntu:18.04-x-arm64          : Ok   aarch64-linux-gnu-gcc (Ubuntu/Linaro 7.3.0-16ubuntu3) 7.3.0
>   59    11.27 ubuntu:18.04-x-m68k           : FAIL m68k-linux-gnu-gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0
>   60    14.18 ubuntu:18.04-x-powerpc        : FAIL powerpc-linux-gnu-gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0
>   61    39.74 ubuntu:18.04-x-powerpc64      : Ok   powerpc64-linux-gnu-gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0
>   62    40.19 ubuntu:18.04-x-powerpc64el    : Ok   powerpc64le-linux-gnu-gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0
>   63    70.67 ubuntu:18.04-x-riscv64        : Ok   riscv64-linux-gnu-gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0
>   64    32.72 ubuntu:18.04-x-s390           : Ok   s390x-linux-gnu-gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0
>   65    14.43 ubuntu:18.04-x-sh4            : FAIL sh4-linux-gnu-gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0
> 
> 

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

* Re: [PATCH v3] perf record: encode -k clockid frequency into Perf trace
  2018-10-06 13:14     ` Alexey Budankov
@ 2018-10-08 17:37       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-08 17:37 UTC (permalink / raw)
  To: Alexey Budankov
  Cc: Jiri Olsa, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
	Namhyung Kim, Andi Kleen, linux-kernel

Em Sat, Oct 06, 2018 at 04:14:11PM +0300, Alexey Budankov escreveu:
> On 06.10.2018 14:51, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Oct 04, 2018 at 02:36:57PM +0200, Jiri Olsa escreveu:
> >> On Wed, Oct 03, 2018 at 07:57:12PM +0300, Alexey Budankov wrote:
> >>>
> >>> Store -k clockid frequency into Perf trace to enable timestamps 
> >>> derived metrics conversion into wall clock time on reporting stage.
> >>>
> >>> Below is the example of perf report output:
> >>>
> >>> tools/perf/perf record -k raw -- ../../matrix/linux/matrix.gcc
> >>> ...
> >>> [ perf record: Captured and wrote 31.222 MB perf.data (818054 samples) ]
> >>>
> >>> tools/perf/perf report --header
> >>> # ========
> >>> ...
> >>> # event : name = cycles:ppp, , size = 112, { sample_period, sample_freq } = 4000, sample_type = IP|TID|TIME|PERIOD, disabled = 1, inherit = 1, mmap = 1, comm = 1, freq = 1, enable_on_exec = 1, task = 1, precise_ip = 3, sample_id_all = 1, exclude_guest = 1, mmap2 = 1, comm_exec = 1, use_clockid = 1, clockid = 4
> >>> ...
> >>> # clockid frequency: 1000 MHz
> >>> ...
> >>> # ========
> >>>
> >>> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> >> Reviewed-by: Jiri Olsa <jolsa@kernel.org>
> > 
> > 
> > 
> >   CC       /tmp/build/perf/builtin-script.o
> >   CC       /tmp/build/perf/util/header.o
> > util/header.c: In function 'print_clockid':
> > util/header.c:1520:38: error: format '%ld' expects argument of type 'long int', but argument 3 has type 'size_t' {aka 'unsigned int'} [-Werror=format=]
> >   fprintf(fp, "# clockid frequency: %ld MHz\n",
> >                                     ~~^
> >                                     %d
> >    ff->ph->env.clockid_res_ns * 1000);
> >    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~   
> > util/header.c: In function 'process_clockid':
> > util/header.c:2552:22: error: passing argument 2 of 'do_read_u64' from incompatible pointer type [-Werror=incompatible-pointer-types]
> >   if (do_read_u64(ff, &clockid_res_ns))
> >                       ^~~~~~~~~~~~~~~
> > util/header.c:229:49: note: expected 'u64 *' {aka 'long long unsigned int *'} but argument is of type 'size_t *' {aka 'unsigned int *'}
> >  static int do_read_u64(struct feat_fd *ff, u64 *addr)
> >                                             ~~~~~^~~~
> > In file included from /usr/mips-linux-gnu/include/string.h:494,
> >                  from util/string2.h:7,
> >                  from util/header.c:5:
> > In function 'memcpy',
> >     inlined from '__do_read_buf' at util/header.c:202:2,
> >     inlined from '__do_read' at util/header.c:213:9,
> >     inlined from 'do_read_u64' at util/header.c:233:8,
> >     inlined from 'process_clockid' at util/header.c:2552:6:
> > /usr/mips-linux-gnu/include/bits/string_fortified.h:34:10: error: '__builtin___memcpy_chk' forming offset [5, 8] is out of the bounds [0, 4] of object 'clockid_res_ns' with type 'size_t' {aka 'unsigned int'} [-Werror=array-bounds]
> >    return __builtin___memcpy_chk (__dest, __src, __len, __bos0 (__dest));
> >           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > util/header.c: In function 'process_clockid':
> > util/header.c:2550:9: note: 'clockid_res_ns' declared here
> >   size_t clockid_res_ns;
> >          ^~~~~~~~~~~~~~
> >   MKDIR    /tmp/build/perf/scripts/
> >   LD       /tmp/build/perf/scripts/libperf-in.o
> >   CC       /tmp/build/perf/builtin-kmem.o
> >   CC       /tmp/build/perf/builtin-lock.o
> >   CC       /tmp/build/perf/ui/browsers/map.o
> >   CC       /tmp/build/perf/builtin-kvm.o
> > cc1: all warnings being treated as errors
> > mv: cannot stat '/tmp/build/perf/util/.header.o.tmp': No such file or directory
> > make[4]: *** [/git/linux/tools/build/Makefile.build:97: /tmp/build/perf/util/header.o] Error 1
> > make[3]: *** [/git/linux/tools/build/Makefile.build:139: util] Error 2
> > make[3]: *** Waiting for unfinished jobs....
> > 
> 
> Hope this helps.
> 
> ---
> tools/perf/util/header.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 4ce5339158f7..afaebbb53035 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -1517,7 +1517,7 @@ static void print_cpu_topology(struct feat_fd *ff, FILE *fp)
>  
>  static void print_clockid(struct feat_fd *ff, FILE *fp)
>  {
> -       fprintf(fp, "# clockid frequency: %ld MHz\n",
> +       fprintf(fp, "# clockid frequency: %zd MHz\n",
>                 ff->ph->env.clockid_res_ns * 1000);
>  }
>  
> @@ -2549,7 +2549,7 @@ static int process_clockid(struct feat_fd *ff,
>  {
>         size_t clockid_res_ns;
>  
> -       if (do_read_u64(ff, &clockid_res_ns))
> +       if (__do_read(ff, &clockid_res_ns, sizeof(clockid_res_ns)))
>                 return -1;
>  
>         ff->ph->env.clockid_res_ns = clockid_res_ns;

Are you sure about that? Isn't this always encoded as an u64? Also, look
at what else do_read_u64 does besides just reading:

static int do_read_u64(struct feat_fd *ff, u64 *addr)
{
        int ret;

        ret = __do_read(ff, addr, sizeof(*addr));
        if (ret)
                return ret;

        if (ff->ph->needs_swap)
                *addr = bswap_64(*addr);
        return 0;
}

/me goes to look at the original patch to see how you encode that
clod_id_res_ns in the perf.data header...

- Arnaldo

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

* Re: [PATCH v3] perf record: encode -k clockid frequency into Perf trace
  2018-10-03 16:57 [PATCH v3] perf record: encode -k clockid frequency into Perf trace Alexey Budankov
  2018-10-04 12:36 ` Jiri Olsa
  2018-10-04 15:27 ` Arnaldo Carvalho de Melo
@ 2018-10-08 17:42 ` Arnaldo Carvalho de Melo
  2018-10-08 18:13   ` Alexey Budankov
  2 siblings, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-08 17:42 UTC (permalink / raw)
  To: Alexey Budankov
  Cc: Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Andi Kleen, linux-kernel

Em Wed, Oct 03, 2018 at 07:57:12PM +0300, Alexey Budankov escreveu:
> 
> Store -k clockid frequency into Perf trace to enable timestamps 
> derived metrics conversion into wall clock time on reporting stage.

<SNIP>

> +++ b/tools/perf/util/env.h
> @@ -63,6 +63,7 @@ struct perf_env {
>  	struct numa_node	*numa_nodes;
>  	struct memory_node	*memory_nodes;
>  	unsigned long long	 memory_bsize;
> +	size_t                   clockid_res_ns;
>  };
>  
>  extern struct perf_env perf_env;
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 1ec1d9bc2d63..4ce5339158f7 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -1034,6 +1034,13 @@ static int write_auxtrace(struct feat_fd *ff,
>  	return err;
>  }
>  
> +static int write_clockid(struct feat_fd *ff,
> +			 struct perf_evlist *evlist __maybe_unused)
> +{
> +	return do_write(ff, &ff->ph->env.clockid_res_ns,
> +			sizeof(ff->ph->env.clockid_res_ns));
> +}
> +

Is sizeof(size_t) the same everywhere? I think you should encode this
always as a u64, read it, assign it to a temp u64 var, write the u64 to
the perf.data header, read it using do_read_u64, that will take care of
endianness, then set it to the size_t in the ff->ph->env.clockid_res_ns,
right?

I'm removing the patch till this gets sorted out,

Thanks,

- Arnaldo

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

* Re: [PATCH v3] perf record: encode -k clockid frequency into Perf trace
  2018-10-08 17:42 ` Arnaldo Carvalho de Melo
@ 2018-10-08 18:13   ` Alexey Budankov
  0 siblings, 0 replies; 9+ messages in thread
From: Alexey Budankov @ 2018-10-08 18:13 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Andi Kleen, linux-kernel


Hi,

On 08.10.2018 20:42, Arnaldo Carvalho de Melo wrote:
> Em Wed, Oct 03, 2018 at 07:57:12PM +0300, Alexey Budankov escreveu:
>>
>> Store -k clockid frequency into Perf trace to enable timestamps 
>> derived metrics conversion into wall clock time on reporting stage.
> 
> <SNIP>
> 
>> +++ b/tools/perf/util/env.h
>> @@ -63,6 +63,7 @@ struct perf_env {
>>  	struct numa_node	*numa_nodes;
>>  	struct memory_node	*memory_nodes;
>>  	unsigned long long	 memory_bsize;
>> +	size_t                   clockid_res_ns;
>>  };
>>  
>>  extern struct perf_env perf_env;
>> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
>> index 1ec1d9bc2d63..4ce5339158f7 100644
>> --- a/tools/perf/util/header.c
>> +++ b/tools/perf/util/header.c
>> @@ -1034,6 +1034,13 @@ static int write_auxtrace(struct feat_fd *ff,
>>  	return err;
>>  }
>>  
>> +static int write_clockid(struct feat_fd *ff,
>> +			 struct perf_evlist *evlist __maybe_unused)
>> +{
>> +	return do_write(ff, &ff->ph->env.clockid_res_ns,
>> +			sizeof(ff->ph->env.clockid_res_ns));
>> +}
>> +
> 
> Is sizeof(size_t) the same everywhere? I think you should encode this
> always as a u64, read it, assign it to a temp u64 var, write the u64 to
> the perf.data header, read it using do_read_u64, that will take care of
> endianness, then set it to the size_t in the ff->ph->env.clockid_res_ns,
> right?

Right. Let me take care of all that.

Thanks!
Alexey

> 
> I'm removing the patch till this gets sorted out,
> 
> Thanks,
> 
> - Arnaldo
> 

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

end of thread, other threads:[~2018-10-08 18:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-03 16:57 [PATCH v3] perf record: encode -k clockid frequency into Perf trace Alexey Budankov
2018-10-04 12:36 ` Jiri Olsa
2018-10-06 11:51   ` Arnaldo Carvalho de Melo
2018-10-06 13:14     ` Alexey Budankov
2018-10-08 17:37       ` Arnaldo Carvalho de Melo
2018-10-04 15:27 ` Arnaldo Carvalho de Melo
2018-10-04 16:13   ` Alexey Budankov
2018-10-08 17:42 ` Arnaldo Carvalho de Melo
2018-10-08 18:13   ` Alexey Budankov

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