lkml.org 
[lkml]   [2022]   [Feb]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 3/3] perf tools: Rework prologue generation code
    On Fri, Feb 18, 2022 at 02:03:28PM +0100, Jiri Olsa wrote:
    > On Fri, Feb 18, 2022 at 10:01:45AM +0100, Jiri Olsa wrote:
    > > On Thu, Feb 17, 2022 at 01:53:16PM -0800, Andrii Nakryiko wrote:
    > > > On Thu, Feb 17, 2022 at 5:19 AM Jiri Olsa <jolsa@kernel.org> wrote:
    > > > >
    > > > > Some functions we use now for bpf prologue generation are
    > > > > going to be deprecated, so reworking the current code not
    > > > > to use them.
    > > > >
    > > > > We need to replace following functions/struct:
    > > > > bpf_program__set_prep
    > > > > bpf_program__nth_fd
    > > > > struct bpf_prog_prep_result
    > > > >
    > > > > Current code uses bpf_program__set_prep to hook perf callback
    > > > > before the program is loaded and provide new instructions with
    > > > > the prologue.
    > > > >
    > > > > We workaround this by using objects's 'unloaded' programs instructions
    > > > > for that specific program and load new ebpf programs with prologue
    > > > > using separate bpf_prog_load calls.
    > > > >
    > > > > We keep new ebpf program instances descriptors in bpf programs
    > > > > private struct.
    > > > >
    > > > > Suggested-by: Andrii Nakryiko <andrii@kernel.org>
    > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
    > > > > ---
    > > > > tools/perf/util/bpf-loader.c | 122 +++++++++++++++++++++++++++++------
    > > > > 1 file changed, 104 insertions(+), 18 deletions(-)
    > > > >
    > > >
    > > > [...]
    > > >
    > > > > errout:
    > > > > @@ -696,7 +718,7 @@ static int hook_load_preprocessor(struct bpf_program *prog)
    > > > > struct bpf_prog_priv *priv = program_priv(prog);
    > > > > struct perf_probe_event *pev;
    > > > > bool need_prologue = false;
    > > > > - int err, i;
    > > > > + int i;
    > > > >
    > > > > if (IS_ERR_OR_NULL(priv)) {
    > > > > pr_debug("Internal error when hook preprocessor\n");
    > > > > @@ -727,6 +749,12 @@ static int hook_load_preprocessor(struct bpf_program *prog)
    > > > > return 0;
    > > > > }
    > > > >
    > > > > + /*
    > > > > + * Do not load programs that need prologue, because we need
    > > > > + * to add prologue first, check bpf_object__load_prologue.
    > > > > + */
    > > > > + bpf_program__set_autoload(prog, false);
    > > >
    > > > if you set autoload to false, program instructions might be invalid in
    > > > the end. Libbpf doesn't apply some (all?) relocations to such
    > > > programs, doesn't resolve CO-RE, etc, etc. You have to let
    > > > "prototypal" BPF program to be loaded before you can grab final
    > > > instructions. It's not great, but in your case it should work, right?
    > >
    > > hum, do we care? it should all be done when the 'new' program with
    > > the prologue is loaded, right?
    > >
    > > I switched it off because the verifier failed to load the program
    > > without the prologue.. because in the originaal program there's no
    > > code to grab the arguments that the rest of the code depends on,
    > > so the verifier sees invalid access
    > >
    > > >
    > > > > +
    > > > > priv->need_prologue = true;
    > > > > priv->insns_buf = malloc(sizeof(struct bpf_insn) * BPF_MAXINSNS);
    > > > > if (!priv->insns_buf) {
    > > > > @@ -734,6 +762,13 @@ static int hook_load_preprocessor(struct bpf_program *prog)
    > > > > return -ENOMEM;
    > > > > }
    > > > >
    > > >
    > > > [...]
    > > >
    > > > > + /*
    > > > > + * For each program that needs prologue we do following:
    > > > > + *
    > > > > + * - take its current instructions and use them
    > > > > + * to generate the new code with prologue
    > > > > + *
    > > > > + * - load new instructions with bpf_prog_load
    > > > > + * and keep the fd in proglogue_fds
    > > > > + *
    > > > > + * - new fd will be used bpf__foreach_event
    > > > > + * to connect this program with perf evsel
    > > > > + */
    > > > > + orig_insns = bpf_program__insns(prog);
    > > > > + orig_insns_cnt = bpf_program__insn_cnt(prog);
    > > > > +
    > > > > + pev = &priv->pev;
    > > > > + for (i = 0; i < pev->ntevs; i++) {
    > > > > + err = preproc_gen_prologue(prog, i, orig_insns,
    > > > > + orig_insns_cnt, &res);
    > > > > + if (err)
    > > > > + return err;
    > > > > +
    > > > > + fd = bpf_prog_load(bpf_program__get_type(prog),
    > > >
    > > > nit: bpf_program__type() is preferred (we are deprecating/discouraging
    > > > "get_" prefixed getters in libbpf 1.0)
    > >
    > > ok, will change
    >
    > hum, I can't see bpf_program__type.. what do I miss?

    nah I was on top of perf/core.. I see it now ;-)

    jirka

    \
     
     \ /
      Last update: 2022-02-18 15:23    [W:3.280 / U:0.772 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site