Hi Jiri and Alexei,

Here is the 2nd version of fprobe. This version uses the
ftrace_set_filter_ips() for reducing the registering overhead.
Note that this also drops per-probe point private data, which
is not used anyway.

This introduces the fprobe, the function entry/exit probe with
multiple probe point support. This also introduces the rethook
for hooking function return as same as kretprobe does. This
abstraction will help us to generalize the fgraph tracer,
because we can just switch it from rethook in fprobe, depending
on the kernel configuration.

The patch [1/8] and [7/8] are from your series[1]. Other libbpf
patches will not be affected by this change.

[1] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#u

I also added an out-of-tree (just for testing) patch at the
end of this series ([8/8]) for adding a wildcard support to
the sample program. With that patch, it shows how long the
registration will take;

# time insmod fprobe_example.ko symbol='btrfs_*'
[   36.130947] fprobe_init: 1028 symbols found
[   36.177901] fprobe_init: Planted fprobe at btrfs_*
real    0m 0.08s
user    0m 0.00s
sys     0m 0.07s

Thank you,

---

Jiri Olsa (2):
      ftrace: Add ftrace_set_filter_ips function
      bpf: Add kprobe link for attaching raw kprobes

Masami Hiramatsu (6):
      fprobe: Add ftrace based probe APIs
      rethook: Add a generic return hook
      rethook: x86: Add rethook x86 implementation
      fprobe: Add exit_handler support
      fprobe: Add sample program for fprobe
      [DO NOT MERGE] Out-of-tree: Support wildcard symbol option to sample


 arch/x86/Kconfig                |    1 
 arch/x86/kernel/Makefile        |    1 
 arch/x86/kernel/rethook.c       |  115 ++++++++++++++++++++
 include/linux/bpf_types.h       |    1 
 include/linux/fprobe.h          |   57 ++++++++++
 include/linux/ftrace.h          |    3 +
 include/linux/rethook.h         |   74 +++++++++++++
 include/linux/sched.h           |    3 +
 include/uapi/linux/bpf.h        |   12 ++
 kernel/bpf/syscall.c            |  195 +++++++++++++++++++++++++++++++++-
 kernel/exit.c                   |    2 
 kernel/fork.c                   |    3 +
 kernel/kallsyms.c               |    1 
 kernel/trace/Kconfig            |   22 ++++
 kernel/trace/Makefile           |    2 
 kernel/trace/fprobe.c           |  168 +++++++++++++++++++++++++++++
 kernel/trace/ftrace.c           |   54 ++++++++-
 kernel/trace/rethook.c          |  226 +++++++++++++++++++++++++++++++++++++++
 samples/Kconfig                 |    7 +
 samples/Makefile                |    1 
 samples/fprobe/Makefile         |    3 +
 samples/fprobe/fprobe_example.c |  154 +++++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h  |   12 ++
 23 files changed, 1103 insertions(+), 14 deletions(-)
 create mode 100644 arch/x86/kernel/rethook.c
 create mode 100644 include/linux/fprobe.h
 create mode 100644 include/linux/rethook.h
 create mode 100644 kernel/trace/fprobe.c
 create mode 100644 kernel/trace/rethook.c
 create mode 100644 samples/fprobe/Makefile
 create mode 100644 samples/fprobe/fprobe_example.c

--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

On Wed, Jan 12, 2022 at 6:02 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> Hi Jiri and Alexei,
>
> Here is the 2nd version of fprobe. This version uses the
> ftrace_set_filter_ips() for reducing the registering overhead.
> Note that this also drops per-probe point private data, which
> is not used anyway.

This per-probe private data is necessary for the feature called BPF
cookie, in which each attachment has a unique user-provided u64 value
associated to it, accessible at runtime through
bpf_get_attach_cookie() helper. One way or another we'll need to
support this to make these multi-attach BPF programs really useful for
generic tracing applications.

Jiri,

We've discussed with Alexei this week how cookies can be supported for
multi-attach fentry (where it seems even more challenging than in
kprobe case), and agreed on rather simple solution, which roughly goes
like this. When multi-attaching either fentry/fexit program, save
sorted array of IP addresses and then sorted in the same order as IPs
list of u64 cookies. At runtime, bpf_get_attach_cookie() helper should
somehow get access to these two arrays and functions IP (that we
already have with bpf_get_func_ip()), perform binary search and locate
necessary cookie. This offloads the overhead of finding this cookie to
actual call site of bpf_get_attach_cookie() (and it's a log(N), so not
bad at all, especially if BPF program can be optimized to call this
helper just once).

I think something like that should be doable for Masami's fprobe-based
multi-attach kprobes, right? That would allow to have super-fast
attachment, but still support BPF cookie per each individual IP/kernel
function attachment. I haven't looked at code thoroughly, though,
please let me know if I'm missing something fundamental.

>
> This introduces the fprobe, the function entry/exit probe with
> multiple probe point support. This also introduces the rethook
> for hooking function return as same as kretprobe does. This
> abstraction will help us to generalize the fgraph tracer,
> because we can just switch it from rethook in fprobe, depending
> on the kernel configuration.
>
> The patch [1/8] and [7/8] are from your series[1]. Other libbpf
> patches will not be affected by this change.
>
> [1] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#u
>
> I also added an out-of-tree (just for testing) patch at the
> end of this series ([8/8]) for adding a wildcard support to
> the sample program. With that patch, it shows how long the
> registration will take;
>
> # time insmod fprobe_example.ko symbol='btrfs_*'
> [   36.130947] fprobe_init: 1028 symbols found
> [   36.177901] fprobe_init: Planted fprobe at btrfs_*
> real    0m 0.08s
> user    0m 0.00s
> sys     0m 0.07s
>
> Thank you,
>
> ---
>
> Jiri Olsa (2):
>       ftrace: Add ftrace_set_filter_ips function
>       bpf: Add kprobe link for attaching raw kprobes
>
> Masami Hiramatsu (6):
>       fprobe: Add ftrace based probe APIs
>       rethook: Add a generic return hook
>       rethook: x86: Add rethook x86 implementation
>       fprobe: Add exit_handler support
>       fprobe: Add sample program for fprobe
>       [DO NOT MERGE] Out-of-tree: Support wildcard symbol option to sample
>
>
>  arch/x86/Kconfig                |    1
>  arch/x86/kernel/Makefile        |    1
>  arch/x86/kernel/rethook.c       |  115 ++++++++++++++++++++
>  include/linux/bpf_types.h       |    1
>  include/linux/fprobe.h          |   57 ++++++++++
>  include/linux/ftrace.h          |    3 +
>  include/linux/rethook.h         |   74 +++++++++++++
>  include/linux/sched.h           |    3 +
>  include/uapi/linux/bpf.h        |   12 ++
>  kernel/bpf/syscall.c            |  195 +++++++++++++++++++++++++++++++++-
>  kernel/exit.c                   |    2
>  kernel/fork.c                   |    3 +
>  kernel/kallsyms.c               |    1
>  kernel/trace/Kconfig            |   22 ++++
>  kernel/trace/Makefile           |    2
>  kernel/trace/fprobe.c           |  168 +++++++++++++++++++++++++++++
>  kernel/trace/ftrace.c           |   54 ++++++++-
>  kernel/trace/rethook.c          |  226 +++++++++++++++++++++++++++++++++++++++
>  samples/Kconfig                 |    7 +
>  samples/Makefile                |    1
>  samples/fprobe/Makefile         |    3 +
>  samples/fprobe/fprobe_example.c |  154 +++++++++++++++++++++++++++
>  tools/include/uapi/linux/bpf.h  |   12 ++
>  23 files changed, 1103 insertions(+), 14 deletions(-)
>  create mode 100644 arch/x86/kernel/rethook.c
>  create mode 100644 include/linux/fprobe.h
>  create mode 100644 include/linux/rethook.h
>  create mode 100644 kernel/trace/fprobe.c
>  create mode 100644 kernel/trace/rethook.c
>  create mode 100644 samples/fprobe/Makefile
>  create mode 100644 samples/fprobe/fprobe_example.c
>
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

On Fri, Jan 14, 2022 at 05:08:32PM -0800, Andrii Nakryiko wrote:
> On Wed, Jan 12, 2022 at 6:02 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >
> > Hi Jiri and Alexei,
> >
> > Here is the 2nd version of fprobe. This version uses the
> > ftrace_set_filter_ips() for reducing the registering overhead.
> > Note that this also drops per-probe point private data, which
> > is not used anyway.
> 
> This per-probe private data is necessary for the feature called BPF
> cookie, in which each attachment has a unique user-provided u64 value
> associated to it, accessible at runtime through
> bpf_get_attach_cookie() helper. One way or another we'll need to
> support this to make these multi-attach BPF programs really useful for
> generic tracing applications.
> 
> Jiri,
> 
> We've discussed with Alexei this week how cookies can be supported for
> multi-attach fentry (where it seems even more challenging than in
> kprobe case), and agreed on rather simple solution, which roughly goes
> like this. When multi-attaching either fentry/fexit program, save
> sorted array of IP addresses and then sorted in the same order as IPs
> list of u64 cookies. At runtime, bpf_get_attach_cookie() helper should
> somehow get access to these two arrays and functions IP (that we
> already have with bpf_get_func_ip()), perform binary search and locate
> necessary cookie. This offloads the overhead of finding this cookie to
> actual call site of bpf_get_attach_cookie() (and it's a log(N), so not
> bad at all, especially if BPF program can be optimized to call this
> helper just once).
> 
> I think something like that should be doable for Masami's fprobe-based
> multi-attach kprobes, right? That would allow to have super-fast
> attachment, but still support BPF cookie per each individual IP/kernel
> function attachment. I haven't looked at code thoroughly, though,
> please let me know if I'm missing something fundamental.

ok, that seems doable, we should be able to get the link struct
in bpf_get_attach_cookie_trace and reach both ips and cookies

jirka

> 
> >
> > This introduces the fprobe, the function entry/exit probe with
> > multiple probe point support. This also introduces the rethook
> > for hooking function return as same as kretprobe does. This
> > abstraction will help us to generalize the fgraph tracer,
> > because we can just switch it from rethook in fprobe, depending
> > on the kernel configuration.
> >
> > The patch [1/8] and [7/8] are from your series[1]. Other libbpf
> > patches will not be affected by this change.
> >
> > [1] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#u
> >
> > I also added an out-of-tree (just for testing) patch at the
> > end of this series ([8/8]) for adding a wildcard support to
> > the sample program. With that patch, it shows how long the
> > registration will take;
> >
> > # time insmod fprobe_example.ko symbol='btrfs_*'
> > [   36.130947] fprobe_init: 1028 symbols found
> > [   36.177901] fprobe_init: Planted fprobe at btrfs_*
> > real    0m 0.08s
> > user    0m 0.00s
> > sys     0m 0.07s
> >
> > Thank you,
> >
> > ---
> >
> > Jiri Olsa (2):
> >       ftrace: Add ftrace_set_filter_ips function
> >       bpf: Add kprobe link for attaching raw kprobes
> >
> > Masami Hiramatsu (6):
> >       fprobe: Add ftrace based probe APIs
> >       rethook: Add a generic return hook
> >       rethook: x86: Add rethook x86 implementation
> >       fprobe: Add exit_handler support
> >       fprobe: Add sample program for fprobe
> >       [DO NOT MERGE] Out-of-tree: Support wildcard symbol option to sample
> >
> >
> >  arch/x86/Kconfig                |    1
> >  arch/x86/kernel/Makefile        |    1
> >  arch/x86/kernel/rethook.c       |  115 ++++++++++++++++++++
> >  include/linux/bpf_types.h       |    1
> >  include/linux/fprobe.h          |   57 ++++++++++
> >  include/linux/ftrace.h          |    3 +
> >  include/linux/rethook.h         |   74 +++++++++++++
> >  include/linux/sched.h           |    3 +
> >  include/uapi/linux/bpf.h        |   12 ++
> >  kernel/bpf/syscall.c            |  195 +++++++++++++++++++++++++++++++++-
> >  kernel/exit.c                   |    2
> >  kernel/fork.c                   |    3 +
> >  kernel/kallsyms.c               |    1
> >  kernel/trace/Kconfig            |   22 ++++
> >  kernel/trace/Makefile           |    2
> >  kernel/trace/fprobe.c           |  168 +++++++++++++++++++++++++++++
> >  kernel/trace/ftrace.c           |   54 ++++++++-
> >  kernel/trace/rethook.c          |  226 +++++++++++++++++++++++++++++++++++++++
> >  samples/Kconfig                 |    7 +
> >  samples/Makefile                |    1
> >  samples/fprobe/Makefile         |    3 +
> >  samples/fprobe/fprobe_example.c |  154 +++++++++++++++++++++++++++
> >  tools/include/uapi/linux/bpf.h  |   12 ++
> >  23 files changed, 1103 insertions(+), 14 deletions(-)
> >  create mode 100644 arch/x86/kernel/rethook.c
> >  create mode 100644 include/linux/fprobe.h
> >  create mode 100644 include/linux/rethook.h
> >  create mode 100644 kernel/trace/fprobe.c
> >  create mode 100644 kernel/trace/rethook.c
> >  create mode 100644 samples/fprobe/Makefile
> >  create mode 100644 samples/fprobe/fprobe_example.c
> >
> > --
> > Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
> 

On Wed, Jan 12, 2022 at 11:02:46PM +0900, Masami Hiramatsu wrote:
> Hi Jiri and Alexei,
> 
> Here is the 2nd version of fprobe. This version uses the
> ftrace_set_filter_ips() for reducing the registering overhead.
> Note that this also drops per-probe point private data, which
> is not used anyway.
> 
> This introduces the fprobe, the function entry/exit probe with
> multiple probe point support. This also introduces the rethook
> for hooking function return as same as kretprobe does. This

nice, I was going through the multi-user-graph support 
and was wondering that this might be a better way

> abstraction will help us to generalize the fgraph tracer,
> because we can just switch it from rethook in fprobe, depending
> on the kernel configuration.
> 
> The patch [1/8] and [7/8] are from your series[1]. Other libbpf
> patches will not be affected by this change.

I'll try the bpf selftests on top of this

> 
> [1] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#u
> 
> I also added an out-of-tree (just for testing) patch at the
> end of this series ([8/8]) for adding a wildcard support to
> the sample program. With that patch, it shows how long the
> registration will take;
> 
> # time insmod fprobe_example.ko symbol='btrfs_*'
> [   36.130947] fprobe_init: 1028 symbols found
> [   36.177901] fprobe_init: Planted fprobe at btrfs_*
> real    0m 0.08s
> user    0m 0.00s
> sys     0m 0.07s

I'll run my bpftrace tests on top of that

thanks,
jirka

> 
> Thank you,
> 
> ---
> 
> Jiri Olsa (2):
>       ftrace: Add ftrace_set_filter_ips function
>       bpf: Add kprobe link for attaching raw kprobes
> 
> Masami Hiramatsu (6):
>       fprobe: Add ftrace based probe APIs
>       rethook: Add a generic return hook
>       rethook: x86: Add rethook x86 implementation
>       fprobe: Add exit_handler support
>       fprobe: Add sample program for fprobe
>       [DO NOT MERGE] Out-of-tree: Support wildcard symbol option to sample
> 
> 
>  arch/x86/Kconfig                |    1 
>  arch/x86/kernel/Makefile        |    1 
>  arch/x86/kernel/rethook.c       |  115 ++++++++++++++++++++
>  include/linux/bpf_types.h       |    1 
>  include/linux/fprobe.h          |   57 ++++++++++
>  include/linux/ftrace.h          |    3 +
>  include/linux/rethook.h         |   74 +++++++++++++
>  include/linux/sched.h           |    3 +
>  include/uapi/linux/bpf.h        |   12 ++
>  kernel/bpf/syscall.c            |  195 +++++++++++++++++++++++++++++++++-
>  kernel/exit.c                   |    2 
>  kernel/fork.c                   |    3 +
>  kernel/kallsyms.c               |    1 
>  kernel/trace/Kconfig            |   22 ++++
>  kernel/trace/Makefile           |    2 
>  kernel/trace/fprobe.c           |  168 +++++++++++++++++++++++++++++
>  kernel/trace/ftrace.c           |   54 ++++++++-
>  kernel/trace/rethook.c          |  226 +++++++++++++++++++++++++++++++++++++++
>  samples/Kconfig                 |    7 +
>  samples/Makefile                |    1 
>  samples/fprobe/Makefile         |    3 +
>  samples/fprobe/fprobe_example.c |  154 +++++++++++++++++++++++++++
>  tools/include/uapi/linux/bpf.h  |   12 ++
>  23 files changed, 1103 insertions(+), 14 deletions(-)
>  create mode 100644 arch/x86/kernel/rethook.c
>  create mode 100644 include/linux/fprobe.h
>  create mode 100644 include/linux/rethook.h
>  create mode 100644 kernel/trace/fprobe.c
>  create mode 100644 kernel/trace/rethook.c
>  create mode 100644 samples/fprobe/Makefile
>  create mode 100644 samples/fprobe/fprobe_example.c
> 
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
> 

On Wed, Jan 12, 2022 at 05:01:15PM +0100, Jiri Olsa wrote:
> On Wed, Jan 12, 2022 at 11:02:46PM +0900, Masami Hiramatsu wrote:
> > Hi Jiri and Alexei,
> > 
> > Here is the 2nd version of fprobe. This version uses the
> > ftrace_set_filter_ips() for reducing the registering overhead.
> > Note that this also drops per-probe point private data, which
> > is not used anyway.
> > 
> > This introduces the fprobe, the function entry/exit probe with
> > multiple probe point support. This also introduces the rethook
> > for hooking function return as same as kretprobe does. This
> 
> nice, I was going through the multi-user-graph support 
> and was wondering that this might be a better way
> 
> > abstraction will help us to generalize the fgraph tracer,
> > because we can just switch it from rethook in fprobe, depending
> > on the kernel configuration.
> > 
> > The patch [1/8] and [7/8] are from your series[1]. Other libbpf
> > patches will not be affected by this change.
> 
> I'll try the bpf selftests on top of this

I'm getting crash and stall when running bpf selftests,
the fprobe sample module works fine, I'll check on that

jirka

On Thu, 13 Jan 2022 13:27:34 +0100
Jiri Olsa <jolsa@redhat.com> wrote:

> On Wed, Jan 12, 2022 at 05:01:15PM +0100, Jiri Olsa wrote:
> > On Wed, Jan 12, 2022 at 11:02:46PM +0900, Masami Hiramatsu wrote:
> > > Hi Jiri and Alexei,
> > > 
> > > Here is the 2nd version of fprobe. This version uses the
> > > ftrace_set_filter_ips() for reducing the registering overhead.
> > > Note that this also drops per-probe point private data, which
> > > is not used anyway.
> > > 
> > > This introduces the fprobe, the function entry/exit probe with
> > > multiple probe point support. This also introduces the rethook
> > > for hooking function return as same as kretprobe does. This
> > 
> > nice, I was going through the multi-user-graph support 
> > and was wondering that this might be a better way
> > 
> > > abstraction will help us to generalize the fgraph tracer,
> > > because we can just switch it from rethook in fprobe, depending
> > > on the kernel configuration.
> > > 
> > > The patch [1/8] and [7/8] are from your series[1]. Other libbpf
> > > patches will not be affected by this change.
> > 
> > I'll try the bpf selftests on top of this
> 
> I'm getting crash and stall when running bpf selftests,
> the fprobe sample module works fine, I'll check on that

Hmm, would you mean tools/testing/selftests/bpf/prog_tests/ ?
Let me check it too.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

Hi Jiri and Alexei,

On Thu, 13 Jan 2022 13:27:34 +0100
Jiri Olsa <jolsa@redhat.com> wrote:

> On Wed, Jan 12, 2022 at 05:01:15PM +0100, Jiri Olsa wrote:
> > On Wed, Jan 12, 2022 at 11:02:46PM +0900, Masami Hiramatsu wrote:
> > > Hi Jiri and Alexei,
> > > 
> > > Here is the 2nd version of fprobe. This version uses the
> > > ftrace_set_filter_ips() for reducing the registering overhead.
> > > Note that this also drops per-probe point private data, which
> > > is not used anyway.
> > > 
> > > This introduces the fprobe, the function entry/exit probe with
> > > multiple probe point support. This also introduces the rethook
> > > for hooking function return as same as kretprobe does. This
> > 
> > nice, I was going through the multi-user-graph support 
> > and was wondering that this might be a better way
> > 
> > > abstraction will help us to generalize the fgraph tracer,
> > > because we can just switch it from rethook in fprobe, depending
> > > on the kernel configuration.
> > > 
> > > The patch [1/8] and [7/8] are from your series[1]. Other libbpf
> > > patches will not be affected by this change.
> > 
> > I'll try the bpf selftests on top of this
> 
> I'm getting crash and stall when running bpf selftests,
> the fprobe sample module works fine, I'll check on that

I've tried to build tools/testing/selftests/bpf on my machine,
but I got below errors. Would you know how I can setup to build
the bpf selftests correctly? (I tried "make M=samples/bpf", but same result)

~/ksrc/linux/tools/testing/selftests/bpf$ make 
[...]
  CLANG   /home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool/pid_iter.bpf.o
skeleton/pid_iter.bpf.c:35:10: error: incomplete definition of type 'struct bpf_link'
                return BPF_CORE_READ((struct bpf_link *)ent, id);
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:403:2: note: expanded from macro 'BPF_CORE_READ'
        ___type((src), a, ##__VA_ARGS__) __r;                               \
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:274:29: note: expanded from macro '___type'
#define ___type(...) typeof(___arrow(__VA_ARGS__))
                            ^~~~~~~~~~~~~~~~~~~~~
/home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:272:23: note: expanded from macro '___arrow'
#define ___arrow(...) ___apply(___arrow, ___narg(__VA_ARGS__))(__VA_ARGS__)
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
/home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:223:25: note: expanded from macro '___concat'
#define ___concat(a, b) a ## b
                        ^
<scratch space>:16:1: note: expanded from here
___arrow2
^
/home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:263:26: note: expanded from macro '___arrow2'
#define ___arrow2(a, b) a->b
                        ~^
skeleton/pid_iter.bpf.c:35:32: note: forward declaration of 'struct bpf_link'
                return BPF_CORE_READ((struct bpf_link *)ent, id);
                                             ^
skeleton/pid_iter.bpf.c:35:10: error: incomplete definition of type 'struct bpf_link'
                return BPF_CORE_READ((struct bpf_link *)ent, id);
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:404:2: note: expanded from macro 'BPF_CORE_READ'
        BPF_CORE_READ_INTO(&__r, (src), a, ##__VA_ARGS__);                  \
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:311:2: note: expanded from macro 'BPF_CORE_READ_INTO'
        ___core_read(bpf_core_read, bpf_core_read,                          \
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:302:2: note: expanded from macro '___core_read'
        ___apply(___core_read, ___empty(__VA_ARGS__))(fn, fn_ptr, dst,      \
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
note: (skipping 3 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
/home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:296:2: note: expanded from macro '___core_read0'
        ___read(fn, dst, ___type(src), src, a);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:277:59: note: expanded from macro '___read'
        read_fn((void *)(dst), sizeof(*(dst)), &((src_type)(src))->accessor)
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
/home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:206:79: note: expanded from macro 'bpf_core_read'
        bpf_probe_read_kernel(dst, sz, (const void *)__builtin_preserve_access_index(src))
                                                                                     ^~~
skeleton/pid_iter.bpf.c:35:32: note: forward declaration of 'struct bpf_link'
                return BPF_CORE_READ((struct bpf_link *)ent, id);
                                             ^
skeleton/pid_iter.bpf.c:35:10: error: returning 'void' from a function with incompatible result type '__u32' (aka 'unsigned int')
                return BPF_CORE_READ((struct bpf_link *)ent, id);
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:402:36: note: expanded from macro 'BPF_CORE_READ'
#define BPF_CORE_READ(src, a, ...) ({                                       \
                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
skeleton/pid_iter.bpf.c:42:17: warning: declaration of 'struct bpf_iter__task_file' will not be visible outside of this function [-Wvisibility]
int iter(struct bpf_iter__task_file *ctx)
                ^
skeleton/pid_iter.bpf.c:44:25: error: incomplete definition of type 'struct bpf_iter__task_file'
        struct file *file = ctx->file;
                            ~~~^
skeleton/pid_iter.bpf.c:42:17: note: forward declaration of 'struct bpf_iter__task_file'
int iter(struct bpf_iter__task_file *ctx)
                ^
skeleton/pid_iter.bpf.c:45:32: error: incomplete definition of type 'struct bpf_iter__task_file'
        struct task_struct *task = ctx->task;
                                   ~~~^
skeleton/pid_iter.bpf.c:42:17: note: forward declaration of 'struct bpf_iter__task_file'
int iter(struct bpf_iter__task_file *ctx)
                ^
skeleton/pid_iter.bpf.c:76:19: error: incomplete definition of type 'struct bpf_iter__task_file'
        bpf_seq_write(ctx->meta->seq, &e, sizeof(e));
                      ~~~^
skeleton/pid_iter.bpf.c:42:17: note: forward declaration of 'struct bpf_iter__task_file'
int iter(struct bpf_iter__task_file *ctx)
                ^
1 warning and 6 errors generated.
make[1]: *** [Makefile:188: /home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool/pid_iter.bpf.o] Error 1
make: *** [Makefile:219: /home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/sbin/bpftool] Error 2


Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

On Fri, Jan 14, 2022 at 11:47:04PM +0900, Masami Hiramatsu wrote:
> Hi Jiri and Alexei,
> 
> On Thu, 13 Jan 2022 13:27:34 +0100
> Jiri Olsa <jolsa@redhat.com> wrote:
> 
> > On Wed, Jan 12, 2022 at 05:01:15PM +0100, Jiri Olsa wrote:
> > > On Wed, Jan 12, 2022 at 11:02:46PM +0900, Masami Hiramatsu wrote:
> > > > Hi Jiri and Alexei,
> > > > 
> > > > Here is the 2nd version of fprobe. This version uses the
> > > > ftrace_set_filter_ips() for reducing the registering overhead.
> > > > Note that this also drops per-probe point private data, which
> > > > is not used anyway.
> > > > 
> > > > This introduces the fprobe, the function entry/exit probe with
> > > > multiple probe point support. This also introduces the rethook
> > > > for hooking function return as same as kretprobe does. This
> > > 
> > > nice, I was going through the multi-user-graph support 
> > > and was wondering that this might be a better way
> > > 
> > > > abstraction will help us to generalize the fgraph tracer,
> > > > because we can just switch it from rethook in fprobe, depending
> > > > on the kernel configuration.
> > > > 
> > > > The patch [1/8] and [7/8] are from your series[1]. Other libbpf
> > > > patches will not be affected by this change.
> > > 
> > > I'll try the bpf selftests on top of this
> > 
> > I'm getting crash and stall when running bpf selftests,
> > the fprobe sample module works fine, I'll check on that
> 
> I've tried to build tools/testing/selftests/bpf on my machine,
> but I got below errors. Would you know how I can setup to build
> the bpf selftests correctly? (I tried "make M=samples/bpf", but same result)

what's your clang version? your distro might be behind,
I'm using clang 14 compiled from sources:

	$ /opt/clang/bin/clang --version
	clang version 14.0.0 (https://github.com/llvm/llvm-project.git 9f8ffaaa0bddcefeec15a3df9858fd50b05fcbae)
	Target: x86_64-unknown-linux-gnu
	Thread model: posix
	InstalledDir: /opt/clang/bin

and compiling bpf selftests with:

	$ CLANG=/opt/clang/bin/clang make

jirka


> 
> ~/ksrc/linux/tools/testing/selftests/bpf$ make 
> [...]
>   CLANG   /home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool/pid_iter.bpf.o
> skeleton/pid_iter.bpf.c:35:10: error: incomplete definition of type 'struct bpf_link'
>                 return BPF_CORE_READ((struct bpf_link *)ent, id);
>                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:403:2: note: expanded from macro 'BPF_CORE_READ'
>         ___type((src), a, ##__VA_ARGS__) __r;                               \
>         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:274:29: note: expanded from macro '___type'
> #define ___type(...) typeof(___arrow(__VA_ARGS__))
>                             ^~~~~~~~~~~~~~~~~~~~~
> /home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:272:23: note: expanded from macro '___arrow'
> #define ___arrow(...) ___apply(___arrow, ___narg(__VA_ARGS__))(__VA_ARGS__)
>                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
> /home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:223:25: note: expanded from macro '___concat'
> #define ___concat(a, b) a ## b
>                         ^
> <scratch space>:16:1: note: expanded from here
> ___arrow2
> ^
> /home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:263:26: note: expanded from macro '___arrow2'
> #define ___arrow2(a, b) a->b
>                         ~^
> skeleton/pid_iter.bpf.c:35:32: note: forward declaration of 'struct bpf_link'
>                 return BPF_CORE_READ((struct bpf_link *)ent, id);
>                                              ^
> skeleton/pid_iter.bpf.c:35:10: error: incomplete definition of type 'struct bpf_link'
>                 return BPF_CORE_READ((struct bpf_link *)ent, id);
>                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:404:2: note: expanded from macro 'BPF_CORE_READ'
>         BPF_CORE_READ_INTO(&__r, (src), a, ##__VA_ARGS__);                  \
>         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:311:2: note: expanded from macro 'BPF_CORE_READ_INTO'
>         ___core_read(bpf_core_read, bpf_core_read,                          \
>         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:302:2: note: expanded from macro '___core_read'
>         ___apply(___core_read, ___empty(__VA_ARGS__))(fn, fn_ptr, dst,      \
>         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> note: (skipping 3 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
> /home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:296:2: note: expanded from macro '___core_read0'
>         ___read(fn, dst, ___type(src), src, a);
>         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:277:59: note: expanded from macro '___read'
>         read_fn((void *)(dst), sizeof(*(dst)), &((src_type)(src))->accessor)
>         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
> /home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:206:79: note: expanded from macro 'bpf_core_read'
>         bpf_probe_read_kernel(dst, sz, (const void *)__builtin_preserve_access_index(src))
>                                                                                      ^~~
> skeleton/pid_iter.bpf.c:35:32: note: forward declaration of 'struct bpf_link'
>                 return BPF_CORE_READ((struct bpf_link *)ent, id);
>                                              ^
> skeleton/pid_iter.bpf.c:35:10: error: returning 'void' from a function with incompatible result type '__u32' (aka 'unsigned int')
>                 return BPF_CORE_READ((struct bpf_link *)ent, id);
>                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool//bootstrap/libbpf//include/bpf/bpf_core_read.h:402:36: note: expanded from macro 'BPF_CORE_READ'
> #define BPF_CORE_READ(src, a, ...) ({                                       \
>                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> skeleton/pid_iter.bpf.c:42:17: warning: declaration of 'struct bpf_iter__task_file' will not be visible outside of this function [-Wvisibility]
> int iter(struct bpf_iter__task_file *ctx)
>                 ^
> skeleton/pid_iter.bpf.c:44:25: error: incomplete definition of type 'struct bpf_iter__task_file'
>         struct file *file = ctx->file;
>                             ~~~^
> skeleton/pid_iter.bpf.c:42:17: note: forward declaration of 'struct bpf_iter__task_file'
> int iter(struct bpf_iter__task_file *ctx)
>                 ^
> skeleton/pid_iter.bpf.c:45:32: error: incomplete definition of type 'struct bpf_iter__task_file'
>         struct task_struct *task = ctx->task;
>                                    ~~~^
> skeleton/pid_iter.bpf.c:42:17: note: forward declaration of 'struct bpf_iter__task_file'
> int iter(struct bpf_iter__task_file *ctx)
>                 ^
> skeleton/pid_iter.bpf.c:76:19: error: incomplete definition of type 'struct bpf_iter__task_file'
>         bpf_seq_write(ctx->meta->seq, &e, sizeof(e));
>                       ~~~^
> skeleton/pid_iter.bpf.c:42:17: note: forward declaration of 'struct bpf_iter__task_file'
> int iter(struct bpf_iter__task_file *ctx)
>                 ^
> 1 warning and 6 errors generated.
> make[1]: *** [Makefile:188: /home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/build/bpftool/pid_iter.bpf.o] Error 1
> make: *** [Makefile:219: /home/mhiramat/ksrc/linux/tools/testing/selftests/bpf/tools/sbin/bpftool] Error 2
> 
> 
> Thank you,
> 
> -- 
> Masami Hiramatsu <mhiramat@kernel.org>
> 

On Fri, Jan 14, 2022 at 7:10 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Fri, Jan 14, 2022 at 11:47:04PM +0900, Masami Hiramatsu wrote:
> > Hi Jiri and Alexei,
> >
> > On Thu, 13 Jan 2022 13:27:34 +0100
> > Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > > On Wed, Jan 12, 2022 at 05:01:15PM +0100, Jiri Olsa wrote:
> > > > On Wed, Jan 12, 2022 at 11:02:46PM +0900, Masami Hiramatsu wrote:
> > > > > Hi Jiri and Alexei,
> > > > >
> > > > > Here is the 2nd version of fprobe. This version uses the
> > > > > ftrace_set_filter_ips() for reducing the registering overhead.
> > > > > Note that this also drops per-probe point private data, which
> > > > > is not used anyway.
> > > > >
> > > > > This introduces the fprobe, the function entry/exit probe with
> > > > > multiple probe point support. This also introduces the rethook
> > > > > for hooking function return as same as kretprobe does. This
> > > >
> > > > nice, I was going through the multi-user-graph support
> > > > and was wondering that this might be a better way
> > > >
> > > > > abstraction will help us to generalize the fgraph tracer,
> > > > > because we can just switch it from rethook in fprobe, depending
> > > > > on the kernel configuration.
> > > > >
> > > > > The patch [1/8] and [7/8] are from your series[1]. Other libbpf
> > > > > patches will not be affected by this change.
> > > >
> > > > I'll try the bpf selftests on top of this
> > >
> > > I'm getting crash and stall when running bpf selftests,
> > > the fprobe sample module works fine, I'll check on that
> >
> > I've tried to build tools/testing/selftests/bpf on my machine,
> > but I got below errors. Would you know how I can setup to build
> > the bpf selftests correctly? (I tried "make M=samples/bpf", but same result)
>
> what's your clang version? your distro might be behind,

If you have very recent Clang, decently recent pahole, and qemu, try
using vmtest.sh. That should build the kernel with all the necessary
kernel config options and start qemu image with that latest image and
build selftests. And even run selftests automatically.

> I'm using clang 14 compiled from sources:
>
>         $ /opt/clang/bin/clang --version
>         clang version 14.0.0 (https://github.com/llvm/llvm-project.git 9f8ffaaa0bddcefeec15a3df9858fd50b05fcbae)
>         Target: x86_64-unknown-linux-gnu
>         Thread model: posix
>         InstalledDir: /opt/clang/bin
>
> and compiling bpf selftests with:
>
>         $ CLANG=/opt/clang/bin/clang make
>
> jirka
>
>
> >

[...]

> >
> > Thank you,
> >
> > --
> > Masami Hiramatsu <mhiramat@kernel.org>
> >
>

Hi Andrii,

On Fri, 14 Jan 2022 17:02:31 -0800
Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:

> On Fri, Jan 14, 2022 at 7:10 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Fri, Jan 14, 2022 at 11:47:04PM +0900, Masami Hiramatsu wrote:
> > > Hi Jiri and Alexei,
> > >
> > > On Thu, 13 Jan 2022 13:27:34 +0100
> > > Jiri Olsa <jolsa@redhat.com> wrote:
> > >
> > > > On Wed, Jan 12, 2022 at 05:01:15PM +0100, Jiri Olsa wrote:
> > > > > On Wed, Jan 12, 2022 at 11:02:46PM +0900, Masami Hiramatsu wrote:
> > > > > > Hi Jiri and Alexei,
> > > > > >
> > > > > > Here is the 2nd version of fprobe. This version uses the
> > > > > > ftrace_set_filter_ips() for reducing the registering overhead.
> > > > > > Note that this also drops per-probe point private data, which
> > > > > > is not used anyway.
> > > > > >
> > > > > > This introduces the fprobe, the function entry/exit probe with
> > > > > > multiple probe point support. This also introduces the rethook
> > > > > > for hooking function return as same as kretprobe does. This
> > > > >
> > > > > nice, I was going through the multi-user-graph support
> > > > > and was wondering that this might be a better way
> > > > >
> > > > > > abstraction will help us to generalize the fgraph tracer,
> > > > > > because we can just switch it from rethook in fprobe, depending
> > > > > > on the kernel configuration.
> > > > > >
> > > > > > The patch [1/8] and [7/8] are from your series[1]. Other libbpf
> > > > > > patches will not be affected by this change.
> > > > >
> > > > > I'll try the bpf selftests on top of this
> > > >
> > > > I'm getting crash and stall when running bpf selftests,
> > > > the fprobe sample module works fine, I'll check on that
> > >
> > > I've tried to build tools/testing/selftests/bpf on my machine,
> > > but I got below errors. Would you know how I can setup to build
> > > the bpf selftests correctly? (I tried "make M=samples/bpf", but same result)
> >
> > what's your clang version? your distro might be behind,

I'm using clang 13.0.0.

$ clang -v
clang version 13.0.0 (https://github.com/llvm/llvm-project.git d7b669b3a30345cfcdb2fde2af6f48aa4b94845d)
Target: x86_64-unknown-linux-gnu

> 
> If you have very recent Clang, decently recent pahole, and qemu, try
> using vmtest.sh. That should build the kernel with all the necessary
> kernel config options and start qemu image with that latest image and
> build selftests. And even run selftests automatically.

OK, vmtest.sh works! :)

So I got the vmtest.sh runs out with some failures. Jiri, did you talked about
these failures, or real crash?

Summary: 212/1033 PASSED, 12 SKIPPED, 14 FAILED

Thanks! 

> 
> > I'm using clang 14 compiled from sources:
> >
> >         $ /opt/clang/bin/clang --version
> >         clang version 14.0.0 (https://github.com/llvm/llvm-project.git 9f8ffaaa0bddcefeec15a3df9858fd50b05fcbae)
> >         Target: x86_64-unknown-linux-gnu
> >         Thread model: posix
> >         InstalledDir: /opt/clang/bin
> >
> > and compiling bpf selftests with:
> >
> >         $ CLANG=/opt/clang/bin/clang make
> >
> > jirka
> >
> >
> > >
> 
> [...]
> 
> > >
> > > Thank you,
> > >
> > > --
> > > Masami Hiramatsu <mhiramat@kernel.org>
> > >
> >


-- 
Masami Hiramatsu <mhiramat@kernel.org>

On Thu, 13 Jan 2022 13:27:34 +0100
Jiri Olsa <jolsa@redhat.com> wrote:

> On Wed, Jan 12, 2022 at 05:01:15PM +0100, Jiri Olsa wrote:
> > On Wed, Jan 12, 2022 at 11:02:46PM +0900, Masami Hiramatsu wrote:
> > > Hi Jiri and Alexei,
> > > 
> > > Here is the 2nd version of fprobe. This version uses the
> > > ftrace_set_filter_ips() for reducing the registering overhead.
> > > Note that this also drops per-probe point private data, which
> > > is not used anyway.
> > > 
> > > This introduces the fprobe, the function entry/exit probe with
> > > multiple probe point support. This also introduces the rethook
> > > for hooking function return as same as kretprobe does. This
> > 
> > nice, I was going through the multi-user-graph support 
> > and was wondering that this might be a better way
> > 
> > > abstraction will help us to generalize the fgraph tracer,
> > > because we can just switch it from rethook in fprobe, depending
> > > on the kernel configuration.
> > > 
> > > The patch [1/8] and [7/8] are from your series[1]. Other libbpf
> > > patches will not be affected by this change.
> > 
> > I'll try the bpf selftests on top of this
> 
> I'm getting crash and stall when running bpf selftests,
> the fprobe sample module works fine, I'll check on that

OK, I got a kernel stall. I missed to enable CONFIG_FPROBE.
I think vmtest.sh should support menuconfig option.

#6 bind_perm:OK
#7 bloom_filter_map:OK
[  107.282403] clocksource: timekeeping watchdog on CPU0: Marking clocksource 'tsc' as unstable because the skew is too large:
[  107.283240] clocksource:                       'hpet' wd_nsec: 496216090 wd_now: 7ddc7120 wd_last: 7ae746b7 mask: ffffffff
[  107.284045] clocksource:                       'tsc' cs_nsec: 495996979 cs_now: 31fdb69b39 cs_last: 31c2d29219 mask: ffffffffffffffff
[  107.284926] clocksource:                       'tsc' is current clocksource.
[  107.285487] tsc: Marking TSC unstable due to clocksource watchdog
[  107.285973] TSC found unstable after boot, most likely due to broken BIOS. Use 'tsc=unstable'.
[  107.286616] sched_clock: Marking unstable (107240582544, 45390230)<-(107291410145, -5437339)
[  107.290408] clocksource: Not enough CPUs to check clocksource 'tsc'.
[  107.290879] clocksource: Switched to clocksource hpet
[  604.210415] INFO: rcu_tasks detected stalls on tasks:
[  604.210830] (____ptrval____): .. nvcsw: 86/86 holdout: 1 idle_cpu: -1/0
[  604.211314] task:test_progs      state:R  running task     stack:    0 pid:   87 ppid:    85 flags:0x00004000
[  604.212058] Call Trace:
[  604.212246]  <TASK>
[  604.212452]  __schedule+0x362/0xbb0
[  604.212723]  ? preempt_schedule_notrace_thunk+0x16/0x18
[  604.213107]  preempt_schedule_notrace+0x48/0x80
[  604.217403]  ? asm_sysvec_apic_timer_interrupt+0x12/0x20
[  604.217790]  ? ftrace_regs_call+0xd/0x52
[  604.218087]  ? bpf_test_finish.isra.0+0x190/0x190
[  604.218461]  ? bpf_fentry_test1+0x5/0x10
[  604.218750]  ? trace_clock_x86_tsc+0x10/0x10
[  604.219064]  ? __sys_bpf+0x8b1/0x2970
[  604.219337]  ? lock_is_held_type+0xd7/0x130
[  604.219680]  ? __x64_sys_bpf+0x1c/0x20
[  604.219957]  ? do_syscall_64+0x35/0x80
[  604.220237]  ? entry_SYSCALL_64_after_hwframe+0x44/0xae
[  604.220653]  </TASK>

Jiri, is that what you had seen? 


Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

On Sat, Jan 15, 2022 at 01:52:19PM +0900, Masami Hiramatsu wrote:
> On Thu, 13 Jan 2022 13:27:34 +0100
> Jiri Olsa <jolsa@redhat.com> wrote:
> 
> > On Wed, Jan 12, 2022 at 05:01:15PM +0100, Jiri Olsa wrote:
> > > On Wed, Jan 12, 2022 at 11:02:46PM +0900, Masami Hiramatsu wrote:
> > > > Hi Jiri and Alexei,
> > > > 
> > > > Here is the 2nd version of fprobe. This version uses the
> > > > ftrace_set_filter_ips() for reducing the registering overhead.
> > > > Note that this also drops per-probe point private data, which
> > > > is not used anyway.
> > > > 
> > > > This introduces the fprobe, the function entry/exit probe with
> > > > multiple probe point support. This also introduces the rethook
> > > > for hooking function return as same as kretprobe does. This
> > > 
> > > nice, I was going through the multi-user-graph support 
> > > and was wondering that this might be a better way
> > > 
> > > > abstraction will help us to generalize the fgraph tracer,
> > > > because we can just switch it from rethook in fprobe, depending
> > > > on the kernel configuration.
> > > > 
> > > > The patch [1/8] and [7/8] are from your series[1]. Other libbpf
> > > > patches will not be affected by this change.
> > > 
> > > I'll try the bpf selftests on top of this
> > 
> > I'm getting crash and stall when running bpf selftests,
> > the fprobe sample module works fine, I'll check on that
> 
> OK, I got a kernel stall. I missed to enable CONFIG_FPROBE.
> I think vmtest.sh should support menuconfig option.
> 
> #6 bind_perm:OK
> #7 bloom_filter_map:OK
> [  107.282403] clocksource: timekeeping watchdog on CPU0: Marking clocksource 'tsc' as unstable because the skew is too large:
> [  107.283240] clocksource:                       'hpet' wd_nsec: 496216090 wd_now: 7ddc7120 wd_last: 7ae746b7 mask: ffffffff
> [  107.284045] clocksource:                       'tsc' cs_nsec: 495996979 cs_now: 31fdb69b39 cs_last: 31c2d29219 mask: ffffffffffffffff
> [  107.284926] clocksource:                       'tsc' is current clocksource.
> [  107.285487] tsc: Marking TSC unstable due to clocksource watchdog
> [  107.285973] TSC found unstable after boot, most likely due to broken BIOS. Use 'tsc=unstable'.
> [  107.286616] sched_clock: Marking unstable (107240582544, 45390230)<-(107291410145, -5437339)
> [  107.290408] clocksource: Not enough CPUs to check clocksource 'tsc'.
> [  107.290879] clocksource: Switched to clocksource hpet
> [  604.210415] INFO: rcu_tasks detected stalls on tasks:
> [  604.210830] (____ptrval____): .. nvcsw: 86/86 holdout: 1 idle_cpu: -1/0
> [  604.211314] task:test_progs      state:R  running task     stack:    0 pid:   87 ppid:    85 flags:0x00004000
> [  604.212058] Call Trace:
> [  604.212246]  <TASK>
> [  604.212452]  __schedule+0x362/0xbb0
> [  604.212723]  ? preempt_schedule_notrace_thunk+0x16/0x18
> [  604.213107]  preempt_schedule_notrace+0x48/0x80
> [  604.217403]  ? asm_sysvec_apic_timer_interrupt+0x12/0x20
> [  604.217790]  ? ftrace_regs_call+0xd/0x52
> [  604.218087]  ? bpf_test_finish.isra.0+0x190/0x190
> [  604.218461]  ? bpf_fentry_test1+0x5/0x10
> [  604.218750]  ? trace_clock_x86_tsc+0x10/0x10
> [  604.219064]  ? __sys_bpf+0x8b1/0x2970
> [  604.219337]  ? lock_is_held_type+0xd7/0x130
> [  604.219680]  ? __x64_sys_bpf+0x1c/0x20
> [  604.219957]  ? do_syscall_64+0x35/0x80
> [  604.220237]  ? entry_SYSCALL_64_after_hwframe+0x44/0xae
> [  604.220653]  </TASK>
> 
> Jiri, is that what you had seen? 

hi,
sorry for late response

I did not get any backtrace for the stall, debugging showed 
that the first probed function was called over and over for
some reason

as for the crash I used the small fix below

do you have any newer version I could play with?

jirka


---
diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
index 3333893e5217..883151275892 100644
--- a/kernel/trace/fprobe.c
+++ b/kernel/trace/fprobe.c
@@ -157,7 +157,8 @@ int unregister_fprobe(struct fprobe *fp)
 	ret = unregister_ftrace_function(&fp->ftrace);
 
 	if (!ret) {
-		rethook_free(fp->rethook);
+		if (fp->rethook)
+			rethook_free(fp->rethook);
 		if (fp->syms) {
 			kfree(fp->addrs);
 			fp->addrs = NULL;

On Tue, 18 Jan 2022 15:25:25 +0100
Jiri Olsa <jolsa@redhat.com> wrote:

> On Sat, Jan 15, 2022 at 01:52:19PM +0900, Masami Hiramatsu wrote:
> > On Thu, 13 Jan 2022 13:27:34 +0100
> > Jiri Olsa <jolsa@redhat.com> wrote:
> > 
> > > On Wed, Jan 12, 2022 at 05:01:15PM +0100, Jiri Olsa wrote:
> > > > On Wed, Jan 12, 2022 at 11:02:46PM +0900, Masami Hiramatsu wrote:
> > > > > Hi Jiri and Alexei,
> > > > > 
> > > > > Here is the 2nd version of fprobe. This version uses the
> > > > > ftrace_set_filter_ips() for reducing the registering overhead.
> > > > > Note that this also drops per-probe point private data, which
> > > > > is not used anyway.
> > > > > 
> > > > > This introduces the fprobe, the function entry/exit probe with
> > > > > multiple probe point support. This also introduces the rethook
> > > > > for hooking function return as same as kretprobe does. This
> > > > 
> > > > nice, I was going through the multi-user-graph support 
> > > > and was wondering that this might be a better way
> > > > 
> > > > > abstraction will help us to generalize the fgraph tracer,
> > > > > because we can just switch it from rethook in fprobe, depending
> > > > > on the kernel configuration.
> > > > > 
> > > > > The patch [1/8] and [7/8] are from your series[1]. Other libbpf
> > > > > patches will not be affected by this change.
> > > > 
> > > > I'll try the bpf selftests on top of this
> > > 
> > > I'm getting crash and stall when running bpf selftests,
> > > the fprobe sample module works fine, I'll check on that
> > 
> > OK, I got a kernel stall. I missed to enable CONFIG_FPROBE.
> > I think vmtest.sh should support menuconfig option.
> > 
> > #6 bind_perm:OK
> > #7 bloom_filter_map:OK
> > [  107.282403] clocksource: timekeeping watchdog on CPU0: Marking clocksource 'tsc' as unstable because the skew is too large:
> > [  107.283240] clocksource:                       'hpet' wd_nsec: 496216090 wd_now: 7ddc7120 wd_last: 7ae746b7 mask: ffffffff
> > [  107.284045] clocksource:                       'tsc' cs_nsec: 495996979 cs_now: 31fdb69b39 cs_last: 31c2d29219 mask: ffffffffffffffff
> > [  107.284926] clocksource:                       'tsc' is current clocksource.
> > [  107.285487] tsc: Marking TSC unstable due to clocksource watchdog
> > [  107.285973] TSC found unstable after boot, most likely due to broken BIOS. Use 'tsc=unstable'.
> > [  107.286616] sched_clock: Marking unstable (107240582544, 45390230)<-(107291410145, -5437339)
> > [  107.290408] clocksource: Not enough CPUs to check clocksource 'tsc'.
> > [  107.290879] clocksource: Switched to clocksource hpet
> > [  604.210415] INFO: rcu_tasks detected stalls on tasks:
> > [  604.210830] (____ptrval____): .. nvcsw: 86/86 holdout: 1 idle_cpu: -1/0
> > [  604.211314] task:test_progs      state:R  running task     stack:    0 pid:   87 ppid:    85 flags:0x00004000
> > [  604.212058] Call Trace:
> > [  604.212246]  <TASK>
> > [  604.212452]  __schedule+0x362/0xbb0
> > [  604.212723]  ? preempt_schedule_notrace_thunk+0x16/0x18
> > [  604.213107]  preempt_schedule_notrace+0x48/0x80
> > [  604.217403]  ? asm_sysvec_apic_timer_interrupt+0x12/0x20
> > [  604.217790]  ? ftrace_regs_call+0xd/0x52
> > [  604.218087]  ? bpf_test_finish.isra.0+0x190/0x190
> > [  604.218461]  ? bpf_fentry_test1+0x5/0x10
> > [  604.218750]  ? trace_clock_x86_tsc+0x10/0x10
> > [  604.219064]  ? __sys_bpf+0x8b1/0x2970
> > [  604.219337]  ? lock_is_held_type+0xd7/0x130
> > [  604.219680]  ? __x64_sys_bpf+0x1c/0x20
> > [  604.219957]  ? do_syscall_64+0x35/0x80
> > [  604.220237]  ? entry_SYSCALL_64_after_hwframe+0x44/0xae
> > [  604.220653]  </TASK>
> > 
> > Jiri, is that what you had seen? 
> 
> hi,
> sorry for late response
> 
> I did not get any backtrace for the stall, debugging showed 
> that the first probed function was called over and over for
> some reason
> 
> as for the crash I used the small fix below

Oops, good catch!

> 
> do you have any newer version I could play with?

Let me update the fprobe and rethook. I'm now trying to integrate
the rethook with kretprobes and find some issues.

Thank you!

> 
> jirka
> 
> 
> ---
> diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
> index 3333893e5217..883151275892 100644
> --- a/kernel/trace/fprobe.c
> +++ b/kernel/trace/fprobe.c
> @@ -157,7 +157,8 @@ int unregister_fprobe(struct fprobe *fp)
>  	ret = unregister_ftrace_function(&fp->ftrace);
>  
>  	if (!ret) {
> -		rethook_free(fp->rethook);
> +		if (fp->rethook)
> +			rethook_free(fp->rethook);
>  		if (fp->syms) {
>  			kfree(fp->addrs);
>  			fp->addrs = NULL;
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>