public inbox for dtrace@lists.linux.dev
 help / color / mirror / Atom feed
From: Kris Van Hees <kris.van.hees@oracle.com>
To: Nick Alcock <nick.alcock@oracle.com>
Cc: Kris Van Hees <kris.van.hees@oracle.com>,
	dtrace@lists.linux.dev, dtrace-devel@oss.oracle.com
Subject: Re: [PATCH 3/4] btf: add dt_btf_func_is_traceable()
Date: Fri, 17 Apr 2026 15:04:11 -0400	[thread overview]
Message-ID: <aeKEKxIGoKdgzHiF@oracle.com> (raw)
In-Reply-To: <87eckdzfb3.fsf@esperi.org.uk>

On Fri, Apr 17, 2026 at 06:07:12PM +0100, Nick Alcock wrote:
> On 14 Apr 2026, Kris Van Hees spake thusly:
> 
> > Add a function to determine whether a function (given by BTF id) can be
> > traced using BPF fentry/fexit probes.  It implements checks inspired by
> > btf_distill_func_proto() in kernel/bpf/btf.c.
> 
> ... looks reasonable.
> 
> > Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> 
> Reviewed-by: Nick Alcock <nick.alcock@oracle.com>
> 
> (though you might possibly want to implement the suggestion below, it's
> very much optional.)
> 
> > ---
> >  libdtrace/dt_btf.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++
> >  libdtrace/dt_btf.h |  2 ++
> >  2 files changed, 67 insertions(+)
> >
> > diff --git a/libdtrace/dt_btf.c b/libdtrace/dt_btf.c
> > index 56eb96a4e..fbf377b4e 100644
> > --- a/libdtrace/dt_btf.c
> > +++ b/libdtrace/dt_btf.c
> > @@ -1020,3 +1020,68 @@ dt_btf_func_is_void(dtrace_hdl_t *dtp, const dt_btf_t *btf, uint32_t id)
> >  
> >  	return 0;
> >  }
> > +
> > +/*
> > + * Return 1 if the function referenced by the BTF id can be traced using the
> > + * fprobe/fexit facility.  Specifically, that the function is not variadic and
> > + * that none of its arguments exceeds the acceptable value size (16 bytes).
> > + * (See btf_distill_func_proto() in kernel/bpf/btf.c for reference.)
> > + */
> > +#define MAX_BPF_FUNC_REG_REGS	5
> 
> A shame we need to hardwire this. More of a shame that we need to
> hardwire it deep in a .c file where it'll almost certainly be overlooked
> when the time comes to change it. Stick it in a suitable header, maybe?

It is an internal (non-UAPI) aspect of BPF that is only relevant here, which is
why I chose to define it here.  I think it would be confusing to have it in a
more generic header file.

> > +int
> > +dt_btf_func_is_traceable(dtrace_hdl_t *dtp, const dt_btf_t *btf, uint32_t id)
> > +{
> > +	btf_type_t	*type = dt_btf_real_type_by_id(dtp, btf, id);
> > +	int		argc;
> > +
> > +	/* If no prototype is found, BPF fprobes do not work. */
> > +	if (!type || BTF_INFO_KIND(type->info) != BTF_KIND_FUNC_PROTO)
> > +		return 0;
> > +
> > +	/* If the return type is a struct or union, BPF fprobes do not work. */
> > +	if (type->type != 0) {
> > +		btf_type_t	*rtype = dt_btf_real_type_by_id(dtp, btf, type->type);
> > +
> > +		if (rtype == NULL)
> > +			return 0;
> > +
> > +		switch (BTF_INFO_KIND(rtype->info)) {
> > +		case BTF_KIND_STRUCT:
> > +		case BTF_KIND_UNION:
> > +			return 0;
> > +		default:
> > +			/* fall-through */
> > +		}
> > +	}
> > +
> > +	/*
> > +	 * BPF fprobes do not support functions that are variadic or that have
> > +	 * any arguments passed by value that are a struct or union of size
> > +	 * greater than 16.
> > +	 */
> > +	argc = BTF_INFO_VLEN(type->info);
> > +	if (argc > 0) {
> > +		btf_param_t	*args = (btf_param_t *)(type + 1);
> > +		int		i;
> > +
> > +		if (args[argc - 1].type == 0)		/* variadic */
> > +			return 0;
> > +
> > +		for (i = 0; i < argc; i++) {
> > +			type = dt_btf_real_type_by_id(dtp, btf, args[i].type);
> > +			if (type == NULL)
> > +				return 0;
> > +
> > +			switch (BTF_INFO_KIND(type->info)) {
> > +			case BTF_KIND_STRUCT:
> > +			case BTF_KIND_UNION:
> > +				if (type->size > 16)	/* value size > 16 */
> > +					return 0;
> > +			default:
> > +				/* fall-through */
> > +			}
> > +		}
> > +	}
> > +
> > +	return 1;
> > +}
> > diff --git a/libdtrace/dt_btf.h b/libdtrace/dt_btf.h
> > index d956ad940..2c921a033 100644
> > --- a/libdtrace/dt_btf.h
> > +++ b/libdtrace/dt_btf.h
> > @@ -28,6 +28,8 @@ extern int dt_btf_func_argc(dtrace_hdl_t *dtp, const dt_btf_t *btf,
> >  			    uint32_t id);
> >  extern int dt_btf_func_is_void(dtrace_hdl_t *dtp, const dt_btf_t *btf,
> >  			       uint32_t id);
> > +extern int dt_btf_func_is_traceable(dtrace_hdl_t *dtp, const dt_btf_t *btf,
> > +				    uint32_t id);
> >  extern int dt_btf_get_module_ids(dtrace_hdl_t *);
> >  extern int dt_btf_module_fd(const dt_module_t *);
> 
> -- 
> NULL && (void)

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

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-14  6:27 [PATCH 3/4] btf: add dt_btf_func_is_traceable() Kris Van Hees
2026-04-17 17:07 ` Nick Alcock
2026-04-17 19:04   ` Kris Van Hees [this message]

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=aeKEKxIGoKdgzHiF@oracle.com \
    --to=kris.van.hees@oracle.com \
    --cc=dtrace-devel@oss.oracle.com \
    --cc=dtrace@lists.linux.dev \
    --cc=nick.alcock@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox