Re: [PATCH dtrace 0/3] fprobe fallback kprobe support, sched fix

[Kris Van Hees <kris.van.hees@oracle.com>]

Hi Alan,

I see what you are doing here and I think that it certainly has merit, but I
would say that the implementation is breaking some assumptions that are part
of the provider design in the code base.  The way that a probe is related to
a provider implementation is rather fundamental and you end up breaking that
by having a fprobe-backed probe actually use the kprobe implementation.  I am
thinking how we can establish the same without breaking that association.

There is also the fact to consider that if we end up attaching a given probe
both as an fprobe-backed FBT probe and a kprobe-based FBT probe, things can
get really interesting because the defined order of execution of clauses will
no longer be satisfied.

Now, what perhaps could work is if we were to first try to determine whether
we can use a fprobe-backed for an FBT probe, and if that is not possible, do
a fall-back to a kprobe-backed probe by removing the probe and re-inserting it
as a kprobe-backed FBT probe.  Since a probe is associated with a provider by
a pointer to the provider implementation, that should work.

It should also still work for probe lookup (though that should no longer be
taking place at that point in processing) because the lookup code is based
on the provider name selecting the correct bucket, which can contain probes
that have that same provider name even if the implementation for them is
different.

This way we can never have a probe that is associated with the fprobe
implementation but really ends up using the kprobe implementation.  The
association between probe and provider implementation would still be kept
as-is.

Do you think that would work?

ONe complication I can think of is cases where both func and func.<suffix>
should be traced when you use an fbt::func:entry (or return) probe.  They
would both need to be done as kprobes, because (again) we cannot have a mix.

On Wed, Oct 09, 2024 at 03:02:33PM +0100, Alan Maguire wrote:
> This series is focused on solving a few issues with fprobe-based
> attachment which prevent us being able to attach to functions
> like finish_task_switch.isra.0.  Such functions are present in
> available_filter_functions, but because they either lack BTF
> representations or those representations are named without the
> .isra suffix, attachment via fentry/fexit is currently impossible.
> Falling back to kprobe attach is the best solution here.
> 
> However providers have been written with the implicit assumption
> that certain aspects of the probes are shared across the provider;
> program type, stack skips etc.  Patch 1 addresses this by providing
> optional provider implementation callbacks to return these values
> for a specific probe.  Patch 2 then updates the fbt provider to
> use these mechanisms to support fallback to kprobe and to support
> "."-suffixed functions.  Finally patch 3 can then specify
> fbt::finish_task_switch*:return as the underlying probe for
> sched:::on-cpu.
> 
> Tested on upstream, UEK7 (5.15-based kernel) and UEK6 (5.4-based).
> 
> Alan Maguire (3):
>   dtrace: add support for probe-specific prog types, stack skips
>   fbt: support ".isra.0" suffixed functions
>   sched: fix on-cpu firing for kernels < 5.16
> 
>  libdtrace/dt_bpf.c         |  4 +-
>  libdtrace/dt_cc.c          |  2 +-
>  libdtrace/dt_probe.c       | 16 ++++++++
>  libdtrace/dt_probe.h       |  2 +
>  libdtrace/dt_prov_fbt.c    | 84 +++++++++++++++++++++++++++++++-------
>  libdtrace/dt_prov_rawtp.c  |  2 +-
>  libdtrace/dt_prov_sched.c  | 23 +----------
>  libdtrace/dt_prov_sdt.c    |  2 +-
>  libdtrace/dt_provider.h    |  4 ++
>  libdtrace/dt_provider_tp.c | 12 +++++-
>  libdtrace/dt_provider_tp.h |  9 +++-
>  11 files changed, 117 insertions(+), 43 deletions(-)
> 
> -- 
> 2.43.5
>