From: David Ahern <dsahern@gmail.com>
To: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Ingo Molnar <mingo@kernel.org>,
linux-kernel@vger.kernel.org,
Arnaldo Carvalho de Melo <acme@redhat.com>,
Frederic Weisbecker <fweisbec@gmail.com>,
Mike Galbraith <efault@gmx.de>, Namhyung Kim <namhyung@gmail.com>,
Paul Mackerras <paulus@samba.org>,
Peter Zijlstra <peterz@infradead.org>,
Stephane Eranian <eranian@google.com>
Subject: Re: [PATCH 3/4] perf top: Fix counter name fixup when fallbacking to cpu-clock
Date: Fri, 25 May 2012 14:26:34 -0600 [thread overview]
Message-ID: <4FBFEAFA.8050004@gmail.com> (raw)
In-Reply-To: <1337975958-24047-4-git-send-email-acme@infradead.org>
On 5/25/12 1:59 PM, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo<acme@redhat.com>
>
> In 40491eaa "perf top: Update event name when falling back to cpu-clock"
> we freed counter->name but didn't reset it to NULL, then when setting it
> to the result of event_name(), event_name() would use the cached value,
> which by now was overwritten and thus we got garbage or a zero lenght
> string.
>
> Fix it by just freeing and setting counter->name to NULL, this way
> event_name() when called afterwards, will find the right counter name
> and cache it again.
>
> Found while trying 'cycles:pp' on a machine were :pp couldn't be
> honoured. Probably the best fallback here is to tell the user that that
> level of precision is not available on the PMU and then go removing 'p',
> levels of precision till we get to play 'cycles' and if even that fails,
> _then_ get to 'cpu-clock'.
>
> But that is the matter for another patch, this one just needs to fix the
> caching issue, which in the end will show 'cpu-clock' when tools ask for
> the event name being used, which clarifies things for the user, that
> will see that 'cycles:pp' or whatever not support event is not being
> used, some sort of fallback happened.
>
> Cc: David Ahern<dsahern@gmail.com>
> Cc: Frederic Weisbecker<fweisbec@gmail.com>
> Cc: Mike Galbraith<efault@gmx.de>
> Cc: Namhyung Kim<namhyung@gmail.com>
> Cc: Paul Mackerras<paulus@samba.org>
> Cc: Peter Zijlstra<peterz@infradead.org>
> Cc: Stephane Eranian<eranian@google.com>
> Link: http://lkml.kernel.org/n/tip-w1neie2dqli89we1bzwkf4id@git.kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo<acme@redhat.com>
> ---
> tools/perf/builtin-top.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 6031dce..d4a5f9b 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -953,7 +953,7 @@ try_again:
> attr->config = PERF_COUNT_SW_CPU_CLOCK;
> if (counter->name) {
> free(counter->name);
> - counter->name = strdup(event_name(counter));
> + counter->name = NULL;
> }
> goto try_again;
> }
The patch I sent had:
+ if (counter->name) {
+ free(counter->name);
+ counter->name = NULL;
+ counter->name = strdup(event_name(counter));
+ }
See http://lkml.indiana.edu/hypermail/linux/kernel/1205.1/00390.html
next prev parent reply other threads:[~2012-05-25 20:26 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-25 19:59 [GIT PULL 0/4] perf/urgent fixes Arnaldo Carvalho de Melo
2012-05-25 19:59 ` [PATCH 1/4] perf tools: Do not use _FORTIFY_SOURCE when DEBUG=1 is specified Arnaldo Carvalho de Melo
2012-05-25 19:59 ` [PATCH 2/4] perf tools: fix thread_map__new_by_pid_str() memory leak in error path Arnaldo Carvalho de Melo
2012-05-25 19:59 ` [PATCH 3/4] perf top: Fix counter name fixup when fallbacking to cpu-clock Arnaldo Carvalho de Melo
2012-05-25 20:26 ` David Ahern [this message]
2012-05-25 21:30 ` Arnaldo Carvalho de Melo
2012-05-25 19:59 ` [PATCH 4/4] perf tools: Reconstruct event with modifiers from perf_event_attr Arnaldo Carvalho de Melo
2012-05-25 21:46 ` [GIT PULL 0/4] perf/urgent fixes 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=4FBFEAFA.8050004@gmail.com \
--to=dsahern@gmail.com \
--cc=acme@ghostprotocols.net \
--cc=acme@redhat.com \
--cc=efault@gmx.de \
--cc=eranian@google.com \
--cc=fweisbec@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=namhyung@gmail.com \
--cc=paulus@samba.org \
--cc=peterz@infradead.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).