From patchwork Wed Aug 24 09:19:05 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Will Deacon X-Patchwork-Id: 74573 Delivered-To: patch@linaro.org Received: by 10.140.29.52 with SMTP id a49csp215292qga; Wed, 24 Aug 2016 02:26:52 -0700 (PDT) X-Received: by 10.98.80.29 with SMTP id e29mr3769839pfb.76.1472030411985; Wed, 24 Aug 2016 02:20:11 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id wm4si8754958pab.235.2016.08.24.02.20.11; Wed, 24 Aug 2016 02:20:11 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754246AbcHXJUI (ORCPT + 27 others); Wed, 24 Aug 2016 05:20:08 -0400 Received: from foss.arm.com ([217.140.101.70]:45702 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751025AbcHXJUE (ORCPT ); Wed, 24 Aug 2016 05:20:04 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 72D53451; Wed, 24 Aug 2016 02:20:44 -0700 (PDT) Received: from edgewater-inn.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id D6E463F32C; Wed, 24 Aug 2016 02:19:04 -0700 (PDT) Received: by edgewater-inn.cambridge.arm.com (Postfix, from userid 1000) id D12C61AE2DB3; Wed, 24 Aug 2016 10:19:05 +0100 (BST) Date: Wed, 24 Aug 2016 10:19:05 +0100 From: Will Deacon To: Alexander Shishkin Cc: Vince Weaver , Linux Kernel Mailing List , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo Subject: Re: perf: WARNING: kernel/events/core.c:4893 perf_mmap_close Message-ID: <20160824091905.GA16944@arm.com> References: <87k2fqmska.fsf@ashishki-desk.ger.corp.intel.com> <20160823151239.GA29462@arm.com> <87vayrl8fs.fsf@ashishki-desk.ger.corp.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87vayrl8fs.fsf@ashishki-desk.ger.corp.intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Alex, On Tue, Aug 23, 2016 at 08:08:23PM +0300, Alexander Shishkin wrote: > Will Deacon writes: > > On Fri, Aug 12, 2016 at 08:54:49PM +0300, Alexander Shishkin wrote: > >> Yes, I tracked to a race between unmapping and set_output, trying to > >> come up with a good fix now. > > > > Did you get anywhere with this? I think I just hit the same issue with > > the intel-pt driver on my broadwell laptop with -rc3+PREEMPT (dump > > below) and I'm more than happy to test a fix. > > Yes, I posted a patchset [1] this morning that should fix this; if this > is PT, 3/3 should suffice. Unfortunately, I still see the same issue with those three patches applied, so perhaps it's not quite the same as the problem reported by Vince. A little bit of digging suggests that the preempt count isn't balanced across the call to perf_pmu_output_stop from perf_mmap_close. Further digging revealed the unbalanced get_cpu_ptr in __perf_pmu_output_stop. Fix below! I also noticed that we go to some effort to return an error code from __perf_pmu_output_stop, but it's completly ignored by the cpu_function_call machinery :( > What's "pt-poker", btw? :) A very boring application I wrote that just opens a pt event with config=0, mmaps the AUX buffer and then spins waiting for the data that never comes. The issue above occurs when I ctrl+c the application under a preemptible kernel. Will --->8 >From 0d59936e07aa9b00f2a5640661bd1153f66914dc Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Wed, 24 Aug 2016 10:07:14 +0100 Subject: [PATCH] perf/core: Use this_cpu_ptr when stopping AUX events When tearing down an aux buf for an event via perf_mmap_close, __perf_event_output_stop is called on the event's CPU to ensure that trace generation is halted before the process of unmapping and freeing the buffer pages begins. The callback is performed via cpu_function_call, which ensures that it runs with interrupts disabled and is therefore not preemptible. Unfortunately, the current code grabs the per-cpu context pointer using get_cpu_ptr, which unnecessarily disables preemption and doesn't pair the call with put_cpu_ptr, leading to a preempt_count() imbalance and a BUG when freeing the aux buffer later on: [ 92.159611] ------------[ cut here ]------------ [ 92.159617] WARNING: CPU: 1 PID: 2249 at kernel/events/ring_buffer.c:539 __rb_free_aux+0x10c/0x120 [ 92.159618] Modules linked in: [ 92.159620] CPU: 1 PID: 2249 Comm: spe-poker Tainted: G W 4.8.0-rc3 #2 [ 92.159621] Hardware name: Dell Inc. XPS 13 9343/0F5KF3, BIOS A07 11/11/2015 [ 92.159622] 0000000000000000 ffff88020f91fad8 ffffffff813379dd 0000000000000000 [ 92.159624] 0000000000000000 ffff88020f91fb18 ffffffff81059ff6 0000021b0f91fb98 [ 92.159625] ffff8802100fe300 ffff88020de1da80 ffff88020de1d800 ffff8802100b8228 [ 92.159627] Call Trace: [ 92.159630] [] dump_stack+0x4f/0x72 [ 92.159632] [] __warn+0xc6/0xe0 [ 92.159633] [] warn_slowpath_null+0x18/0x20 [ 92.159634] [] __rb_free_aux+0x10c/0x120 [ 92.159636] [] rb_free_aux+0x13/0x20 [ 92.159637] [] perf_mmap_close+0x29e/0x2f0 [ 92.159638] [] ? perf_iterate_ctx+0xe0/0xe0 [ 92.159640] [] remove_vma+0x25/0x60 [ 92.159641] [] exit_mmap+0x106/0x140 [ 92.159643] [] mmput+0x1c/0xd0 [ 92.159644] [] do_exit+0x253/0xbf0 [ 92.159645] [] do_group_exit+0x3e/0xb0 [ 92.159647] [] get_signal+0x249/0x640 [ 92.159648] [] do_signal+0x23/0x640 [ 92.159651] [] ? _raw_write_unlock_irq+0x12/0x30 [ 92.159652] [] ? _raw_spin_unlock_irq+0x9/0x10 [ 92.159653] [] ? __schedule+0x2c6/0x710 [ 92.159655] [] exit_to_usermode_loop+0x74/0x90 [ 92.159656] [] prepare_exit_to_usermode+0x26/0x30 [ 92.159658] [] retint_user+0x8/0x10 [ 92.159659] ---[ end trace 926e713af6f1575e ]--- This patch uses this_cpu_ptr instead of get_cpu_ptr, since preemption is already disabled by the caller. Fixes: 95ff4ca26c49 ("perf/core: Free AUX pages in unmap path") Cc: Alexander Shishkin Cc: Peter Zijlstra Signed-off-by: Will Deacon --- kernel/events/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.5.0 diff --git a/kernel/events/core.c b/kernel/events/core.c index 5650f5317e0c..3cfabdf7b942 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -6166,7 +6166,7 @@ static int __perf_pmu_output_stop(void *info) { struct perf_event *event = info; struct pmu *pmu = event->pmu; - struct perf_cpu_context *cpuctx = get_cpu_ptr(pmu->pmu_cpu_context); + struct perf_cpu_context *cpuctx = this_cpu_ptr(pmu->pmu_cpu_context); struct remote_output ro = { .rb = event->rb, };