From: Namhyung Kim <namhyung@kernel.org>
To: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>,
Paul Mackerras <paulus@samba.org>, Ingo Molnar <mingo@kernel.org>,
LKML <linux-kernel@vger.kernel.org>, Jiri Olsa <jolsa@redhat.com>
Subject: Re: [PATCHSET RESEND 0/5] perf tools: Cleanup hist printing code (v4)
Date: Sat, 08 Sep 2012 23:00:36 +0900 [thread overview]
Message-ID: <87ipboaa2j.fsf@kernel.org> (raw)
In-Reply-To: <20120908004854.GD20401@ghostprotocols.net> (Arnaldo Carvalho de Melo's message of "Fri, 7 Sep 2012 17:48:54 -0700")
Hi, Arnaldo
On Fri, 7 Sep 2012 17:48:54 -0700, Arnaldo Carvalho de Melo wrote:
> Em Mon, Sep 03, 2012 at 11:53:05AM +0900, Namhyung Kim escreveu:
>> This is a cleanup and refactoring patchset for the hist printing code
>> by adding perf_hpp__format functions and perf_hpp. I believe it makes
>> the code easy to maintain and to add new features like upcoming group
>> viewing and callchain accumulation.
>
> I applied this patch series to then get some patches from Jiri's 'perf
> diff' series, so that he can use what you did here, as you noticed the
> overlap when reviewing his series.
>
> But then 'perf diff' segfaults :-\
>
> I left your patch series + with that overhead column fixlet at my tree,
> branch tmp.perf/hpp.
>
> The segfaults happens here:
>
> Program received signal SIGSEGV, Segmentation fault.
> 0x0000000000498837 in hpp__entry_delta (hpp=0x7fffffffdeb0, he=0xcd6310) at ui/hist.c:200
> 200 old_percent = 100.0 * he->pair->period / old_total;
> Missing separate debuginfos, use: debuginfo-install atk-1.28.0-2.el6.x86_64 bzip2-libs-1.0.5-7.el6_0.x86_64 elfutils-libelf-0.152-1.el6.x86_64 elfutils-libs-0.152-1.el6.x86_64 expat-2.0.1-11.el6_2.x86_64 fontconfig-2.8.0-3.el6.x86_64 freetype-2.3.11-6.el6_2.9.x86_64 glib2-2.22.5-7.el6.x86_64 gtk2-2.18.9-10.el6.x86_64 libX11-1.3-2.el6.x86_64 libXau-1.0.5-1.el6.x86_64 libXcomposite-0.4.1-2.el6.x86_64 libXcursor-1.1.10-2.el6.x86_64 libXdamage-1.1.2-1.el6.x86_64 libXext-1.1-3.el6.x86_64 libXfixes-4.0.4-1.el6.x86_64 libXi-1.3-3.el6.x86_64 libXinerama-1.1-1.el6.x86_64 libXrandr-1.3.0-4.el6.x86_64 libXrender-0.9.5-1.el6.x86_64 libpng-1.2.49-1.el6_2.x86_64 libselinux-2.0.94-5.3.el6.x86_64 libxcb-1.5-1.el6.x86_64 nss-softokn-freebl-3.12.9-11.el6.x86_64 pango-1.28.1-3.el6_0.5.x86_64 perl-libs-5.10.1
> -127.el6.x86_64 pixman-0.18.4-1.el6_0.1.x86_64 xz-libs-4.999.9-0.3.beta.20091007git.el6.x86_64 zlib-1.2.3-27.el6.x86_64
> (gdb) p he->pair
> $1 = (struct hist_entry *) 0x0
>
> Please try with:
>
> perf record -a usleep 1
> perf record -a usleep 1
> perf diff
>
> it will use perf.data.old and perf.data and will segfault in that branch.
I see the problem. I overlooked he->pair can be NULL when perf diff is
running. So the fix will be adding a NULL check before the line. In
case of NULL, it will default to 0, so no problem.
Thanks,
Namhyung
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 16dc486d02be..3c2701fa6be1 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -196,7 +196,7 @@ static int hpp__entry_delta(struct perf_hpp *hpp, struct hist_entry *he)
char buf[32] = " ";
old_total = pair_hists->stats.total_period;
- if (old_total > 0)
+ if (old_total > 0 && he->pair)
old_percent = 100.0 * he->pair->period / old_total;
new_total = hpp->total_period;
next prev parent reply other threads:[~2012-09-08 14:00 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-03 2:53 [PATCHSET RESEND 0/5] perf tools: Cleanup hist printing code (v4) Namhyung Kim
2012-09-03 2:53 ` [PATCH 1/5] perf hists: Introduce perf_hpp for hist period printing Namhyung Kim
2012-09-09 8:54 ` [tip:perf/core] " tip-bot for Namhyung Kim
2012-09-12 17:26 ` Robert Richter
2012-09-12 18:48 ` Arnaldo Carvalho de Melo
2012-09-13 2:43 ` Namhyung Kim
2012-09-13 3:51 ` Arnaldo Carvalho de Melo
2012-09-13 4:14 ` [PATCH] perf record: Add missing perf_hpp__init for pipe-mode Namhyung Kim
2012-09-13 4:21 ` Namhyung Kim
2012-09-13 9:48 ` Robert Richter
2012-09-19 15:25 ` [tip:perf/core] perf report: " tip-bot for Namhyung Kim
2012-09-03 2:53 ` [PATCH 2/5] perf hists: Handle field separator properly Namhyung Kim
2012-09-09 8:55 ` [tip:perf/core] " tip-bot for Namhyung Kim
2012-09-03 2:53 ` [PATCH 3/5] perf hists: Use perf_hpp__format->width to calculate the column widths Namhyung Kim
2012-09-09 8:56 ` [tip:perf/core] perf hists: Use perf_hpp__format-> width " tip-bot for Namhyung Kim
2012-09-03 2:53 ` [PATCH 4/5] perf ui/browser: Use perf_hpp__format functions Namhyung Kim
2012-09-08 0:32 ` Arnaldo Carvalho de Melo
2012-09-08 14:05 ` Namhyung Kim
2012-09-12 6:25 ` Namhyung Kim
2012-09-09 8:57 ` [tip:perf/core] perf hists browser: " tip-bot for Namhyung Kim
2012-09-03 2:53 ` [PATCH 5/5] perf gtk/browser: " Namhyung Kim
2012-09-09 8:58 ` [tip:perf/core] " tip-bot for Namhyung Kim
2012-09-08 0:48 ` [PATCHSET RESEND 0/5] perf tools: Cleanup hist printing code (v4) Arnaldo Carvalho de Melo
2012-09-08 14:00 ` Namhyung Kim [this message]
2012-09-08 15:08 ` Arnaldo Carvalho de Melo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87ipboaa2j.fsf@kernel.org \
--to=namhyung@kernel.org \
--cc=a.p.zijlstra@chello.nl \
--cc=acme@ghostprotocols.net \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=paulus@samba.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).