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

[Eugene Loh]

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?

Also, how about a comment mentioning the .->_ conversion, analogous to 
the comment that was in rawtp detach() or that is still in kprobe_attach()?
> +		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);
>   }