[DTrace-devel] [PATCH 8/8] fbt, rawfbt: consolidate code to avoid duplication

[Kris Van Hees]

On Tue, Mar 18, 2025 at 01:10:21AM -0400, Eugene Loh via DTrace-devel wrote:
> Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
> 
> A few minor notes...
> 
> On 3/7/25 16:34, Kris Van Hees via DTrace-devel wrote:
> > diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
> > @@ -6,17 +6,26 @@
> >    *
> >    * The Function Boundary Tracing (FBT) provider for DTrace.
> >    *
> > - * FBT probes are exposed by the kernel as kprobes.  They are listed in the
> > - * TRACEFS/available_filter_functions file.  Some kprobes are associated with
> > - * a specific kernel module, while most are in the core kernel.
> > + * Kernnel functions can be traced through fentry/fexit probes (when available)
> s/Kernnel/Kernel/
> > + * and kprobes.  The FBT provider supports both implementations and will use
> > + * fentry/fexit probes if the kernel supports them, and fallback to kprobes
> > + * otherwise.  The FBT provider does not support tracing synthetic functions
> > + * (i.e. compiler-generated functions with a . in their name).
> > + *
> > + * The rawfbt provider implements a variant of the FBT provider and always uses
> > + * kprobes.  This provider allow tracing of synthetic function.
> s/allow/allows/
> >    *
> >    * Mapping from event name to DTrace probe name:
> >    *
> >    *	<name>					fbt:vmlinux:<name>:entry
> >    *						fbt:vmlinux:<name>:return
> > + *						rawfbt:vmlinux:<name>:entry
> > + *						rawfbt:vmlinux:<name>:return
> >    *   or
> >    *	<name> [<modname>]			fbt:<modname>:<name>:entry
> >    *						fbt:<modname>:<name>:return
> > + *						rawfbt:<modname>:<name>:entry
> > + *						rawfbt:<modname>:<name>:return
> >    */
> >   #include <assert.h>
> >   #include <errno.h>
> > @@ -514,9 +552,20 @@ static void kprobe_detach(dtrace_hdl_t *dtp, const dt_probe_t *prp)
> >   	if (fd == -1)
> >   		return;
> > -	dprintf(fd, "-:" FBT_GROUP_FMT "/%s\n", FBT_GROUP_DATA,
> > -		prp->desc->fun);
> > +	if (strcmp(prp->desc->prv, dt_rawfbt.name) == 0) {
> > +		char	*p;
> > +
> Do you want to make tpn a strdup() here before modifying it?

Ah yes indeed.  It doesn't break anything as it is now because detach is
done when things are shutting down, but correctness would demand doing a
strdup() here.  Thanks.

> Also, how about a comment mentioning the .->_ conversion, analogous to the
> comment that was in rawtp detach() or that is still in kprobe_attach()?

Sure.

> > +		for (p = tpn; *p; p++) {
> > +			if (*p == '.')
> > +				*p = '_';
> > +		}
> > +	}
> > +
> > +	dprintf(fd, "-:" FBT_GROUP_FMT "/%s\n", FBT_GROUP_DATA, tpn);
> >   	close(fd);
> > +
> > +	if (tpn != prp->desc->fun)
> > +		free(tpn);
> >   }
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel