[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <41a9c8e5-044e-d42d-0e42-058b974d4f00@codeaurora.org>
Date: Thu, 27 Oct 2016 16:14:55 -0700
From: Joonwoo Park <joonwoop@...eaurora.org>
To: Arnaldo Carvalho de Melo <acme@...nel.org>,
Ingo Molnar <mingo@...nel.org>
Cc: linux-kernel@...r.kernel.org, Namhyung Kim <namhyung@...nel.org>,
David Ahern <dsahern@...il.com>, Jiri Olsa <jolsa@...nel.org>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Arnaldo Carvalho de Melo <acme@...hat.com>
Subject: Re: [PATCH 08/15] perf tools: Introduce timestamp_in_usec()
On 10/27/2016 01:40 PM, Arnaldo Carvalho de Melo wrote:
> From: Namhyung Kim <namhyung@...nel.org>
>
> Joonwoo reported that there's a mismatch between timestamps in script
> and sched commands. This was because of difference in printing the
> timestamp. Factor out the code and share it so that they can be in
> sync. Also I found that sched map has similar problem, fix it too.
>
> Reported-and-Acked-by: Joonwoo Park <joonwoop@...eaurora.org>
Hmm.. I didn't ACK this patch because of bug I commented at
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1256724.html
s/work_list->max_lat/work_list->max_lat_at/
Do we have fix for this?
Thanks,
Joonwoo
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> Cc: David Ahern <dsahern@...il.com>
> Cc: Jiri Olsa <jolsa@...nel.org>
> Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
> Link: http://lkml.kernel.org/r/20161024020246.14928-3-namhyung@kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
> ---
> tools/perf/builtin-sched.c | 9 ++++++---
> tools/perf/builtin-script.c | 9 ++++++---
> tools/perf/util/util.c | 9 +++++++++
> tools/perf/util/util.h | 3 +++
> 4 files changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 1f33d15314a5..f0ab715b4923 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -1191,6 +1191,7 @@ static void output_lat_thread(struct perf_sched *sched, struct work_atoms *work_
> int i;
> int ret;
> u64 avg;
> + char buf[32];
>
> if (!work_list->nb_atoms)
> return;
> @@ -1213,11 +1214,11 @@ static void output_lat_thread(struct perf_sched *sched, struct work_atoms *work_
>
> avg = work_list->total_lat / work_list->nb_atoms;
>
> - printf("|%11.3f ms |%9" PRIu64 " | avg:%9.3f ms | max:%9.3f ms | max at: %13.6f s\n",
> + printf("|%11.3f ms |%9" PRIu64 " | avg:%9.3f ms | max:%9.3f ms | max at: %13s s\n",
> (double)work_list->total_runtime / NSEC_PER_MSEC,
> work_list->nb_atoms, (double)avg / NSEC_PER_MSEC,
> (double)work_list->max_lat / NSEC_PER_MSEC,
> - (double)work_list->max_lat_at / NSEC_PER_SEC);
> + timestamp_in_usec(buf, sizeof(buf), work_list->max_lat));
> }
>
> static int pid_cmp(struct work_atoms *l, struct work_atoms *r)
> @@ -1402,6 +1403,7 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
> int cpus_nr;
> bool new_cpu = false;
> const char *color = PERF_COLOR_NORMAL;
> + char buf[32];
>
> BUG_ON(this_cpu >= MAX_CPUS || this_cpu < 0);
>
> @@ -1492,7 +1494,8 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
> if (sched->map.cpus && !cpu_map__has(sched->map.cpus, this_cpu))
> goto out;
>
> - color_fprintf(stdout, color, " %12.6f secs ", (double)timestamp / NSEC_PER_SEC);
> + color_fprintf(stdout, color, " %12s secs ",
> + timestamp_in_usec(buf, sizeof(buf), timestamp));
> if (new_shortname || (verbose && sched_in->tid)) {
> const char *pid_color = color;
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 412fb6e65ac0..dae4d1013c33 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -441,7 +441,6 @@ static void print_sample_start(struct perf_sample *sample,
> {
> struct perf_event_attr *attr = &evsel->attr;
> unsigned long secs;
> - unsigned long usecs;
> unsigned long long nsecs;
>
> if (PRINT_FIELD(COMM)) {
> @@ -468,14 +467,18 @@ static void print_sample_start(struct perf_sample *sample,
> }
>
> if (PRINT_FIELD(TIME)) {
> + char buf[32];
> + size_t sz = sizeof(buf);
> +
> nsecs = sample->time;
> secs = nsecs / NSEC_PER_SEC;
> nsecs -= secs * NSEC_PER_SEC;
> - usecs = nsecs / NSEC_PER_USEC;
> +
> if (nanosecs)
> printf("%5lu.%09llu: ", secs, nsecs);
> else
> - printf("%5lu.%06lu: ", secs, usecs);
> + printf("%12s: ", timestamp_in_usec(buf, sz,
> + sample->time));
> }
> }
>
> diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
> index 85c56800f17a..aa3e778989ce 100644
> --- a/tools/perf/util/util.c
> +++ b/tools/perf/util/util.c
> @@ -433,6 +433,15 @@ int parse_nsec_time(const char *str, u64 *ptime)
> return 0;
> }
>
> +char *timestamp_in_usec(char *buf, size_t sz, u64 timestamp)
> +{
> + u64 sec = timestamp / NSEC_PER_SEC;
> + u64 usec = (timestamp % NSEC_PER_SEC) / NSEC_PER_USEC;
> +
> + scnprintf(buf, sz, "%"PRIu64".%06"PRIu64, sec, usec);
> + return buf;
> +}
> +
> unsigned long parse_tag_value(const char *str, struct parse_tag *tags)
> {
> struct parse_tag *i = tags;
> diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
> index 71b6992f1d98..ece974f1c538 100644
> --- a/tools/perf/util/util.h
> +++ b/tools/perf/util/util.h
> @@ -362,4 +362,7 @@ extern int sched_getcpu(void);
> #endif
>
> int is_printable_array(char *p, unsigned int len);
> +
> +char *timestamp_in_usec(char *buf, size_t sz, u64 timestamp);
> +
> #endif /* GIT_COMPAT_UTIL_H */
>
Powered by blists - more mailing lists