* [PATCH] perf: Detach group events when removing event from ctx
@ 2010-10-03 19:35 Matt Fleming
2010-10-11 10:25 ` Matt Fleming
2010-10-15 9:42 ` Peter Zijlstra
0 siblings, 2 replies; 5+ messages in thread
From: Matt Fleming @ 2010-10-03 19:35 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Frederic Weisbecker
When removing an event from a task's list of events we need to make sure
that we also detach all group events. If we don't, then when we later
call perf_group_attach() we'll hit the WARN_ON_ONCE() like so,
[ 1200.449161] ------------[ cut here ]------------
[ 1200.449171] WARNING: at kernel/perf_event.c:313 add_event_to_ctx+0xe1/0x159()
[ 1200.449174] Hardware name: MacBook2,1
[ 1200.449176] Modules linked in: dm_mod sg sd_mod
[ 1200.449183] Pid: 7619, comm: perf Not tainted 2.6.36-rc6-tip+ #2
[ 1200.449186] Call Trace:
[ 1200.449193] [<ffffffff8103b71e>] warn_slowpath_common+0x85/0x9d
[ 1200.449197] [<ffffffff8103b750>] warn_slowpath_null+0x1a/0x1c
[ 1200.449201] [<ffffffff810a16fb>] add_event_to_ctx+0xe1/0x159
[ 1200.449206] [<ffffffff810a2f01>] perf_install_in_context+0x85/0x99
[ 1200.449210] [<ffffffff810a7811>] sys_perf_event_open+0x4da/0x633
[ 1200.449216] [<ffffffff81002b1b>] system_call_fastpath+0x16/0x1b
[ 1200.449218] ---[ end trace 9df38c30c6cacb50 ]---
Signed-off-by: Matt Fleming <matt@console-pimps.org>
---
kernel/perf_event.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/kernel/perf_event.c b/kernel/perf_event.c
index c16158c..c13d869 100644
--- a/kernel/perf_event.c
+++ b/kernel/perf_event.c
@@ -550,8 +550,11 @@ retry:
* can remove the event safely, if the call above did not
* succeed.
*/
- if (!list_empty(&event->group_entry))
+ if (!list_empty(&event->group_entry)) {
+ perf_group_detach(event);
list_del_event(event, ctx);
+ }
+
raw_spin_unlock_irq(&ctx->lock);
}
--
1.7.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] perf: Detach group events when removing event from ctx
2010-10-03 19:35 [PATCH] perf: Detach group events when removing event from ctx Matt Fleming
@ 2010-10-11 10:25 ` Matt Fleming
2010-10-15 9:42 ` Peter Zijlstra
1 sibling, 0 replies; 5+ messages in thread
From: Matt Fleming @ 2010-10-11 10:25 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Frederic Weisbecker
On Sun, Oct 03, 2010 at 08:35:50PM +0100, Matt Fleming wrote:
> When removing an event from a task's list of events we need to make sure
> that we also detach all group events. If we don't, then when we later
> call perf_group_attach() we'll hit the WARN_ON_ONCE() like so,
>
> [ 1200.449161] ------------[ cut here ]------------
> [ 1200.449171] WARNING: at kernel/perf_event.c:313 add_event_to_ctx+0xe1/0x159()
> [ 1200.449174] Hardware name: MacBook2,1
> [ 1200.449176] Modules linked in: dm_mod sg sd_mod
> [ 1200.449183] Pid: 7619, comm: perf Not tainted 2.6.36-rc6-tip+ #2
> [ 1200.449186] Call Trace:
> [ 1200.449193] [<ffffffff8103b71e>] warn_slowpath_common+0x85/0x9d
> [ 1200.449197] [<ffffffff8103b750>] warn_slowpath_null+0x1a/0x1c
> [ 1200.449201] [<ffffffff810a16fb>] add_event_to_ctx+0xe1/0x159
> [ 1200.449206] [<ffffffff810a2f01>] perf_install_in_context+0x85/0x99
> [ 1200.449210] [<ffffffff810a7811>] sys_perf_event_open+0x4da/0x633
> [ 1200.449216] [<ffffffff81002b1b>] system_call_fastpath+0x16/0x1b
> [ 1200.449218] ---[ end trace 9df38c30c6cacb50 ]---
>
> Signed-off-by: Matt Fleming <matt@console-pimps.org>
> ---
> kernel/perf_event.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/perf_event.c b/kernel/perf_event.c
> index c16158c..c13d869 100644
> --- a/kernel/perf_event.c
> +++ b/kernel/perf_event.c
> @@ -550,8 +550,11 @@ retry:
> * can remove the event safely, if the call above did not
> * succeed.
> */
> - if (!list_empty(&event->group_entry))
> + if (!list_empty(&event->group_entry)) {
> + perf_group_detach(event);
> list_del_event(event, ctx);
> + }
> +
> raw_spin_unlock_irq(&ctx->lock);
> }
>
> --
> 1.7.1
Ping?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf: Detach group events when removing event from ctx
2010-10-03 19:35 [PATCH] perf: Detach group events when removing event from ctx Matt Fleming
2010-10-11 10:25 ` Matt Fleming
@ 2010-10-15 9:42 ` Peter Zijlstra
2010-10-16 11:12 ` Matt Fleming
2010-10-18 19:19 ` [tip:perf/core] perf: Fix group moving tip-bot for Peter Zijlstra
1 sibling, 2 replies; 5+ messages in thread
From: Peter Zijlstra @ 2010-10-15 9:42 UTC (permalink / raw)
To: Matt Fleming
Cc: linux-kernel, Ingo Molnar, Arnaldo Carvalho de Melo, Frederic Weisbecker
On Sun, 2010-10-03 at 20:35 +0100, Matt Fleming wrote:
> When removing an event from a task's list of events we need to make sure
> that we also detach all group events. If we don't, then when we later
> call perf_group_attach() we'll hit the WARN_ON_ONCE() like so,
>
> [ 1200.449161] ------------[ cut here ]------------
> [ 1200.449171] WARNING: at kernel/perf_event.c:313 add_event_to_ctx+0xe1/0x159()
> [ 1200.449174] Hardware name: MacBook2,1
> [ 1200.449176] Modules linked in: dm_mod sg sd_mod
> [ 1200.449183] Pid: 7619, comm: perf Not tainted 2.6.36-rc6-tip+ #2
> [ 1200.449186] Call Trace:
> [ 1200.449193] [<ffffffff8103b71e>] warn_slowpath_common+0x85/0x9d
> [ 1200.449197] [<ffffffff8103b750>] warn_slowpath_null+0x1a/0x1c
> [ 1200.449201] [<ffffffff810a16fb>] add_event_to_ctx+0xe1/0x159
> [ 1200.449206] [<ffffffff810a2f01>] perf_install_in_context+0x85/0x99
> [ 1200.449210] [<ffffffff810a7811>] sys_perf_event_open+0x4da/0x633
> [ 1200.449216] [<ffffffff81002b1b>] system_call_fastpath+0x16/0x1b
> [ 1200.449218] ---[ end trace 9df38c30c6cacb50 ]---
I think its due to the new and fancy move groups around context code
from: b04243ef70 (perf: Complete software pmu grouping).
Does the below cure it?
It only detaches the group when we move it from one context to another,
the perf_install_in_context() later on will then re-assemble the group
on the other context.
---
Index: linux-2.6/kernel/perf_event.c
===================================================================
--- linux-2.6.orig/kernel/perf_event.c
+++ linux-2.6/kernel/perf_event.c
@@ -5592,6 +5592,7 @@ SYSCALL_DEFINE5(perf_event_open,
list_for_each_entry(sibling, &group_leader->sibling_list,
group_entry) {
perf_event_remove_from_context(sibling);
+ perf_group_detach(sibling);
put_ctx(gctx);
}
mutex_unlock(&gctx->mutex);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf: Detach group events when removing event from ctx
2010-10-15 9:42 ` Peter Zijlstra
@ 2010-10-16 11:12 ` Matt Fleming
2010-10-18 19:19 ` [tip:perf/core] perf: Fix group moving tip-bot for Peter Zijlstra
1 sibling, 0 replies; 5+ messages in thread
From: Matt Fleming @ 2010-10-16 11:12 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, Ingo Molnar, Arnaldo Carvalho de Melo, Frederic Weisbecker
On Fri, Oct 15, 2010 at 11:42:37AM +0200, Peter Zijlstra wrote:
> On Sun, 2010-10-03 at 20:35 +0100, Matt Fleming wrote:
> > When removing an event from a task's list of events we need to make sure
> > that we also detach all group events. If we don't, then when we later
> > call perf_group_attach() we'll hit the WARN_ON_ONCE() like so,
> >
> > [ 1200.449161] ------------[ cut here ]------------
> > [ 1200.449171] WARNING: at kernel/perf_event.c:313 add_event_to_ctx+0xe1/0x159()
> > [ 1200.449174] Hardware name: MacBook2,1
> > [ 1200.449176] Modules linked in: dm_mod sg sd_mod
> > [ 1200.449183] Pid: 7619, comm: perf Not tainted 2.6.36-rc6-tip+ #2
> > [ 1200.449186] Call Trace:
> > [ 1200.449193] [<ffffffff8103b71e>] warn_slowpath_common+0x85/0x9d
> > [ 1200.449197] [<ffffffff8103b750>] warn_slowpath_null+0x1a/0x1c
> > [ 1200.449201] [<ffffffff810a16fb>] add_event_to_ctx+0xe1/0x159
> > [ 1200.449206] [<ffffffff810a2f01>] perf_install_in_context+0x85/0x99
> > [ 1200.449210] [<ffffffff810a7811>] sys_perf_event_open+0x4da/0x633
> > [ 1200.449216] [<ffffffff81002b1b>] system_call_fastpath+0x16/0x1b
> > [ 1200.449218] ---[ end trace 9df38c30c6cacb50 ]---
>
> I think its due to the new and fancy move groups around context code
> from: b04243ef70 (perf: Complete software pmu grouping).
>
> Does the below cure it?
>
> It only detaches the group when we move it from one context to another,
> the perf_install_in_context() later on will then re-assemble the group
> on the other context.
>
> ---
> Index: linux-2.6/kernel/perf_event.c
> ===================================================================
> --- linux-2.6.orig/kernel/perf_event.c
> +++ linux-2.6/kernel/perf_event.c
> @@ -5592,6 +5592,7 @@ SYSCALL_DEFINE5(perf_event_open,
> list_for_each_entry(sibling, &group_leader->sibling_list,
> group_entry) {
> perf_event_remove_from_context(sibling);
> + perf_group_detach(sibling);
> put_ctx(gctx);
> }
> mutex_unlock(&gctx->mutex);
>
Unfortunately this still hits the WARN_ON_ONCE() :-(
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip:perf/core] perf: Fix group moving
2010-10-15 9:42 ` Peter Zijlstra
2010-10-16 11:12 ` Matt Fleming
@ 2010-10-18 19:19 ` tip-bot for Peter Zijlstra
1 sibling, 0 replies; 5+ messages in thread
From: tip-bot for Peter Zijlstra @ 2010-10-18 19:19 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, a.p.zijlstra, matt, tglx, mingo
Commit-ID: 74c3337c2fc6389d3a57a622a936036b6db6b2e8
Gitweb: http://git.kernel.org/tip/74c3337c2fc6389d3a57a622a936036b6db6b2e8
Author: Peter Zijlstra <a.p.zijlstra@chello.nl>
AuthorDate: Fri, 15 Oct 2010 11:40:29 +0200
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 18 Oct 2010 19:58:51 +0200
perf: Fix group moving
Matt found we trigger the WARN_ON_ONCE() in perf_group_attach() when we take
the move_group path in perf_event_open().
Since we cannot de-construct the group (we rely on it to move the events), we
have to simply ignore the double attach. The group state is context invariant
and doesn't need changing.
Reported-by: Matt Fleming <matt@console-pimps.org>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <1287135757.29097.1368.camel@twins>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
kernel/perf_event.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/kernel/perf_event.c b/kernel/perf_event.c
index 99b9700..346dc0e 100644
--- a/kernel/perf_event.c
+++ b/kernel/perf_event.c
@@ -315,7 +315,12 @@ static void perf_group_attach(struct perf_event *event)
{
struct perf_event *group_leader = event->group_leader;
- WARN_ON_ONCE(event->attach_state & PERF_ATTACH_GROUP);
+ /*
+ * We can have double attach due to group movement in perf_event_open.
+ */
+ if (event->attach_state & PERF_ATTACH_GROUP)
+ return;
+
event->attach_state |= PERF_ATTACH_GROUP;
if (group_leader == event)
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-10-18 19:20 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-03 19:35 [PATCH] perf: Detach group events when removing event from ctx Matt Fleming
2010-10-11 10:25 ` Matt Fleming
2010-10-15 9:42 ` Peter Zijlstra
2010-10-16 11:12 ` Matt Fleming
2010-10-18 19:19 ` [tip:perf/core] perf: Fix group moving tip-bot for Peter Zijlstra
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.