Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758626AbeAISCR (ORCPT + 1 other); Tue, 9 Jan 2018 13:02:17 -0500 Received: from bombadil.infradead.org ([65.50.211.133]:55797 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758491AbeAISCP (ORCPT ); Tue, 9 Jan 2018 13:02:15 -0500 Date: Tue, 9 Jan 2018 19:02:07 +0100 From: Peter Zijlstra To: Steven Rostedt Cc: Vince Weaver , Ingo Molnar , linux-kernel@vger.kernel.org, Ingo Molnar , Arnaldo Carvalho de Melo , Thomas Gleixner Subject: Re: perf: perf_fuzzer quickly locks up on 4.15-rc7 Message-ID: <20180109180207.GM6176@hirez.programming.kicks-ass.net> References: <20180108173005.lkglqrixb2ota6g2@gmail.com> <20180109102507.GG6176@hirez.programming.kicks-ass.net> <20180109132602.GA2369@hirez.programming.kicks-ass.net> <20180109151253.GK6176@hirez.programming.kicks-ass.net> <20180109161400.GC2369@hirez.programming.kicks-ass.net> <20180109125346.17cad4a5@vmware.local.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180109125346.17cad4a5@vmware.local.home> User-Agent: Mutt/1.9.2 (2017-12-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On Tue, Jan 09, 2018 at 12:53:46PM -0500, Steven Rostedt wrote: > Looking at ftrace_profile_set_filter(), I see it starts with: > > mutex_lock(&event_mutex); > > How much of a big deal would it be if we move taking event_mutex() into > perf_ioctl(), and then make ftrace_profile_set_filter() not take the > event_mutex. This is the only place that function is used. Would that > work? > diff --git a/kernel/events/core.c b/kernel/events/core.c > index 4df5b695bf0d..9fac7ac14b32 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -4741,9 +4741,11 @@ static long perf_ioctl(struct file *file, unsigned int cmd, unsigned long arg) > struct perf_event_context *ctx; > long ret; > > + mutex_lock(&event_mutex); > ctx = perf_event_ctx_lock(event); > ret = _perf_ioctl(event, cmd, arg); > perf_event_ctx_unlock(event, ctx); > + mutex_unlock(&event_mutex); > > return ret; > } This would globally serialize all perf_ioctl()'s, also that event_mutex is for trace_events and really does not belong in perf. So no, I really rather would not do this. The alternative I was thinking of was lifting the cpuhp lock out from under event_mutex, that would also break the chain, but would probably be lots of work for trace bits.