From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754222AbcJENXQ (ORCPT ); Wed, 5 Oct 2016 09:23:16 -0400 Received: from mail.kernel.org ([198.145.29.136]:38266 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753250AbcJENXO (ORCPT ); Wed, 5 Oct 2016 09:23:14 -0400 Date: Wed, 5 Oct 2016 10:23:07 -0300 From: Arnaldo Carvalho de Melo To: Jiri Olsa Cc: Jiri Olsa , lkml , Don Zickus , Joe Mario , Ingo Molnar , Peter Zijlstra , Namhyung Kim , David Ahern , Andi Kleen Subject: Re: [PATCH 20/57] perf c2c report: Add dcacheline dimension key Message-ID: <20161005132307.GB30363@kernel.org> References: <1474558645-19956-1-git-send-email-jolsa@kernel.org> <1474558645-19956-21-git-send-email-jolsa@kernel.org> <20161005110141.GM7143@kernel.org> <20161005124537.GB14500@krava> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161005124537.GB14500@krava> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.7.0 (2016-08-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Wed, Oct 05, 2016 at 02:45:37PM +0200, Jiri Olsa escreveu: > On Wed, Oct 05, 2016 at 08:01:41AM -0300, Arnaldo Carvalho de Melo wrote: > > Em Thu, Sep 22, 2016 at 05:36:48PM +0200, Jiri Olsa escreveu: > > > Adding dcacheline dimension key support. It > > > displays cacheline address as hex number. > > > > > > Using c2c wrapper to standard 'dcacheline' object > > > to defined own header and simple (just address) > > > cacheline output. > > > > > > Link: http://lkml.kernel.org/n/tip-j5enppr8e7h27nskqhgq33lu@git.kernel.org > > > Signed-off-by: Jiri Olsa > > > --- > > > tools/perf/builtin-c2c.c | 38 ++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 38 insertions(+) > > > > > > diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c > > > index cfa12930b77b..335c0fd30757 100644 > > > --- a/tools/perf/builtin-c2c.c > > > +++ b/tools/perf/builtin-c2c.c > > > @@ -1,5 +1,6 @@ > > > #include > > > #include > > > +#include > > > #include "util.h" > > > #include "debug.h" > > > #include "builtin.h" > > > @@ -7,6 +8,7 @@ > > > #include "mem-events.h" > > > #include "session.h" > > > #include "hist.h" > > > +#include "sort.h" > > > #include "tool.h" > > > #include "data.h" > > > #include "sort.h" > > > @@ -271,6 +273,33 @@ static int c2c_header(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp, > > > return scnprintf(hpp->buf, hpp->size, "%*s", width, text); > > > } > > > > > > +static char *hex_str(u64 val) > > > +{ > > > + static char buf[20]; > > > > Ouch, what for? > > it's being used later on for short time hex number string > which is psased right away to another buffer ;-) > > having just one thread I didn't see any harm, esp when it > saved some code lines I have no problems with gotos, but hate static vars used to return vals... :-) > > > > > + > > > + snprintf(buf, 20, "0x%" PRIx64, val); > > > + return buf; > > > +} > > > + > > > +static int64_t > > > +dcacheline_cmp(struct perf_hpp_fmt *fmt __maybe_unused, > > > + struct hist_entry *left, struct hist_entry *right) > > > +{ > > > + return sort__dcacheline_cmp(left, right); > > > +} > > > + > > > +static int dcacheline_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp, > > > + struct hist_entry *he) > > > +{ > > > + uint64_t addr = 0; > > > + int width = c2c_width(fmt, hpp, he->hists); > > > + > > > + if (he->mem_info) > > > + addr = cl_address(he->mem_info->daddr.addr); > > > + > > > + return snprintf(hpp->buf, hpp->size, "%*s", width, hex_str(addr)); > > > > So here you get that static buffer and then truncate it? Wouldn't the > > perf_hpp stuff take care of this? Can't we stop using that static buffer > > and this truncation at such a level? > > I think we need to cut it on this level, but I actualy might recall some > change you did for perf_hpp to cut this on column width later on? > > I'll check on that.. Ok, would be great to have this sorted out in a better way.