From: Will Deacon <will.deacon@arm.com>
To: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: "mingo@elte.hu" <mingo@elte.hu>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"gregkh@suse.de" <gregkh@suse.de>,
Arnaldo Carvalho de Melo <acme@infradead.org>
Subject: Re: perf event group siblings not counting in mainline
Date: Tue, 10 Jan 2012 11:56:02 +0000 [thread overview]
Message-ID: <20120110115602.GC24180@mudshark.cambridge.arm.com> (raw)
In-Reply-To: <1326194188.2442.106.camel@twins>
On Tue, Jan 10, 2012 at 11:16:28AM +0000, Peter Zijlstra wrote:
> On Tue, 2012-01-10 at 11:09 +0000, Will Deacon wrote:
> > On Sat, Dec 24, 2011 at 12:54:15PM +0000, Will Deacon wrote:
> > > If I try using perf with event groups on a mainline kernel, only the group
> > > leader counts:
[...]
> > Now that 3.2 is out with this regression, would it be possible to get this
> > (b79387ef ("perf: Fix enable_on_exec for sibling events")) into -stable please?
>
> How can this be a regression? Its always been like that. At worst its a
> broken new feature.
My mistake, for some reason I thought this had always had the new behaviour.
> Also, you really shouldn't set enable_on_exec on non group leaders,
> that's just daft. But yes that patch sorts it. Something like the below
> should too.
>From a perf stat perspective, it should probably do the right thing then.
>
> ---
> tools/perf/builtin-stat.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index f5d2a63..623519f 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -298,7 +298,7 @@ static int create_perf_stat_counter(struct perf_evsel *evsel,
> group, group_fd);
> if (target_pid == -1 && target_tid == -1) {
> attr->disabled = 1;
> - attr->enable_on_exec = 1;
> + attr->enable_on_exec = !group_fd;
> }
>
> return perf_evsel__open_per_thread(evsel, evsel_list->threads,
Had to make a small change so that that the siblings don't get stuck as
disabled:
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 955930e..a0f4b26 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -296,10 +296,9 @@ static int create_perf_stat_counter(struct perf_evsel *evsel,
if (system_wide)
return perf_evsel__open_per_cpu(evsel, evsel_list->cpus,
group, group_fd);
- if (target_pid == -1 && target_tid == -1) {
- attr->disabled = 1;
- attr->enable_on_exec = 1;
- }
+
+ if (target_pid == -1 && target_tid == -1)
+ attr->disabled = attr->enable_on_exec = !group_fd;
return perf_evsel__open_per_thread(evsel, evsel_list->threads,
group, group_fd);
but that seems to do it - thanks!
Tested-by: Will Deacon <will.deacon@arm.com>
It would be nice to see this in -stable, but as it's not a regression I'll
leave it up to you.
Cheers,
Will
prev parent reply other threads:[~2012-01-10 11:57 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-24 12:54 perf event group siblings not counting in mainline Will Deacon
2012-01-10 11:09 ` Will Deacon
2012-01-10 11:16 ` Peter Zijlstra
2012-01-10 11:56 ` Will Deacon [this message]
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=20120110115602.GC24180@mudshark.cambridge.arm.com \
--to=will.deacon@arm.com \
--cc=a.p.zijlstra@chello.nl \
--cc=acme@infradead.org \
--cc=gregkh@suse.de \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
/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).