Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965401AbcJXU4L (ORCPT ); Mon, 24 Oct 2016 16:56:11 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:55771 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932554AbcJXU4K (ORCPT ); Mon, 24 Oct 2016 16:56:10 -0400 DMARC-Filter: OpenDMARC Filter v1.3.1 smtp.codeaurora.org 3C80D61B8B Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=pass smtp.mailfrom=joonwoop@codeaurora.org Subject: Re: [PATCH 3/3] perf tools: Introduce timestamp_in_usec() To: Namhyung Kim , Arnaldo Carvalho de Melo References: <20161024020246.14928-1-namhyung@kernel.org> <20161024020246.14928-3-namhyung@kernel.org> Cc: Ingo Molnar , Peter Zijlstra , Jiri Olsa , LKML , David Ahern From: Joonwoo Park Message-ID: <8d5bccf4-df81-c4f0-a836-d717b7ebb137@codeaurora.org> Date: Mon, 24 Oct 2016 13:56:07 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20161024020246.14928-3-namhyung@kernel.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4746 Lines: 141 On 10/23/2016 07:02 PM, Namhyung Kim wrote: > 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-by: Joonwoo Park Sorry I was busy with something else so didn't have chance to follow up on my initial proposal and thanks for take caring of this. > Signed-off-by: Namhyung Kim > --- > 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 97d6cbf486bb..c88d64ae997e 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)); This should be : s/work_list->max_lat/work_list->max_lat_at/ > } > > 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 7228d141a789..c848c74bdc90 100644 > --- a/tools/perf/builtin-script.c > +++ b/tools/perf/builtin-script.c > @@ -437,7 +437,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)) { > @@ -464,14 +463,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) I agree with Jirka. timestamp_usec__scnprintf looks better. Thanks, Joonwoo > +{ > + 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 */ >