From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932315AbbKDIfA (ORCPT ); Wed, 4 Nov 2015 03:35:00 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41799 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753682AbbKDIe7 (ORCPT ); Wed, 4 Nov 2015 03:34:59 -0500 Date: Wed, 4 Nov 2015 09:34:56 +0100 From: Jiri Olsa To: Andi Kleen Cc: jolsa@kernel.org, acme@kernel.org, mingo@kernel.org, linux-kernel@vger.kernel.org, Andi Kleen Subject: Re: [PATCH 3/9] perf, tools, stat: Abstract stat metrics printing Message-ID: <20151104083456.GE29671@krava.brq.redhat.com> References: <1446515428-7450-1-git-send-email-andi@firstfloor.org> <1446515428-7450-4-git-send-email-andi@firstfloor.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1446515428-7450-4-git-send-email-andi@firstfloor.org> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Nov 02, 2015 at 05:50:22PM -0800, Andi Kleen wrote: SNIP > @@ -307,119 +302,145 @@ void perf_stat__print_shadow_stats(FILE *out, struct perf_evsel *evsel, > total = avg_stats(&runtime_cycles_stats[ctx][cpu]); > if (total) { > ratio = avg / total; > - fprintf(out, " # %5.2f insns per cycle ", ratio); > + print_metric(ctxp, NULL, "%7.2f ", > + "insn per cycle", ratio); > } else { > - fprintf(out, " "); > + print_metric(ctxp, NULL, NULL, "insn per cycle", 0); > } > total = avg_stats(&runtime_stalled_cycles_front_stats[ctx][cpu]); > total = max(total, avg_stats(&runtime_stalled_cycles_back_stats[ctx][cpu])); > > + out->new_line(ctxp); I think this needs to be in the condition below (check patch below) otherwise you'll get empty new line in case there's no stalled-cycles events > if (total && avg) { > ratio = total / avg; > - fprintf(out, "\n"); > - if (aggr == AGGR_NONE) > - fprintf(out, " "); > - fprintf(out, " # %5.2f stalled cycles per insn", ratio); > + print_metric(ctxp, NULL, "%7.2f ", > + "stalled cycles per insn", > + ratio); > + } else { > + print_metric(ctxp, NULL, NULL, > + "stalled cycles per insn", 0); > } also I dont understand the reason for the else case in here jirka > - > - } else if (perf_evsel__match(evsel, HARDWARE, HW_BRANCH_MISSES) && > - runtime_branches_stats[ctx][cpu].n != 0) { > - print_branch_misses(out, cpu, evsel, avg); --- diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c index 4d8f18581b9b..367e220e93d5 100644 --- a/tools/perf/util/stat-shadow.c +++ b/tools/perf/util/stat-shadow.c @@ -310,8 +310,8 @@ void perf_stat__print_shadow_stats(struct perf_evsel *evsel, total = avg_stats(&runtime_stalled_cycles_front_stats[ctx][cpu]); total = max(total, avg_stats(&runtime_stalled_cycles_back_stats[ctx][cpu])); - out->new_line(ctxp); if (total && avg) { + out->new_line(ctxp); ratio = total / avg; print_metric(ctxp, NULL, "%7.2f ", "stalled cycles per insn",