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:     Kairui Song <kasong@redhat.com>
CC:     Josh Poimboeuf <jpoimboe@redhat.com>,
        Alexei Starovoitov <ast@fb.com>,
        Peter Zijlstra <peterz@infradead.org>,
        lkml <linux-kernel@vger.kernel.org>,
        Kernel Team <Kernel-team@fb.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+ICAAAFQgIAAD2QAgAO6dwCABHK6gIAADQKAgAAxOYCAAGTygIAAdcuAgAAb14A=
Date:   Thu, 23 May 2019 08:27:50 +0000
Message-ID: <39AB1404-1C9A-43E9-A3EC-AED4AA26DC8C@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>
 <CACPcB9cpNp5CBqoRs+XMCwufzAFa8Pj-gbmj9fb+g5wVdue=ig@mail.gmail.com>
 <20190522140233.GC16275@worktop.programming.kicks-ass.net>
 <ab047883-69f6-1175-153f-5ad9462c6389@fb.com>
 <20190522174517.pbdopvookggen3d7@treble>
 <20190522234635.a47bettklcf5gt7c@treble>
 <CACPcB9dRJ89YAMDQdKoDMU=vFfpb5AaY0mWC_Xzw1ZMTFBf6ng@mail.gmail.com>
In-Reply-To: <CACPcB9dRJ89YAMDQdKoDMU=vFfpb5AaY0mWC_Xzw1ZMTFBf6ng@mail.gmail.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: <369C0EAFB3BD2943A719CC1D7F6C5BB1@namprd15.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 044b308b-45c0-498a-81cb-08d6df588b5d
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 May 2019 08:27:50.6359
 (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: MWHPR15MB1136
X-OriginatorOrg: fb.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2019-05-23_08:,,
 signatures=0
X-Proofpoint-Spam-Details: rule=fb_default_notspam policy=fb_default score=0 priorityscore=1501
 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0
 clxscore=1015 lowpriorityscore=0 mlxscore=0 impostorscore=0
 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx
 scancount=1 engine=8.0.1-1810050000 definitions=main-1905230061
X-FB-Internal: deliver
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 22, 2019, at 11:48 PM, Kairui Song <kasong@redhat.com> wrote:
>=20
> On Thu, May 23, 2019 at 7:46 AM Josh Poimboeuf <jpoimboe@redhat.com> wrot=
e:
>>=20
>> On Wed, May 22, 2019 at 12:45:17PM -0500, Josh Poimboeuf wrote:
>>> On Wed, May 22, 2019 at 02:49:07PM +0000, Alexei Starovoitov wrote:
>>>> The one that is broken is prog_tests/stacktrace_map.c
>>>> There we attach bpf to standard tracepoint where
>>>> kernel suppose to collect pt_regs before calling into bpf.
>>>> And that's what bpf_get_stackid_tp() is doing.
>>>> It passes pt_regs (that was collected before any bpf)
>>>> into bpf_get_stackid() which calls get_perf_callchain().
>>>> Same thing with kprobes, uprobes.
>>>=20
>>> Is it trying to unwind through ___bpf_prog_run()?
>>>=20
>>> If so, that would at least explain why ORC isn't working.  Objtool
>>> currently ignores that function because it can't follow the jump table.
>>=20
>> Here's a tentative fix (for ORC, at least).  I'll need to make sure this
>> doesn't break anything else.
>>=20
>> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
>> index 242a643af82f..1d9a7cc4b836 100644
>> --- a/kernel/bpf/core.c
>> +++ b/kernel/bpf/core.c
>> @@ -1562,7 +1562,6 @@ static u64 ___bpf_prog_run(u64 *regs, const struct=
 bpf_insn *insn, u64 *stack)
>>                BUG_ON(1);
>>                return 0;
>> }
>> -STACK_FRAME_NON_STANDARD(___bpf_prog_run); /* jump table */
>>=20
>> #define PROG_NAME(stack_size) __bpf_prog_run##stack_size
>> #define DEFINE_BPF_PROG_RUN(stack_size) \
>> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
>> index 172f99195726..2567027fce95 100644
>> --- a/tools/objtool/check.c
>> +++ b/tools/objtool/check.c
>> @@ -1033,13 +1033,6 @@ static struct rela *find_switch_table(struct objt=
ool_file *file,
>>                if (text_rela->type =3D=3D R_X86_64_PC32)
>>                        table_offset +=3D 4;
>>=20
>> -               /*
>> -                * Make sure the .rodata address isn't associated with a
>> -                * symbol.  gcc jump tables are anonymous data.
>> -                */
>> -               if (find_symbol_containing(rodata_sec, table_offset))
>> -                       continue;
>> -
>>                rodata_rela =3D find_rela_by_dest(rodata_sec, table_offse=
t);
>>                if (rodata_rela) {
>>                        /*
>=20
> Hi Josh, this still won't fix the problem.
>=20
> Problem is not (or not only) with ___bpf_prog_run, what actually went
> wrong is with the JITed bpf code.
>=20
> For frame pointer unwinder, it seems the JITed bpf code will have a
> shifted "BP" register? (arch/x86/net/bpf_jit_comp.c:217), so if we can
> unshift it properly then it will work.
>=20
> I tried below code, and problem is fixed (only for frame pointer
> unwinder though). Need to find a better way to detect and do any
> similar trick for bpf part, if this is a feasible way to fix it:
>=20
> diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_fram=
e.c
> index 9b9fd4826e7a..2c0fa2aaa7e4 100644
> --- a/arch/x86/kernel/unwind_frame.c
> +++ b/arch/x86/kernel/unwind_frame.c
> @@ -330,8 +330,17 @@ bool unwind_next_frame(struct unwind_state *state)
>        }
>=20
>        /* Move to the next frame if it's safe: */
> -       if (!update_stack_state(state, next_bp))
> -               goto bad_address;
> +       if (!update_stack_state(state, next_bp)) {
> +               // Try again with shifted BP
> +               state->bp +=3D 5; // see AUX_STACK_SPACE
> +               next_bp =3D (unsigned long
> *)READ_ONCE_TASK_STACK(state->task, *state->bp);
> +               // Clean and refetch stack info, it's marked as error out=
ed
> +               state->stack_mask =3D 0;
> +               get_stack_info(next_bp, state->task,
> &state->stack_info, &state->stack_mask);
> +               if (!update_stack_state(state, next_bp)) {
> +                       goto bad_address;
> +               }
> +       }
>=20
>        return true;
>=20
> For ORC unwinder, I think the unwinder can't find any info about the
> JITed part. Maybe if can let it just skip the JITed part and go to
> kernel context, then should be good enough.

In this case (tracepoint), the callchain bpf_get_stackid() fetches is the=20
callchain at the tracepoint. So we don't need the JITed part.=20

BPF program passes the regs at the tracepoint to perf_callchain_kernel().=20
However, perf_callchain_kernel() only uses regs->sp for !perf_hw_regs()
case. This is probably expected, as passing regs in doesn't really help.=20

There are multiple cases in unwind_orc.c:__unwind_start(), which I don't=20
understand very well.=20

Does the above make sense? Did I mis-understand anything?

@Alexei, do you remember some rough time/version that ORC unwinder works
well for tracepoints? Maybe we can dig into that version to see the
difference.  =20

Thanks,
Song

>=20
>=20
>=20
>=20
>=20
> --
> Best Regards,
> Kairui Song