Re: perf: perf_fuzzer quickly locks up on 4.15-rc7

[Peter Zijlstra <peterz@infradead.org>]

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.