Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Song Liu <songliubraving@fb.com>
To:     Alexei Starovoitov <ast@fb.com>
CC:     Peter Zijlstra <peterz@infradead.org>,
        Kairui Song <kasong@redhat.com>,
        lkml <linux-kernel@vger.kernel.org>,
        Kernel Team <Kernel-team@fb.com>,
        "Josh Poimboeuf" <jpoimboe@redhat.com>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        "bpf@vger.kernel.org" <bpf@vger.kernel.org>
Subject: Re: Getting empty callchain from perf_callchain_kernel()
Thread-Topic: Getting empty callchain from perf_callchain_kernel()
Thread-Index: AQHVDEJXbyjc1nknl06FlgTGlvLrzKZu8J8AgAAG+ICAAAFQgIAAD2QAgACfPoCAACi4gIAAC66A
Date:   Fri, 17 May 2019 21:48:14 +0000
Message-ID: <8449BBF3-E754-4ABC-BFEF-A8F264297F2D@fb.com>
References: <3CD3EE63-0CD2-404A-A403-E11DCF2DF8D9@fb.com>
 <20190517074600.GJ2623@hirez.programming.kicks-ass.net>
 <20190517081057.GQ2650@hirez.programming.kicks-ass.net>
 <CACPcB9cB5n1HOmZcVpusJq8rAV5+KfmZ-Lxv3tgsSoy7vNrk7w@mail.gmail.com>
 <20190517091044.GM2606@hirez.programming.kicks-ass.net>
 <8C814E68-B0B6-47E4-BDD6-917B01EC62D0@fb.com>
 <c881767d-b6f3-c53e-5c70-556d09ea8d89@fb.com>
In-Reply-To: <c881767d-b6f3-c53e-5c70-556d09ea8d89@fb.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: fb.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-ID: <84CD2ECF61D29244B5029CBEE63D56A4@namprd15.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: b181ee31-5ce4-4350-9292-08d6db115d11
X-MS-Exchange-CrossTenant-originalarrivaltime: 17 May 2019 21:48:14.0672
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR15MB1648
X-OriginatorOrg: fb.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2019-05-17_14:,,
 signatures=0
X-Proofpoint-Spam-Reason: safe
X-FB-Internal: Safe
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> On May 17, 2019, at 2:06 PM, Alexei Starovoitov <ast@fb.com> wrote:
>=20
> On 5/17/19 11:40 AM, Song Liu wrote:
>> +Alexei, Daniel, and bpf
>>=20
>>> On May 17, 2019, at 2:10 AM, Peter Zijlstra <peterz@infradead.org> wrot=
e:
>>>=20
>>> On Fri, May 17, 2019 at 04:15:39PM +0800, Kairui Song wrote:
>>>> Hi, I think the actual problem is that bpf_get_stackid_tp (and maybe
>>>> some other bfp functions) is now broken, or, strating an unwind
>>>> directly inside a bpf program will end up strangely. It have following
>>>> kernel message:
>>>=20
>>> Urgh, what is that bpf_get_stackid_tp() doing to get the regs? I can't
>>> follow.
>>=20
>> I guess we need something like the following? (we should be able to
>> optimize the PER_CPU stuff).
>>=20
>> Thanks,
>> Song
>>=20
>>=20
>> diff --git i/kernel/trace/bpf_trace.c w/kernel/trace/bpf_trace.c
>> index f92d6ad5e080..c525149028a7 100644
>> --- i/kernel/trace/bpf_trace.c
>> +++ w/kernel/trace/bpf_trace.c
>> @@ -696,11 +696,13 @@ static const struct bpf_func_proto bpf_perf_event_=
output_proto_tp =3D {
>>         .arg5_type      =3D ARG_CONST_SIZE_OR_ZERO,
>>  };
>>=20
>> +static DEFINE_PER_CPU(struct pt_regs, bpf_stackid_tp_regs);
>>  BPF_CALL_3(bpf_get_stackid_tp, void *, tp_buff, struct bpf_map *, map,
>>            u64, flags)
>>  {
>> -       struct pt_regs *regs =3D *(struct pt_regs **)tp_buff;
>> +       struct pt_regs *regs =3D this_cpu_ptr(&bpf_stackid_tp_regs);
>>=20
>> +       perf_fetch_caller_regs(regs);
>=20
> No. pt_regs is already passed in. It's the first argument.
> If we call perf_fetch_caller_regs() again the stack trace will be wrong.
> bpf prog should not see itself, interpreter or all the frames in between.

Thanks Alexei! I get it now.=20

In bpf_get_stackid_tp(), the pt_regs is get by dereferencing the first fiel=
d
of tp_buff:

	struct pt_regs *regs =3D *(struct pt_regs **)tp_buff;

tp_buff points to something like

	struct sched_switch_args {
        	unsigned long long pad;
	        char prev_comm[16];
        	int prev_pid;
	        int prev_prio;
        	long long prev_state;
	        char next_comm[16];
        	int next_pid;
	        int next_prio;
	};

where the first field "pad" is a pointer to pt_regs.=20

@Kairui, I think you confirmed that current code will give empty call trace=
=20
with ORC unwinder? If that's the case, can we add regs->ip back? (as in the=
=20
first email of this thread.=20

Thanks,
Song