From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755096AbaEORnK (ORCPT ); Thu, 15 May 2014 13:43:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:4061 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754074AbaEORnH (ORCPT ); Thu, 15 May 2014 13:43:07 -0400 Date: Thu, 15 May 2014 19:42:53 +0200 From: Jiri Olsa To: linux-kernel@vger.kernel.org Cc: Corey Ashford , Frederic Weisbecker , Ingo Molnar , Paul Mackerras , Peter Zijlstra , Arnaldo Carvalho de Melo , Fengguang Wu Subject: Re: [PATCH] perf: Prevent false warning in perf_swevent_add Message-ID: <20140515174253.GF4009@krava.brq.redhat.com> References: <1396861448-10097-1-git-send-email-jolsa@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1396861448-10097-1-git-send-email-jolsa@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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: > [] dump_stack+0x4f/0x7c > [] warn_slowpath_common+0x8c/0xc0 > [] warn_slowpath_null+0x1a/0x20 > [] perf_swevent_add+0x18d/0x1a0 > [] event_sched_in.isra.75+0x9e/0x1f0 > [] group_sched_in+0x6a/0x1f0 > [] ? sched_clock_local+0x25/0xa0 > [] ctx_sched_in+0x1f6/0x450 > [] perf_event_sched_in+0x6b/0xa0 > [] perf_event_context_sched_in+0x7b/0xc0 > [] __perf_event_task_sched_in+0x43e/0x460 > [] ? put_lock_stats.isra.18+0xe/0x30 > [] finish_task_switch+0xb8/0x100 > [] __schedule+0x30e/0xad0 > [] ? pipe_read+0x3e2/0x560 > [] ? preempt_schedule_irq+0x3e/0x70 > [] ? preempt_schedule_irq+0x3e/0x70 > [] preempt_schedule_irq+0x44/0x70 > [] retint_kernel+0x20/0x30 > [] ? lockdep_sys_exit+0x1a/0x90 > [] lockdep_sys_exit_thunk+0x35/0x67 > [] ? 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 > Signed-off-by: Jiri Olsa > Cc: Corey Ashford > Cc: Frederic Weisbecker > Cc: Ingo Molnar > Cc: Paul Mackerras > Cc: Peter Zijlstra > Cc: Arnaldo Carvalho de Melo > Cc: Fengguang Wu > --- > 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 >