* [PATCH 1/2] perf: Clone child context from parent context pmu
@ 2013-07-09 15:44 Jiri Olsa
2013-07-09 15:44 ` [PATCH 2/2] perf: Remove WARN_ON_ONCE check in __perf_event_enable for valid scenario Jiri Olsa
2013-07-12 13:28 ` [tip:perf/core] perf: Clone child context from parent context pmu tip-bot for Jiri Olsa
0 siblings, 2 replies; 5+ messages in thread
From: Jiri Olsa @ 2013-07-09 15:44 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Corey Ashford, Frederic Weisbecker, Ingo Molnar,
Namhyung Kim, Paul Mackerras, Peter Zijlstra,
Arnaldo Carvalho de Melo, Vince Weaver
Currently when the child context for inherited events is
created, it's based on the pmu object of the first event
of the parent context.
This is wrong for following scenario:
- HW context having HW and SW event
- HW event got removed (closed)
- SW event stays in HW context as the only event
and its pmu is used to clone the child context
The issue starts when the cpu context object is touched
based on the pmu context object (__get_cpu_context). In
this case the HW context will work with SW cpu context
ending up with following WARN below.
Fixing this by using parent context pmu object to clone
from child context.
[ 2716.472065] ------------[ cut here ]------------
[ 2716.476035] WARNING: at kernel/events/core.c:2122 task_ctx_sched_out+0x3c/0x)
[ 2716.476035] Modules linked in: nfsd auth_rpcgss oid_registry nfs_acl nfs locn
[ 2716.476035] CPU: 0 PID: 3164 Comm: perf_fuzzer Not tainted 3.10.0-rc4 #2
[ 2716.476035] Hardware name: AOpen DE7000/nMCP7ALPx-DE R1.06 Oct.19.2012, BI2
[ 2716.476035] 0000000000000000 ffffffff8102e215 0000000000000000 ffff88011fc18
[ 2716.476035] ffff8801175557f0 0000000000000000 ffff880119fda88c ffffffff810ad
[ 2716.476035] ffff880119fda880 ffffffff810af02a 0000000000000009 ffff880117550
[ 2716.476035] Call Trace:
[ 2716.476035] [<ffffffff8102e215>] ? warn_slowpath_common+0x5b/0x70
[ 2716.476035] [<ffffffff810ab2bd>] ? task_ctx_sched_out+0x3c/0x5f
[ 2716.476035] [<ffffffff810af02a>] ? perf_event_exit_task+0xbf/0x194
[ 2716.476035] [<ffffffff81032a37>] ? do_exit+0x3e7/0x90c
[ 2716.476035] [<ffffffff810cd5ab>] ? __do_fault+0x359/0x394
[ 2716.476035] [<ffffffff81032fe6>] ? do_group_exit+0x66/0x98
[ 2716.476035] [<ffffffff8103dbcd>] ? get_signal_to_deliver+0x479/0x4ad
[ 2716.476035] [<ffffffff810ac05c>] ? __perf_event_task_sched_out+0x230/0x2d1
[ 2716.476035] [<ffffffff8100205d>] ? do_signal+0x3c/0x432
[ 2716.476035] [<ffffffff810abbf9>] ? ctx_sched_in+0x43/0x141
[ 2716.476035] [<ffffffff810ac2ca>] ? perf_event_context_sched_in+0x7a/0x90
[ 2716.476035] [<ffffffff810ac311>] ? __perf_event_task_sched_in+0x31/0x118
[ 2716.476035] [<ffffffff81050dd9>] ? mmdrop+0xd/0x1c
[ 2716.476035] [<ffffffff81051a39>] ? finish_task_switch+0x7d/0xa6
[ 2716.476035] [<ffffffff81002473>] ? do_notify_resume+0x20/0x5d
[ 2716.476035] [<ffffffff813654f5>] ? retint_signal+0x3d/0x78
[ 2716.476035] ---[ end trace 827178d8a5966c3d ]---
Reported-by: Vince Weaver <vincent.weaver@maine.edu>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Vince Weaver <vincent.weaver@maine.edu>
---
kernel/events/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 20b3dad..446ec3d 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7478,7 +7478,7 @@ inherit_task_group(struct perf_event *event, struct task_struct *parent,
* child.
*/
- child_ctx = alloc_perf_context(event->pmu, child);
+ child_ctx = alloc_perf_context(parent_ctx->pmu, child);
if (!child_ctx)
return -ENOMEM;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] perf: Remove WARN_ON_ONCE check in __perf_event_enable for valid scenario
2013-07-09 15:44 [PATCH 1/2] perf: Clone child context from parent context pmu Jiri Olsa
@ 2013-07-09 15:44 ` Jiri Olsa
2013-07-09 15:49 ` Jiri Olsa
2013-07-12 13:28 ` [tip:perf/core] perf: Remove WARN_ON_ONCE() check in __perf_event_enable() " tip-bot for Jiri Olsa
2013-07-12 13:28 ` [tip:perf/core] perf: Clone child context from parent context pmu tip-bot for Jiri Olsa
1 sibling, 2 replies; 5+ messages in thread
From: Jiri Olsa @ 2013-07-09 15:44 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Corey Ashford, Frederic Weisbecker, Ingo Molnar,
Namhyung Kim, Paul Mackerras, Peter Zijlstra,
Arnaldo Carvalho de Melo, Vince Weaver
The '!ctx->is_active' check has a valid scenario, so
there's no need for the WARN macro.
The reason is that there's a time window between
'ctx->is_active' check in perf_event_enable function
and __perf_event_enable function having:
- IRQs on
- ctx->lock unlocked
where the task could be killed and 'ctx' deactivated by
perf_event_exit_task, ending up with WARN below.
Removing the WARN_ON_ONCE check.
[ 324.983534] ------------[ cut here ]------------
[ 324.984420] WARNING: at kernel/events/core.c:1953 __perf_event_enable+0x187/0x190()
[ 324.984420] Modules linked in:
[ 324.984420] CPU: 19 PID: 2715 Comm: nmi_bug_snb Not tainted 3.10.0+ #246
[ 324.984420] Hardware name: Supermicro X8DTN/X8DTN, BIOS 4.6.3 01/08/2010
[ 324.984420] 0000000000000009 ffff88043fce3ec8 ffffffff8160ea0b ffff88043fce3f00
[ 324.984420] ffffffff81080ff0 ffff8802314fdc00 ffff880231a8f800 ffff88043fcf7860
[ 324.984420] 0000000000000286 ffff880231a8f800 ffff88043fce3f10 ffffffff8108103a
[ 324.984420] Call Trace:
[ 324.984420] <IRQ> [<ffffffff8160ea0b>] dump_stack+0x19/0x1b
[ 324.984420] [<ffffffff81080ff0>] warn_slowpath_common+0x70/0xa0
[ 324.984420] [<ffffffff8108103a>] warn_slowpath_null+0x1a/0x20
[ 324.984420] [<ffffffff81134437>] __perf_event_enable+0x187/0x190
[ 324.984420] [<ffffffff81130030>] remote_function+0x40/0x50
[ 324.984420] [<ffffffff810e51de>] generic_smp_call_function_single_interrupt+0xbe/0x130
[ 324.984420] [<ffffffff81066a47>] smp_call_function_single_interrupt+0x27/0x40
[ 324.984420] [<ffffffff8161fd2f>] call_function_single_interrupt+0x6f/0x80
[ 324.984420] <EOI> [<ffffffff816161a1>] ? _raw_spin_unlock_irqrestore+0x41/0x70
[ 324.984420] [<ffffffff8113799d>] perf_event_exit_task+0x14d/0x210
[ 324.984420] [<ffffffff810acd04>] ? switch_task_namespaces+0x24/0x60
[ 324.984420] [<ffffffff81086946>] do_exit+0x2b6/0xa40
[ 324.984420] [<ffffffff8161615c>] ? _raw_spin_unlock_irq+0x2c/0x30
[ 324.984420] [<ffffffff81087279>] do_group_exit+0x49/0xc0
[ 324.984420] [<ffffffff81096854>] get_signal_to_deliver+0x254/0x620
[ 324.984420] [<ffffffff81043057>] do_signal+0x57/0x5a0
[ 324.984420] [<ffffffff8161a164>] ? __do_page_fault+0x2a4/0x4e0
[ 324.984420] [<ffffffff8161665c>] ? retint_restore_args+0xe/0xe
[ 324.984420] [<ffffffff816166cd>] ? retint_signal+0x11/0x84
[ 324.984420] [<ffffffff81043605>] do_notify_resume+0x65/0x80
[ 324.984420] [<ffffffff81616702>] retint_signal+0x46/0x84
[ 324.984420] ---[ end trace 442ec2f04db3771a ]---
Reported-by: Vince Weaver <vincent.weaver@maine.edu>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Vince Weaver <vincent.weaver@maine.edu>
---
kernel/events/core.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 446ec3d..eba8fb5 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1963,7 +1963,16 @@ static int __perf_event_enable(void *info)
struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
int err;
- if (WARN_ON_ONCE(!ctx->is_active))
+ /*
+ * There's a time window between 'ctx->is_active' check
+ * in perf_event_enable function and this place having:
+ * - IRQs on
+ * - ctx->lock unlocked
+ *
+ * where the task could be killed and 'ctx' deactivated
+ * by perf_event_exit_task.
+ */
+ if (!ctx->is_active)
return -EINVAL;
raw_spin_lock(&ctx->lock);
--
1.7.11.7
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] perf: Remove WARN_ON_ONCE check in __perf_event_enable for valid scenario
2013-07-09 15:44 ` [PATCH 2/2] perf: Remove WARN_ON_ONCE check in __perf_event_enable for valid scenario Jiri Olsa
@ 2013-07-09 15:49 ` Jiri Olsa
2013-07-12 13:28 ` [tip:perf/core] perf: Remove WARN_ON_ONCE() check in __perf_event_enable() " tip-bot for Jiri Olsa
1 sibling, 0 replies; 5+ messages in thread
From: Jiri Olsa @ 2013-07-09 15:49 UTC (permalink / raw)
To: linux-kernel
Cc: Corey Ashford, Frederic Weisbecker, Ingo Molnar, Namhyung Kim,
Paul Mackerras, Peter Zijlstra, Arnaldo Carvalho de Melo,
Vince Weaver
On Tue, Jul 09, 2013 at 05:44:11PM +0200, Jiri Olsa wrote:
> The '!ctx->is_active' check has a valid scenario, so
> there's no need for the WARN macro.
>
> The reason is that there's a time window between
> 'ctx->is_active' check in perf_event_enable function
> and __perf_event_enable function having:
> - IRQs on
> - ctx->lock unlocked
>
> where the task could be killed and 'ctx' deactivated by
> perf_event_exit_task, ending up with WARN below.
>
> Removing the WARN_ON_ONCE check.
>
> [ 324.983534] ------------[ cut here ]------------
> [ 324.984420] WARNING: at kernel/events/core.c:1953 __perf_event_enable+0x187/0x190()
> [ 324.984420] Modules linked in:
> [ 324.984420] CPU: 19 PID: 2715 Comm: nmi_bug_snb Not tainted 3.10.0+ #246
> [ 324.984420] Hardware name: Supermicro X8DTN/X8DTN, BIOS 4.6.3 01/08/2010
> [ 324.984420] 0000000000000009 ffff88043fce3ec8 ffffffff8160ea0b ffff88043fce3f00
> [ 324.984420] ffffffff81080ff0 ffff8802314fdc00 ffff880231a8f800 ffff88043fcf7860
> [ 324.984420] 0000000000000286 ffff880231a8f800 ffff88043fce3f10 ffffffff8108103a
> [ 324.984420] Call Trace:
> [ 324.984420] <IRQ> [<ffffffff8160ea0b>] dump_stack+0x19/0x1b
> [ 324.984420] [<ffffffff81080ff0>] warn_slowpath_common+0x70/0xa0
> [ 324.984420] [<ffffffff8108103a>] warn_slowpath_null+0x1a/0x20
> [ 324.984420] [<ffffffff81134437>] __perf_event_enable+0x187/0x190
> [ 324.984420] [<ffffffff81130030>] remote_function+0x40/0x50
> [ 324.984420] [<ffffffff810e51de>] generic_smp_call_function_single_interrupt+0xbe/0x130
> [ 324.984420] [<ffffffff81066a47>] smp_call_function_single_interrupt+0x27/0x40
> [ 324.984420] [<ffffffff8161fd2f>] call_function_single_interrupt+0x6f/0x80
> [ 324.984420] <EOI> [<ffffffff816161a1>] ? _raw_spin_unlock_irqrestore+0x41/0x70
> [ 324.984420] [<ffffffff8113799d>] perf_event_exit_task+0x14d/0x210
> [ 324.984420] [<ffffffff810acd04>] ? switch_task_namespaces+0x24/0x60
> [ 324.984420] [<ffffffff81086946>] do_exit+0x2b6/0xa40
> [ 324.984420] [<ffffffff8161615c>] ? _raw_spin_unlock_irq+0x2c/0x30
> [ 324.984420] [<ffffffff81087279>] do_group_exit+0x49/0xc0
> [ 324.984420] [<ffffffff81096854>] get_signal_to_deliver+0x254/0x620
> [ 324.984420] [<ffffffff81043057>] do_signal+0x57/0x5a0
> [ 324.984420] [<ffffffff8161a164>] ? __do_page_fault+0x2a4/0x4e0
> [ 324.984420] [<ffffffff8161665c>] ? retint_restore_args+0xe/0xe
> [ 324.984420] [<ffffffff816166cd>] ? retint_signal+0x11/0x84
> [ 324.984420] [<ffffffff81043605>] do_notify_resume+0x65/0x80
> [ 324.984420] [<ffffffff81616702>] retint_signal+0x46/0x84
> [ 324.984420] ---[ end trace 442ec2f04db3771a ]---
>
> Reported-by: Vince Weaver <vincent.weaver@maine.edu>
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Suggested-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Vince Weaver <vincent.weaver@maine.edu>
> ---
> kernel/events/core.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 446ec3d..eba8fb5 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -1963,7 +1963,16 @@ static int __perf_event_enable(void *info)
> struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
> int err;
>
> - if (WARN_ON_ONCE(!ctx->is_active))
> + /*
> + * There's a time window between 'ctx->is_active' check
> + * in perf_event_enable function and this place having:
> + * - IRQs on
> + * - ctx->lock unlocked
> + *
> + * where the task could be killed and 'ctx' deactivated
> + * by perf_event_exit_task.
> + */
> + if (!ctx->is_active)
> return -EINVAL;
>
> raw_spin_lock(&ctx->lock);
> --
> 1.7.11.7
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip:perf/core] perf: Clone child context from parent context pmu
2013-07-09 15:44 [PATCH 1/2] perf: Clone child context from parent context pmu Jiri Olsa
2013-07-09 15:44 ` [PATCH 2/2] perf: Remove WARN_ON_ONCE check in __perf_event_enable for valid scenario Jiri Olsa
@ 2013-07-12 13:28 ` tip-bot for Jiri Olsa
1 sibling, 0 replies; 5+ messages in thread
From: tip-bot for Jiri Olsa @ 2013-07-12 13:28 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, acme, paulus, hpa, mingo, peterz, namhyung, jolsa,
vincent.weaver, fweisbec, stable, tglx, cjashfor, mingo
Commit-ID: 734df5ab549ca44f40de0f07af1c8803856dfb18
Gitweb: http://git.kernel.org/tip/734df5ab549ca44f40de0f07af1c8803856dfb18
Author: Jiri Olsa <jolsa@redhat.com>
AuthorDate: Tue, 9 Jul 2013 17:44:10 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 12 Jul 2013 11:10:47 +0200
perf: Clone child context from parent context pmu
Currently when the child context for inherited events is
created, it's based on the pmu object of the first event
of the parent context.
This is wrong for the following scenario:
- HW context having HW and SW event
- HW event got removed (closed)
- SW event stays in HW context as the only event
and its pmu is used to clone the child context
The issue starts when the cpu context object is touched
based on the pmu context object (__get_cpu_context). In
this case the HW context will work with SW cpu context
ending up with following WARN below.
Fixing this by using parent context pmu object to clone
from child context.
Addresses the following warning reported by Vince Weaver:
[ 2716.472065] ------------[ cut here ]------------
[ 2716.476035] WARNING: at kernel/events/core.c:2122 task_ctx_sched_out+0x3c/0x)
[ 2716.476035] Modules linked in: nfsd auth_rpcgss oid_registry nfs_acl nfs locn
[ 2716.476035] CPU: 0 PID: 3164 Comm: perf_fuzzer Not tainted 3.10.0-rc4 #2
[ 2716.476035] Hardware name: AOpen DE7000/nMCP7ALPx-DE R1.06 Oct.19.2012, BI2
[ 2716.476035] 0000000000000000 ffffffff8102e215 0000000000000000 ffff88011fc18
[ 2716.476035] ffff8801175557f0 0000000000000000 ffff880119fda88c ffffffff810ad
[ 2716.476035] ffff880119fda880 ffffffff810af02a 0000000000000009 ffff880117550
[ 2716.476035] Call Trace:
[ 2716.476035] [<ffffffff8102e215>] ? warn_slowpath_common+0x5b/0x70
[ 2716.476035] [<ffffffff810ab2bd>] ? task_ctx_sched_out+0x3c/0x5f
[ 2716.476035] [<ffffffff810af02a>] ? perf_event_exit_task+0xbf/0x194
[ 2716.476035] [<ffffffff81032a37>] ? do_exit+0x3e7/0x90c
[ 2716.476035] [<ffffffff810cd5ab>] ? __do_fault+0x359/0x394
[ 2716.476035] [<ffffffff81032fe6>] ? do_group_exit+0x66/0x98
[ 2716.476035] [<ffffffff8103dbcd>] ? get_signal_to_deliver+0x479/0x4ad
[ 2716.476035] [<ffffffff810ac05c>] ? __perf_event_task_sched_out+0x230/0x2d1
[ 2716.476035] [<ffffffff8100205d>] ? do_signal+0x3c/0x432
[ 2716.476035] [<ffffffff810abbf9>] ? ctx_sched_in+0x43/0x141
[ 2716.476035] [<ffffffff810ac2ca>] ? perf_event_context_sched_in+0x7a/0x90
[ 2716.476035] [<ffffffff810ac311>] ? __perf_event_task_sched_in+0x31/0x118
[ 2716.476035] [<ffffffff81050dd9>] ? mmdrop+0xd/0x1c
[ 2716.476035] [<ffffffff81051a39>] ? finish_task_switch+0x7d/0xa6
[ 2716.476035] [<ffffffff81002473>] ? do_notify_resume+0x20/0x5d
[ 2716.476035] [<ffffffff813654f5>] ? retint_signal+0x3d/0x78
[ 2716.476035] ---[ end trace 827178d8a5966c3d ]---
Reported-by: Vince Weaver <vincent.weaver@maine.edu>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: <stable@kernel.org>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1373384651-6109-1-git-send-email-jolsa@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/events/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 1833bc5..1d1f030 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7465,7 +7465,7 @@ inherit_task_group(struct perf_event *event, struct task_struct *parent,
* child.
*/
- child_ctx = alloc_perf_context(event->pmu, child);
+ child_ctx = alloc_perf_context(parent_ctx->pmu, child);
if (!child_ctx)
return -ENOMEM;
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [tip:perf/core] perf: Remove WARN_ON_ONCE() check in __perf_event_enable() for valid scenario
2013-07-09 15:44 ` [PATCH 2/2] perf: Remove WARN_ON_ONCE check in __perf_event_enable for valid scenario Jiri Olsa
2013-07-09 15:49 ` Jiri Olsa
@ 2013-07-12 13:28 ` tip-bot for Jiri Olsa
1 sibling, 0 replies; 5+ messages in thread
From: tip-bot for Jiri Olsa @ 2013-07-12 13:28 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, acme, paulus, hpa, mingo, a.p.zijlstra, peterz,
namhyung, jolsa, vincent.weaver, fweisbec, stable, tglx, cjashfor,
mingo
Commit-ID: 06f417968beac6e6b614e17b37d347aa6a6b1d30
Gitweb: http://git.kernel.org/tip/06f417968beac6e6b614e17b37d347aa6a6b1d30
Author: Jiri Olsa <jolsa@redhat.com>
AuthorDate: Tue, 9 Jul 2013 17:44:11 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 12 Jul 2013 11:11:01 +0200
perf: Remove WARN_ON_ONCE() check in __perf_event_enable() for valid scenario
The '!ctx->is_active' check has a valid scenario, so
there's no need for the warning.
The reason is that there's a time window between the
'ctx->is_active' check in the perf_event_enable() function
and the __perf_event_enable() function having:
- IRQs on
- ctx->lock unlocked
where the task could be killed and 'ctx' deactivated by
perf_event_exit_task(), ending up with the warning below.
So remove the WARN_ON_ONCE() check and add comments to
explain it all.
This addresses the following warning reported by Vince Weaver:
[ 324.983534] ------------[ cut here ]------------
[ 324.984420] WARNING: at kernel/events/core.c:1953 __perf_event_enable+0x187/0x190()
[ 324.984420] Modules linked in:
[ 324.984420] CPU: 19 PID: 2715 Comm: nmi_bug_snb Not tainted 3.10.0+ #246
[ 324.984420] Hardware name: Supermicro X8DTN/X8DTN, BIOS 4.6.3 01/08/2010
[ 324.984420] 0000000000000009 ffff88043fce3ec8 ffffffff8160ea0b ffff88043fce3f00
[ 324.984420] ffffffff81080ff0 ffff8802314fdc00 ffff880231a8f800 ffff88043fcf7860
[ 324.984420] 0000000000000286 ffff880231a8f800 ffff88043fce3f10 ffffffff8108103a
[ 324.984420] Call Trace:
[ 324.984420] <IRQ> [<ffffffff8160ea0b>] dump_stack+0x19/0x1b
[ 324.984420] [<ffffffff81080ff0>] warn_slowpath_common+0x70/0xa0
[ 324.984420] [<ffffffff8108103a>] warn_slowpath_null+0x1a/0x20
[ 324.984420] [<ffffffff81134437>] __perf_event_enable+0x187/0x190
[ 324.984420] [<ffffffff81130030>] remote_function+0x40/0x50
[ 324.984420] [<ffffffff810e51de>] generic_smp_call_function_single_interrupt+0xbe/0x130
[ 324.984420] [<ffffffff81066a47>] smp_call_function_single_interrupt+0x27/0x40
[ 324.984420] [<ffffffff8161fd2f>] call_function_single_interrupt+0x6f/0x80
[ 324.984420] <EOI> [<ffffffff816161a1>] ? _raw_spin_unlock_irqrestore+0x41/0x70
[ 324.984420] [<ffffffff8113799d>] perf_event_exit_task+0x14d/0x210
[ 324.984420] [<ffffffff810acd04>] ? switch_task_namespaces+0x24/0x60
[ 324.984420] [<ffffffff81086946>] do_exit+0x2b6/0xa40
[ 324.984420] [<ffffffff8161615c>] ? _raw_spin_unlock_irq+0x2c/0x30
[ 324.984420] [<ffffffff81087279>] do_group_exit+0x49/0xc0
[ 324.984420] [<ffffffff81096854>] get_signal_to_deliver+0x254/0x620
[ 324.984420] [<ffffffff81043057>] do_signal+0x57/0x5a0
[ 324.984420] [<ffffffff8161a164>] ? __do_page_fault+0x2a4/0x4e0
[ 324.984420] [<ffffffff8161665c>] ? retint_restore_args+0xe/0xe
[ 324.984420] [<ffffffff816166cd>] ? retint_signal+0x11/0x84
[ 324.984420] [<ffffffff81043605>] do_notify_resume+0x65/0x80
[ 324.984420] [<ffffffff81616702>] retint_signal+0x46/0x84
[ 324.984420] ---[ end trace 442ec2f04db3771a ]---
Reported-by: Vince Weaver <vincent.weaver@maine.edu>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Suggested-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: <stable@kernel.org>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1373384651-6109-2-git-send-email-jolsa@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/events/core.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 1d1f030..ef5e7cc 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1950,7 +1950,16 @@ static int __perf_event_enable(void *info)
struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
int err;
- if (WARN_ON_ONCE(!ctx->is_active))
+ /*
+ * There's a time window between 'ctx->is_active' check
+ * in perf_event_enable function and this place having:
+ * - IRQs on
+ * - ctx->lock unlocked
+ *
+ * where the task could be killed and 'ctx' deactivated
+ * by perf_event_exit_task.
+ */
+ if (!ctx->is_active)
return -EINVAL;
raw_spin_lock(&ctx->lock);
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-07-12 13:31 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-09 15:44 [PATCH 1/2] perf: Clone child context from parent context pmu Jiri Olsa
2013-07-09 15:44 ` [PATCH 2/2] perf: Remove WARN_ON_ONCE check in __perf_event_enable for valid scenario Jiri Olsa
2013-07-09 15:49 ` Jiri Olsa
2013-07-12 13:28 ` [tip:perf/core] perf: Remove WARN_ON_ONCE() check in __perf_event_enable() " tip-bot for Jiri Olsa
2013-07-12 13:28 ` [tip:perf/core] perf: Clone child context from parent context pmu tip-bot for Jiri Olsa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox