All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nick Alcock <nick.alcock@oracle.com>
To: Kris Van Hees <kris.van.hees@oracle.com>
Cc: dtrace@lists.linux.dev, dtrace-devel@oss.oracle.com
Subject: Re: [PATCH 4/4] fbt: do not provide untraceable functins (for fprobes)
Date: Fri, 17 Apr 2026 18:11:48 +0100	[thread overview]
Message-ID: <87a4v1zf3f.fsf@esperi.org.uk> (raw)
In-Reply-To: <IA0PR10MB7325BFFD3DC90AC2FEFCADD9C2252@IA0PR10MB7325.namprd10.prod.outlook.com> (Kris Van Hees's message of "Tue, 14 Apr 2026 02:27:48 -0400")

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 <kris.van.hees@oracle.com>
> ---
>  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)

  reply	other threads:[~2026-04-17 17:12 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-14  6:27 [PATCH 4/4] fbt: do not provide untraceable functins (for fprobes) Kris Van Hees
2026-04-17 17:11 ` Nick Alcock [this message]
2026-04-17 19:28   ` Kris Van Hees

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87a4v1zf3f.fsf@esperi.org.uk \
    --to=nick.alcock@oracle.com \
    --cc=dtrace-devel@oss.oracle.com \
    --cc=dtrace@lists.linux.dev \
    --cc=kris.van.hees@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.