Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 13 Sep 2021 12:36:36 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     =?utf-8?B?546L6LSH?= <yun.wang@linux.alibaba.com>
Cc:     Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@redhat.com>,
        Namhyung Kim <namhyung@kernel.org>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>,
        "open list:PERFORMANCE EVENTS SUBSYSTEM" 
        <linux-perf-users@vger.kernel.org>,
        "open list:PERFORMANCE EVENTS SUBSYSTEM" 
        <linux-kernel@vger.kernel.org>,
        "open list:BPF (Safe dynamic programs and tools)" 
        <netdev@vger.kernel.org>,
        "open list:BPF (Safe dynamic programs and tools)" 
        <bpf@vger.kernel.org>
Subject: Re: [RFC PATCH] perf: fix panic by mark recursion inside
 perf_log_throttle
Message-ID: <YT8ptOoN0age04PQ@hirez.programming.kicks-ass.net>
References: <ff979a43-045a-dc56-64d1-2c31dd4db381@linux.alibaba.com>
 <20210910153839.GH4323@worktop.programming.kicks-ass.net>
 <f38987a5-dc36-a20d-8c5e-81e8ead5b4dc@linux.alibaba.com>
 <YT8m2B6D2yWc5Umq@hirez.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YT8m2B6D2yWc5Umq@hirez.programming.kicks-ass.net>
Precedence: bulk

On Mon, Sep 13, 2021 at 12:24:24PM +0200, Peter Zijlstra wrote:

FWIW:

> I'm confused tho; where does the #DF come from? Because taking a #PF
> from NMI should be perfectly fine.
> 
> AFAICT that callchain is something like:
> 
> 	NMI
> 	  perf_event_nmi_handler()
> 	    (part of the chain is missing here)
> 	      perf_log_throttle()
> 	        perf_output_begin() /* events/ring_buffer.c */
> 		  rcu_read_lock()
> 		    rcu_lock_acquire()
> 		      lock_acquire()
> 		        trace_lock_acquire() --> perf_trace_foo

This function also calls perf_trace_buf_alloc(), and will have
incremented the recursion count, such that:

> 
> 			  ...
> 			    perf_callchain()
> 			      perf_callchain_user()
> 			        #PF (fully expected during a userspace callchain)
> 				  (some stuff, until the first __fentry)
> 				    perf_trace_function_call
> 				      perf_trace_buf_alloc()
> 				        perf_swevent_get_recursion_context()
> 					  *BOOM*

this one, if it wouldn't mysteriously explode, would find recursion and
terminate, except that seems to be going side-ways.

> Now, supposedly we then take another #PF from get_recursion_context() or
> something, but that doesn't make sense. That should just work...
> 
> Can you figure out what's going wrong there? going with the RIP, this
> almost looks like 'swhash->recursion' goes splat, but again that makes
> no sense, that's a per-cpu variable.
> 
>