Re: [PATCH 4/4] fbt: do not provide untraceable functins (for fprobes)
Nick Alcock <[email protected]> Fri, 17 Apr 2026 18:11:48 +0100
| Newsgroups | dev.linux.lists.dtrace |
|---|---|
| Message-ID | <[email protected]> |
On 14 Apr 2026, Kris Van Hees uttered the following: > Subject: Re: [PATCH 4/4] fbt: do not provide untraceable functins (for fprobes) Typo. > Ensure that we do not provide FBT probes for functions that cannot be > probed using BPF fentry/fexit probes if fprobes are the default. I guess falling back to kprobes for those single functions alone is either too complicated or has unpleasantly visible side-effects? > Signed-off-by: Kris Van Hees <[email protected]> > --- > libdtrace/dt_prov_fbt.c | 47 ++++++++++++++++++++++++++--------------- > 1 file changed, 30 insertions(+), 17 deletions(-) > > diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c > index 59e4583b2..a2f20c9bc 100644 > --- a/libdtrace/dt_prov_fbt.c > +++ b/libdtrace/dt_prov_fbt.c > @@ -86,14 +86,30 @@ static int populate(dtrace_hdl_t *dtp) > } > > /* Create a probe (if it does not exist yet). */ > -static int provide_probe(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp) > +static int provide_probe(dtrace_hdl_t *dtp, dt_module_t *dmp, > + dtrace_probedesc_t *pdp) > { > dt_provider_t *prv = dt_provider_lookup(dtp, pdp->prv); > > if (prv == NULL) > return 0; > + pdp->mod = dmp->dm_name; > if (dt_probe_lookup(dtp, pdp) != NULL) > return 0; > + > + /* > + * The BPF verifier does not allow fentry/fexit probing of variadic > + * functions. This comment is wrong now that this is being implemented via another function: the fact that dt_btf_func_is_traceable() checks variadicity belongs above that function (and is indeed there). Something like /* * Do not provide probes for functions with BTF that indicates that they * are not traceable. */ maybe? > + */ > + if (prv->impl->prog_type == BPF_PROG_TYPE_TRACING) { > + int32_t btf_id; > + > + btf_id = dt_btf_lookup_name_kind(dtp, dmp, pdp->fun, BTF_KIND_FUNC); > + if (btf_id <= 0 || > + !dt_btf_func_is_traceable(dtp, dmp->dm_btf, btf_id)) > + return -1; > + } > + > if (dt_tp_probe_insert(dtp, prv, pdp->prv, pdp->mod, pdp->fun, pdp->prb)) > return 1; > The hunks below appear to be doing something not called out in the commit log. I think this is getting the symbol module right? > @@ -173,16 +189,15 @@ static int provide(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp) > > pd.id = DTRACE_IDNONE; > pd.prv = pdp->prv; > - pd.mod = dmp->dm_name; > pd.fun = pdp->fun; > > if (prb & FBT_ENTRY) { > pd.prb = "entry"; > - n += provide_probe(dtp, &pd); > + n += provide_probe(dtp, dmp, &pd); > } > if (prb & FBT_RETURN) { > pd.prb = "return"; > - n += provide_probe(dtp, &pd); > + n += provide_probe(dtp, dmp, &pd); > } > > return n; > @@ -190,22 +205,20 @@ static int provide(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp) > > sym = dt_symbol_by_name(dtp, pdp->fun); > while (sym != NULL) { > - const char *mod = dt_symbol_module(sym)->dm_name; > - > + dmp = dt_symbol_module(sym); > if (dt_symbol_traceable(sym) && > - dt_gmatch(mod, pdp->mod)) { > + dt_gmatch(dmp->dm_name, pdp->mod)) { > pd.id = DTRACE_IDNONE; > pd.prv = pdp->prv; > - pd.mod = mod; > pd.fun = pdp->fun; > > if (prb & FBT_ENTRY) { > pd.prb = "entry"; > - n += provide_probe(dtp, &pd); > + n += provide_probe(dtp, dmp, &pd); > } > if (prb & FBT_RETURN) { > pd.prb = "return"; > - n += provide_probe(dtp, &pd); > + n += provide_probe(dtp, dmp, &pd); > } > > } > @@ -242,16 +255,15 @@ static int provide(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp) > > pd.id = DTRACE_IDNONE; > pd.prv = pdp->prv; > - pd.mod = smp->dm_name; > pd.fun = fun; > > if (prb & FBT_ENTRY) { > pd.prb = "entry"; > - n += provide_probe(dtp, &pd); > + n += provide_probe(dtp, smp, &pd); > } > if (prb & FBT_RETURN) { > pd.prb = "return"; > - n += provide_probe(dtp, &pd); > + n += provide_probe(dtp, smp, &pd); > } > } > The need for this trampoline change is also rather opaque. > @@ -323,10 +335,11 @@ static int fprobe_trampoline(dt_pcb_t *pcb, uint_t exitlbl) > */ > dmp = dt_module_lookup_by_name(dtp, prp->desc->mod); > if (dmp && prp->argc == 2) { > - int32_t btf_id = dt_tp_probe_get_id(prp); > - int i = dt_btf_func_argc(dtp, dmp->dm_btf, btf_id); > - > - emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_8, i * 8)); > + emit(dlp, BPF_MOV_REG(BPF_REG_1, BPF_REG_8)); > + emit(dlp, BPF_MOV_REG(BPF_REG_2, BPF_REG_FP)); > + emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, DT_TRAMP_SP_SLOT(0))); > + emit(dlp, BPF_CALL_HELPER(dtp->dt_bpfhelper[BPF_FUNC_get_func_ret])); > + emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_FP, DT_TRAMP_SP_SLOT(0))); > emit(dlp, BPF_STORE(BPF_DW, BPF_REG_7, DMST_ARG(1), BPF_REG_0)); > } > } -- NULL && (void)