From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755983AbcCCJyE (ORCPT ); Thu, 3 Mar 2016 04:54:04 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36860 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751279AbcCCJx6 (ORCPT ); Thu, 3 Mar 2016 04:53:58 -0500 Date: Thu, 3 Mar 2016 10:53:48 +0100 From: Jiri Olsa To: Ingo Molnar Cc: Arnaldo Carvalho de Melo , linux-kernel@vger.kernel.org, Alexei Starovoitov , Andi Kleen , Brendan Gregg , David Ahern , He Kuang , Jeff Bastian , Jeremie Galarneau , Jiri Olsa , Josh Boyer , Lai Jiangshan , Li Zefan , Masami Hiramatsu , Namhyung Kim , Peter Zijlstra , pi3orama@163.com, Stephane Eranian , Steven Rostedt , Taeung Song , Thomas Gleixner , Wang Nan , Arnaldo Carvalho de Melo Subject: [PATCH] perf tools: Fix locale handling in pmu parsing Message-ID: <20160303095348.GA24511@krava.redhat.com> References: <1456773727-3005-1-git-send-email-acme@kernel.org> <20160303082130.GA13523@gmail.com> <20160303091522.GA14355@krava.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160303091522.GA14355@krava.redhat.com> 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 Thu, Mar 03, 2016 at 10:15:22AM +0100, Jiri Olsa wrote: SNIP > > > > 11989.280397 task-clock (msec) # 11.981 CPUs utilized > > 1299 context-switches # 0.108 K/sec > > 6 cpu-migrations # 0.001 K/sec > > 70 page-faults # 0.006 K/sec > > 127008602 cycles # 0.011 GHz > > 279538533 stalled-cycles-frontend # 220.09% frontend cycles idle > > 119213269 instructions # 0.94 insn per cycle > > # 2.34 stalled cycles per insn > > 24166678 branches # 2.016 M/sec > > 505681 branch-misses # 2.09% of all branches > > > > 1.000684278 seconds time elapsed > > > > > > ... see how the numbers became human-unreadable, losing the big-number separator? > > > > I suspect it's due to the following commit: > > > > fa184776ac27 perf stat: Check existence of frontend/backed stalled cycles > > yea, it used the pmu parsing which screwes locales, > following patch fixed that for me.. > resending with full changelog jirka --- Ingo reported regression on display format of big numbers, which is missing separators (in default perf stat output). triton:~/tip> perf stat -a sleep 1 ... 127008602 cycles # 0.011 GHz 279538533 stalled-cycles-frontend # 220.09% frontend cycles idle 119213269 instructions # 0.94 insn per cycle This is caused by recent change: perf stat: Check existence of frontend/backed stalled cycles that added call to pmu_have_event, that subsequently calls perf_pmu__parse_scale, which has a bug in locale handling. The lc string returned from setlocale, that we use to store old locale value, may be allocated in static storage. Getting a dynamic copy to make it survive another setlocale call. [jolsa@krava perf]$ perf stat ls ... 2,360,602 cycles # 3.080 GHz 2,703,090 instructions # 1.15 insn per cycle 546,031 branches # 712.511 M/sec Reported-by: Ingo Molnar Cc: David Ahern Cc: Namhyung Kim Cc: Peter Zijlstra Signed-off-by: Jiri Olsa --- tools/perf/util/pmu.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index ce61f79dbaae..d8cd038baed2 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -124,6 +124,17 @@ static int perf_pmu__parse_scale(struct perf_pmu_alias *alias, char *dir, char * lc = setlocale(LC_NUMERIC, NULL); /* + * The lc string may be allocated in static storage, + * so get a dynamic copy to make it survive setlocale + * call below. + */ + lc = strdup(lc); + if (!lc) { + ret = -ENOMEM; + goto error; + } + + /* * force to C locale to ensure kernel * scale string is converted correctly. * kernel uses default C locale. @@ -135,6 +146,8 @@ static int perf_pmu__parse_scale(struct perf_pmu_alias *alias, char *dir, char * /* restore locale */ setlocale(LC_NUMERIC, lc); + free((char *) lc); + ret = 0; error: close(fd); -- 2.4.3