bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kairui Song <kasong@redhat.com>
To: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Alexei Starovoitov <ast@fb.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Song Liu <songliubraving@fb.com>,
	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()
Date: Thu, 23 May 2019 14:48:11 +0800	[thread overview]
Message-ID: <CACPcB9dRJ89YAMDQdKoDMU=vFfpb5AaY0mWC_Xzw1ZMTFBf6ng@mail.gmail.com> (raw)
In-Reply-To: <20190522234635.a47bettklcf5gt7c@treble>

On Thu, May 23, 2019 at 7:46 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> 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.
> >
> > Is it trying to unwind through ___bpf_prog_run()?
> >
> > 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.
>
> Here's a tentative fix (for ORC, at least).  I'll need to make sure this
> doesn't break anything else.
>
> 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 */
>
>  #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 objtool_file *file,
>                 if (text_rela->type == R_X86_64_PC32)
>                         table_offset += 4;
>
> -               /*
> -                * 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 = find_rela_by_dest(rodata_sec, table_offset);
>                 if (rodata_rela) {
>                         /*

Hi Josh, this still won't fix the problem.

Problem is not (or not only) with ___bpf_prog_run, what actually went
wrong is with the JITed bpf code.

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.

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:

diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.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)
        }

        /* 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 += 5; // see AUX_STACK_SPACE
+               next_bp = (unsigned long
*)READ_ONCE_TASK_STACK(state->task, *state->bp);
+               // Clean and refetch stack info, it's marked as error outed
+               state->stack_mask = 0;
+               get_stack_info(next_bp, state->task,
&state->stack_info, &state->stack_mask);
+               if (!update_stack_state(state, next_bp)) {
+                       goto bad_address;
+               }
+       }

        return true;

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.





--
Best Regards,
Kairui Song

  reply	other threads:[~2019-05-23  6:48 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <3CD3EE63-0CD2-404A-A403-E11DCF2DF8D9@fb.com>
     [not found] ` <20190517074600.GJ2623@hirez.programming.kicks-ass.net>
     [not found]   ` <20190517081057.GQ2650@hirez.programming.kicks-ass.net>
     [not found]     ` <CACPcB9cB5n1HOmZcVpusJq8rAV5+KfmZ-Lxv3tgsSoy7vNrk7w@mail.gmail.com>
     [not found]       ` <20190517091044.GM2606@hirez.programming.kicks-ass.net>
2019-05-17 18:40         ` Getting empty callchain from perf_callchain_kernel() Song Liu
2019-05-17 21:06           ` Alexei Starovoitov
2019-05-17 21:48             ` Song Liu
2019-05-19 18:07               ` Kairui Song
2019-05-20 17:22                 ` Song Liu
2019-05-22 13:51                   ` Peter Zijlstra
2019-05-19 18:06         ` Kairui Song
2019-05-20 17:16           ` Song Liu
2019-05-20 17:19           ` Song Liu
2019-05-22 14:02           ` Peter Zijlstra
2019-05-22 14:49             ` Alexei Starovoitov
2019-05-22 17:45               ` Josh Poimboeuf
2019-05-22 23:46                 ` Josh Poimboeuf
2019-05-23  6:48                   ` Kairui Song [this message]
2019-05-23  8:27                     ` Song Liu
2019-05-23  9:11                       ` Kairui Song
2019-05-23 13:32                     ` Josh Poimboeuf
2019-05-23 14:50                       ` Kairui Song
2019-05-23 15:24                         ` Josh Poimboeuf
2019-05-23 16:41                           ` Kairui Song
2019-05-23 17:27                             ` Josh Poimboeuf
2019-05-24  2:20                               ` Kairui Song
2019-05-24 23:23                                 ` Josh Poimboeuf
2019-05-27 11:57                                   ` Kairui Song
2019-06-06 16:04                                     ` Song Liu
2019-06-06 23:58                                       ` Josh Poimboeuf
2019-06-11 21:03                                       ` Josh Poimboeuf
2019-05-24  8:53                           ` Peter Zijlstra
2019-05-24 13:05                             ` Josh Poimboeuf
2019-06-12  3:05                             ` Josh Poimboeuf
2019-06-12  8:54                               ` Peter Zijlstra
2019-06-12 14:50                                 ` Josh Poimboeuf
2019-06-13 20:26                                   ` Josh Poimboeuf
2019-06-12 13:10                               ` Steven Rostedt
2019-06-12 14:26                                 ` Josh Poimboeuf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CACPcB9dRJ89YAMDQdKoDMU=vFfpb5AaY0mWC_Xzw1ZMTFBf6ng@mail.gmail.com' \
    --to=kasong@redhat.com \
    --cc=Kernel-team@fb.com \
    --cc=ast@fb.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=songliubraving@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).