Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754587AbbFPLhR (ORCPT ); Tue, 16 Jun 2015 07:37:17 -0400 Received: from mga01.intel.com ([192.55.52.88]:12810 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750990AbbFPLhK (ORCPT ); Tue, 16 Jun 2015 07:37:10 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,626,1427785200"; d="scan'208";a="728346655" From: Alexander Shishkin To: Peter Zijlstra , Vince Weaver Cc: linux-kernel@vger.kernel.org, Ingo Molnar , Arnaldo Carvalho de Melo , Stephane Eranian Subject: Re: perf: aux area related crash and warnings In-Reply-To: <87egld2l2o.fsf@ashishki-desk.ger.corp.intel.com> References:

<20150615122054.GY3644@twins.programming.kicks-ass.net> <87egld2l2o.fsf@ashishki-desk.ger.corp.intel.com> User-Agent: Notmuch/0.20.1 (http://notmuchmail.org) Emacs/24.4.1 (x86_64-pc-linux-gnu) Date: Tue, 16 Jun 2015 14:37:06 +0300 Message-ID: <87381rg9zx.fsf@ashishki-desk.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5850 Lines: 156 Alexander Shishkin writes: > Peter Zijlstra writes: > >> Alex, any clue? > > Let me look into it. Definitely haven't seen anything like that in my > tests. > >> On Fri, Jun 12, 2015 at 02:42:36PM -0400, Vince Weaver wrote: >>> On Thu, 11 Jun 2015, Vince Weaver wrote: >>> >>> > and while I was trying to cut and paste that, the locked haswell just took >>> > down the network switch so I can't get the rest until tomorrow. >>> >>> here are the full dumps if anyone is interested >>> >>> the warning is reproducible, the spinlock disaster is not. >>> >>> [36298.986117] BUG: spinlock recursion on CPU#4, perf_fuzzer/3410 >>> [36298.992915] lock: 0xffff88011edf7cd0, .magic: dead4ead, .owner: perf_fuzzer/3410, .owner_cpu: 4 >>> [36299.002919] CPU: 4 PID: 3410 Comm: perf_fuzzer Tainted: G W 4.1.0-rc7+ #155 >>> [36299.012152] Hardware name: LENOVO 10AM000AUS/SHARKBAY, BIOS FBKT72AUS 01/26/2014 >>> [36299.020606] ffff88011edf7cd0 ffff88011eb059a0 ffffffff816d7229 0000000000000054 >>> [36299.029199] ffff8800c2f4ac50 ffff88011eb059c0 ffffffff810c2895 ffff88011edf7cd0 >>> [36299.037796] ffffffff81a1e481 ffff88011eb059e0 ffffffff810c2916 ffff88011edf7cd0 >>> [36299.046338] Call Trace: >>> [36299.049501] [] dump_stack+0x45/0x57 >>> [36299.056284] [] spin_dump+0x85/0xe0 >>> [36299.062282] [] spin_bug+0x26/0x30 >>> [36299.068111] [] do_raw_spin_lock+0x13f/0x180 >>> [36299.074897] [] _raw_spin_lock+0x39/0x40 >>> [36299.081276] [] ? free_pcppages_bulk+0x39/0x620 >>> [36299.088340] [] free_pcppages_bulk+0x39/0x620 >>> [36299.095182] [] ? free_pages_prepare+0x3a4/0x550 >>> [36299.102291] [] ? kfree_debugcheck+0x16/0x40 >>> [36299.108987] [] free_hot_cold_page+0x178/0x1a0 >>> [36299.115850] [] __free_pages+0x37/0x50 >>> [36299.121991] [] rb_free_aux+0xba/0xf0 > > This one goes to free aux pages from nmi context, looks like aux buffer > was unmapped while the event was running, so here it dropped the last > reference. Ok, here's what I propose for this one. >From b8cd18bc440c318e8b30825bf654c815b42aa1e0 Mon Sep 17 00:00:00 2001 From: Alexander Shishkin Date: Tue, 16 Jun 2015 14:14:04 +0300 Subject: [PATCH] perf: Free AUX area pages in rcu callback Currently, if the user unmaps AUX area while the corresponding event is active, perf_aux_output_end() may be the last one to drop the aux area refcount, and end up freeing the pages in NMI context or scheduler's fast path. Same can happen in the error path of perf_aux_output_begin(). To avoid the bug, this patch moves actual freeing code to rb_free_rcu(), which will know whether it is called for AUX area or the ring buffer proper and act accordingly. Signed-off-by: Alexander Shishkin Reported-by: Vince Weaver --- kernel/events/core.c | 31 ++++++++++++++++++++++++++++++- kernel/events/internal.h | 1 + kernel/events/ring_buffer.c | 8 +------- 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/kernel/events/core.c b/kernel/events/core.c index eddf1ed415..5f1cc5976f 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -4381,7 +4381,36 @@ static void rb_free_rcu(struct rcu_head *rcu_head) struct ring_buffer *rb; rb = container_of(rcu_head, struct ring_buffer, rcu_head); - rb_free(rb); + + /* + * are we called for AUX or the rb: + * AUX always goes first, then if rb::refcount drops to zero, + * free rb synchronously + */ + if (atomic_read(&rb->refcount)) { + __rb_free_aux(rb); + + /* matches the increment in rb_free_aux() */ + if (atomic_dec_and_test(&rb->refcount)) + rb_free(rb); + } else { + rb_free(rb); + } +} + +void rb_free_aux(struct ring_buffer *rb) +{ + /* + * hold rb::refcount to make sure rb doesn't disappear + * before aux pages are freed + */ + if (WARN_ON_ONCE(!atomic_inc_not_zero(&rb->refcount))) + return; + + if (atomic_dec_and_test(&rb->aux_refcount)) + call_rcu(&rb->rcu_head, rb_free_rcu); + else + ring_buffer_put(rb); /* matches the increment above */ } struct ring_buffer *ring_buffer_get(struct perf_event *event) diff --git a/kernel/events/internal.h b/kernel/events/internal.h index 9f6ce9ba4a..7f8242ed85 100644 --- a/kernel/events/internal.h +++ b/kernel/events/internal.h @@ -61,6 +61,7 @@ extern void perf_event_wakeup(struct perf_event *event); extern int rb_alloc_aux(struct ring_buffer *rb, struct perf_event *event, pgoff_t pgoff, int nr_pages, long watermark, int flags); extern void rb_free_aux(struct ring_buffer *rb); +extern void __rb_free_aux(struct ring_buffer *rb); extern struct ring_buffer *ring_buffer_get(struct perf_event *event); extern void ring_buffer_put(struct ring_buffer *rb); diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c index 725c416085..343121e943 100644 --- a/kernel/events/ring_buffer.c +++ b/kernel/events/ring_buffer.c @@ -537,7 +537,7 @@ out: return ret; } -static void __rb_free_aux(struct ring_buffer *rb) +void __rb_free_aux(struct ring_buffer *rb) { int pg; @@ -554,12 +554,6 @@ static void __rb_free_aux(struct ring_buffer *rb) rb->aux_nr_pages = 0; } -void rb_free_aux(struct ring_buffer *rb) -{ - if (atomic_dec_and_test(&rb->aux_refcount)) - __rb_free_aux(rb); -} - #ifndef CONFIG_PERF_USE_VMALLOC /* -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/