* [RFC PATCH 1/2] perf: Enable group siblings on exec if attr::enable_on_exec set
@ 2010-11-10 6:15 Lin Ming
2010-11-10 12:19 ` Peter Zijlstra
0 siblings, 1 reply; 6+ messages in thread
From: Lin Ming @ 2010-11-10 6:15 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Matt Fleming
Cc: Zhang Rui, Frederic Weisbecker, lkml, Arnaldo Carvalho de Melo
Currently, only group leader is enabled on exec.
This patch enables all group events on exec if attr::enable_on_exec is
set.
This is needed by next patch.
Signed-off-by: Lin Ming <ming.m.lin@intel.com>
---
kernel/perf_event.c | 24 +++++++++++++++++++++---
1 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/kernel/perf_event.c b/kernel/perf_event.c
index 517d827..b94c18d 100644
--- a/kernel/perf_event.c
+++ b/kernel/perf_event.c
@@ -1700,11 +1700,29 @@ static int event_enable_on_exec(struct perf_event *event,
if (event->state >= PERF_EVENT_STATE_INACTIVE)
return 0;
- __perf_event_mark_enabled(event, ctx);
+ event->state = PERF_EVENT_STATE_INACTIVE;
return 1;
}
+static int group_enable_on_exec(struct perf_event *event,
+ struct perf_event_context *ctx)
+{
+ int enabled = 0;
+ struct perf_event *sub;
+
+ list_for_each_entry(sub, &event->sibling_list, group_entry)
+ if (event_enable_on_exec(sub, ctx))
+ enabled++;
+
+ if (event_enable_on_exec(event, ctx))
+ enabled++;
+
+ __perf_event_mark_enabled(event, ctx);
+
+ return enabled;
+}
+
/*
* Enable all of a task's events that have been marked enable-on-exec.
* This expects task == current.
@@ -1725,13 +1743,13 @@ static void perf_event_enable_on_exec(struct perf_event_context *ctx)
raw_spin_lock(&ctx->lock);
list_for_each_entry(event, &ctx->pinned_groups, group_entry) {
- ret = event_enable_on_exec(event, ctx);
+ ret = group_enable_on_exec(event, ctx);
if (ret)
enabled = 1;
}
list_for_each_entry(event, &ctx->flexible_groups, group_entry) {
- ret = event_enable_on_exec(event, ctx);
+ ret = group_enable_on_exec(event, ctx);
if (ret)
enabled = 1;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 1/2] perf: Enable group siblings on exec if attr::enable_on_exec set
2010-11-10 6:15 [RFC PATCH 1/2] perf: Enable group siblings on exec if attr::enable_on_exec set Lin Ming
@ 2010-11-10 12:19 ` Peter Zijlstra
2010-11-10 14:17 ` Lin Ming
0 siblings, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2010-11-10 12:19 UTC (permalink / raw)
To: Lin Ming
Cc: Ingo Molnar, Matt Fleming, Zhang Rui, Frederic Weisbecker, lkml,
Arnaldo Carvalho de Melo
On Wed, 2010-11-10 at 14:15 +0800, Lin Ming wrote:
> Currently, only group leader is enabled on exec.
That's enough, right? If all sibling events are already enabled enabling
the group leader makes the whole thing go.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 1/2] perf: Enable group siblings on exec if attr::enable_on_exec set
2010-11-10 12:19 ` Peter Zijlstra
@ 2010-11-10 14:17 ` Lin Ming
2010-11-10 14:38 ` Peter Zijlstra
0 siblings, 1 reply; 6+ messages in thread
From: Lin Ming @ 2010-11-10 14:17 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Ingo Molnar, Matt Fleming, Zhang, Rui, Frederic Weisbecker, lkml,
Arnaldo Carvalho de Melo
On Wed, 2010-11-10 at 20:19 +0800, Peter Zijlstra wrote:
> On Wed, 2010-11-10 at 14:15 +0800, Lin Ming wrote:
> > Currently, only group leader is enabled on exec.
>
> That's enough, right? If all sibling events are already enabled enabling
> the group leader makes the whole thing go.
No.
If the event group is disabled by default("perf stat" case) and will be
enabled at next exec, then actually only the group leader will be
enabled , because all siblings are explicitly disabled(->state ==
PERF_EVENT_STATE_OFF). So the siblings will never be enabled.
/*
* Put a event into inactive state and update time fields.
* Enabling the leader of a group effectively enables all
* the group members that aren't explicitly disabled, so we
* have to update their ->tstamp_enabled also.
* Note: this works for group members as well as group leaders
* since the non-leader members' sibling_lists will be empty.
*/
static void __perf_event_mark_enabled(struct perf_event *event,
struct perf_event_context *ctx)
{
struct perf_event *sub;
event->state = PERF_EVENT_STATE_INACTIVE;
event->tstamp_enabled = ctx->time - event->total_time_enabled;
list_for_each_entry(sub, &event->sibling_list, group_entry)
if (sub->state >= PERF_EVENT_STATE_INACTIVE)
sub->tstamp_enabled =
ctx->time - sub->total_time_enabled;
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 1/2] perf: Enable group siblings on exec if attr::enable_on_exec set
2010-11-10 14:17 ` Lin Ming
@ 2010-11-10 14:38 ` Peter Zijlstra
2010-11-10 14:49 ` Lin Ming
0 siblings, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2010-11-10 14:38 UTC (permalink / raw)
To: Lin Ming
Cc: Ingo Molnar, Matt Fleming, Zhang, Rui, Frederic Weisbecker, lkml,
Arnaldo Carvalho de Melo
On Wed, 2010-11-10 at 22:17 +0800, Lin Ming wrote:
> On Wed, 2010-11-10 at 20:19 +0800, Peter Zijlstra wrote:
> > On Wed, 2010-11-10 at 14:15 +0800, Lin Ming wrote:
> > > Currently, only group leader is enabled on exec.
> >
> > That's enough, right? If all sibling events are already enabled enabling
> > the group leader makes the whole thing go.
>
> No.
>
> If the event group is disabled by default("perf stat" case) and will be
> enabled at next exec, then actually only the group leader will be
> enabled , because all siblings are explicitly disabled(->state ==
> PERF_EVENT_STATE_OFF). So the siblings will never be enabled.
Right, so what I was saying is, don't disable all the siblings by
default, so that only the group leader will be disabled. In that case
nothing will get scheduled because the group leader is not enabled.
So: {disabled, enabled, enabled} will not actually get scheduled and the
group as a whole is effectively disabled. Once the enabled_on_exec flips
the group leader to 'enabled' the whole group becomes enabled.
I don't mind the patch too much (although I would like to avoid that
double sibling iteration in both group_enable_on_exec() and
__perf_event_mark_enabled()), but I think it can already work as is.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 1/2] perf: Enable group siblings on exec if attr::enable_on_exec set
2010-11-10 14:38 ` Peter Zijlstra
@ 2010-11-10 14:49 ` Lin Ming
2010-11-10 14:54 ` Peter Zijlstra
0 siblings, 1 reply; 6+ messages in thread
From: Lin Ming @ 2010-11-10 14:49 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Ingo Molnar, Matt Fleming, Zhang, Rui, Frederic Weisbecker, lkml,
Arnaldo Carvalho de Melo
On Wed, 2010-11-10 at 22:38 +0800, Peter Zijlstra wrote:
> On Wed, 2010-11-10 at 22:17 +0800, Lin Ming wrote:
> > On Wed, 2010-11-10 at 20:19 +0800, Peter Zijlstra wrote:
> > > On Wed, 2010-11-10 at 14:15 +0800, Lin Ming wrote:
> > > > Currently, only group leader is enabled on exec.
> > >
> > > That's enough, right? If all sibling events are already enabled enabling
> > > the group leader makes the whole thing go.
> >
> > No.
> >
> > If the event group is disabled by default("perf stat" case) and will be
> > enabled at next exec, then actually only the group leader will be
> > enabled , because all siblings are explicitly disabled(->state ==
> > PERF_EVENT_STATE_OFF). So the siblings will never be enabled.
>
> Right, so what I was saying is, don't disable all the siblings by
> default, so that only the group leader will be disabled. In that case
> nothing will get scheduled because the group leader is not enabled.
>
> So: {disabled, enabled, enabled} will not actually get scheduled and the
> group as a whole is effectively disabled. Once the enabled_on_exec flips
> the group leader to 'enabled' the whole group becomes enabled.
>
> I don't mind the patch too much (although I would like to avoid that
> double sibling iteration in both group_enable_on_exec() and
> __perf_event_mark_enabled()), but I think it can already work as is.
I got it now!
Another stupid patch, sorry.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 1/2] perf: Enable group siblings on exec if attr::enable_on_exec set
2010-11-10 14:49 ` Lin Ming
@ 2010-11-10 14:54 ` Peter Zijlstra
0 siblings, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2010-11-10 14:54 UTC (permalink / raw)
To: Lin Ming
Cc: Ingo Molnar, Matt Fleming, Zhang, Rui, Frederic Weisbecker, lkml,
Arnaldo Carvalho de Melo
On Wed, 2010-11-10 at 22:49 +0800, Lin Ming wrote:
> > I don't mind the patch too much (although I would like to avoid that
> > double sibling iteration in both group_enable_on_exec() and
> > __perf_event_mark_enabled()), but I think it can already work as is.
>
> I got it now!
>
> Another stupid patch, sorry.
Well, I wouldn't call it a stupid patch, it does fix something that is
evidently non-obvious so it might be worth considering it on that merit.
I'm simply pointing out that what you're trying to achieve might be
possible without changing code.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-11-10 14:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-10 6:15 [RFC PATCH 1/2] perf: Enable group siblings on exec if attr::enable_on_exec set Lin Ming
2010-11-10 12:19 ` Peter Zijlstra
2010-11-10 14:17 ` Lin Ming
2010-11-10 14:38 ` Peter Zijlstra
2010-11-10 14:49 ` Lin Ming
2010-11-10 14:54 ` Peter Zijlstra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox