[DTrace-devel] [PATCH dtrace 0/3] fprobe fallback kprobe support, sched fix

[Alan Maguire]

hi Kris

thanks for the detailed explanation! Replies below..

On 09/10/2024 18:34, Kris Van Hees wrote:
> 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?

I've been looking at the code, and the problem I see is that it's hard
to find a place where we can look at the actually _used_ probes (as
opposed to those we populate()) and make determinations around kprobe
support. My first go at this was simply to use the kprobe implementation
for all functions containing a ".". However, as you observed, that could
potentially result in a mix of fprobe and kprobe for a particular probe
context which isn't allowed.

And for programs where a probe participates in multiple contexts we'd
need to worry about whether probe replacement in one then had knock-on
effects for fprobe/kprobe mixing, timing etc. Specifically if we
replaced a fprobe by a kprobe for one clause, we'd need to do the same
for it in other clauses I think.

So I _think_ we're stuck with an either-or; either we can use fprobes
everywhere, or we have to use kprobes everywhere in a program. Is that
the case do you think?

If so, the question then becomes is there a way to iterate over the
actually used fbt probes to make sure they are usable as fprobes; if not
replacing all probes with a kprobe backend. The problem is the only
place I see that is in the compile of the various clauses. We'd need (I
think) some kind of prepare phase where we walk the clauses and figure
out which probes are associated to make this determination. Once we
start compiling clauses, I think it's too late to backpedal and replace
an implementation.

Or are there other places/ways to solve this I'm missing? Thanks!

Alan

> 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
>>