From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753494AbaIDKoN (ORCPT ); Thu, 4 Sep 2014 06:44:13 -0400 Received: from bombadil.infradead.org ([198.137.202.9]:47072 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752825AbaIDKoK (ORCPT ); Thu, 4 Sep 2014 06:44:10 -0400 Date: Thu, 4 Sep 2014 12:44:02 +0200 From: Peter Zijlstra To: Mark Rutland Cc: "linux-kernel@vger.kernel.org" , Yan Zheng , Stephane Eranian , Ingo Molnar , Vince Weaver , Linus Torvalds Subject: Re: Possible race between CPU hotplug and perf_pmu_migrate_context Message-ID: <20140904104402.GS4783@worktop.ger.corp.intel.com> References: <20140901181808.GA6427@leverpostej> <20140901190534.GC5806@worktop.ger.corp.intel.com> <20140902185807.GA7434@leverpostej> <20140903115013.GA3127@leverpostej> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140903115013.GA3127@leverpostej> User-Agent: Mutt/1.5.22.1 (2013-10-16) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Sep 03, 2014 at 12:50:14PM +0100, Mark Rutland wrote: > From 6465beace3ad9b12039127468f4596b8e87a53e8 Mon Sep 17 00:00:00 2001 > From: Mark Rutland > Date: Wed, 3 Sep 2014 11:06:22 +0100 > Subject: [PATCH] perf: prevent hotplug race on event->ctx > > The perf_pmu_migrate_context code introduced in 0cda4c023132 (perf: > Introduce perf_pmu_migrate_context()) didn't take the tear-down of > events into account, and left open a race with put_event on event->ctx. > A resulting duplicate put_ctx of an event's original context can lead to > the context being erroneously kfreed via RCU, resulting in the below > splat with the intel uncore_imc PMU driver: > In response to a CPU notifier an uncore PMU driver calls > perf_pmu_migrate context, which will remove all events from the old CPU > context before placing them all into the new CPU context. For a short > period the events are in limbo and are part of neither context, though > their event->ctx pointers still point at the old context. > > During this period another CPU may enter put_event, which will try to > remove the event from event->ctx. As this may still point at the old > context, put_ctx can be called twice for a given event on the original > context. The context's refcount may drop to zero unexpectedly, whereupon > put_ctx will queue up a kfree with RCU. This blows up at the end of the > next grace period as the uncore PMU contexts are housed within > perf_cpu_context and weren't directly allocated with k*alloc. > > This patch prevents the issue by inhibiting hotplug for the portion of > put_event which must access event->ctx, preventing the notifiers which > call perf_pmu_migrate_context from running concurrently. Once the event > has been removed from its context perf_pmu_migrate_context will no > longer be able to access it, so it is not necessary to inhibit hotplug > for the duration of event tear-down. Right, so that works I suppose. The thing is, get_online_cpus() is a global lock and we can potentially do a lot of put_event()s. I had a patch a while back that rewrote the cpuhotplug locking, but Linus didn't particularly like that at the time. I'll try and see if I can come up with anything else, but so far I've only discovered a lot of ways that don't work (like I'm sure you did too).