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.
