ping
thanks,
jirka
On Mon, Apr 07, 2014 at 11:04:08AM +0200, Jiri Olsa wrote:
> The perf cpu offline callback takes down all cpu context
> events and releases swhash->swevent_hlist.
>
> This could race with task context software event being just
> scheduled on this cpu via perf_swevent_add while cpu hotplug
> code already cleaned up event's data.
>
> The race happens in the gap between the cpu notifier code
> and the cpu being actually taken down. Note that only cpu
> ctx events are terminated in the perf cpu hotplug code.
>
> It's easily reproduced with:
> $ perf record -e faults perf bench sched pipe
>
> while putting one of the cpus offline:
> # echo 0 > /sys/devices/system/cpu/cpu1/online
>
> Console emits following warning:
> WARNING: CPU: 1 PID: 2845 at kernel/events/core.c:5672
> perf_swevent_add+0x18d/0x1a0()
> Modules linked in:
> CPU: 1 PID: 2845 Comm: sched-pipe Tainted: G W 3.14.0+ #256
> Hardware name: Intel Corporation Montevina platform/To be filled by O.E.M.,
> BIOS AMVACRB1.86C.0066.B00.0805070703 05/07/2008
> 0000000000000009 ffff880077233ab8 ffffffff81665a23 0000000000200005
> 0000000000000000 ffff880077233af8 ffffffff8104732c 0000000000000046
> ffff88007467c800 0000000000000002 ffff88007a9cf2a0 0000000000000001
> Call Trace:
> [<ffffffff81665a23>] dump_stack+0x4f/0x7c
> [<ffffffff8104732c>] warn_slowpath_common+0x8c/0xc0
> [<ffffffff8104737a>] warn_slowpath_null+0x1a/0x20
> [<ffffffff8110fb3d>] perf_swevent_add+0x18d/0x1a0
> [<ffffffff811162ae>] event_sched_in.isra.75+0x9e/0x1f0
> [<ffffffff8111646a>] group_sched_in+0x6a/0x1f0
> [<ffffffff81083dd5>] ? sched_clock_local+0x25/0xa0
> [<ffffffff811167e6>] ctx_sched_in+0x1f6/0x450
> [<ffffffff8111757b>] perf_event_sched_in+0x6b/0xa0
> [<ffffffff81117a4b>] perf_event_context_sched_in+0x7b/0xc0
> [<ffffffff81117ece>] __perf_event_task_sched_in+0x43e/0x460
> [<ffffffff81096f1e>] ? put_lock_stats.isra.18+0xe/0x30
> [<ffffffff8107b3c8>] finish_task_switch+0xb8/0x100
> [<ffffffff8166a7de>] __schedule+0x30e/0xad0
> [<ffffffff81172dd2>] ? pipe_read+0x3e2/0x560
> [<ffffffff8166b45e>] ? preempt_schedule_irq+0x3e/0x70
> [<ffffffff8166b45e>] ? preempt_schedule_irq+0x3e/0x70
> [<ffffffff8166b464>] preempt_schedule_irq+0x44/0x70
> [<ffffffff816707f0>] retint_kernel+0x20/0x30
> [<ffffffff8109e60a>] ? lockdep_sys_exit+0x1a/0x90
> [<ffffffff812a4234>] lockdep_sys_exit_thunk+0x35/0x67
> [<ffffffff81679321>] ? sysret_check+0x5/0x56
>
> Fixing this by tracking the cpu hotplug state and displaying
> the WARN only if current cpu is initialized properly.
>
> Reported-by: Fengguang Wu <fengguang...@intel.com>
> Signed-off-by: Jiri Olsa <jo...@redhat.com>
> Cc: Corey Ashford <cjash...@linux.vnet.ibm.com>
> Cc: Frederic Weisbecker <fweis...@gmail.com>
> Cc: Ingo Molnar <mi...@kernel.org>
> Cc: Paul Mackerras <pau...@samba.org>
> Cc: Peter Zijlstra <a.p.zijls...@chello.nl>
> Cc: Arnaldo Carvalho de Melo <a...@redhat.com>
> Cc: Fengguang Wu <fengguang...@intel.com>
> ---
> kernel/events/core.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 661951a..0747389 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -5423,6 +5423,9 @@ struct swevent_htable {
>
> /* Recursion avoidance in each contexts */
> int recursion[PERF_NR_CONTEXTS];
> +
> + /* Keeps track of cpu being initialized/exited */
> + bool online;
> };
>
> static DEFINE_PER_CPU(struct swevent_htable, swevent_htable);
> @@ -5669,8 +5672,14 @@ static int perf_swevent_add(struct perf_event *event,
> int flags)
> hwc->state = !(flags & PERF_EF_START);
>
> head = find_swevent_head(swhash, event);
> - if (WARN_ON_ONCE(!head))
> + if (!head) {
> + /*
> + * We can race with cpu hotplug code. Do not
> + * WARN if the cpu just got unplugged.
> + */
> + WARN_ON_ONCE(swhash->online);
> return -EINVAL;
> + }
>
> hlist_add_head_rcu(&event->hlist_entry, head);
>
> @@ -7850,6 +7859,7 @@ static void perf_event_init_cpu(int cpu)
> struct swevent_htable *swhash = &per_cpu(swevent_htable, cpu);
>
> mutex_lock(&swhash->hlist_mutex);
> + swhash->online = true;
> if (swhash->hlist_refcount > 0) {
> struct swevent_hlist *hlist;
>
> @@ -7907,6 +7917,7 @@ static void perf_event_exit_cpu(int cpu)
> perf_event_exit_cpu_context(cpu);
>
> mutex_lock(&swhash->hlist_mutex);
> + swhash->online = false;
> swevent_hlist_release(swhash);
> mutex_unlock(&swhash->hlist_mutex);
> }
> --
> 1.7.11.7
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
